| Message ID | 20200204095208.269131-6-k.jensen@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nvme: support NVMe v1.3d, SGLs and multiple namespaces | expand |
On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > Required for compliance with NVMe revision 1.2.1 or later. See NVM > Express 1.2.1, Section 5.11 ("Identify command"), Figure 90 and Section > 7.9 ("NVMe Qualified Names"). > > This also bumps the supported version to 1.2.1. > > Signed-off-by: Klaus Jensen <klaus.jensen@cnexlabs.com> > --- > hw/block/nvme.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index f05ebcce3f53..9abf74da20f2 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -9,9 +9,9 @@ > */ > > /** > - * Reference Specs: http://www.nvmexpress.org, 1.2, 1.1, 1.0e > + * Reference Specification: NVM Express 1.2.1 > * > - * http://www.nvmexpress.org/resources/ > + * https://nvmexpress.org/resources/specifications/ To be honest that redirects to https://nvmexpress.org/specifications/ Not a problem though. > */ > > /** > @@ -43,6 +43,8 @@ > #include "trace.h" > #include "nvme.h" > > +#define NVME_SPEC_VER 0x00010201 > + > #define NVME_GUEST_ERR(trace, fmt, ...) \ > do { \ > (trace_##trace)(__VA_ARGS__); \ > @@ -1365,6 +1367,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) > id->ieee[0] = 0x00; > id->ieee[1] = 0x02; > id->ieee[2] = 0xb3; > + id->ver = cpu_to_le32(NVME_SPEC_VER); This is indeed 1.2 addition > id->oacs = cpu_to_le16(0); > id->frmw = 7 << 1; > id->lpa = 1 << 0; > @@ -1372,6 +1375,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) > id->cqes = (0x4 << 4) | 0x4; > id->nn = cpu_to_le32(n->num_namespaces); > id->oncs = cpu_to_le16(NVME_ONCS_WRITE_ZEROS | NVME_ONCS_TIMESTAMP); > + > + strcpy((char *) id->subnqn, "nqn.2019-08.org.qemu:"); > + pstrcat((char *) id->subnqn, sizeof(id->subnqn), n->params.serial); Looks OK, this is first format according to the spec. > + > id->psd[0].mp = cpu_to_le16(0x9c4); > id->psd[0].enlat = cpu_to_le32(0x10); > id->psd[0].exlat = cpu_to_le32(0x4); > @@ -1386,7 +1393,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) > NVME_CAP_SET_CSS(n->bar.cap, 1); > NVME_CAP_SET_MPSMAX(n->bar.cap, 4); > > - n->bar.vs = 0x00010200; > + n->bar.vs = NVME_SPEC_VER; > n->bar.intmc = n->bar.intms = 0; > > if (n->params.cmb_size_mb) { To be really pedantic, the 1.2 spec also requires at least: * wctemp and cctemp to be nonzero in Identify Controller (yea, this is stupid to report temperature for virtual controller) * NVME_ADM_CMD_GET_LOG_PAGE, with some mandatory log pages * NVME_ADM_CMD_SET_FEATURES/NVME_ADM_CMD_GET_FEATURES - The device currently doesn't implement some mandatory features. And there are probably more. This is what I can recall from my nvme-mdev. However I see that you implmented these in following patches, so I suggest you first put patches that implement all that features, and then bump the NVME version. Most of these features I mentioned were mandatory even in version 1.0 of the spec, so current version is not even compliant with 1.0 IMHO. Best regards, Maxim Levitsky
diff --git a/hw/block/nvme.c b/hw/block/nvme.c index f05ebcce3f53..9abf74da20f2 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -9,9 +9,9 @@ */ /** - * Reference Specs: http://www.nvmexpress.org, 1.2, 1.1, 1.0e + * Reference Specification: NVM Express 1.2.1 * - * http://www.nvmexpress.org/resources/ + * https://nvmexpress.org/resources/specifications/ */ /** @@ -43,6 +43,8 @@ #include "trace.h" #include "nvme.h" +#define NVME_SPEC_VER 0x00010201 + #define NVME_GUEST_ERR(trace, fmt, ...) \ do { \ (trace_##trace)(__VA_ARGS__); \ @@ -1365,6 +1367,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) id->ieee[0] = 0x00; id->ieee[1] = 0x02; id->ieee[2] = 0xb3; + id->ver = cpu_to_le32(NVME_SPEC_VER); id->oacs = cpu_to_le16(0); id->frmw = 7 << 1; id->lpa = 1 << 0; @@ -1372,6 +1375,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) id->cqes = (0x4 << 4) | 0x4; id->nn = cpu_to_le32(n->num_namespaces); id->oncs = cpu_to_le16(NVME_ONCS_WRITE_ZEROS | NVME_ONCS_TIMESTAMP); + + strcpy((char *) id->subnqn, "nqn.2019-08.org.qemu:"); + pstrcat((char *) id->subnqn, sizeof(id->subnqn), n->params.serial); + id->psd[0].mp = cpu_to_le16(0x9c4); id->psd[0].enlat = cpu_to_le32(0x10); id->psd[0].exlat = cpu_to_le32(0x4); @@ -1386,7 +1393,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) NVME_CAP_SET_CSS(n->bar.cap, 1); NVME_CAP_SET_MPSMAX(n->bar.cap, 4); - n->bar.vs = 0x00010200; + n->bar.vs = NVME_SPEC_VER; n->bar.intmc = n->bar.intms = 0; if (n->params.cmb_size_mb) {
Required for compliance with NVMe revision 1.2.1 or later. See NVM Express 1.2.1, Section 5.11 ("Identify command"), Figure 90 and Section 7.9 ("NVMe Qualified Names"). This also bumps the supported version to 1.2.1. Signed-off-by: Klaus Jensen <klaus.jensen@cnexlabs.com> --- hw/block/nvme.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)