1. Patchew
  2. linux
  3. [PATCH V2 00/20] Add CXL LSA 2.1 format support in nvdimm and cxl pmem
  4. View patch

[PATCH V2 03/20] nvdimm/namespace_label: Add namespace label changes as per CXL LSA v2.1

Neeraj Kumar posted 20 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH V2 03/20] nvdimm/namespace_label: Add namespace label changes as per CXL LSA v2.1
Posted by Neeraj Kumar 2 months, 1 week ago 
CXL 3.2 Spec mentions CXL LSA 2.1 Namespace Labels at section 9.13.2.5
Modified __pmem_label_update function using setter functions to update
namespace label as per CXL LSA 2.1

Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
---
 drivers/nvdimm/label.c |  3 +++
 drivers/nvdimm/nd.h    | 27 +++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
index 75bc11da4c11..3f8a6bdb77c7 100644
--- a/drivers/nvdimm/label.c
+++ b/drivers/nvdimm/label.c
@@ -933,6 +933,7 @@ static int __pmem_label_update(struct nd_region *nd_region,
 	memset(lsa_label, 0, sizeof_namespace_label(ndd));
 
 	nd_label = &lsa_label->ns_label;
+	nsl_set_type(ndd, nd_label);
 	nsl_set_uuid(ndd, nd_label, nspm->uuid);
 	nsl_set_name(ndd, nd_label, nspm->alt_name);
 	nsl_set_flags(ndd, nd_label, flags);
@@ -944,7 +945,9 @@ static int __pmem_label_update(struct nd_region *nd_region,
 	nsl_set_lbasize(ndd, nd_label, nspm->lbasize);
 	nsl_set_dpa(ndd, nd_label, res->start);
 	nsl_set_slot(ndd, nd_label, slot);
+	nsl_set_alignment(ndd, nd_label, 0);
 	nsl_set_type_guid(ndd, nd_label, &nd_set->type_guid);
+	nsl_set_region_uuid(ndd, nd_label, NULL);
 	nsl_set_claim_class(ndd, nd_label, ndns->claim_class);
 	nsl_calculate_checksum(ndd, nd_label);
 	nd_dbg_dpa(nd_region, ndd, res, "\n");
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index 61348dee687d..651847f1bbf9 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -295,6 +295,33 @@ static inline const u8 *nsl_uuid_raw(struct nvdimm_drvdata *ndd,
 	return nd_label->efi.uuid;
 }
 
+static inline void nsl_set_type(struct nvdimm_drvdata *ndd,
+				struct nd_namespace_label *ns_label)
+{
+	uuid_t tmp;
+
+	if (ndd->cxl) {
+		uuid_parse(CXL_NAMESPACE_UUID, &tmp);
+		export_uuid(ns_label->cxl.type, &tmp);
+	}
+}
+
+static inline void nsl_set_alignment(struct nvdimm_drvdata *ndd,
+				     struct nd_namespace_label *ns_label,
+				     u32 align)
+{
+	if (ndd->cxl)
+		ns_label->cxl.align = __cpu_to_le16(align);
+}
+
+static inline void nsl_set_region_uuid(struct nvdimm_drvdata *ndd,
+				       struct nd_namespace_label *ns_label,
+				       const uuid_t *uuid)
+{
+	if (ndd->cxl)
+		export_uuid(ns_label->cxl.region_uuid, uuid);
+}
+
 bool nsl_validate_type_guid(struct nvdimm_drvdata *ndd,
 			    struct nd_namespace_label *nd_label, guid_t *guid);
 enum nvdimm_claim_class nsl_get_claim_class(struct nvdimm_drvdata *ndd,
-- 
2.34.1
Re: [PATCH V2 03/20] nvdimm/namespace_label: Add namespace label changes as per CXL LSA v2.1
Posted by Ira Weiny 1 month, 2 weeks ago 
Neeraj Kumar wrote:
> CXL 3.2 Spec mentions CXL LSA 2.1 Namespace Labels at section 9.13.2.5
> Modified __pmem_label_update function using setter functions to update
> namespace label as per CXL LSA 2.1
> 
> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>

[snip]

>  
> +static inline void nsl_set_type(struct nvdimm_drvdata *ndd,
> +				struct nd_namespace_label *ns_label)
> +{
> +	uuid_t tmp;
> +
> +	if (ndd->cxl) {
> +		uuid_parse(CXL_NAMESPACE_UUID, &tmp);
> +		export_uuid(ns_label->cxl.type, &tmp);

One more thing why can't uuid_parse put the UUID directly into type?

I think this is done at least 1 other place.

Ira


[snip]
Re: [PATCH V2 03/20] nvdimm/namespace_label: Add namespace label changes as per CXL LSA v2.1
Posted by Neeraj Kumar 1 month ago 
On 19/08/25 02:36PM, Ira Weiny wrote:
>Neeraj Kumar wrote:
>> CXL 3.2 Spec mentions CXL LSA 2.1 Namespace Labels at section 9.13.2.5
>> Modified __pmem_label_update function using setter functions to update
>> namespace label as per CXL LSA 2.1
>>
>> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
>
>[snip]
>
>>
>> +static inline void nsl_set_type(struct nvdimm_drvdata *ndd,
>> +				struct nd_namespace_label *ns_label)
>> +{
>> +	uuid_t tmp;
>> +
>> +	if (ndd->cxl) {
>> +		uuid_parse(CXL_NAMESPACE_UUID, &tmp);
>> +		export_uuid(ns_label->cxl.type, &tmp);
>
>One more thing why can't uuid_parse put the UUID directly into type?

Thanks Ira for your suggestion. I have used because of uuid_parse() and
export_uuid() signature difference.
But yes we can only use uuid_parse() using typecasting. I will modify it
in next patch-set.

>
>I think this is done at least 1 other place.
>
>Ira

Sure, I will fix it at the other place as well.


Regards,
Neeraj
Re: [PATCH V2 03/20] nvdimm/namespace_label: Add namespace label changes as per CXL LSA v2.1
Posted by Ira Weiny 1 month, 2 weeks ago 
Neeraj Kumar wrote:
> CXL 3.2 Spec mentions CXL LSA 2.1 Namespace Labels at section 9.13.2.5
> Modified __pmem_label_update function using setter functions to update
> namespace label as per CXL LSA 2.1

But why?  And didn't we just remove nd_namespace_label in patch 2?

Why are we now defining accessor functions for it?

> 
> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
> ---
>  drivers/nvdimm/label.c |  3 +++
>  drivers/nvdimm/nd.h    | 27 +++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
> index 75bc11da4c11..3f8a6bdb77c7 100644
> --- a/drivers/nvdimm/label.c
> +++ b/drivers/nvdimm/label.c
> @@ -933,6 +933,7 @@ static int __pmem_label_update(struct nd_region *nd_region,
>  	memset(lsa_label, 0, sizeof_namespace_label(ndd));
>  
>  	nd_label = &lsa_label->ns_label;
> +	nsl_set_type(ndd, nd_label);
>  	nsl_set_uuid(ndd, nd_label, nspm->uuid);
>  	nsl_set_name(ndd, nd_label, nspm->alt_name);
>  	nsl_set_flags(ndd, nd_label, flags);
> @@ -944,7 +945,9 @@ static int __pmem_label_update(struct nd_region *nd_region,
>  	nsl_set_lbasize(ndd, nd_label, nspm->lbasize);
>  	nsl_set_dpa(ndd, nd_label, res->start);
>  	nsl_set_slot(ndd, nd_label, slot);
> +	nsl_set_alignment(ndd, nd_label, 0);
>  	nsl_set_type_guid(ndd, nd_label, &nd_set->type_guid);
> +	nsl_set_region_uuid(ndd, nd_label, NULL);
>  	nsl_set_claim_class(ndd, nd_label, ndns->claim_class);
>  	nsl_calculate_checksum(ndd, nd_label);
>  	nd_dbg_dpa(nd_region, ndd, res, "\n");
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index 61348dee687d..651847f1bbf9 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -295,6 +295,33 @@ static inline const u8 *nsl_uuid_raw(struct nvdimm_drvdata *ndd,
>  	return nd_label->efi.uuid;
>  }
>  
> +static inline void nsl_set_type(struct nvdimm_drvdata *ndd,
> +				struct nd_namespace_label *ns_label)

Set type to what?

Why is driver data passed here?

This reads as an accessor function for some sort of label class but seems
to do some back checking into ndd to set the uuid of the label?

At a minimum this should be *_set_uuid(..., uuid_t uuid)  But I'm not
following this chunk of changes so don't just change it without more
explaination.

> +{
> +	uuid_t tmp;
> +
> +	if (ndd->cxl) {
> +		uuid_parse(CXL_NAMESPACE_UUID, &tmp);
> +		export_uuid(ns_label->cxl.type, &tmp);
> +	}
> +}
> +
> +static inline void nsl_set_alignment(struct nvdimm_drvdata *ndd,
> +				     struct nd_namespace_label *ns_label,
> +				     u32 align)

Why is this needed?

> +{
> +	if (ndd->cxl)
> +		ns_label->cxl.align = __cpu_to_le16(align);
> +}
> +
> +static inline void nsl_set_region_uuid(struct nvdimm_drvdata *ndd,
> +				       struct nd_namespace_label *ns_label,
> +				       const uuid_t *uuid)

Again why?

> +{
> +	if (ndd->cxl)
> +		export_uuid(ns_label->cxl.region_uuid, uuid);

export does a memcpy() and you are passing it NULL.  Is that safe?

Ira

[snip]
Re: [PATCH V2 03/20] nvdimm/namespace_label: Add namespace label changes as per CXL LSA v2.1
Posted by Neeraj Kumar 1 month ago 
On 19/08/25 10:57AM, Ira Weiny wrote:
>Neeraj Kumar wrote:
>> CXL 3.2 Spec mentions CXL LSA 2.1 Namespace Labels at section 9.13.2.5
>> Modified __pmem_label_update function using setter functions to update
>> namespace label as per CXL LSA 2.1
>
>But why?  And didn't we just remove nd_namespace_label in patch 2?
>
>Why are we now defining accessor functions for it?
>

Hi Ira,

No we haven't removed nd_namespace_label in patch 2. In patch 2, we have
introduced
lsa_label which contains nd_namespace_label as well as cxl_region_label
under union.

Actually, LSA 2.1 spec introduced new region label along with existing
(v1.1 & v1.2) namespace label
But in v2.1 namespace label members are also modified compared to v1.1 &
v1.2.

New members introduced in namespace label are following

	struct nvdimm_cxl_label {
		u8 type[NSLABEL_UUID_LEN];	--> Filling it with nsl_set_type()
		u8 uuid[NSLABEL_UUID_LEN];
		u8 name[NSLABEL_NAME_LEN];
		__le32 flags;
		__le16 nrange;
		__le16 position;
		__le64 dpa;
		__le64 rawsize;
		__le32 slot;
		__le32 align;			--> Filling it with
		nsl_set_alignment()
		u8 region_uuid[16];		--> Filling it with
		nsl_set_region_uuid()
		u8 abstraction_uuid[16];
		__le16 lbasize;
		u8 reserved[0x56];
		__le64 checksum;
	};

Therefore this patch-set address this modification in namespace label as per v2.1

>>
>> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
>> ---
>>  drivers/nvdimm/label.c |  3 +++
>>  drivers/nvdimm/nd.h    | 27 +++++++++++++++++++++++++++
>>  2 files changed, 30 insertions(+)
>>
>> diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
>> index 75bc11da4c11..3f8a6bdb77c7 100644
>> --- a/drivers/nvdimm/label.c
>> +++ b/drivers/nvdimm/label.c
>> @@ -933,6 +933,7 @@ static int __pmem_label_update(struct nd_region *nd_region,
>>  	memset(lsa_label, 0, sizeof_namespace_label(ndd));
>>
>>  	nd_label = &lsa_label->ns_label;
>> +	nsl_set_type(ndd, nd_label);
>>  	nsl_set_uuid(ndd, nd_label, nspm->uuid);
>>  	nsl_set_name(ndd, nd_label, nspm->alt_name);
>>  	nsl_set_flags(ndd, nd_label, flags);
>> @@ -944,7 +945,9 @@ static int __pmem_label_update(struct nd_region *nd_region,
>>  	nsl_set_lbasize(ndd, nd_label, nspm->lbasize);
>>  	nsl_set_dpa(ndd, nd_label, res->start);
>>  	nsl_set_slot(ndd, nd_label, slot);
>> +	nsl_set_alignment(ndd, nd_label, 0);
>>  	nsl_set_type_guid(ndd, nd_label, &nd_set->type_guid);
>> +	nsl_set_region_uuid(ndd, nd_label, NULL);
>>  	nsl_set_claim_class(ndd, nd_label, ndns->claim_class);
>>  	nsl_calculate_checksum(ndd, nd_label);
>>  	nd_dbg_dpa(nd_region, ndd, res, "\n");
>> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
>> index 61348dee687d..651847f1bbf9 100644
>> --- a/drivers/nvdimm/nd.h
>> +++ b/drivers/nvdimm/nd.h
>> @@ -295,6 +295,33 @@ static inline const u8 *nsl_uuid_raw(struct nvdimm_drvdata *ndd,
>>  	return nd_label->efi.uuid;
>>  }
>>
>> +static inline void nsl_set_type(struct nvdimm_drvdata *ndd,
>> +				struct nd_namespace_label *ns_label)
>
>Set type to what?
>

LSA 2.1 spec mentions seperate UUID for namespace label and region
label.

#define CXL_REGION_UUID "529d7c61-da07-47c4-a93f-ecdf2c06f444"
#define CXL_NAMESPACE_UUID "68bb2c0a-5a77-4937-9f85-3caf41a0f93c"
Here we are setting label->type with CXL_NAMESPACE_UUID

In following patch, accordingly we are setting region label->type with
CXL_REGION_UUID

May be I will rename it to nsl_set_type_uuid() in next patch-set.

>Why is driver data passed here?

ndd->cxl is used to segregate between EFI labels (v1.1 & v1.2) and CXL
Labels (v2.1). It was introduced in 5af96835e4daf

>
>This reads as an accessor function for some sort of label class but seems
>to do some back checking into ndd to set the uuid of the label?
>
>At a minimum this should be *_set_uuid(..., uuid_t uuid)  But I'm not
>following this chunk of changes so don't just change it without more
>explaination.

I have created setter function taking inspiration from other members
setter helpers introduced in 8176f14789125

>
>> +{
>> +	uuid_t tmp;
>> +
>> +	if (ndd->cxl) {
>> +		uuid_parse(CXL_NAMESPACE_UUID, &tmp);
>> +		export_uuid(ns_label->cxl.type, &tmp);
>> +	}
>> +}
>> +
>> +static inline void nsl_set_alignment(struct nvdimm_drvdata *ndd,
>> +				     struct nd_namespace_label *ns_label,
>> +				     u32 align)
>
>Why is this needed?

As per CXL spec 3.2 Table - 9-11. Namespace Label Layout
The desired region alignment in multiples of 256 MB:
• 0 = No desired alignment
• 1 = 256-MB desired alignment
• 2 = 512-MB desired alignment
• etc.

In this patch-set we are using 0.

>
>> +{
>> +	if (ndd->cxl)
>> +		ns_label->cxl.align = __cpu_to_le16(align);
>> +}
>> +
>> +static inline void nsl_set_region_uuid(struct nvdimm_drvdata *ndd,
>> +				       struct nd_namespace_label *ns_label,
>> +				       const uuid_t *uuid)
>
>Again why?

This field is used to track namespace label associated with perticular
region label. It stores the region label's UUID

>
>> +{
>> +	if (ndd->cxl)
>> +		export_uuid(ns_label->cxl.region_uuid, uuid);
>
>export does a memcpy() and you are passing it NULL.  Is that safe?
>
>Ira

Thanks Ira for pointing this, Yes it will not be safe with NULL.
Sure I will fix this in next patch-set.


Regards,
Neeraj
Re: [PATCH V2 03/20] nvdimm/namespace_label: Add namespace label changes as per CXL LSA v2.1
Posted by Jonathan Cameron 1 month, 3 weeks ago 
On Wed, 30 Jul 2025 17:41:52 +0530
Neeraj Kumar <s.neeraj@samsung.com> wrote:

> CXL 3.2 Spec mentions CXL LSA 2.1 Namespace Labels at section 9.13.2.5
> Modified __pmem_label_update function using setter functions to update
> namespace label as per CXL LSA 2.1
> 
> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>

> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index 61348dee687d..651847f1bbf9 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -295,6 +295,33 @@ static inline const u8 *nsl_uuid_raw(struct nvdimm_drvdata *ndd,

> +static inline void nsl_set_alignment(struct nvdimm_drvdata *ndd,
> +				     struct nd_namespace_label *ns_label,
> +				     u32 align)
> +{
> +	if (ndd->cxl)
> +		ns_label->cxl.align = __cpu_to_le16(align);

The bot caught this one, it should be __cpu_to_le32(align);

> +}
Re: [PATCH V2 03/20] nvdimm/namespace_label: Add namespace label changes as per CXL LSA v2.1
Posted by Neeraj Kumar 1 month ago 
On 13/08/25 02:27PM, Jonathan Cameron wrote:
>On Wed, 30 Jul 2025 17:41:52 +0530
>Neeraj Kumar <s.neeraj@samsung.com> wrote:
>
>> CXL 3.2 Spec mentions CXL LSA 2.1 Namespace Labels at section 9.13.2.5
>> Modified __pmem_label_update function using setter functions to update
>> namespace label as per CXL LSA 2.1
>>
>> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
>
>> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
>> index 61348dee687d..651847f1bbf9 100644
>> --- a/drivers/nvdimm/nd.h
>> +++ b/drivers/nvdimm/nd.h
>> @@ -295,6 +295,33 @@ static inline const u8 *nsl_uuid_raw(struct nvdimm_drvdata *ndd,
>
>> +static inline void nsl_set_alignment(struct nvdimm_drvdata *ndd,
>> +				     struct nd_namespace_label *ns_label,
>> +				     u32 align)
>> +{
>> +	if (ndd->cxl)
>> +		ns_label->cxl.align = __cpu_to_le16(align);
>
>The bot caught this one, it should be __cpu_to_le32(align);
>

Yes Jonathan, I will fix this in next patch-set

Regards,
Neeraj
Re: [PATCH V2 03/20] nvdimm/namespace_label: Add namespace label changes as per CXL LSA v2.1
Posted by kernel test robot 2 months ago 
Hi Neeraj,

kernel test robot noticed the following build warnings:

[auto build test WARNING on f11a5f89910a7ae970fbce4fdc02d86a8ba8570f]

url:    https://github.com/intel-lab-lkp/linux/commits/Neeraj-Kumar/nvdimm-label-Introduce-NDD_CXL_LABEL-flag-to-set-cxl-label-format/20250730-202209
base:   f11a5f89910a7ae970fbce4fdc02d86a8ba8570f
patch link:    https://lore.kernel.org/r/20250730121209.303202-4-s.neeraj%40samsung.com
patch subject: [PATCH V2 03/20] nvdimm/namespace_label: Add namespace label changes as per CXL LSA v2.1
config: x86_64-randconfig-121-20250731 (https://download.01.org/0day-ci/archive/20250731/202507312016.4ga8UpF5-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250731/202507312016.4ga8UpF5-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507312016.4ga8UpF5-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   drivers/nvdimm/label.c: note: in included file (through drivers/nvdimm/nd-core.h):
>> drivers/nvdimm/nd.h:314:37: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le32 [usertype] align @@     got restricted __le16 [usertype] @@
   drivers/nvdimm/nd.h:314:37: sparse:     expected restricted __le32 [usertype] align
   drivers/nvdimm/nd.h:314:37: sparse:     got restricted __le16 [usertype]

vim +314 drivers/nvdimm/nd.h

   308	
   309	static inline void nsl_set_alignment(struct nvdimm_drvdata *ndd,
   310					     struct nd_namespace_label *ns_label,
   311					     u32 align)
   312	{
   313		if (ndd->cxl)
 > 314			ns_label->cxl.align = __cpu_to_le16(align);
   315	}
   316	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.