diff mbox series

[v2,05/18] hw/block/nvme: Introduce the Namespace Types definitions

Message ID 20200617213415.22417-6-dmitry.fomichev@wdc.com (mailing list archive)
State New, archived
Headers show
Series hw/block/nvme: Support Namespace Types and Zoned Namespace Command Set | expand

Commit Message

Dmitry Fomichev June 17, 2020, 9:34 p.m. UTC
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(-)

Comments

Alistair Francis June 30, 2020, 2:12 a.m. UTC | #1
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?

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
>
>
Klaus Jensen June 30, 2020, 4:57 a.m. UTC | #2
On Jun 18 06:34, Dmitry Fomichev 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,

Maybe we could call this CSS_CSI, since it just specifies that one or
more command sets are supported, not that ALL namespace types are
supported.

Otherwise,
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

> +    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;
> +    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
> 
>
Niklas Cassel June 30, 2020, 10:02 a.m. UTC | #3
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
> >
> >
Niklas Cassel June 30, 2020, 4:04 p.m. UTC | #4
On Tue, Jun 30, 2020 at 06:57:16AM +0200, Klaus Jensen wrote:
> On Jun 18 06:34, Dmitry Fomichev 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,
> 
> Maybe we could call this CSS_CSI, since it just specifies that one or
> more command sets are supported, not that ALL namespace types are
> supported.

The enum name here is CcCss, so this represents CC.CSS,
which specifies which Command Sets to enable,
not which Command Sets that are supported.

(Supported Command Sets are defined by CAP.CSS and the
I/O Command Set data structure.)

So it indeed says to enable ALL command sets supported by the
controller. (Although for the CSI case, you need to check the
I/O Command Set data structure to know what is actually supported.)


However, I agree, the name CSS_ALL_NSTYPES is a bit misleading.
ALL_SUPPORTED_CSI would have been a more precise name.
However, simply naming it CSS_CSI, like you suggest, is more intuitive,
and is what we use in the Linux kernel patches, so let's use that :)


Kind regards,
Niklas

> 
> Otherwise,
> Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
> 
> > +    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;
> > +    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
> > 
> >
Keith Busch June 30, 2020, 5:02 p.m. UTC | #5
On Tue, Jun 30, 2020 at 10:02:15AM +0000, Niklas Cassel wrote:
> 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:
> > > +    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.

They shouldn't have shortened controller to "CNT". For those of us that
can't help but pronounce these as words, that is a vulgarity in English.
diff mbox series

Patch

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;
+    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);
 }