riscv
/
qemu
mirror of https://gitea.goldendeliverer.com/riscv/qemu.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

hw/nvme: fix namespace atomic parameter setup 

Coverity complains about a possible copy-paste error in the verification
of the namespace atomic parameters (CID 1642811). While the check is
correct, the code (and the intention) is unclear.

Fix this by reworking how the parameters are verified. Peter also
identified that the realize function was not correctly erroring out if
parameters were misconfigured, so fix that too.

Lastly, change the error messages to be more describing.

Coverity: CID 1642811
Fixes: bce51b8370 ("hw/nvme: add atomic boundary support")
Fixes: 3b41acc962 ("hw/nvme: enable ns atomic writes")
Reviewed-by: Jesper Wendel Devantier <foss@defmacro.it>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
pull/312/head
Klaus Jensen 5 months ago
parent
de074358e9
commit
3050b34921
4 changed files with 183 additions and 144 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 115
      hw/nvme/ctrl.c
  2. 189
      hw/nvme/ns.c
  3. 25
      hw/nvme/nvme.h
  4. 2
      include/block/nvme.h

115
hw/nvme/ctrl.c
View File

@ -6524,6 +6524,53 @@ static uint16_t nvme_set_feature_fdp_events(NvmeCtrl *n, NvmeNamespace *ns,
return NVME_SUCCESS;
}
void nvme_atomic_configure_max_write_size(bool dn, uint16_t awun,
uint16_t awupf, NvmeAtomic *atomic)
{
atomic->atomic_max_write_size = (dn ? awupf : awun) + 1;
if (atomic->atomic_max_write_size > 1) {
atomic->atomic_writes = 1;
}
}
static uint16_t nvme_set_feature_write_atomicity(NvmeCtrl *n, NvmeRequest *req)
{
NvmeCmd *cmd = &req->cmd;
uint32_t dw11 = le32_to_cpu(cmd->cdw11);
uint16_t awun = le16_to_cpu(n->id_ctrl.awun);
uint16_t awupf = le16_to_cpu(n->id_ctrl.awupf);
n->dn = dw11 & 0x1;
nvme_atomic_configure_max_write_size(n->dn, awun, awupf, &n->atomic);
for (int i = 1; i <= NVME_MAX_NAMESPACES; i++) {
uint16_t nawun, nawupf, nabsn, nabspf;
NvmeNamespace *ns = nvme_ns(n, i);
if (!ns) {
continue;
}
nawun = le16_to_cpu(ns->id_ns.nawun);
nawupf = le16_to_cpu(ns->id_ns.nawupf);
nvme_atomic_configure_max_write_size(n->dn, nawun, nawupf,
&ns->atomic);
nabsn = le16_to_cpu(ns->id_ns.nabsn);
nabspf = le16_to_cpu(ns->id_ns.nabspf);
nvme_ns_atomic_configure_boundary(n->dn, nabsn, nabspf,
&ns->atomic);
}
return NVME_SUCCESS;
}
static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest *req)
{
NvmeNamespace *ns = NULL;
@ -6536,8 +6583,6 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest *req)
uint8_t save = NVME_SETFEAT_SAVE(dw10);
uint16_t status;
int i;
NvmeIdCtrl *id = &n->id_ctrl;
NvmeAtomic *atomic = &n->atomic;
trace_pci_nvme_setfeat(nvme_cid(req), nsid, fid, save, dw11);
@ -6691,50 +6736,7 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest *req)
case NVME_FDP_EVENTS:
return nvme_set_feature_fdp_events(n, ns, req);
case NVME_WRITE_ATOMICITY:
n->dn = 0x1 & dw11;
if (n->dn) {
atomic->atomic_max_write_size = le16_to_cpu(id->awupf) + 1;
} else {
atomic->atomic_max_write_size = le16_to_cpu(id->awun) + 1;
}
if (atomic->atomic_max_write_size == 1) {
atomic->atomic_writes = 0;
} else {
atomic->atomic_writes = 1;
}
for (i = 1; i <= NVME_MAX_NAMESPACES; i++) {
ns = nvme_ns(n, i);
if (ns && ns->atomic.atomic_writes) {
if (n->dn) {
ns->atomic.atomic_max_write_size =
le16_to_cpu(ns->id_ns.nawupf) + 1;
if (ns->id_ns.nabspf) {
ns->atomic.atomic_boundary =
le16_to_cpu(ns->id_ns.nabspf) + 1;
} else {
ns->atomic.atomic_boundary = 0;
}
} else {
ns->atomic.atomic_max_write_size =
le16_to_cpu(ns->id_ns.nawun) + 1;
if (ns->id_ns.nabsn) {
ns->atomic.atomic_boundary =
le16_to_cpu(ns->id_ns.nabsn) + 1;
} else {
ns->atomic.atomic_boundary = 0;
}
}
if (ns->atomic.atomic_max_write_size == 1) {
ns->atomic.atomic_writes = 0;
} else {
ns->atomic.atomic_writes = 1;
}
}
}
break;
return nvme_set_feature_write_atomicity(n, req);
default:
return NVME_FEAT_NOT_CHANGEABLE | NVME_DNR;
}
@ -7669,6 +7671,10 @@ static int nvme_atomic_boundary_check(NvmeCtrl *n, NvmeCmd *cmd,
imask = ~(atomic->atomic_boundary - 1);
if ((slba & imask) != (elba & imask)) {
/*
* The write crosses an atomic boundary and the controller provides
* no atomicity guarantees unless AWUN/AWUPF are non-zero.
*/
if (n->atomic.atomic_max_write_size &&
((nlb + 1) <= n->atomic.atomic_max_write_size)) {
return 1;
@ -8709,7 +8715,6 @@ static void nvme_init_state(NvmeCtrl *n)
NvmeSecCtrlEntry *list = n->sec_ctrl_list;
NvmeSecCtrlEntry *sctrl;
PCIDevice *pci = PCI_DEVICE(n);
NvmeAtomic *atomic = &n->atomic;
NvmeIdCtrl *id = &n->id_ctrl;
uint8_t max_vfs;
int i;
@ -8781,19 +8786,9 @@ static void nvme_init_state(NvmeCtrl *n)
id->awupf = 0;
}
if (n->dn) {
atomic->atomic_max_write_size = id->awupf + 1;
} else {
atomic->atomic_max_write_size = id->awun + 1;
}
if (atomic->atomic_max_write_size == 1) {
atomic->atomic_writes = 0;
} else {
atomic->atomic_writes = 1;
}
atomic->atomic_boundary = 0;
atomic->atomic_nabo = 0;
nvme_atomic_configure_max_write_size(n->dn, n->params.atomic_awun,
n->params.atomic_awupf,
&n->atomic);
}
}

189
hw/nvme/ns.c
View File

@ -718,85 +718,119 @@ static void nvme_ns_unrealize(DeviceState *dev)
nvme_ns_cleanup(ns);
}
static void nvme_ns_realize(DeviceState *dev, Error **errp)
void nvme_ns_atomic_configure_boundary(bool dn, uint16_t nabsn,
uint16_t nabspf, NvmeAtomic *atomic)
{
atomic->atomic_boundary = dn ? nabspf : nabsn;
if (atomic->atomic_boundary > 0) {
atomic->atomic_boundary += 1;
}
}
static bool nvme_ns_set_nab(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
{
NvmeNamespace *ns = NVME_NS(dev);
BusState *s = qdev_get_parent_bus(dev);
NvmeCtrl *n = NVME(s->parent);
NvmeSubsystem *subsys = n->subsys;
NvmeIdCtrl *id = &n->id_ctrl;
NvmeIdNs *id_ns = &ns->id_ns;
uint32_t nsid = ns->params.nsid;
int i;
NvmeIdCtrl *id_ctrl = &n->id_ctrl;
assert(subsys);
uint16_t nabsn = ns->params.atomic.nabsn;
uint16_t nabspf = ns->params.atomic.nabspf;
uint16_t nabo = ns->params.atomic.nabo;
/* Set atomic write parameters */
if (ns->params.atomic_nsfeat) {
id_ns->nsfeat |= NVME_ID_NS_NSFEAT_NSABPNS;
id_ns->nawun = cpu_to_le16(ns->params.atomic_nawun);
if (!id->awupf || (id_ns->nawun && (id_ns->nawun < id->awun))) {
error_report("Invalid NAWUN: %x AWUN=%x", id_ns->nawun, id->awun);
}
id_ns->nawupf = cpu_to_le16(ns->params.atomic_nawupf);
if (!id->awupf || (id_ns->nawupf && (id_ns->nawupf < id->awupf))) {
error_report("Invalid NAWUPF: %x AWUPF=%x",
id_ns->nawupf, id->awupf);
}
if (id_ns->nawupf > id_ns->nawun) {
error_report("Invalid: NAWUN=%x NAWUPF=%x",
id_ns->nawun, id_ns->nawupf);
}
id_ns->nabsn = cpu_to_le16(ns->params.atomic_nabsn);
id_ns->nabspf = cpu_to_le16(ns->params.atomic_nabspf);
id_ns->nabo = cpu_to_le16(ns->params.atomic_nabo);
if (!id->awun || (id_ns->nabsn && ((id_ns->nabsn < id_ns->nawun) ||
(id_ns->nabsn < id->awun)))) {
error_report("Invalid NABSN: %x NAWUN=%x AWUN=%x",
id_ns->nabsn, id_ns->nawun, id->awun);
}
if (!id->awupf || (id_ns->nabspf && ((id_ns->nabspf < id_ns->nawupf) ||
(id_ns->nawupf < id->awupf)))) {
error_report("Invalid NABSPF: %x NAWUPF=%x AWUPF=%x",
id_ns->nabspf, id_ns->nawupf, id->awupf);
}
if (id_ns->nabo && ((id_ns->nabo > id_ns->nabsn) ||
(id_ns->nabo > id_ns->nabspf))) {
error_report("Invalid NABO: %x NABSN=%x NABSPF=%x",
id_ns->nabo, id_ns->nabsn, id_ns->nabspf);
}
if (id_ns->nawupf > id_ns->nawun) {
error_report("Invalid: NAWUN=%x NAWUPF=%x", id_ns->nawun,
id_ns->nawupf);
}
}
if (id_ns->nawun || id_ns->nawupf) {
NvmeAtomic *atomic = &ns->atomic;
if (n->dn) {
atomic->atomic_max_write_size = cpu_to_le16(id_ns->nawupf) + 1;
if (id_ns->nabspf) {
atomic->atomic_boundary = cpu_to_le16(id_ns->nabspf) + 1;
} else {
atomic->atomic_boundary = 0;
if (nabsn && nabsn < le16_to_cpu(id_ctrl->awun)) {
error_setg(errp, "nabsn must be greater than or equal to awun");
return false;
}
} else {
atomic->atomic_max_write_size = cpu_to_le16(id_ns->nawun) + 1;
if (id_ns->nabsn) {
atomic->atomic_boundary = cpu_to_le16(id_ns->nabsn) + 1;
} else {
atomic->atomic_boundary = 0;
if (nabspf && nabspf < le16_to_cpu(id_ctrl->awupf)) {
error_setg(errp, "nabspf must be greater than or equal to awupf");
return false;
}
if (id_ns->nsfeat & NVME_ID_NS_NSFEAT_NSABP) {
if (nabsn && nabsn < le16_to_cpu(id_ns->nawun)) {
error_setg(errp, "nabsn must be greater than or equal to nawun");
return false;
}
if (nabspf && nabspf < le16_to_cpu(id_ns->nawupf)) {
error_setg(errp, "nabspf must be great than or equal to nawupf");
return false;
}
}
if (nabo && (nabo > nabsn || nabo > nabspf)) {
error_setg(errp, "nabo must be less than or equal to nabsn and nabspf");
return false;
}
if (atomic->atomic_max_write_size == 1) {
atomic->atomic_writes = 0;
id_ns->nabsn = cpu_to_le16(nabsn);
id_ns->nabspf = cpu_to_le16(nabspf);
id_ns->nabo = cpu_to_le16(nabo);
ns->atomic.atomic_nabo = nabo;
nvme_ns_atomic_configure_boundary(n->dn, nabsn, nabspf, &ns->atomic);
return true;
}
static bool nvme_ns_set_nsabp(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
{
NvmeIdNs *id_ns = &ns->id_ns;
NvmeIdCtrl *id_ctrl = &n->id_ctrl;
uint16_t awun = le16_to_cpu(id_ctrl->awun);
uint16_t awupf = le16_to_cpu(id_ctrl->awupf);
uint16_t nawun = ns->params.atomic.nawun;
uint16_t nawupf = ns->params.atomic.nawupf;
if (nawupf > nawun) {
if (nawun == 0) {
nawun = nawupf;
} else {
atomic->atomic_writes = 1;
error_setg(errp, "nawupf must be less than or equal to nawun");
return false;
}
}
/* neither nawun or nawupf is set */
if (nawun == 0) {
return true;
}
if (nawun < awun) {
error_setg(errp, "nawun must be greater than or equal to awun");
return false;
}
atomic->atomic_nabo = cpu_to_le16(id_ns->nabo);
if (nawupf < awupf) {
error_setg(errp, "nawupf must be greater than or equal to awupf");
return false;
}
id_ns->nsfeat |= NVME_ID_NS_NSFEAT_NSABP;
id_ns->nawun = cpu_to_le16(nawun);
id_ns->nawupf = cpu_to_le16(nawupf);
nvme_atomic_configure_max_write_size(n->dn, nawun, nawupf, &ns->atomic);
return true;
}
static void nvme_ns_realize(DeviceState *dev, Error **errp)
{
NvmeNamespace *ns = NVME_NS(dev);
BusState *s = qdev_get_parent_bus(dev);
NvmeCtrl *n = NVME(s->parent);
NvmeSubsystem *subsys = n->subsys;
uint32_t nsid = ns->params.nsid;
int i;
assert(subsys);
/* reparent to subsystem bus */
if (!qdev_set_parent_bus(dev, &subsys->bus.parent_bus, errp)) {
return;
@ -804,6 +838,14 @@ static void nvme_ns_realize(DeviceState *dev, Error **errp)
ns->subsys = subsys;
ns->endgrp = &subsys->endgrp;
if (!nvme_ns_set_nsabp(n, ns, errp)) {
return;
}
if (!nvme_ns_set_nab(n, ns, errp)) {
return;
}
if (nvme_ns_setup(ns, errp)) {
return;
}
@ -872,12 +914,11 @@ static const Property nvme_ns_props[] = {
DEFINE_PROP_BOOL("eui64-default", NvmeNamespace, params.eui64_default,
false),
DEFINE_PROP_STRING("fdp.ruhs", NvmeNamespace, params.fdp.ruhs),
DEFINE_PROP_UINT16("atomic.nawun", NvmeNamespace, params.atomic_nawun, 0),
DEFINE_PROP_UINT16("atomic.nawupf", NvmeNamespace, params.atomic_nawupf, 0),
DEFINE_PROP_UINT16("atomic.nabspf", NvmeNamespace, params.atomic_nabspf, 0),
DEFINE_PROP_UINT16("atomic.nabsn", NvmeNamespace, params.atomic_nabsn, 0),
DEFINE_PROP_UINT16("atomic.nabo", NvmeNamespace, params.atomic_nabo, 0),
DEFINE_PROP_BOOL("atomic.nsfeat", NvmeNamespace, params.atomic_nsfeat, 0),
DEFINE_PROP_UINT16("atomic.nawun", NvmeNamespace, params.atomic.nawun, 0),
DEFINE_PROP_UINT16("atomic.nawupf", NvmeNamespace, params.atomic.nawupf, 0),
DEFINE_PROP_UINT16("atomic.nabsn", NvmeNamespace, params.atomic.nabsn, 0),
DEFINE_PROP_UINT16("atomic.nabspf", NvmeNamespace, params.atomic.nabspf, 0),
DEFINE_PROP_UINT16("atomic.nabo", NvmeNamespace, params.atomic.nabo, 0),
};
static void nvme_ns_class_init(ObjectClass *oc, const void *data)

25
hw/nvme/nvme.h
View File

@ -218,12 +218,14 @@ typedef struct NvmeNamespaceParams {
struct {
char *ruhs;
} fdp;
uint16_t atomic_nawun;
uint16_t atomic_nawupf;
uint16_t atomic_nabsn;
uint16_t atomic_nabspf;
uint16_t atomic_nabo;
bool atomic_nsfeat;
struct {
uint16_t nawun;
uint16_t nawupf;
uint16_t nabsn;
uint16_t nabspf;
uint16_t nabo;
} atomic;
} NvmeNamespaceParams;
typedef struct NvmeAtomic {
@ -288,11 +290,7 @@ typedef struct NvmeNamespace {
/* reclaim unit handle identifiers indexed by placement handle */
uint16_t *phs;
} fdp;
uint16_t atomic_nawun;
uint16_t atomic_nawupf;
uint16_t atomic_nabsn;
uint16_t atomic_nabspf;
uint16_t atomic_nabo;
NvmeAtomic atomic;
} NvmeNamespace;
@ -742,4 +740,9 @@ void nvme_rw_complete_cb(void *opaque, int ret);
uint16_t nvme_map_dptr(NvmeCtrl *n, NvmeSg *sg, size_t len,
NvmeCmd *cmd);
void nvme_atomic_configure_max_write_size(bool dn, uint16_t awun,
uint16_t awupf, NvmeAtomic *atomic);
void nvme_ns_atomic_configure_boundary(bool dn, uint16_t nabsn,
uint16_t nabspf, NvmeAtomic *atomic);
#endif /* HW_NVME_NVME_H */

2
include/block/nvme.h
View File

@ -1589,7 +1589,7 @@ enum NvmeIdNsMc {
enum NvmeIdNsNsfeat {
NVME_ID_NS_NSFEAT_THINP = 1 << 0,
NVME_ID_NS_NSFEAT_NSABPNS = 1 << 1,
NVME_ID_NS_NSFEAT_NSABP = 1 << 1,
NVME_ID_NS_NSFEAT_DAE = 1 << 2,
NVME_ID_NS_NSFEAT_UIDREUSE = 1 << 3,
NVME_ID_NS_NSFEAT_OPTPERF_ALL = 3 << 4,

Write Preview
Loading…
Cancel
Save