On Sep 28 11:35, Dmitry Fomichev wrote: > From: Niklas Cassel <niklas.cas...@wdc.com> > > Namespace Types introduce a new command set, "I/O Command Sets", > that allows the host to retrieve the command sets associated with > a namespace. Introduce support for the command set and enable > detection for the NVM Command Set. > > The new workflows for identify commands rely heavily on zero-filled > identify structs. E.g., certain CNS commands are defined to return > a zero-filled identify struct when an inactive namespace NSID > is supplied. > > Add a helper function in order to avoid code duplication when > reporting zero-filled identify structures. > > Signed-off-by: Niklas Cassel <niklas.cas...@wdc.com> > Signed-off-by: Dmitry Fomichev <dmitry.fomic...@wdc.com> > --- > hw/block/nvme-ns.c | 3 + > hw/block/nvme.c | 210 +++++++++++++++++++++++++++++++++++++-------- > 2 files changed, 175 insertions(+), 38 deletions(-) > > diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c > index bbd7879492..31b7f986c3 100644 > --- a/hw/block/nvme-ns.c > +++ b/hw/block/nvme-ns.c
The following looks like a rebase gone wrong. There are some redundant checks and wrong return values. > static uint16_t nvme_identify_ns_descr_list(NvmeCtrl *n, NvmeRequest *req) > { > NvmeIdentify *c = (NvmeIdentify *)&req->cmd; > + NvmeNamespace *ns; > uint32_t nsid = le32_to_cpu(c->nsid); > - uint8_t list[NVME_IDENTIFY_DATA_SIZE]; > - > - struct data { > - struct { > - NvmeIdNsDescr hdr; > - uint8_t v[16]; > - } uuid; > - }; > - > - struct data *ns_descrs = (struct data *)list; > + NvmeIdNsDescr *desc; > + uint8_t list[NVME_IDENTIFY_DATA_SIZE] = {}; > + static const int data_len = sizeof(list); > + void *list_ptr = list; Oh maaan, please do not replace my nicely cleaned up code with pointer arithmetics :( > > trace_pci_nvme_identify_ns_descr_list(nsid); > > - if (!nvme_nsid_valid(n, nsid) || nsid == NVME_NSID_BROADCAST) { > - return NVME_INVALID_NSID | NVME_DNR; > - } > - This removal looks wrong. > if (unlikely(!nvme_ns(n, nsid))) { > return NVME_INVALID_FIELD | NVME_DNR; > } > > - memset(list, 0x0, sizeof(list)); > + ns = nvme_ns(n, nsid); > + if (unlikely(!ns)) { > + return nvme_rpt_empty_id_struct(n, req); > + } > And this doesnt look like it belongs (its checked just a few lines before, and it returns an error status as it should). > /* > * Because the NGUID and EUI64 fields are 0 in the Identify Namespace > data > @@ -1597,12 +1667,31 @@ static uint16_t nvme_identify_ns_descr_list(NvmeCtrl > *n, NvmeRequest *req) > * Namespace Identification Descriptor. Add a very basic Namespace UUID > * here. > */ > - ns_descrs->uuid.hdr.nidt = NVME_NIDT_UUID; > - ns_descrs->uuid.hdr.nidl = NVME_NIDL_UUID; > - stl_be_p(&ns_descrs->uuid.v, nsid); > + desc = list_ptr; > + desc->nidt = NVME_NIDT_UUID; > + desc->nidl = NVME_NIDL_UUID; > + list_ptr += sizeof(*desc); > + memcpy(list_ptr, ns->params.uuid.data, NVME_NIDL_UUID); > + list_ptr += NVME_NIDL_UUID; > > - return nvme_dma(n, list, NVME_IDENTIFY_DATA_SIZE, > - DMA_DIRECTION_FROM_DEVICE, req); > + desc = list_ptr; > + desc->nidt = NVME_NIDT_CSI; > + desc->nidl = NVME_NIDL_CSI; > + list_ptr += sizeof(*desc); > + *(uint8_t *)list_ptr = NVME_CSI_NVM; > + > + return nvme_dma(n, list, data_len, DMA_DIRECTION_FROM_DEVICE, req); > +} > +
signature.asc
Description: PGP signature