[PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

[PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

[Steven Seungcheol Lee]

lbaf, lbafe used on nvme driver
Base spec:
    NVMe-NVM-Command-Set-Specification-1.0a-2021.07.26-Ratified
    NVMe-Zoned-Namespace-Command-Set-Specification-1.1a-2021.07.26-Ratified

Signed-off-by: Steven Seungcheol Lee <sc108.lee@@gmail.com>
---
 include/linux/nvme.h | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index b7c4c4130b65..c11eda4be426 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -368,7 +368,10 @@ struct nvme_id_ns {
  __le16 npdg;
  __le16 npda;
  __le16 nows;
- __u8 rsvd74[18];
+ __le16 mssrl;
+ __le32 mcl;
+ __u8 msrc;
+ __u8 rsvd81[11];
  __le32 anagrpid;
  __u8 rsvd96[3];
  __u8 nsattr;
@@ -376,8 +379,7 @@ struct nvme_id_ns {
  __le16 endgid;
  __u8 nguid[16];
  __u8 eui64[8];
- struct nvme_lbaf lbaf[16];
- __u8 rsvd192[192];
+ struct nvme_lbaf lbaf[64];
  __u8 vs[3712];
 };

@@ -387,16 +389,21 @@ struct nvme_zns_lbafe {
  __u8 rsvd9[7];
 };

-struct nvme_id_ns_zns {
+struct nvme_zns_id_ns {
  __le16 zoc;
  __le16 ozcs;
  __le32 mar;
  __le32 mor;
  __le32 rrl;
  __le32 frl;
- __u8 rsvd20[2796];
- struct nvme_zns_lbafe lbafe[16];
- __u8 rsvd3072[768];
+ __le32 rrl1;
+ __le32 rrl2;
+ __le32 rrl3;
+ __le32 frl1;
+ __le32 frl2;
+ __le32 frl3;
+ __u8 rsvd44[2772];
+ struct nvme_zns_lbafe lbafe[64];
  __u8 vs[256];
 };

--
2.17.0.windows.1

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

[Niklas Cassel]

On Sun, Sep 26, 2021 at 01:34:10PM +0900, Steven Seungcheol Lee wrote:
> lbaf, lbafe used on nvme driver
> Base spec:
>     NVMe-NVM-Command-Set-Specification-1.0a-2021.07.26-Ratified
>     NVMe-Zoned-Namespace-Command-Set-Specification-1.1a-2021.07.26-Ratified

Please read:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes

You should explain what the patch changes, and why the patch is necessary.

> 
> Signed-off-by: Steven Seungcheol Lee <sc108.lee@@gmail.com>
> ---
>  include/linux/nvme.h | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
> index b7c4c4130b65..c11eda4be426 100644
> --- a/include/linux/nvme.h
> +++ b/include/linux/nvme.h
> @@ -368,7 +368,10 @@ struct nvme_id_ns {
>   __le16 npdg;
>   __le16 npda;
>   __le16 nows;
> - __u8 rsvd74[18];
> + __le16 mssrl;
> + __le32 mcl;
> + __u8 msrc;
> + __u8 rsvd81[11];
>   __le32 anagrpid;
>   __u8 rsvd96[3];
>   __u8 nsattr;
> @@ -376,8 +379,7 @@ struct nvme_id_ns {
>   __le16 endgid;
>   __u8 nguid[16];
>   __u8 eui64[8];
> - struct nvme_lbaf lbaf[16];
> - __u8 rsvd192[192];
> + struct nvme_lbaf lbaf[64];
>   __u8 vs[3712];
>  };
> 
> @@ -387,16 +389,21 @@ struct nvme_zns_lbafe {
>   __u8 rsvd9[7];
>  };
> 
> -struct nvme_id_ns_zns {
> +struct nvme_zns_id_ns {

You cannot simply rename the struct like this.

You didn't provide any reason for renaming it in the changelog..

..and even if you provided a really good reason for renaming it,
you didn't change the variables that uses this struct, so patch
should not compile.

>   __le16 zoc;
>   __le16 ozcs;
>   __le32 mar;
>   __le32 mor;
>   __le32 rrl;
>   __le32 frl;
> - __u8 rsvd20[2796];
> - struct nvme_zns_lbafe lbafe[16];
> - __u8 rsvd3072[768];
> + __le32 rrl1;
> + __le32 rrl2;
> + __le32 rrl3;
> + __le32 frl1;
> + __le32 frl2;
> + __le32 frl3;
> + __u8 rsvd44[2772];
> + struct nvme_zns_lbafe lbafe[64];
>   __u8 vs[256];
>  };
> 
> --
> 2.17.0.windows.1
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

[Chaitanya Kulkarni]

On 9/25/21 9:34 PM, Steven Seungcheol Lee wrote:
> lbaf, lbafe used on nvme driver
> Base spec:
>      NVMe-NVM-Command-Set-Specification-1.0a-2021.07.26-Ratified
>      NVMe-Zoned-Namespace-Command-Set-Specification-1.1a-2021.07.26-Ratified
> 
> Signed-off-by: Steven Seungcheol Lee <sc108.lee@@gmail.com>


In its current state Nackd.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

[Keith Busch]

On Sun, Sep 26, 2021 at 01:34:10PM +0900, Steven Seungcheol Lee wrote:
> lbaf, lbafe used on nvme driver
> Base spec:
>     NVMe-NVM-Command-Set-Specification-1.0a-2021.07.26-Ratified
>     NVMe-Zoned-Namespace-Command-Set-Specification-1.1a-2021.07.26-Ratified

In order to ease the driver maintenance burden, we typically don't
update spec fields unless the driver has a real use for them. If this
patch is part of a larger series that eventually does make use of these
new formats, please send this update when that happens.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

[Steven Seungcheol Lee]

@Keith Busch
As I mentioned on commit, Its already been used from kernel source.

@Niklas Cassel
Thanks for the chekcing details, I made mistake for the structure name
since I brought those changes from nvme-cli.

Added more details on commit message about both of you guy's question.


=========== modified commit below=============
modification for supporting more LBA format from the device
if the nvme device does support over 16 LBA format,
current nvme driver will occur error.

lbaf, lbafe currently used on nvme driver
lbaf:
    function nvme_configure_metadata
    function nvme_update_ns_info
    function nvmet_passthru_override_id_ns

lbafe:
    function nvme_update_zone_info

The structure chages based on spec:
    NVMe-NVM-Command-Set-Specification-1.0a-2021.07.26-Ratified
    NVMe-Zoned-Namespace-Command-Set-Specification-1.1a-2021.07.26-Ratified

Signed-off-by: Steven Seungcheol Lee <sc108.lee@gmail.com>
---
 include/linux/nvme.h | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index b7c4c4130b65..2d03c7322ac9 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -368,7 +368,10 @@ struct nvme_id_ns {
  __le16 npdg;
  __le16 npda;
  __le16 nows;
- __u8 rsvd74[18];
+ __le16 mssrl;
+ __le32 mcl;
+ __u8 msrc;
+ __u8 rsvd81[11];
  __le32 anagrpid;
  __u8 rsvd96[3];
  __u8 nsattr;
@@ -376,8 +379,7 @@ struct nvme_id_ns {
  __le16 endgid;
  __u8 nguid[16];
  __u8 eui64[8];
- struct nvme_lbaf lbaf[16];
- __u8 rsvd192[192];
+ struct nvme_lbaf lbaf[64];
  __u8 vs[3712];
 };

@@ -394,9 +396,14 @@ struct nvme_id_ns_zns {
  __le32 mor;
  __le32 rrl;
  __le32 frl;
- __u8 rsvd20[2796];
- struct nvme_zns_lbafe lbafe[16];
- __u8 rsvd3072[768];
+ __le32 rrl1;
+ __le32 rrl2;
+ __le32 rrl3;
+ __le32 frl1;
+ __le32 frl2;
+ __le32 frl3;
+ __u8 rsvd44[2772];
+ struct nvme_zns_lbafe lbafe[64];
  __u8 vs[256];
 };

--
2.25.1

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

[Keith Busch]

On Tue, Sep 28, 2021 at 11:19:40AM +0900, Steven Seungcheol Lee wrote:
> @Keith Busch
> As I mentioned on commit, Its already been used from kernel source.

You are introducing new things with his patch. How is the kernel already
using them before they exist?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

[Steven Seungcheol Lee]

I only mean lbaf, lbafe field.
other than lbaf, lbafe fields are not used.
I thought It would be nice to add other fields as well, since the
structure will be changed with lbaf, lbafe.
should I comment about other fields instead of just mentioning spec ?


>
> On Tue, Sep 28, 2021 at 11:19:40AM +0900, Steven Seungcheol Lee wrote:
> > @Keith Busch
> > As I mentioned on commit, Its already been used from kernel source.
>
> You are introducing new things with his patch. How is the kernel already
> using them before they exist?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

[Keith Busch]

On Tue, Sep 28, 2021 at 11:51:30AM +0900, Steven Seungcheol Lee wrote:
> I only mean lbaf, lbafe field.
> other than lbaf, lbafe fields are not used.
> I thought It would be nice to add other fields as well, since the
> structure will be changed with lbaf, lbafe.
> should I comment about other fields instead of just mentioning spec ?

The driver currently can't access the new elements, though, so what's
the point of defining them? If you want to enable these formats, then
this change needs to be part of a larger patch that properly enables the
feature.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

[Steven Seungcheol Lee]

That means leave other elements and fix lbf only?

The changes for lbaf, lbafe happens.
so I thought same structure should be updated as same version of spec.

If not recommanded, I wll change as below.

id_ns
mssrl, mcl, msrc => remove
rsvd area -> lbaf[64]

id_zns
rrl1~3, frl1~3 => remove
rsvd area -> lbafe[64]

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme.h: expend lbaf on nvme_id_ns, lbafe on nvme_zns_id_ns

[Steven Seungcheol Lee]

Sorry guys misunderstood, I just realized that changes need other
functionality that you guys mention.
Thanks for the chekcing :)
Just ignore this patch ~

2021년 9월 28일 (화) 오후 12:44, Steven Seungcheol Lee <sc108.lee@gmail.com>님이 작성:
>
> That means leave other elements and fix lbf only?
>
> The changes for lbaf, lbafe happens.
> so I thought same structure should be updated as same version of spec.
>
> If not recommanded, I wll change as below.
>
> id_ns
> mssrl, mcl, msrc => remove
> rsvd area -> lbaf[64]
>
> id_zns
> rrl1~3, frl1~3 => remove
> rsvd area -> lbafe[64]

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme