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.nee...@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
