On Mar 25 12:37, Maxim Levitsky wrote: > On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > > From: Klaus Jensen <k.jen...@samsung.com> > > > > Add missing fields in the Identify Controller and Identify Namespace > > data structures to bring them in line with NVMe v1.3. > > > > This also adds data structures and defines for SGL support which > > requires a couple of trivial changes to the nvme block driver as well. > > > > Signed-off-by: Klaus Jensen <k.jen...@samsung.com> > > Acked-by: Fam Zheng <f...@euphon.net> > > --- > > block/nvme.c | 18 ++--- > > hw/block/nvme.c | 12 ++-- > > include/block/nvme.h | 153 ++++++++++++++++++++++++++++++++++++++----- > > 3 files changed, 151 insertions(+), 32 deletions(-) > > > > diff --git a/block/nvme.c b/block/nvme.c > > index d41c4bda6e39..99b9bb3dac96 100644 > > --- a/block/nvme.c > > +++ b/block/nvme.c > > @@ -589,6 +675,16 @@ enum NvmeIdCtrlOncs { > > #define NVME_CTRL_CQES_MIN(cqes) ((cqes) & 0xf) > > #define NVME_CTRL_CQES_MAX(cqes) (((cqes) >> 4) & 0xf) > > > > +#define NVME_CTRL_SGLS_SUPPORTED_MASK (0x3 << 0) > > +#define NVME_CTRL_SGLS_SUPPORTED_NO_ALIGNMENT (0x1 << 0) > > +#define NVME_CTRL_SGLS_SUPPORTED_DWORD_ALIGNMENT (0x1 << 1) > > +#define NVME_CTRL_SGLS_KEYED (0x1 << 2) > > +#define NVME_CTRL_SGLS_BITBUCKET (0x1 << 16) > > +#define NVME_CTRL_SGLS_MPTR_CONTIGUOUS (0x1 << 17) > > +#define NVME_CTRL_SGLS_EXCESS_LENGTH (0x1 << 18) > > +#define NVME_CTRL_SGLS_MPTR_SGL (0x1 << 19) > > +#define NVME_CTRL_SGLS_ADDR_OFFSET (0x1 << 20) > OK > > + > > typedef struct NvmeFeatureVal { > > uint32_t arbitration; > > uint32_t power_mgmt; > > @@ -611,6 +707,10 @@ typedef struct NvmeFeatureVal { > > #define NVME_INTC_THR(intc) (intc & 0xff) > > #define NVME_INTC_TIME(intc) ((intc >> 8) & 0xff) > > > > +#define NVME_TEMP_THSEL(temp) ((temp >> 20) & 0x3) > Nitpick: If we are adding this, I'll add a #define for the values as well >
Done. And used in the subsequent "nvme: add temperature threshold feature" patch. > > +#define NVME_TEMP_TMPSEL(temp) ((temp >> 16) & 0xf) > > +#define NVME_TEMP_TMPTH(temp) ((temp >> 0) & 0xffff) > > + > > enum NvmeFeatureIds { > > NVME_ARBITRATION = 0x1, > > NVME_POWER_MANAGEMENT = 0x2, > > @@ -653,18 +753,37 @@ typedef struct NvmeIdNs { > > uint8_t mc; > > uint8_t dpc; > > uint8_t dps; > > - > > uint8_t nmic; > > uint8_t rescap; > > uint8_t fpi; > > uint8_t dlfeat; > > - > > - uint8_t res34[94]; > > + uint16_t nawun; > > + uint16_t nawupf; > > + uint16_t nacwu; > > + uint16_t nabsn; > > + uint16_t nabo; > > + uint16_t nabspf; > > + uint16_t noiob; > > + uint8_t nvmcap[16]; > > + uint8_t rsvd64[40]; > > + uint8_t nguid[16]; > > + uint64_t eui64; > > NvmeLBAF lbaf[16]; > > - uint8_t res192[192]; > > + uint8_t rsvd192[192]; > > uint8_t vs[3712]; > > } NvmeIdNs; > Also checked this against V5, looks OK now > > > > > +typedef struct NvmeIdNsDescr { > > + uint8_t nidt; > > + uint8_t nidl; > > + uint8_t rsvd2[2]; > > +} NvmeIdNsDescr; > OK > > > > > + > > +#define NVME_NIDT_UUID_LEN 16 > > + > > +enum { > > + NVME_NIDT_UUID = 0x3, > Very minor nitpick: I'll would add others as well just for the sake > of better understanding what this is > Done. > > +}; > > > > /*Deallocate Logical Block Features*/ > > #define NVME_ID_NS_DLFEAT_GUARD_CRC(dlfeat) ((dlfeat) & 0x10) > > Looks very good. > > Reviewed-by: Maxim Levitsky <mlevi...@redhat.com> > > Best regards, > Maxim Levitsky >