| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1983025922.01757055782611.JavaMail.epsvc@epcpadp2new>
Date: Thu, 4 Sep 2025 19:21:13 +0530
From: Neeraj Kumar <s.neeraj@...sung.com>
To: Ira Weiny <ira.weiny@...el.com>
Cc: linux-cxl@...r.kernel.org, nvdimm@...ts.linux.dev,
linux-kernel@...r.kernel.org, gost.dev@...sung.com,
a.manzanares@...sung.com, vishak.g@...sung.com, neeraj.kernel@...il.com,
cpgs@...sung.com
Subject: Re: [PATCH V2 03/20] nvdimm/namespace_label: Add namespace label
changes as per CXL LSA v2.1
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@...sung.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
Powered by blists - more mailing lists