Prior to LSA 2.1 Support, label in slot means only namespace
label. But with LSA 2.1 a label can be either namespace or
region label.
Slot validation routine validates label slot by calculating
label checksum. It was only validating namespace label.
This changeset also validates region label if present.
In previous patch to_lsa_label() was introduced along with
to_label(). to_label() returns only namespace label whereas
to_lsa_label() returns union nd_lsa_label*
In this patch We have converted all usage of to_label()
to to_lsa_label()
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
---
drivers/nvdimm/label.c | 94 ++++++++++++++++++++++++++++--------------
1 file changed, 64 insertions(+), 30 deletions(-)
diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
index 17e2a1f5a6da..9854cb45fb62 100644
--- a/drivers/nvdimm/label.c
+++ b/drivers/nvdimm/label.c
@@ -312,16 +312,6 @@ static union nd_lsa_label *to_lsa_label(struct nvdimm_drvdata *ndd, int slot)
return (union nd_lsa_label *) label;
}
-static struct nd_namespace_label *to_label(struct nvdimm_drvdata *ndd, int slot)
-{
- unsigned long label, base;
-
- base = (unsigned long) nd_label_base(ndd);
- label = base + sizeof_namespace_label(ndd) * slot;
-
- return (struct nd_namespace_label *) label;
-}
-
#define for_each_clear_bit_le(bit, addr, size) \
for ((bit) = find_next_zero_bit_le((addr), (size), 0); \
(bit) < (size); \
@@ -382,7 +372,7 @@ static bool nsl_validate_checksum(struct nvdimm_drvdata *ndd,
{
u64 sum, sum_save;
- if (!ndd->cxl && !efi_namespace_label_has(ndd, checksum))
+ if (!efi_namespace_label_has(ndd, checksum))
return true;
sum_save = nsl_get_checksum(ndd, nd_label);
@@ -397,13 +387,25 @@ static void nsl_calculate_checksum(struct nvdimm_drvdata *ndd,
{
u64 sum;
- if (!ndd->cxl && !efi_namespace_label_has(ndd, checksum))
+ if (!efi_namespace_label_has(ndd, checksum))
return;
nsl_set_checksum(ndd, nd_label, 0);
sum = nd_fletcher64(nd_label, sizeof_namespace_label(ndd), 1);
nsl_set_checksum(ndd, nd_label, sum);
}
+static bool region_label_validate_checksum(struct nvdimm_drvdata *ndd,
+ struct cxl_region_label *region_label)
+{
+ u64 sum, sum_save;
+
+ sum_save = __le64_to_cpu(region_label->checksum);
+ region_label->checksum = __cpu_to_le64(0);
+ sum = nd_fletcher64(region_label, sizeof_namespace_label(ndd), 1);
+ region_label->checksum = __cpu_to_le64(sum_save);
+ return sum == sum_save;
+}
+
static void region_label_calculate_checksum(struct nvdimm_drvdata *ndd,
struct cxl_region_label *region_label)
{
@@ -415,16 +417,34 @@ static void region_label_calculate_checksum(struct nvdimm_drvdata *ndd,
}
static bool slot_valid(struct nvdimm_drvdata *ndd,
- struct nd_namespace_label *nd_label, u32 slot)
+ union nd_lsa_label *lsa_label, u32 slot)
{
+ struct cxl_region_label *region_label = &lsa_label->region_label;
+ struct nd_namespace_label *nd_label = &lsa_label->ns_label;
+ enum label_type type;
bool valid;
+ static const char * const label_name[] = {
+ [RG_LABEL_TYPE] = "region",
+ [NS_LABEL_TYPE] = "namespace",
+ };
/* check that we are written where we expect to be written */
- if (slot != nsl_get_slot(ndd, nd_label))
- return false;
- valid = nsl_validate_checksum(ndd, nd_label);
+ if (is_region_label(ndd, lsa_label)) {
+ type = RG_LABEL_TYPE;
+ if (slot != __le32_to_cpu(region_label->slot))
+ return false;
+ valid = region_label_validate_checksum(ndd, region_label);
+ } else {
+ type = NS_LABEL_TYPE;
+ if (slot != nsl_get_slot(ndd, nd_label))
+ return false;
+ valid = nsl_validate_checksum(ndd, nd_label);
+ }
+
if (!valid)
- dev_dbg(ndd->dev, "fail checksum. slot: %d\n", slot);
+ dev_dbg(ndd->dev, "%s label checksum fail. slot: %d\n",
+ label_name[type], slot);
+
return valid;
}
@@ -440,14 +460,16 @@ int nd_label_reserve_dpa(struct nvdimm_drvdata *ndd)
for_each_clear_bit_le(slot, free, nslot) {
struct nd_namespace_label *nd_label;
struct nd_region *nd_region = NULL;
+ union nd_lsa_label *lsa_label;
struct nd_label_id label_id;
struct resource *res;
uuid_t label_uuid;
u32 flags;
- nd_label = to_label(ndd, slot);
+ lsa_label = to_lsa_label(ndd, slot);
+ nd_label = &lsa_label->ns_label;
- if (!slot_valid(ndd, nd_label, slot))
+ if (!slot_valid(ndd, lsa_label, slot))
continue;
nsl_get_uuid(ndd, nd_label, &label_uuid);
@@ -598,18 +620,30 @@ int nd_label_active_count(struct nvdimm_drvdata *ndd)
return 0;
for_each_clear_bit_le(slot, free, nslot) {
+ struct cxl_region_label *region_label;
struct nd_namespace_label *nd_label;
-
- nd_label = to_label(ndd, slot);
-
- if (!slot_valid(ndd, nd_label, slot)) {
- u32 label_slot = nsl_get_slot(ndd, nd_label);
- u64 size = nsl_get_rawsize(ndd, nd_label);
- u64 dpa = nsl_get_dpa(ndd, nd_label);
+ union nd_lsa_label *lsa_label;
+ u32 lslot;
+ u64 size, dpa;
+
+ lsa_label = to_lsa_label(ndd, slot);
+ nd_label = &lsa_label->ns_label;
+ region_label = &lsa_label->region_label;
+
+ if (!slot_valid(ndd, lsa_label, slot)) {
+ if (is_region_label(ndd, lsa_label)) {
+ lslot = __le32_to_cpu(region_label->slot);
+ size = __le64_to_cpu(region_label->rawsize);
+ dpa = __le64_to_cpu(region_label->dpa);
+ } else {
+ lslot = nsl_get_slot(ndd, nd_label);
+ size = nsl_get_rawsize(ndd, nd_label);
+ dpa = nsl_get_dpa(ndd, nd_label);
+ }
dev_dbg(ndd->dev,
"slot%d invalid slot: %d dpa: %llx size: %llx\n",
- slot, label_slot, dpa, size);
+ slot, lslot, dpa, size);
continue;
}
count++;
@@ -627,10 +661,10 @@ union nd_lsa_label *nd_label_active(struct nvdimm_drvdata *ndd, int n)
return NULL;
for_each_clear_bit_le(slot, free, nslot) {
- struct nd_namespace_label *nd_label;
+ union nd_lsa_label *lsa_label;
- nd_label = to_label(ndd, slot);
- if (!slot_valid(ndd, nd_label, slot))
+ lsa_label = to_lsa_label(ndd, slot);
+ if (!slot_valid(ndd, lsa_label, slot))
continue;
if (n-- == 0)
--
2.34.1Neeraj Kumar wrote: > Prior to LSA 2.1 Support, label in slot means only namespace > label. But with LSA 2.1 a label can be either namespace or > region label. > > Slot validation routine validates label slot by calculating > label checksum. It was only validating namespace label. > This changeset also validates region label if present. > > In previous patch to_lsa_label() was introduced along with > to_label(). to_label() returns only namespace label whereas > to_lsa_label() returns union nd_lsa_label* > > In this patch We have converted all usage of to_label() > to to_lsa_label() > Reviewed-by: Ira Weiny <ira.weiny@intel.com> [snip]
On Fri, 9 Jan 2026 18:14:24 +0530
Neeraj Kumar <s.neeraj@samsung.com> wrote:
> Prior to LSA 2.1 Support, label in slot means only namespace
> label. But with LSA 2.1 a label can be either namespace or
> region label.
>
> Slot validation routine validates label slot by calculating
> label checksum. It was only validating namespace label.
> This changeset also validates region label if present.
>
> In previous patch to_lsa_label() was introduced along with
> to_label(). to_label() returns only namespace label whereas
> to_lsa_label() returns union nd_lsa_label*
>
> In this patch We have converted all usage of to_label()
> to to_lsa_label()
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
Hi
A few more things given you are going around again!
Jonathan
> ---
> drivers/nvdimm/label.c | 94 ++++++++++++++++++++++++++++--------------
> 1 file changed, 64 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
> index 17e2a1f5a6da..9854cb45fb62 100644
> --- a/drivers/nvdimm/label.c
> +++ b/drivers/nvdimm/label.c
> @@ -312,16 +312,6 @@ static union nd_lsa_label *to_lsa_label(struct nvdimm_drvdata *ndd, int slot)
> +
> static void region_label_calculate_checksum(struct nvdimm_drvdata *ndd,
> struct cxl_region_label *region_label)
> {
> @@ -415,16 +417,34 @@ static void region_label_calculate_checksum(struct nvdimm_drvdata *ndd,
> }
>
> static bool slot_valid(struct nvdimm_drvdata *ndd,
> - struct nd_namespace_label *nd_label, u32 slot)
> + union nd_lsa_label *lsa_label, u32 slot)
> {
> + struct cxl_region_label *region_label = &lsa_label->region_label;
> + struct nd_namespace_label *nd_label = &lsa_label->ns_label;
Move the assignments of these so we only do them if they are relevant.
That should make the code more resilient as any use outside those scopes
will fire compiler warnings.
> + enum label_type type;
> bool valid;
> + static const char * const label_name[] = {
> + [RG_LABEL_TYPE] = "region",
> + [NS_LABEL_TYPE] = "namespace",
> + };
>
> /* check that we are written where we expect to be written */
> - if (slot != nsl_get_slot(ndd, nd_label))
> - return false;
> - valid = nsl_validate_checksum(ndd, nd_label);
> + if (is_region_label(ndd, lsa_label)) {
> + type = RG_LABEL_TYPE;
> + if (slot != __le32_to_cpu(region_label->slot))
> + return false;
> + valid = region_label_validate_checksum(ndd, region_label);
> + } else {
> + type = NS_LABEL_TYPE;
> + if (slot != nsl_get_slot(ndd, nd_label))
> + return false;
> + valid = nsl_validate_checksum(ndd, nd_label);
> + }
> @@ -598,18 +620,30 @@ int nd_label_active_count(struct nvdimm_drvdata *ndd)
> return 0;
>
> for_each_clear_bit_le(slot, free, nslot) {
> + struct cxl_region_label *region_label;
> struct nd_namespace_label *nd_label;
> -
> - nd_label = to_label(ndd, slot);
> -
> - if (!slot_valid(ndd, nd_label, slot)) {
> - u32 label_slot = nsl_get_slot(ndd, nd_label);
> - u64 size = nsl_get_rawsize(ndd, nd_label);
> - u64 dpa = nsl_get_dpa(ndd, nd_label);
> + union nd_lsa_label *lsa_label;
> + u32 lslot;
Trivial but if you keep the label_slot naming, then the diff gets a little
smaller, and it's easier to see where the code moved. Don't worry about going just
over 80 chars. We are more relaxed these days as long as it helps readability.
> + u64 size, dpa;
> +
> + lsa_label = to_lsa_label(ndd, slot);
> + nd_label = &lsa_label->ns_label;
Move this
> + region_label = &lsa_label->region_label;
and this down to the scopes where they are meaningful.
So under if (is_region_label() and the else.
> +
> + if (!slot_valid(ndd, lsa_label, slot)) {
> + if (is_region_label(ndd, lsa_label)) {
> + lslot = __le32_to_cpu(region_label->slot);
> + size = __le64_to_cpu(region_label->rawsize);
> + dpa = __le64_to_cpu(region_label->dpa);
> + } else {
> + lslot = nsl_get_slot(ndd, nd_label);
> + size = nsl_get_rawsize(ndd, nd_label);
> + dpa = nsl_get_dpa(ndd, nd_label);
> + }
>
> dev_dbg(ndd->dev,
> "slot%d invalid slot: %d dpa: %llx size: %llx\n",
> - slot, label_slot, dpa, size);
> + slot, lslot, dpa, size);
> continue;
> }
> count++;
=
Neeraj Kumar wrote:
> Prior to LSA 2.1 Support, label in slot means only namespace
> label. But with LSA 2.1 a label can be either namespace or
> region label.
>
> Slot validation routine validates label slot by calculating
> label checksum. It was only validating namespace label.
> This changeset also validates region label if present.
>
> In previous patch to_lsa_label() was introduced along with
> to_label(). to_label() returns only namespace label whereas
> to_lsa_label() returns union nd_lsa_label*
>
> In this patch We have converted all usage of to_label()
NIT: don't use 'We'
> to to_lsa_label()
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
> ---
> drivers/nvdimm/label.c | 94 ++++++++++++++++++++++++++++--------------
> 1 file changed, 64 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
> index 17e2a1f5a6da..9854cb45fb62 100644
> --- a/drivers/nvdimm/label.c
> +++ b/drivers/nvdimm/label.c
> @@ -312,16 +312,6 @@ static union nd_lsa_label *to_lsa_label(struct nvdimm_drvdata *ndd, int slot)
> return (union nd_lsa_label *) label;
> }
>
> -static struct nd_namespace_label *to_label(struct nvdimm_drvdata *ndd, int slot)
> -{
> - unsigned long label, base;
> -
> - base = (unsigned long) nd_label_base(ndd);
> - label = base + sizeof_namespace_label(ndd) * slot;
> -
> - return (struct nd_namespace_label *) label;
> -}
> -
> #define for_each_clear_bit_le(bit, addr, size) \
> for ((bit) = find_next_zero_bit_le((addr), (size), 0); \
> (bit) < (size); \
> @@ -382,7 +372,7 @@ static bool nsl_validate_checksum(struct nvdimm_drvdata *ndd,
> {
> u64 sum, sum_save;
>
> - if (!ndd->cxl && !efi_namespace_label_has(ndd, checksum))
> + if (!efi_namespace_label_has(ndd, checksum))
What does this change have to do with region label validation during slot
validation?
> return true;
>
> sum_save = nsl_get_checksum(ndd, nd_label);
> @@ -397,13 +387,25 @@ static void nsl_calculate_checksum(struct nvdimm_drvdata *ndd,
> {
> u64 sum;
>
> - if (!ndd->cxl && !efi_namespace_label_has(ndd, checksum))
> + if (!efi_namespace_label_has(ndd, checksum))
This and the above seem like cleanups because efi_namespace_label_has()
already checks !ndd->cxl? Was that the intent? Perhaps as a separate
cleanup?
Rest of the patch looks reasonable,
Ira
> return;
> nsl_set_checksum(ndd, nd_label, 0);
> sum = nd_fletcher64(nd_label, sizeof_namespace_label(ndd), 1);
> nsl_set_checksum(ndd, nd_label, sum);
> }
>
> +static bool region_label_validate_checksum(struct nvdimm_drvdata *ndd,
> + struct cxl_region_label *region_label)
> +{
> + u64 sum, sum_save;
> +
> + sum_save = __le64_to_cpu(region_label->checksum);
> + region_label->checksum = __cpu_to_le64(0);
> + sum = nd_fletcher64(region_label, sizeof_namespace_label(ndd), 1);
> + region_label->checksum = __cpu_to_le64(sum_save);
> + return sum == sum_save;
> +}
> +
> static void region_label_calculate_checksum(struct nvdimm_drvdata *ndd,
> struct cxl_region_label *region_label)
> {
> @@ -415,16 +417,34 @@ static void region_label_calculate_checksum(struct nvdimm_drvdata *ndd,
> }
>
> static bool slot_valid(struct nvdimm_drvdata *ndd,
> - struct nd_namespace_label *nd_label, u32 slot)
> + union nd_lsa_label *lsa_label, u32 slot)
> {
> + struct cxl_region_label *region_label = &lsa_label->region_label;
> + struct nd_namespace_label *nd_label = &lsa_label->ns_label;
> + enum label_type type;
> bool valid;
> + static const char * const label_name[] = {
> + [RG_LABEL_TYPE] = "region",
> + [NS_LABEL_TYPE] = "namespace",
> + };
>
> /* check that we are written where we expect to be written */
> - if (slot != nsl_get_slot(ndd, nd_label))
> - return false;
> - valid = nsl_validate_checksum(ndd, nd_label);
> + if (is_region_label(ndd, lsa_label)) {
> + type = RG_LABEL_TYPE;
> + if (slot != __le32_to_cpu(region_label->slot))
> + return false;
> + valid = region_label_validate_checksum(ndd, region_label);
> + } else {
> + type = NS_LABEL_TYPE;
> + if (slot != nsl_get_slot(ndd, nd_label))
> + return false;
> + valid = nsl_validate_checksum(ndd, nd_label);
> + }
> +
> if (!valid)
> - dev_dbg(ndd->dev, "fail checksum. slot: %d\n", slot);
> + dev_dbg(ndd->dev, "%s label checksum fail. slot: %d\n",
> + label_name[type], slot);
> +
> return valid;
> }
>
> @@ -440,14 +460,16 @@ int nd_label_reserve_dpa(struct nvdimm_drvdata *ndd)
> for_each_clear_bit_le(slot, free, nslot) {
> struct nd_namespace_label *nd_label;
> struct nd_region *nd_region = NULL;
> + union nd_lsa_label *lsa_label;
> struct nd_label_id label_id;
> struct resource *res;
> uuid_t label_uuid;
> u32 flags;
>
> - nd_label = to_label(ndd, slot);
> + lsa_label = to_lsa_label(ndd, slot);
> + nd_label = &lsa_label->ns_label;
>
> - if (!slot_valid(ndd, nd_label, slot))
> + if (!slot_valid(ndd, lsa_label, slot))
> continue;
>
> nsl_get_uuid(ndd, nd_label, &label_uuid);
> @@ -598,18 +620,30 @@ int nd_label_active_count(struct nvdimm_drvdata *ndd)
> return 0;
>
> for_each_clear_bit_le(slot, free, nslot) {
> + struct cxl_region_label *region_label;
> struct nd_namespace_label *nd_label;
> -
> - nd_label = to_label(ndd, slot);
> -
> - if (!slot_valid(ndd, nd_label, slot)) {
> - u32 label_slot = nsl_get_slot(ndd, nd_label);
> - u64 size = nsl_get_rawsize(ndd, nd_label);
> - u64 dpa = nsl_get_dpa(ndd, nd_label);
> + union nd_lsa_label *lsa_label;
> + u32 lslot;
> + u64 size, dpa;
> +
> + lsa_label = to_lsa_label(ndd, slot);
> + nd_label = &lsa_label->ns_label;
> + region_label = &lsa_label->region_label;
> +
> + if (!slot_valid(ndd, lsa_label, slot)) {
> + if (is_region_label(ndd, lsa_label)) {
> + lslot = __le32_to_cpu(region_label->slot);
> + size = __le64_to_cpu(region_label->rawsize);
> + dpa = __le64_to_cpu(region_label->dpa);
> + } else {
> + lslot = nsl_get_slot(ndd, nd_label);
> + size = nsl_get_rawsize(ndd, nd_label);
> + dpa = nsl_get_dpa(ndd, nd_label);
> + }
>
> dev_dbg(ndd->dev,
> "slot%d invalid slot: %d dpa: %llx size: %llx\n",
> - slot, label_slot, dpa, size);
> + slot, lslot, dpa, size);
> continue;
> }
> count++;
> @@ -627,10 +661,10 @@ union nd_lsa_label *nd_label_active(struct nvdimm_drvdata *ndd, int n)
> return NULL;
>
> for_each_clear_bit_le(slot, free, nslot) {
> - struct nd_namespace_label *nd_label;
> + union nd_lsa_label *lsa_label;
>
> - nd_label = to_label(ndd, slot);
> - if (!slot_valid(ndd, nd_label, slot))
> + lsa_label = to_lsa_label(ndd, slot);
> + if (!slot_valid(ndd, lsa_label, slot))
> continue;
>
> if (n-- == 0)
> --
> 2.34.1
>
>
On 14/01/26 03:20PM, Ira Weiny wrote:
>Neeraj Kumar wrote:
>> Prior to LSA 2.1 Support, label in slot means only namespace
>> label. But with LSA 2.1 a label can be either namespace or
>> region label.
>>
>> Slot validation routine validates label slot by calculating
>> label checksum. It was only validating namespace label.
>> This changeset also validates region label if present.
>>
>> In previous patch to_lsa_label() was introduced along with
>> to_label(). to_label() returns only namespace label whereas
>> to_lsa_label() returns union nd_lsa_label*
>>
>> In this patch We have converted all usage of to_label()
>
>NIT: don't use 'We'
Fixed it in V6
>
>> to to_lsa_label()
>>
>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
>> ---
>> drivers/nvdimm/label.c | 94 ++++++++++++++++++++++++++++--------------
>> 1 file changed, 64 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
>> index 17e2a1f5a6da..9854cb45fb62 100644
>> --- a/drivers/nvdimm/label.c
>> +++ b/drivers/nvdimm/label.c
>> @@ -312,16 +312,6 @@ static union nd_lsa_label *to_lsa_label(struct nvdimm_drvdata *ndd, int slot)
>> return (union nd_lsa_label *) label;
>> }
>>
>> -static struct nd_namespace_label *to_label(struct nvdimm_drvdata *ndd, int slot)
>> -{
>> - unsigned long label, base;
>> -
>> - base = (unsigned long) nd_label_base(ndd);
>> - label = base + sizeof_namespace_label(ndd) * slot;
>> -
>> - return (struct nd_namespace_label *) label;
>> -}
>> -
>> #define for_each_clear_bit_le(bit, addr, size) \
>> for ((bit) = find_next_zero_bit_le((addr), (size), 0); \
>> (bit) < (size); \
>> @@ -382,7 +372,7 @@ static bool nsl_validate_checksum(struct nvdimm_drvdata *ndd,
>> {
>> u64 sum, sum_save;
>>
>> - if (!ndd->cxl && !efi_namespace_label_has(ndd, checksum))
>> + if (!efi_namespace_label_has(ndd, checksum))
>
>What does this change have to do with region label validation during slot
>validation?
>
>> return true;
>>
>> sum_save = nsl_get_checksum(ndd, nd_label);
>> @@ -397,13 +387,25 @@ static void nsl_calculate_checksum(struct nvdimm_drvdata *ndd,
>> {
>> u64 sum;
>>
>> - if (!ndd->cxl && !efi_namespace_label_has(ndd, checksum))
>> + if (!efi_namespace_label_has(ndd, checksum))
>
>This and the above seem like cleanups because efi_namespace_label_has()
>already checks !ndd->cxl? Was that the intent? Perhaps as a separate
>cleanup?
Hi Ira,
Actually above is required changes and not the cleanup.
Earlier condition (!ndd->cxl && !efi_namespace_label_has(ndd, checksum))
was getting true (no further processing) in first case only if its a
region label which we don't want in current case.
And this condition (!efi_namespace_label_has(ndd, checksum)) returns false
(means proceed further) in case if its a region label (!!ndd->cxl).
Regards,
Neeraj
© 2016 - 2026 Red Hat, Inc.