Re: [PATCH v5 04/26] nvme: add missing fields in the identify data structures

2020-02-12 Thread Maxim Levitsky
On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote:
> Not used by the device model but added for completeness. See NVM Express
> 1.2.1, Section 5.11 ("Identify command"), Figure 90 and Figure 93.
> 
> Signed-off-by: Klaus Jensen 
> ---
>  include/block/nvme.h | 48 
>  1 file changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index 8fb941c6537c..d2f65e8fe496 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -543,7 +543,13 @@ typedef struct NvmeIdCtrl {
>  uint8_t ieee[3];
>  uint8_t cmic;
>  uint8_t mdts;
> -uint8_t rsvd255[178];
> +uint16_tcntlid;
> +uint32_tver;
> +uint32_trtd3r;
> +uint32_trtd3e;
> +uint32_toaes;
> +uint32_tctratt;
> +uint8_t rsvd100[156];
>  uint16_toacs;
>  uint8_t acl;
>  uint8_t aerl;
> @@ -551,10 +557,22 @@ typedef struct NvmeIdCtrl {
>  uint8_t lpa;
>  uint8_t elpe;
>  uint8_t npss;
> -uint8_t rsvd511[248];
> +uint8_t avscc;
> +uint8_t apsta;
> +uint16_twctemp;
> +uint16_tcctemp;
> +uint16_tmtfa;
> +uint32_thmpre;
> +uint32_thmmin;
> +uint8_t tnvmcap[16];
> +uint8_t unvmcap[16];
> +uint32_trpmbs;
> +uint8_t rsvd316[4];
> +uint16_tkas;
> +uint8_t rsvd322[190];
>  uint8_t sqes;
>  uint8_t cqes;
> -uint16_trsvd515;
> +uint16_tmaxcmd;
>  uint32_tnn;
>  uint16_toncs;
>  uint16_tfuses;
> @@ -562,8 +580,14 @@ typedef struct NvmeIdCtrl {
>  uint8_t vwc;
>  uint16_tawun;
>  uint16_tawupf;
> -uint8_t rsvd703[174];
> -uint8_t rsvd2047[1344];
> +uint8_t nvscc;
> +uint8_t rsvd531;
> +uint16_tacwu;
> +uint8_t rsvd534[2];
> +uint32_tsgls;
> +uint8_t rsvd540[228];
> +uint8_t subnqn[256];
> +uint8_t rsvd1024[1024];
>  NvmePSD psd[32];
>  uint8_t vs[1024];
>  } NvmeIdCtrl;
> @@ -653,13 +677,21 @@ 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];
> +uint8_t rsvd33;
This is wrong. nawun comes right after dlfeat
> +uint16_tnawun;
> +uint16_tnawupf;
And here the error cancels out since here there should be 'nacwu' field.
> +uint16_tnabsn;
> +uint16_tnabo;
> +uint16_tnabspf;
> +uint8_t rsvd46[2];
> +uint8_t nvmcap[16];
> +uint8_t rsvd64[40];
> +uint8_t nguid[16];
> +uint64_teui64;
>  NvmeLBAFlbaf[16];
>  uint8_t res192[192];
Not related to the patch, but maybe rename this to rsvd192 for the sake of 
consistency?
>  uint8_t vs[3712];


I reviewed this patch by cross referencing the nvme structures as defined in 
the kernel,
and the spec.

I prefer to merge this patch with all other spec updates you do in following 
patches,
to bring nvme.h up to date to 1.3d,
so that it will be easier to review this and remove some noise from other 
patches.

Best regards,
Maxim Levitsky




[PATCH v5 04/26] nvme: add missing fields in the identify data structures

2020-02-04 Thread Klaus Jensen
Not used by the device model but added for completeness. See NVM Express
1.2.1, Section 5.11 ("Identify command"), Figure 90 and Figure 93.

Signed-off-by: Klaus Jensen 
---
 include/block/nvme.h | 48 
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/include/block/nvme.h b/include/block/nvme.h
index 8fb941c6537c..d2f65e8fe496 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -543,7 +543,13 @@ typedef struct NvmeIdCtrl {
 uint8_t ieee[3];
 uint8_t cmic;
 uint8_t mdts;
-uint8_t rsvd255[178];
+uint16_tcntlid;
+uint32_tver;
+uint32_trtd3r;
+uint32_trtd3e;
+uint32_toaes;
+uint32_tctratt;
+uint8_t rsvd100[156];
 uint16_toacs;
 uint8_t acl;
 uint8_t aerl;
@@ -551,10 +557,22 @@ typedef struct NvmeIdCtrl {
 uint8_t lpa;
 uint8_t elpe;
 uint8_t npss;
-uint8_t rsvd511[248];
+uint8_t avscc;
+uint8_t apsta;
+uint16_twctemp;
+uint16_tcctemp;
+uint16_tmtfa;
+uint32_thmpre;
+uint32_thmmin;
+uint8_t tnvmcap[16];
+uint8_t unvmcap[16];
+uint32_trpmbs;
+uint8_t rsvd316[4];
+uint16_tkas;
+uint8_t rsvd322[190];
 uint8_t sqes;
 uint8_t cqes;
-uint16_trsvd515;
+uint16_tmaxcmd;
 uint32_tnn;
 uint16_toncs;
 uint16_tfuses;
@@ -562,8 +580,14 @@ typedef struct NvmeIdCtrl {
 uint8_t vwc;
 uint16_tawun;
 uint16_tawupf;
-uint8_t rsvd703[174];
-uint8_t rsvd2047[1344];
+uint8_t nvscc;
+uint8_t rsvd531;
+uint16_tacwu;
+uint8_t rsvd534[2];
+uint32_tsgls;
+uint8_t rsvd540[228];
+uint8_t subnqn[256];
+uint8_t rsvd1024[1024];
 NvmePSD psd[32];
 uint8_t vs[1024];
 } NvmeIdCtrl;
@@ -653,13 +677,21 @@ 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];
+uint8_t rsvd33;
+uint16_tnawun;
+uint16_tnawupf;
+uint16_tnabsn;
+uint16_tnabo;
+uint16_tnabspf;
+uint8_t rsvd46[2];
+uint8_t nvmcap[16];
+uint8_t rsvd64[40];
+uint8_t nguid[16];
+uint64_teui64;
 NvmeLBAFlbaf[16];
 uint8_t res192[192];
 uint8_t vs[3712];
-- 
2.25.0