All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Formatted LBA SIZE[FLBAS]
       [not found] <CGME20210803061146epcas5p4c8970e93f59edec1d2293f8c6b4d4cb9@epcas5p4.samsung.com>
@ 2021-08-03  5:29 ` Sathyavathi M
  2021-08-09 12:00   ` Niklas Cassel
  2021-08-09 14:48   ` Keith Busch
  0 siblings, 2 replies; 3+ messages in thread
From: Sathyavathi M @ 2021-08-03  5:29 UTC (permalink / raw)
  To: linux-nvme; +Cc: jg123.choi, kbusch, Sathyavathi M

The NLBAF(number of LBA formats) support in older spec was 16, but the new spec says the LBA support can be extended to 64.
The namespace data structure FLBAS(Formated LBA size) field bits have been changed to support more LBAF.

Signed-off-by: Sathyavathi M <sathya.m@samsung.com>
Reviewed-by: Jaegyu Choi <jg123.choi@samsung.com>

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 11779be..49cca79 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1683,6 +1683,9 @@ static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 static int nvme_configure_metadata(struct nvme_ns *ns, struct nvme_id_ns *id)
 {
 	struct nvme_ctrl *ctrl = ns->ctrl;
+	unsigned ls_lbaf = id->flbas & NVME_NS_FLBAS_LBA_LSMASK;
+	unsigned ms_lbaf = (id->flbas & NVME_NS_FLBAS_LBA_MSMASK) >> 1;
+	unsigned lbaf = ms_lbaf | ls_lbaf;
 
 	/*
 	 * The PI implementation requires the metadata size to be equal to the
@@ -1855,7 +1858,9 @@ static void nvme_set_chunk_sectors(struct nvme_ns *ns, struct nvme_id_ns *id)
 
 static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
 {
-	unsigned lbaf = id->flbas & NVME_NS_FLBAS_LBA_MASK;
+	unsigned ls_lbaf = id->flbas & NVME_NS_FLBAS_LBA_LSMASK;
+	unsigned ms_lbaf = (id->flbas & NVME_NS_FLBAS_LBA_MSMASK) >> 1;
+	unsigned lbaf = ms_lbaf | ls_lbaf;
 	int ret;
 
 	blk_mq_freeze_queue(ns->disk->queue);
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index b7c4c41..e64194a 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -376,7 +376,7 @@ struct nvme_id_ns {
 	__le16			endgid;
 	__u8			nguid[16];
 	__u8			eui64[8];
-	struct nvme_lbaf	lbaf[16];
+	struct nvme_lbaf	lbaf[64];
 	__u8			rsvd192[192];
 	__u8			vs[3712];
 };
@@ -395,7 +395,7 @@ struct nvme_id_ns_zns {
 	__le32			rrl;
 	__le32			frl;
 	__u8			rsvd20[2796];
-	struct nvme_zns_lbafe	lbafe[16];
+	struct nvme_zns_lbafe	lbafe[64];
 	__u8			rsvd3072[768];
 	__u8			vs[256];
 };
@@ -454,7 +454,8 @@ enum {
 	NVME_NS_FEAT_ATOMICS	= 1 << 1,
 	NVME_NS_FEAT_IO_OPT	= 1 << 4,
 	NVME_NS_ATTR_RO		= 1 << 0,
-	NVME_NS_FLBAS_LBA_MASK	= 0xf,
+	NVME_NS_FLBAS_LBA_LSMASK	= 0xf,
+	NVME_NS_FLBAS_LBA_MSMASK	= 0x60,
 	NVME_NS_FLBAS_META_EXT	= 0x10,
 	NVME_NS_NMIC_SHARED	= 1 << 0,
 	NVME_LBAF_RP_BEST	= 0,

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] Formatted LBA SIZE[FLBAS]
  2021-08-03  5:29 ` [PATCH] Formatted LBA SIZE[FLBAS] Sathyavathi M
@ 2021-08-09 12:00   ` Niklas Cassel
  2021-08-09 14:48   ` Keith Busch
  1 sibling, 0 replies; 3+ messages in thread
From: Niklas Cassel @ 2021-08-09 12:00 UTC (permalink / raw)
  To: Sathyavathi M; +Cc: linux-nvme, jg123.choi, kbusch

On Tue, Aug 03, 2021 at 10:59:12AM +0530, Sathyavathi M wrote:
> The NLBAF(number of LBA formats) support in older spec was 16, but the new spec says the LBA support can be extended to 64.
> The namespace data structure FLBAS(Formated LBA size) field bits have been changed to support more LBAF.
> 
> Signed-off-by: Sathyavathi M <sathya.m@samsung.com>
> Reviewed-by: Jaegyu Choi <jg123.choi@samsung.com>
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 11779be..49cca79 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1683,6 +1683,9 @@ static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
>  static int nvme_configure_metadata(struct nvme_ns *ns, struct nvme_id_ns *id)
>  {
>  	struct nvme_ctrl *ctrl = ns->ctrl;
> +	unsigned ls_lbaf = id->flbas & NVME_NS_FLBAS_LBA_LSMASK;
> +	unsigned ms_lbaf = (id->flbas & NVME_NS_FLBAS_LBA_MSMASK) >> 1;
> +	unsigned lbaf = ms_lbaf | ls_lbaf;
>  
>  	/*
>  	 * The PI implementation requires the metadata size to be equal to the
> @@ -1855,7 +1858,9 @@ static void nvme_set_chunk_sectors(struct nvme_ns *ns, struct nvme_id_ns *id)
>  
>  static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
>  {
> -	unsigned lbaf = id->flbas & NVME_NS_FLBAS_LBA_MASK;
> +	unsigned ls_lbaf = id->flbas & NVME_NS_FLBAS_LBA_LSMASK;
> +	unsigned ms_lbaf = (id->flbas & NVME_NS_FLBAS_LBA_MSMASK) >> 1;
> +	unsigned lbaf = ms_lbaf | ls_lbaf;
>  	int ret;
>  
>  	blk_mq_freeze_queue(ns->disk->queue);
> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
> index b7c4c41..e64194a 100644
> --- a/include/linux/nvme.h
> +++ b/include/linux/nvme.h
> @@ -376,7 +376,7 @@ struct nvme_id_ns {
>  	__le16			endgid;
>  	__u8			nguid[16];
>  	__u8			eui64[8];
> -	struct nvme_lbaf	lbaf[16];
> +	struct nvme_lbaf	lbaf[64];
>  	__u8			rsvd192[192];
>  	__u8			vs[3712];
>  };

Hello Sathyavathi,

Has this patch even been build tested?

I would expect the

BUILD_BUG_ON(sizeof(struct nvme_id_ns) != NVME_IDENTIFY_DATA_SIZE);
and
BUILD_BUG_ON(sizeof(struct nvme_id_ns_zns) != NVME_IDENTIFY_DATA_SIZE);

in drivers/nvme/host/core.c:_nvme_check_size() to fail with this patch.


Structs in NVMe usually have a fixed 4k size, therefore you cannot simply
add new fields without decreasing the number of reserved bytes accordingly.

In this specific case, the additional formats take up all the previously
reserved bytes, therefore you should have dropped the rsvd192 struct member
completely.

> @@ -395,7 +395,7 @@ struct nvme_id_ns_zns {
>  	__le32			rrl;
>  	__le32			frl;
>  	__u8			rsvd20[2796];
> -	struct nvme_zns_lbafe	lbafe[16];
> +	struct nvme_zns_lbafe	lbafe[64];
>  	__u8			rsvd3072[768];

Same thing here, rsvd3072 should be dropped completely.

Kind regards,
Niklas

>  	__u8			vs[256];
>  };
> @@ -454,7 +454,8 @@ enum {
>  	NVME_NS_FEAT_ATOMICS	= 1 << 1,
>  	NVME_NS_FEAT_IO_OPT	= 1 << 4,
>  	NVME_NS_ATTR_RO		= 1 << 0,
> -	NVME_NS_FLBAS_LBA_MASK	= 0xf,
> +	NVME_NS_FLBAS_LBA_LSMASK	= 0xf,
> +	NVME_NS_FLBAS_LBA_MSMASK	= 0x60,
>  	NVME_NS_FLBAS_META_EXT	= 0x10,
>  	NVME_NS_NMIC_SHARED	= 1 << 0,
>  	NVME_LBAF_RP_BEST	= 0,
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] Formatted LBA SIZE[FLBAS]
  2021-08-03  5:29 ` [PATCH] Formatted LBA SIZE[FLBAS] Sathyavathi M
  2021-08-09 12:00   ` Niklas Cassel
@ 2021-08-09 14:48   ` Keith Busch
  1 sibling, 0 replies; 3+ messages in thread
From: Keith Busch @ 2021-08-09 14:48 UTC (permalink / raw)
  To: Sathyavathi M; +Cc: linux-nvme, jg123.choi

On Tue, Aug 03, 2021 at 10:59:12AM +0530, Sathyavathi M wrote:
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 11779be..49cca79 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1683,6 +1683,9 @@ static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
>  static int nvme_configure_metadata(struct nvme_ns *ns, struct nvme_id_ns *id)
>  {
>  	struct nvme_ctrl *ctrl = ns->ctrl;
> +	unsigned ls_lbaf = id->flbas & NVME_NS_FLBAS_LBA_LSMASK;
> +	unsigned ms_lbaf = (id->flbas & NVME_NS_FLBAS_LBA_MSMASK) >> 1;
> +	unsigned lbaf = ms_lbaf | ls_lbaf;

I don't think you tested this. In addition to what Niklas mentioned, the
above is useless without informing the controller the host supports the
extended LBA format. You have to enable that in the Host Behavior
Support feature LBAFEE field.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-09 14:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210803061146epcas5p4c8970e93f59edec1d2293f8c6b4d4cb9@epcas5p4.samsung.com>
2021-08-03  5:29 ` [PATCH] Formatted LBA SIZE[FLBAS] Sathyavathi M
2021-08-09 12:00   ` Niklas Cassel
2021-08-09 14:48   ` Keith Busch

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.