From: Klaus Jensen <its@irrelevant.dk>
To: Dmitry Fomichev <dmitry.fomichev@wdc.com>
Cc: "Kevin Wolf" <kwolf@redhat.com>,
"Niklas Cassel" <niklas.cassel@wdc.com>,
"Damien Le Moal" <damien.lemoal@wdc.com>,
qemu-block@nongnu.org, qemu-devel@nongnu.org,
"Keith Busch" <kbusch@kernel.org>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
"Maxim Levitsky" <mlevitsky@redhat.com>,
"Matias Bjorling" <matias.bjorling@wdc.com>
Subject: Re: [PATCH v2 07/18] hw/block/nvme: Add support for Namespace Types
Date: Tue, 30 Jun 2020 13:31:08 +0200 [thread overview]
Message-ID: <20200630113108.2g6jpa2pwfxumjqh@apples.localdomain> (raw)
In-Reply-To: <20200617213415.22417-8-dmitry.fomichev@wdc.com>
On Jun 18 06:34, Dmitry Fomichev wrote:
> From: Niklas Cassel <niklas.cassel@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.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> ---
> hw/block/nvme.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++--
> hw/block/nvme.h | 11 +++
> 2 files changed, 216 insertions(+), 5 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 03b8deee85..453f4747a5 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -686,6 +686,26 @@ static uint16_t nvme_identify_ctrl(NvmeCtrl *n, NvmeIdentify *c)
> prp1, prp2);
> }
>
> +static uint16_t nvme_identify_ctrl_csi(NvmeCtrl *n, NvmeIdentify *c)
> +{
> + uint64_t prp1 = le64_to_cpu(c->prp1);
> + uint64_t prp2 = le64_to_cpu(c->prp2);
> + static const int data_len = NVME_IDENTIFY_DATA_SIZE;
> + uint32_t *list;
> + uint16_t ret;
> +
> + trace_pci_nvme_identify_ctrl_csi(c->csi);
> +
> + if (c->csi == NVME_CSI_NVM) {
> + list = g_malloc0(data_len);
> + ret = nvme_dma_read_prp(n, (uint8_t *)list, data_len, prp1, prp2);
> + g_free(list);
> + return ret;
> + } else {
> + return NVME_INVALID_FIELD | NVME_DNR;
> + }
> +}
> +
> static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeIdentify *c)
> {
> NvmeNamespace *ns;
> @@ -701,11 +721,42 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeIdentify *c)
> }
>
> ns = &n->namespaces[nsid - 1];
> + assert(nsid == ns->nsid);
>
> return nvme_dma_read_prp(n, (uint8_t *)&ns->id_ns, sizeof(ns->id_ns),
> prp1, prp2);
> }
>
> +static uint16_t nvme_identify_ns_csi(NvmeCtrl *n, NvmeIdentify *c)
> +{
> + NvmeNamespace *ns;
> + uint32_t nsid = le32_to_cpu(c->nsid);
> + uint64_t prp1 = le64_to_cpu(c->prp1);
> + uint64_t prp2 = le64_to_cpu(c->prp2);
> + static const int data_len = NVME_IDENTIFY_DATA_SIZE;
> + uint32_t *list;
> + uint16_t ret;
> +
> + trace_pci_nvme_identify_ns_csi(nsid, c->csi);
> +
> + if (unlikely(nsid == 0 || nsid > n->num_namespaces)) {
> + trace_pci_nvme_err_invalid_ns(nsid, n->num_namespaces);
> + return NVME_INVALID_NSID | NVME_DNR;
> + }
> +
> + ns = &n->namespaces[nsid - 1];
> + assert(nsid == ns->nsid);
> +
> + if (c->csi == NVME_CSI_NVM) {
> + list = g_malloc0(data_len);
> + ret = nvme_dma_read_prp(n, (uint8_t *)list, data_len, prp1, prp2);
> + g_free(list);
> + return ret;
> + } else {
> + return NVME_INVALID_FIELD | NVME_DNR;
> + }
> +}
> +
> static uint16_t nvme_identify_nslist(NvmeCtrl *n, NvmeIdentify *c)
> {
> static const int data_len = NVME_IDENTIFY_DATA_SIZE;
> @@ -733,6 +784,99 @@ static uint16_t nvme_identify_nslist(NvmeCtrl *n, NvmeIdentify *c)
> return ret;
> }
>
> +static uint16_t nvme_identify_nslist_csi(NvmeCtrl *n, NvmeIdentify *c)
> +{
> + static const int data_len = NVME_IDENTIFY_DATA_SIZE;
> + uint32_t min_nsid = le32_to_cpu(c->nsid);
> + uint64_t prp1 = le64_to_cpu(c->prp1);
> + uint64_t prp2 = le64_to_cpu(c->prp2);
> + uint32_t *list;
> + uint16_t ret;
> + int i, j = 0;
> +
> + trace_pci_nvme_identify_nslist_csi(min_nsid, c->csi);
> +
> + if (c->csi != NVME_CSI_NVM) {
> + return NVME_INVALID_FIELD | NVME_DNR;
> + }
> +
> + list = g_malloc0(data_len);
> + for (i = 0; i < n->num_namespaces; i++) {
> + if (i < min_nsid) {
> + continue;
> + }
> + list[j++] = cpu_to_le32(i + 1);
> + if (j == data_len / sizeof(uint32_t)) {
> + break;
> + }
> + }
> + ret = nvme_dma_read_prp(n, (uint8_t *)list, data_len, prp1, prp2);
> + g_free(list);
> + return ret;
> +}
> +
> +static uint16_t nvme_list_ns_descriptors(NvmeCtrl *n, NvmeIdentify *c)
> +{
> + NvmeNamespace *ns;
> + uint32_t nsid = le32_to_cpu(c->nsid);
> + uint64_t prp1 = le64_to_cpu(c->prp1);
> + uint64_t prp2 = le64_to_cpu(c->prp2);
> + void *buf_ptr;
> + NvmeNsIdDesc *desc;
> + static const int data_len = NVME_IDENTIFY_DATA_SIZE;
> + uint8_t *buf;
> + uint16_t status;
> +
> + trace_pci_nvme_list_ns_descriptors();
> +
> + if (unlikely(nsid == 0 || nsid > n->num_namespaces)) {
> + trace_pci_nvme_err_invalid_ns(nsid, n->num_namespaces);
> + return NVME_INVALID_NSID | NVME_DNR;
> + }
> +
> + ns = &n->namespaces[nsid - 1];
> + assert(nsid == ns->nsid);
> +
> + buf = g_malloc0(data_len);
> + buf_ptr = buf;
> +
> + desc = buf_ptr;
> + desc->nidt = NVME_NIDT_UUID;
> + desc->nidl = NVME_NIDL_UUID;
> + buf_ptr += sizeof(*desc);
> + memcpy(buf_ptr, ns->uuid.data, NVME_NIDL_UUID);
> + buf_ptr += NVME_NIDL_UUID;
> +
> + desc = buf_ptr;
> + desc->nidt = NVME_NIDT_CSI;
> + desc->nidl = NVME_NIDL_CSI;
> + buf_ptr += sizeof(*desc);
> + *(uint8_t *)buf_ptr = NVME_CSI_NVM;
> +
> + status = nvme_dma_read_prp(n, buf, data_len, prp1, prp2);
> + g_free(buf);
> + return status;
> +}
> +
> +static uint16_t nvme_identify_cmd_set(NvmeCtrl *n, NvmeIdentify *c)
> +{
> + uint64_t prp1 = le64_to_cpu(c->prp1);
> + uint64_t prp2 = le64_to_cpu(c->prp2);
> + static const int data_len = NVME_IDENTIFY_DATA_SIZE;
> + uint32_t *list;
> + uint8_t *ptr;
> + uint16_t status;
> +
> + trace_pci_nvme_identify_cmd_set();
> +
> + list = g_malloc0(data_len);
> + ptr = (uint8_t *)list;
> + NVME_SET_CSI(*ptr, NVME_CSI_NVM);
> + status = nvme_dma_read_prp(n, (uint8_t *)list, data_len, prp1, prp2);
> + g_free(list);
> + return status;
> +}
> +
> static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
> {
> NvmeIdentify *c = (NvmeIdentify *)cmd;
> @@ -740,10 +884,20 @@ static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
> switch (le32_to_cpu(c->cns)) {
> case NVME_ID_CNS_NS:
> return nvme_identify_ns(n, c);
> + case NVME_ID_CNS_CS_NS:
> + return nvme_identify_ns_csi(n, c);
> case NVME_ID_CNS_CTRL:
> return nvme_identify_ctrl(n, c);
> + case NVME_ID_CNS_CS_CTRL:
> + return nvme_identify_ctrl_csi(n, c);
> case NVME_ID_CNS_NS_ACTIVE_LIST:
> return nvme_identify_nslist(n, c);
> + case NVME_ID_CNS_CS_NS_ACTIVE_LIST:
> + return nvme_identify_nslist_csi(n, c);
> + case NVME_ID_CNS_NS_DESC_LIST:
> + return nvme_list_ns_descriptors(n, c);
> + case NVME_ID_CNS_IO_COMMAND_SET:
> + return nvme_identify_cmd_set(n, c);
> default:
> trace_pci_nvme_err_invalid_identify_cns(le32_to_cpu(c->cns));
> return NVME_INVALID_FIELD | NVME_DNR;
> @@ -818,6 +972,9 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> break;
> case NVME_TIMESTAMP:
> return nvme_get_feature_timestamp(n, cmd);
> + case NVME_COMMAND_SET_PROFILE:
> + result = 0;
> + break;
> default:
> trace_pci_nvme_err_invalid_getfeat(dw10);
> return NVME_INVALID_FIELD | NVME_DNR;
> @@ -864,6 +1021,15 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> break;
> case NVME_TIMESTAMP:
> return nvme_set_feature_timestamp(n, cmd);
> + break;
> +
> + case NVME_COMMAND_SET_PROFILE:
> + if (dw11 & 0x1ff) {
> + trace_pci_nvme_err_invalid_iocsci(dw11 & 0x1ff);
> + return NVME_CMD_SET_CMB_REJECTED | NVME_DNR;
> + }
> + break;
> +
> default:
> trace_pci_nvme_err_invalid_setfeat(dw10);
> return NVME_INVALID_FIELD | NVME_DNR;
> @@ -1149,6 +1315,29 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
> break;
> case 0x14: /* CC */
> trace_pci_nvme_mmio_cfg(data & 0xffffffff);
> +
> + if (NVME_CC_CSS(data) != NVME_CC_CSS(n->bar.cc)) {
> + if (NVME_CC_EN(n->bar.cc)) {
> + NVME_GUEST_ERR(pci_nvme_err_change_css_when_enabled,
> + "changing selected command set when enabled");
> + break;
> + }
> + switch (NVME_CC_CSS(data)) {
> + case CSS_NVM_ONLY:
> + trace_pci_nvme_css_nvm_cset_selected_by_host(data & 0xffffffff);
> + break;
> + case CSS_ALL_NSTYPES:
> + NVME_SET_CC_CSS(n->bar.cc, CSS_ALL_NSTYPES);
> + trace_pci_nvme_css_all_csets_sel_by_host(data & 0xffffffff);
> + break;
> + case CSS_ADMIN_ONLY:
> + break;
> + default:
> + NVME_GUEST_ERR(pci_nvme_ub_unknown_css_value,
> + "unknown value in CC.CSS field");
> + }
> + }
> +
> /* Windows first sends data, then sends enable bit */
> if (!NVME_CC_EN(data) && !NVME_CC_EN(n->bar.cc) &&
> !NVME_CC_SHN(data) && !NVME_CC_SHN(n->bar.cc))
> @@ -1496,6 +1685,7 @@ static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
> {
> int64_t bs_size;
> NvmeIdNs *id_ns = &ns->id_ns;
> + int lba_index;
>
> bs_size = blk_getlength(n->conf.blk);
> if (bs_size < 0) {
> @@ -1505,7 +1695,10 @@ static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
>
> n->ns_size = bs_size;
>
> - id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
> + ns->csi = NVME_CSI_NVM;
> + qemu_uuid_generate(&ns->uuid); /* TODO make UUIDs persistent */
> + lba_index = NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas);
There is only one LBA format at this point anyway, so I don't think this
is needed.
> + id_ns->lbaf[lba_index].ds = nvme_ilog2(n->conf.logical_block_size);
Would be nice to have this in a separate patch.
> id_ns->nsze = cpu_to_le64(nvme_ns_nlbas(n, ns));
>
> /* no thin provisioning */
> @@ -1616,7 +1809,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
> id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
> id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
> strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
> - strpadcpy((char *)id->fr, sizeof(id->fr), "1.0", ' ');
> + strpadcpy((char *)id->fr, sizeof(id->fr), "2.0", ' ');
Out of curiosity, any specific reason for bumping the firmware revision?
> strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' ');
> id->rab = 6;
> id->ieee[0] = 0x00;
> @@ -1640,7 +1833,11 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
> NVME_CAP_SET_MQES(n->bar.cap, 0x7ff);
> NVME_CAP_SET_CQR(n->bar.cap, 1);
> NVME_CAP_SET_TO(n->bar.cap, 0xf);
> - NVME_CAP_SET_CSS(n->bar.cap, 1);
> + /*
> + * The driver now always supports NS Types, but all commands that
s/driver/device
> + * support CSI field will only handle NVM Command Set.
> + */
> + NVME_CAP_SET_CSS(n->bar.cap, (CAP_CSS_NVM | CAP_CSS_CSI_SUPP));
> NVME_CAP_SET_MPSMAX(n->bar.cap, 4);
>
> n->bar.vs = 0x00010200;
> @@ -1650,6 +1847,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
> static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> {
> NvmeCtrl *n = NVME(pci_dev);
> + NvmeNamespace *ns;
> Error *local_err = NULL;
>
> int i;
> @@ -1675,8 +1873,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
>
> nvme_init_ctrl(n, pci_dev);
>
> - for (i = 0; i < n->num_namespaces; i++) {
> - nvme_init_namespace(n, &n->namespaces[i], &local_err);
> + ns = n->namespaces;
> + for (i = 0; i < n->num_namespaces; i++, ns++) {
> + ns->nsid = i + 1;
n->num_namespaces is hardcoded to 1, so no real need for this change,
but cleanup is always nice I guess :)
> + nvme_init_namespace(n, ns, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> index 4fd155c409..0d29f75475 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -121,4 +121,15 @@ static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns)
> return n->ns_size >> nvme_ns_lbads(ns);
> }
>
> +static inline int nvme_ilog2(uint64_t i)
> +{
> + int log = -1;
> +
> + while (i) {
> + i >>= 1;
> + log++;
> + }
> + return log;
> +}
> +
> #endif /* HW_NVME_H */
> --
> 2.21.0
>
>
next prev parent reply other threads:[~2020-06-30 11:33 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-17 21:33 [PATCH v2 00/18] hw/block/nvme: Support Namespace Types and Zoned Namespace Command Set Dmitry Fomichev
2020-06-17 21:33 ` [PATCH v2 01/18] hw/block/nvme: Move NvmeRequest has_sg field to a bit flag Dmitry Fomichev
2020-06-30 0:56 ` Alistair Francis
2020-06-30 4:09 ` Klaus Jensen
2020-06-17 21:33 ` [PATCH v2 02/18] hw/block/nvme: Define 64 bit cqe.result Dmitry Fomichev
2020-06-30 0:58 ` Alistair Francis
2020-06-30 4:15 ` Klaus Jensen
2020-06-17 21:34 ` [PATCH v2 03/18] hw/block/nvme: Clean up unused AER definitions Dmitry Fomichev
2020-06-30 1:00 ` Alistair Francis
2020-06-30 4:40 ` Klaus Jensen
2020-06-17 21:34 ` [PATCH v2 04/18] hw/block/nvme: Add Commands Supported and Effects log Dmitry Fomichev
2020-06-30 1:35 ` Alistair Francis
2020-06-30 4:46 ` Klaus Jensen
2020-06-17 21:34 ` [PATCH v2 05/18] hw/block/nvme: Introduce the Namespace Types definitions Dmitry Fomichev
2020-06-30 2:12 ` Alistair Francis
2020-06-30 10:02 ` Niklas Cassel
2020-06-30 17:02 ` Keith Busch
2020-06-30 4:57 ` Klaus Jensen
2020-06-30 16:04 ` Niklas Cassel
2020-06-17 21:34 ` [PATCH v2 06/18] hw/block/nvme: Define trace events related to NS Types Dmitry Fomichev
2020-06-30 10:20 ` Klaus Jensen
2020-06-30 20:18 ` Alistair Francis
2020-06-17 21:34 ` [PATCH v2 07/18] hw/block/nvme: Add support for Namespace Types Dmitry Fomichev
2020-06-30 11:31 ` Klaus Jensen [this message]
2020-06-17 21:34 ` [PATCH v2 08/18] hw/block/nvme: Make Zoned NS Command Set definitions Dmitry Fomichev
2020-06-30 11:44 ` Klaus Jensen
2020-06-30 12:08 ` Klaus Jensen
2020-06-30 22:11 ` Alistair Francis
2020-06-17 21:34 ` [PATCH v2 09/18] hw/block/nvme: Define Zoned NS Command Set trace events Dmitry Fomichev
2020-06-30 12:14 ` Klaus Jensen
2020-06-17 21:34 ` [PATCH v2 10/18] hw/block/nvme: Support Zoned Namespace Command Set Dmitry Fomichev
2020-06-30 13:31 ` Klaus Jensen
2020-06-17 21:34 ` [PATCH v2 11/18] hw/block/nvme: Introduce max active and open zone limits Dmitry Fomichev
2020-07-01 0:26 ` Alistair Francis
2020-07-01 6:41 ` Klaus Jensen
2020-06-17 21:34 ` [PATCH v2 12/18] hw/block/nvme: Simulate Zone Active excursions Dmitry Fomichev
2020-07-01 0:30 ` Alistair Francis
2020-07-01 6:12 ` Klaus Jensen
2020-06-17 21:34 ` [PATCH v2 13/18] hw/block/nvme: Set Finish/Reset Zone Recommended attributes Dmitry Fomichev
2020-07-01 16:23 ` Klaus Jensen
2020-06-17 21:34 ` [PATCH v2 14/18] hw/block/nvme: Generate zone AENs Dmitry Fomichev
2020-07-01 11:44 ` Klaus Jensen
2020-06-17 21:34 ` [PATCH v2 15/18] hw/block/nvme: Support Zone Descriptor Extensions Dmitry Fomichev
2020-07-01 16:32 ` Klaus Jensen
2020-06-17 21:34 ` [PATCH v2 16/18] hw/block/nvme: Add injection of Offline/Read-Only zones Dmitry Fomichev
2020-06-17 21:34 ` [PATCH v2 17/18] hw/block/nvme: Use zone metadata file for persistence Dmitry Fomichev
2020-07-01 17:26 ` Klaus Jensen
2020-06-17 21:34 ` [PATCH v2 18/18] hw/block/nvme: Document zoned parameters in usage text Dmitry Fomichev
2020-06-29 20:26 ` [PATCH v2 00/18] hw/block/nvme: Support Namespace Types and Zoned Namespace Command Set Dmitry Fomichev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200630113108.2g6jpa2pwfxumjqh@apples.localdomain \
--to=its@irrelevant.dk \
--cc=damien.lemoal@wdc.com \
--cc=dmitry.fomichev@wdc.com \
--cc=kbusch@kernel.org \
--cc=kwolf@redhat.com \
--cc=matias.bjorling@wdc.com \
--cc=mlevitsky@redhat.com \
--cc=niklas.cassel@wdc.com \
--cc=philmd@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.