[PATCH] Formatted LBA SIZE[FLBAS]

[Niklas Cassel]

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 at samsung.com>
> Reviewed-by: Jaegyu Choi <jg123.choi at 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme