[PATCH 0/2]nvme-cli: id-ctrl supports NVMe 1.3 spec

[PATCH 0/2]nvme-cli: id-ctrl supports NVMe 1.3 spec

[Guan Junxiong]

NVMe 1.3 inctrodues new fields such as EDSTT, DSTO, FWUG, HCTMA, MNTMT, MXTMT, SANICAP
into the idenfity controller data structure. The KAS field is included in the previous 
NVMe 1.2.1 spec, but not itemized in the nvme-cli.Those two patch add support to show 
those fileds in the nvme id-ctrl command.



Guan Junxiong (2):
  nvme-cli: add fields into identify controller data structure
  nvme-cli: show more fields for id-ctrl

 linux/nvme.h | 10 ++++++++--
 nvme-print.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 2 deletions(-)

-- 
2.11.1

[PATCH 1/2] nvme-cli: add fields into identify controller data structure

[Guan Junxiong]

NVMe 1.3 inctrodues new fields such as EDSTT, DSTO, FWUG, HCTMA, MNTMT, MXTMT, SANICAP
into the idenfity controller data structure. So we add them. Note that KAS field is
included in the previous NVMe 1.2.1 spec, but not itemized in the nvme-cli.
---
 linux/nvme.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/linux/nvme.h b/linux/nvme.h
index 397c0f1..b2c8dbb 100644
--- a/linux/nvme.h
+++ b/linux/nvme.h
@@ -207,9 +207,15 @@ struct nvme_id_ctrl {
 	__u8			tnvmcap[16];
 	__u8			unvmcap[16];
 	__le32			rpmbs;
-	__u8			rsvd316[4];
+	__le16			edstt;
+	__u8			dsto;
+	__u8			fwug;
 	__le16			kas;
-	__u8			rsvd322[190];
+	__le16			hctma;
+	__le16			mntmt;
+	__le16			mxtmt;
+	__le32			sanicap;
+	__u8			rsvd332[180];
 	__u8			sqes;
 	__u8			cqes;
 	__le16			maxcmd;
-- 
2.11.1

[PATCH 2/2] nvme-cli: show more fields for id-ctrl

[Guan Junxiong]

NVMe 1.3 inctrodues new fields such as EDSTT, DSTO, FWUG, HCTMA, MNTMT,
MXTMT, SANICAP into the idenfity controller data structure. The id-ctrl
command should print them or write them into json format. Note that
although KAS field is included in the previous NVMe 1.2.1 spec, this
patch also supports this field.
---
 nvme-print.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/nvme-print.c b/nvme-print.c
index 60fe119..ffa63ad 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -198,6 +198,38 @@ static void show_nvme_id_ctrl_rpmbs(__le32 ctrl_rpmbs)
 	printf("\n");
 }
 
+static void show_nvme_id_ctrl_hctma(__le16 ctrl_hctma)
+{
+	__u16 hctma = le16_to_cpu(ctrl_hctma);
+	__u16 rsvd = (hctma & 0xFFFE) >> 1;
+	__u16 hctm = hctma & 0x1;
+
+	if (rsvd)
+		printf(" [15:1] : %#x\tReserved\n", rsvd);
+	printf("  [0:0] : %#x\tHost Controlled Thermal Management %sSupported\n",
+		hctm, hctm ? "" : "Not ");
+	printf("\n");
+}
+
+static void show_nvme_id_ctrl_sanicap(__le32 ctrl_sanicap)
+{
+	__u32 sanicap = le32_to_cpu(ctrl_sanicap);
+	__u32 rsvd = (sanicap & 0xFFFFFFF8) >> 3;
+	__u32 owr = (sanicap & 0x4) >> 2;
+	__u32 ber = (sanicap & 0x2) >> 1;
+	__u32 cer = sanicap & 0x1;
+
+	if (rsvd)
+		printf(" [31:3] : %#x\tReserved\n", rsvd);
+	printf("  [2:2] : %#x\tOverwrite Sanitize Operation %sSupported\n",
+		owr, owr ? "" : "Not ");
+	printf("  [1:1] : %#x\tBlock Erase Sanitize Operation %sSupported\n",
+		ber, ber ? "" : "Not ");
+	printf("  [0:0] : %#x\tCrypto Erase Sanitize Operation %sSupported\n",
+		cer, cer ? "" : "Not ");
+	printf("\n");
+}
+
 static void show_nvme_id_ctrl_sqes(__u8 sqes)
 {
 	__u8 msqes = (sqes & 0xF0) >> 4;
@@ -643,6 +675,18 @@ void __show_nvme_id_ctrl(struct nvme_id_ctrl *ctrl, unsigned int mode, void (*ve
 	printf("rpmbs   : %#x\n", le32_to_cpu(ctrl->rpmbs));
 	if (human)
 		show_nvme_id_ctrl_rpmbs(ctrl->rpmbs);
+	printf("edstt   : %d\n", le16_to_cpu(ctrl->edstt));
+	printf("dsto    : %d\n", ctrl->dsto);
+	printf("fwug    : %d\n", ctrl->fwug);
+	printf("kas     : %d\n", le16_to_cpu(ctrl->kas));
+	printf("hctma   : %#x\n", le16_to_cpu(ctrl->hctma));
+	if (human)
+		show_nvme_id_ctrl_hctma(ctrl->hctma);
+	printf("mntmt   : %d\n", le16_to_cpu(ctrl->mntmt));
+	printf("mxtmt   : %d\n", le16_to_cpu(ctrl->mxtmt));
+	printf("sanicap : %#x\n", le32_to_cpu(ctrl->sanicap));
+	if (human)
+		show_nvme_id_ctrl_sanicap(ctrl->sanicap);
 	printf("sqes    : %#x\n", ctrl->sqes);
 	if (human)
 		show_nvme_id_ctrl_sqes(ctrl->sqes);
@@ -1269,6 +1313,14 @@ void json_nvme_id_ctrl(struct nvme_id_ctrl *ctrl, unsigned int mode, void (*vs)(
 	json_object_add_value_float(root, "tnvmcap", tnvmcap);
 	json_object_add_value_float(root, "unvmcap", unvmcap);
 	json_object_add_value_int(root, "rpmbs", le32_to_cpu(ctrl->rpmbs));
+	json_object_add_value_int(root, "edstt", le16_to_cpu(ctrl->edstt));
+	json_object_add_value_int(root, "dsto", ctrl->dsto);
+	json_object_add_value_int(root, "fwug", ctrl->fwug);
+	json_object_add_value_int(root, "kas", le16_to_cpu(ctrl->kas));
+	json_object_add_value_int(root, "hctma", le16_to_cpu(ctrl->hctma));
+	json_object_add_value_int(root, "mntmt", le16_to_cpu(ctrl->mntmt));
+	json_object_add_value_int(root, "mxtmt", le16_to_cpu(ctrl->mxtmt));
+	json_object_add_value_int(root, "sanicap", le32_to_cpu(ctrl->sanicap));
 	json_object_add_value_int(root, "sqes", ctrl->sqes);
 	json_object_add_value_int(root, "cqes", ctrl->cqes);
 	json_object_add_value_int(root, "nn", le32_to_cpu(ctrl->nn));
-- 
2.11.1

[PATCH 1/2] nvme-cli: add fields into identify controller data structure

[Christoph Hellwig]

Please also add them to the kernel copy of the header so that they
stay in sync.  Except for that his looks fine to me:

Reviewed-by: Christoph Hellwig <hch at lst.de>

[PATCH 2/2] nvme-cli: show more fields for id-ctrl

[Christoph Hellwig]

Looks fine:

Reviewed-by: Christoph Hellwig <hch at lst.de>

[PATCH 1/2] nvme-cli: add fields into identify controller data structure

[guanjx09]

Hi,Christoph

On Friday, June 02, 2017 03:46 PM, Christoph Hellwig wrote:
> Please also add them to the kernel copy of the header so that they
> stay in sync.  Except for that his looks fine to me:
> 
> Reviewed-by: Christoph Hellwig <hch at lst.de>
> 

Here is the patch which adds those fields into nvme_id_ctrl structure
in the kernel copy of the header. Does it looks good?

Thanks for your review.
Best ,
Guan Junxiong

---
>From 5f6fa71d885e7cb64cdf5a5ef5e03cee5a5f6f52 Mon Sep 17 00:00:00 2001
From: Guan Junxiong <guanjunxiong@huawei.com>
Date: Fri, 2 Jun 2017 23:13:28 +0800
Subject: [PATCH] nvme: add fields into identify controller data structure

NVMe 1.3 inctrodues new fields such as EDSTT, DSTO, FWUG, HCTMA, MNTMT,
MXTMT, SANICAP into the idenfity controller data structure. So we add
them for later use.
---
 include/linux/nvme.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index b625bac..e5f09f8 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -207,9 +207,15 @@ struct nvme_id_ctrl {
 	__u8			tnvmcap[16];
 	__u8			unvmcap[16];
 	__le32			rpmbs;
-	__u8			rsvd316[4];
+	__le16			edstt;
+	__u8			dsto;
+	__u8			fwug;
 	__le16			kas;
-	__u8			rsvd322[190];
+	__le16			hctma;
+	__le16			mntmt;
+	__le16			mxtmt;
+	__le32			sanicap;
+	__u8			rsvd332[180];
 	__u8			sqes;
 	__u8			cqes;
 	__le16			maxcmd;
-- 
2.7.4

[PATCH 1/2] nvme-cli: add fields into identify controller data structure

[Christoph Hellwig]

On Fri, Jun 02, 2017@11:57:17PM +0800, ??? wrote:
> Hi, Christoph
> 
> On Friday, June 02, 2017 03:46 PM, Christoph Hellwig wrote:
> > Please also add them to the kernel copy of the header so that they
> > stay in sync.  Except for that his looks fine to me:
> > 
> 
> Here is the patch which adds those fields into nvme_id_ctrl structure
> in the kernel copy of the header. Does it looks good?

Thanks, this looks great.  Please send it out formally as a separate
mail and your signoff.

[PATCH 1/2] nvme-cli: add fields into identify controller data structure

[Christoph Hellwig]

On Sat, Jun 03, 2017@11:08:54AM +0800, guanjx09 wrote:
> Hi,Christoph
> 
> On Friday, June 02, 2017 03:46 PM, Christoph Hellwig wrote:
> > Please also add them to the kernel copy of the header so that they
> > stay in sync.  Except for that his looks fine to me:
> > 
> > Reviewed-by: Christoph Hellwig <hch at lst.de>
> > 
> 
> Here is the patch which adds those fields into nvme_id_ctrl structure
> in the kernel copy of the header. Does it looks good?

It does.  I'll apply it once the branch for Linux 4.13 is open.

[PATCH 1/2] nvme-cli: add fields into identify controller data structure

[Sagi Grimberg]

>> Here is the patch which adds those fields into nvme_id_ctrl structure
>> in the kernel copy of the header. Does it looks good?
> 
> Thanks, this looks great.  Please send it out formally as a separate
> mail and your signoff.

You can include my:

Reviewed-by: Sagi Grimberg <sagi at grimberg.me>

Cheers,

[PATCH 1/2] nvme-cli: add fields into identify controller data structure

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi at grimberg.me>

[PATCH 2/2] nvme-cli: show more fields for id-ctrl

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi at grimberg.me>

[PATCH 2/2] nvme-cli: show more fields for id-ctrl

[Keith Busch]

On Fri, Jun 02, 2017@03:18:19PM +0800, Guan Junxiong wrote:
> NVMe 1.3 inctrodues new fields such as EDSTT, DSTO, FWUG, HCTMA, MNTMT,
> MXTMT, SANICAP into the idenfity controller data structure. The id-ctrl
> command should print them or write them into json format. Note that
> although KAS field is included in the previous NVMe 1.2.1 spec, this
> patch also supports this field.

Applied.

For future patches, please add you're "Signed-off-by" for all patches.

[PATCH 1/2] nvme-cli: add fields into identify controller data structure

[Christoph Hellwig]

On Fri, Jun 02, 2017@10:25:11PM -0700, Christoph Hellwig wrote:
> On Fri, Jun 02, 2017@11:57:17PM +0800, ??? wrote:
> > Hi, Christoph
> > 
> > On Friday, June 02, 2017 03:46 PM, Christoph Hellwig wrote:
> > > Please also add them to the kernel copy of the header so that they
> > > stay in sync.  Except for that his looks fine to me:
> > > 
> > 
> > Here is the patch which adds those fields into nvme_id_ctrl structure
> > in the kernel copy of the header. Does it looks good?
> 
> Thanks, this looks great.  Please send it out formally as a separate
> mail and your signoff.

Can I get your Signed-off-by?  Just noticed that it didn't seem to
be provided anywhere when trying to apply it.

[PATCH 1/2] nvme-cli: add fields into identify controller data structure

[Guan Junxiong]

Hi, Christoph

On 2017/6/13 0:11, Christoph Hellwig wrote:
> On Fri, Jun 02, 2017@10:25:11PM -0700, Christoph Hellwig wrote:
>> On Fri, Jun 02, 2017@11:57:17PM +0800, ??? wrote:
>>> Hi, Christoph
>>>
>>> On Friday, June 02, 2017 03:46 PM, Christoph Hellwig wrote:
>>>> Please also add them to the kernel copy of the header so that they
>>>> stay in sync.  Except for that his looks fine to me:
>>>>
>>>
>>> Here is the patch which adds those fields into nvme_id_ctrl structure
>>> in the kernel copy of the header. Does it looks good?
>>
>> Thanks, this looks great.  Please send it out formally as a separate
>> mail and your signoff.
> 
> Can I get your Signed-off-by?  Just noticed that it didn't seem to
> be provided anywhere when trying to apply it.
> 
Sure. An updated patch was sent as a separate formal patch a few minutes ago.
Future patches will not be lack of my Signed-off-by.
Thanks

Cheers,
Guan
.

[PATCH 2/2] nvme-cli: show more fields for id-ctrl

[Guan Junxiong]

Hi,Keith

On 2017/6/12 22:30, Keith Busch wrote:
> On Fri, Jun 02, 2017@03:18:19PM +0800, Guan Junxiong wrote:
>> NVMe 1.3 inctrodues new fields such as EDSTT, DSTO, FWUG, HCTMA, MNTMT,
>> MXTMT, SANICAP into the idenfity controller data structure. The id-ctrl
>> command should print them or write them into json format. Note that
>> although KAS field is included in the previous NVMe 1.2.1 spec, this
>> patch also supports this field.
> 
> Applied.
> 
> For future patches, please add you're "Signed-off-by" for all patches.
> 

Thanks. And all future patches will include my "Signed-off-by".
.