Re: [PATCH V3 08/20] nvdimm/label: Include region label in slot validation

Dave Jiang
2025-09-22T22:17:46Z

On 9/17/25 6:41 AM, Neeraj Kumar wrote:
> Slot validation routine validates label slot by calculating label
> checksum. It was only validating namespace label. This changeset also
> validates region label if present.
>
> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> ---
> drivers/nvdimm/label.c | 72 ++++++++++++++++++++++++++++++++----------
> drivers/nvdimm/nd.h | 5 +++
> 2 files changed, 60 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
> index d33db96ba8ba..5e476154cf81 100644
> --- a/drivers/nvdimm/label.c
> +++ b/drivers/nvdimm/label.c
> @@ -359,7 +359,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);
> @@ -374,13 +374,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 = region_label_get_checksum(region_label);
> + region_label_set_checksum(region_label, 0);
> + sum = nd_fletcher64(region_label, sizeof_namespace_label(ndd), 1);
> + region_label_set_checksum(region_label, sum_save);
> + return sum == sum_save;
> +}
> +
> static void region_label_calculate_checksum(struct nvdimm_drvdata *ndd,
> struct cxl_region_label *region_label)
> {
> @@ -392,16 +404,30 @@ 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;
> + char *label_name;
> bool valid;
>
> /* 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)) {
> + label_name = "rg";

I suggest create a static string table enumerated by 'enum label_type'. That way you can directly address it by 'label_name[ltype]' when being used instead of assigning at run time. And maybe just use "region" and "namespace" since it's for debug output and we don't need to shorten it.

DJ

> + if (slot != region_label_get_slot(region_label))
> + return false;
> + valid = region_label_validate_checksum(ndd, region_label);
> + } else {
> + label_name = "ns";
> + 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, slot);
> +
> return valid;
> }
>
> @@ -424,7 +450,7 @@ int nd_label_reserve_dpa(struct nvdimm_drvdata *ndd)
>
> nd_label = to_label(ndd, slot);
>
> - if (!slot_valid(ndd, nd_label, slot))
> + if (!slot_valid(ndd, (union nd_lsa_label *) nd_label, slot))
> continue;
>
> nsl_get_uuid(ndd, nd_label, &label_uuid);
> @@ -575,18 +601,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 = (union nd_lsa_label *) to_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++;
> @@ -607,7 +645,7 @@ struct nd_namespace_label *nd_label_active(struct nvdimm_drvdata *ndd, int n)
> struct nd_namespace_label *nd_label;
>
> nd_label = to_label(ndd, slot);
> - if (!slot_valid(ndd, nd_label, slot))
> + if (!slot_valid(ndd, (union nd_lsa_label *) nd_label, slot))
> continue;
>
> if (n-- == 0)
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index 046063ea08b6..c985f91728dd 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -344,6 +344,11 @@ region_label_uuid_equal(struct cxl_region_label *region_label,
> return uuid_equal((uuid_t *) region_label->uuid, uuid);
> }
>
> +static inline u32 region_label_get_slot(struct cxl_region_label *region_label)
> +{
> + return __le32_to_cpu(region_label->slot);
> +}
> +
> static inline u64
> region_label_get_checksum(struct cxl_region_label *region_label)
> {