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

Neeraj Kumar posted 20 patches 2 weeks ago
[PATCH V3 08/20] nvdimm/label: Include region label in slot validation
Posted by Neeraj Kumar 2 weeks ago
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";
+		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)
 {
-- 
2.34.1
Re: [PATCH V3 08/20] nvdimm/label: Include region label in slot validation
Posted by Dave Jiang 1 week, 2 days ago

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)
>  {