From: Niklas Cassel <Niklas.Cassel@wdc.com>
To: Alistair Francis <alistair23@gmail.com>
Cc: "Kevin Wolf" <kwolf@redhat.com>,
"Damien Le Moal" <Damien.LeMoal@wdc.com>,
Qemu-block <qemu-block@nongnu.org>,
"Dmitry Fomichev" <Dmitry.Fomichev@wdc.com>,
"qemu-devel@nongnu.org Developers" <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 05/18] hw/block/nvme: Introduce the Namespace Types definitions
Date: Tue, 30 Jun 2020 10:02:15 +0000 [thread overview]
Message-ID: <20200630100214.GA548602@localhost.localdomain> (raw)
In-Reply-To: <CAKmqyKO-O5kVkb-mKmBeCCtmaS8uR+0oau=5FfS_gYrXXX0nHA@mail.gmail.com>
On Mon, Jun 29, 2020 at 07:12:47PM -0700, Alistair Francis wrote:
> On Wed, Jun 17, 2020 at 2:47 PM Dmitry Fomichev <dmitry.fomichev@wdc.com> wrote:
> >
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> >
> > Define the structures and constants required to implement
> > Namespace Types support.
> >
> > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> > Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> > ---
> > hw/block/nvme.h | 3 ++
> > include/block/nvme.h | 75 +++++++++++++++++++++++++++++++++++++++++---
> > 2 files changed, 73 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> > index 4f0dac39ae..4fd155c409 100644
> > --- a/hw/block/nvme.h
> > +++ b/hw/block/nvme.h
> > @@ -63,6 +63,9 @@ typedef struct NvmeCQueue {
> >
> > typedef struct NvmeNamespace {
> > NvmeIdNs id_ns;
> > + uint32_t nsid;
> > + uint8_t csi;
> > + QemuUUID uuid;
> > } NvmeNamespace;
> >
> > static inline NvmeLBAF *nvme_ns_lbaf(NvmeNamespace *ns)
> > diff --git a/include/block/nvme.h b/include/block/nvme.h
> > index 6a58bac0c2..5a1e5e137c 100644
> > --- a/include/block/nvme.h
> > +++ b/include/block/nvme.h
> > @@ -50,6 +50,11 @@ enum NvmeCapMask {
> > CAP_PMR_MASK = 0x1,
> > };
> >
> > +enum NvmeCapCssBits {
> > + CAP_CSS_NVM = 0x01,
> > + CAP_CSS_CSI_SUPP = 0x40,
> > +};
> > +
> > #define NVME_CAP_MQES(cap) (((cap) >> CAP_MQES_SHIFT) & CAP_MQES_MASK)
> > #define NVME_CAP_CQR(cap) (((cap) >> CAP_CQR_SHIFT) & CAP_CQR_MASK)
> > #define NVME_CAP_AMS(cap) (((cap) >> CAP_AMS_SHIFT) & CAP_AMS_MASK)
> > @@ -101,6 +106,12 @@ enum NvmeCcMask {
> > CC_IOCQES_MASK = 0xf,
> > };
> >
> > +enum NvmeCcCss {
> > + CSS_NVM_ONLY = 0,
> > + CSS_ALL_NSTYPES = 6,
> > + CSS_ADMIN_ONLY = 7,
> > +};
> > +
> > #define NVME_CC_EN(cc) ((cc >> CC_EN_SHIFT) & CC_EN_MASK)
> > #define NVME_CC_CSS(cc) ((cc >> CC_CSS_SHIFT) & CC_CSS_MASK)
> > #define NVME_CC_MPS(cc) ((cc >> CC_MPS_SHIFT) & CC_MPS_MASK)
> > @@ -109,6 +120,21 @@ enum NvmeCcMask {
> > #define NVME_CC_IOSQES(cc) ((cc >> CC_IOSQES_SHIFT) & CC_IOSQES_MASK)
> > #define NVME_CC_IOCQES(cc) ((cc >> CC_IOCQES_SHIFT) & CC_IOCQES_MASK)
> >
> > +#define NVME_SET_CC_EN(cc, val) \
> > + (cc |= (uint32_t)((val) & CC_EN_MASK) << CC_EN_SHIFT)
> > +#define NVME_SET_CC_CSS(cc, val) \
> > + (cc |= (uint32_t)((val) & CC_CSS_MASK) << CC_CSS_SHIFT)
> > +#define NVME_SET_CC_MPS(cc, val) \
> > + (cc |= (uint32_t)((val) & CC_MPS_MASK) << CC_MPS_SHIFT)
> > +#define NVME_SET_CC_AMS(cc, val) \
> > + (cc |= (uint32_t)((val) & CC_AMS_MASK) << CC_AMS_SHIFT)
> > +#define NVME_SET_CC_SHN(cc, val) \
> > + (cc |= (uint32_t)((val) & CC_SHN_MASK) << CC_SHN_SHIFT)
> > +#define NVME_SET_CC_IOSQES(cc, val) \
> > + (cc |= (uint32_t)((val) & CC_IOSQES_MASK) << CC_IOSQES_SHIFT)
> > +#define NVME_SET_CC_IOCQES(cc, val) \
> > + (cc |= (uint32_t)((val) & CC_IOCQES_MASK) << CC_IOCQES_SHIFT)
> > +
> > enum NvmeCstsShift {
> > CSTS_RDY_SHIFT = 0,
> > CSTS_CFS_SHIFT = 1,
> > @@ -482,10 +508,41 @@ typedef struct NvmeIdentify {
> > uint64_t rsvd2[2];
> > uint64_t prp1;
> > uint64_t prp2;
> > - uint32_t cns;
> > - uint32_t rsvd11[5];
> > + uint8_t cns;
> > + uint8_t rsvd4;
> > + uint16_t ctrlid;
>
> Shouldn't this be CNTID?
From the NVMe spec:
https://nvmexpress.org/wp-content/uploads/NVM-Express-1_4-2019.06.10-Ratified.pdf
Figure 241:
Controller Identifier (CNTID)
So you are correct, this is the official abbreviation.
I guess that I tried wanted to keep it in sync with Linux:
https://github.com/torvalds/linux/blob/master/include/linux/nvme.h#L974
Which uses ctrlid.
Looking further at the NVMe spec:
In Figure 247 (Identify Controller Data Structure) they use other names
for fields:
Controller ID (CNTLID)
Controller Attributes (CTRATT)
I can understand if they want to have unique names for fields, but it
seems like they have trouble deciding how to abbreviate controller :)
Personally I think that ctrlid is more obvious that we are talking about
a controller and not a count. But I'm fine regardless.
Kind regards,
Niklas
>
> Alistair
>
> > + uint16_t nvmsetid;
> > + uint8_t rsvd3;
> > + uint8_t csi;
> > + uint32_t rsvd12[4];
> > } NvmeIdentify;
> >
> > +typedef struct NvmeNsIdDesc {
> > + uint8_t nidt;
> > + uint8_t nidl;
> > + uint16_t rsvd2;
> > +} NvmeNsIdDesc;
> > +
> > +enum NvmeNidType {
> > + NVME_NIDT_EUI64 = 0x01,
> > + NVME_NIDT_NGUID = 0x02,
> > + NVME_NIDT_UUID = 0x03,
> > + NVME_NIDT_CSI = 0x04,
> > +};
> > +
> > +enum NvmeNidLength {
> > + NVME_NIDL_EUI64 = 8,
> > + NVME_NIDL_NGUID = 16,
> > + NVME_NIDL_UUID = 16,
> > + NVME_NIDL_CSI = 1,
> > +};
> > +
> > +enum NvmeCsi {
> > + NVME_CSI_NVM = 0x00,
> > +};
> > +
> > +#define NVME_SET_CSI(vec, csi) (vec |= (uint8_t)(1 << (csi)))
> > +
> > typedef struct NvmeRwCmd {
> > uint8_t opcode;
> > uint8_t flags;
> > @@ -603,6 +660,7 @@ enum NvmeStatusCodes {
> > NVME_CMD_ABORT_MISSING_FUSE = 0x000a,
> > NVME_INVALID_NSID = 0x000b,
> > NVME_CMD_SEQ_ERROR = 0x000c,
> > + NVME_CMD_SET_CMB_REJECTED = 0x002b,
> > NVME_LBA_RANGE = 0x0080,
> > NVME_CAP_EXCEEDED = 0x0081,
> > NVME_NS_NOT_READY = 0x0082,
> > @@ -729,9 +787,14 @@ typedef struct NvmePSD {
> > #define NVME_IDENTIFY_DATA_SIZE 4096
> >
> > enum {
> > - NVME_ID_CNS_NS = 0x0,
> > - NVME_ID_CNS_CTRL = 0x1,
> > - NVME_ID_CNS_NS_ACTIVE_LIST = 0x2,
> > + NVME_ID_CNS_NS = 0x0,
> > + NVME_ID_CNS_CTRL = 0x1,
> > + NVME_ID_CNS_NS_ACTIVE_LIST = 0x2,
> > + NVME_ID_CNS_NS_DESC_LIST = 0x03,
> > + NVME_ID_CNS_CS_NS = 0x05,
> > + NVME_ID_CNS_CS_CTRL = 0x06,
> > + NVME_ID_CNS_CS_NS_ACTIVE_LIST = 0x07,
> > + NVME_ID_CNS_IO_COMMAND_SET = 0x1c,
> > };
> >
> > typedef struct NvmeIdCtrl {
> > @@ -825,6 +888,7 @@ enum NvmeFeatureIds {
> > NVME_WRITE_ATOMICITY = 0xa,
> > NVME_ASYNCHRONOUS_EVENT_CONF = 0xb,
> > NVME_TIMESTAMP = 0xe,
> > + NVME_COMMAND_SET_PROFILE = 0x19,
> > NVME_SOFTWARE_PROGRESS_MARKER = 0x80
> > };
> >
> > @@ -914,6 +978,7 @@ static inline void _nvme_check_size(void)
> > QEMU_BUILD_BUG_ON(sizeof(NvmeFwSlotInfoLog) != 512);
> > QEMU_BUILD_BUG_ON(sizeof(NvmeSmartLog) != 512);
> > QEMU_BUILD_BUG_ON(sizeof(NvmeIdCtrl) != 4096);
> > + QEMU_BUILD_BUG_ON(sizeof(NvmeNsIdDesc) != 4);
> > QEMU_BUILD_BUG_ON(sizeof(NvmeIdNs) != 4096);
> > QEMU_BUILD_BUG_ON(sizeof(NvmeEffectsLog) != 4096);
> > }
> > --
> > 2.21.0
> >
> >
next prev parent reply other threads:[~2020-06-30 14:23 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 [this message]
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
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=20200630100214.GA548602@localhost.localdomain \
--to=niklas.cassel@wdc.com \
--cc=Damien.LeMoal@wdc.com \
--cc=Dmitry.Fomichev@wdc.com \
--cc=Matias.Bjorling@wdc.com \
--cc=alistair23@gmail.com \
--cc=kbusch@kernel.org \
--cc=kwolf@redhat.com \
--cc=mlevitsky@redhat.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.