Re: [PATCH v2 1/3] cxl/pmem: Format the nvdimm serial number as unsigned decimal

[Richard Cheng <icheng@nvidia.com>]

On Wed, Jul 01, 2026 at 05:30:44PM +0800, Alison Schofield wrote:
> The CXL NVDIMM security passphrase key description and the nvdimm 'id'
> sysfs attribute are both derived from the CXL device serial number,
> but the serial number is not formatted consistently.
> 
> The key description is formatted in hexadecimal while the 'id'
> attribute is formatted in decimal. As a result, ndctl stores the key
> using a decimal description while the kernel later looks it up using
> a hexadecimal description. For serial numbers of 10 and above, the
> descriptions no longer match, preventing automatic unlock after
> reboot.
> 
> The decimal formatting has a second problem. Both the key description
> and the 'id' attribute use the signed %lld format for a u64 PCIe
> Device Serial Number. Devices whose vendor OUI sets bit 63, such as
> Montage CXL devices, appear with negative decimal serial numbers.
> 
> Format the security key description and 'id' attribute as unsigned
> decimal, %llu, and document that the 'id' attribute is an unsigned
> decimal value.
> 
> The key lookup mismatch was exposed by CXL unit test cxl-security.sh
> when cxl_test mock serial numbers were extended to 10 and above.
> 
> Cc: <stable@vger.kernel.org>
> Fixes: b5807c80b5bc ("cxl: add dimm_id support for __nvdimm_create()")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-nvdimm |  3 ++-
>  drivers/cxl/core/pmem.c                    | 10 ++++++----
>  drivers/cxl/cxl.h                          |  3 ++-
>  drivers/cxl/pmem.c                         |  2 +-
>  4 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-nvdimm b/Documentation/ABI/testing/sysfs-bus-nvdimm
> index 64eb8f4c6a41..46dafd8482b9 100644
> --- a/Documentation/ABI/testing/sysfs-bus-nvdimm
> +++ b/Documentation/ABI/testing/sysfs-bus-nvdimm
> @@ -48,7 +48,8 @@ What:		/sys/bus/nd/devices/nmemX/cxl/id
>  Date:		November 2022
>  KernelVersion:	6.2
>  Contact:	Dave Jiang <dave.jiang@intel.com>
> -Description:	(RO) Show the id (serial) of the device. This is CXL specific.
> +Description:	(RO) Show the id (serial) of the device, formatted as an
> +		unsigned 64-bit decimal value. This is CXL specific.
>  
>  What:		/sys/bus/nd/devices/nmemX/cxl/provider
>  Date:		November 2022
> diff --git a/drivers/cxl/core/pmem.c b/drivers/cxl/core/pmem.c
> index 68462e38a977..5a3bb7e8a1f1 100644
> --- a/drivers/cxl/core/pmem.c
> +++ b/drivers/cxl/core/pmem.c
> @@ -219,12 +219,14 @@ static struct cxl_nvdimm *cxl_nvdimm_alloc(struct cxl_nvdimm_bridge *cxl_nvb,
>  	dev->bus = &cxl_bus_type;
>  	dev->type = &cxl_nvdimm_type;
>  	/*
> -	 * A "%llx" string is 17-bytes vs dimm_id that is max
> -	 * NVDIMM_KEY_DESC_LEN
> +	 * dev_id is the nvdimm dimm_id used for security key lookup.
> +	 * It must match id_show(), which emits the CXL serial as an
> +	 * unsigned decimal. A u64 decimal string is at most 20 digits
> +	 * plus NUL.
>  	 */
> -	BUILD_BUG_ON(sizeof(cxl_nvd->dev_id) < 17 ||
> +	BUILD_BUG_ON(sizeof(cxl_nvd->dev_id) < 21 ||
>  		     sizeof(cxl_nvd->dev_id) > NVDIMM_KEY_DESC_LEN);
> -	sprintf(cxl_nvd->dev_id, "%llx", cxlmd->cxlds->serial);
> +	sprintf(cxl_nvd->dev_id, "%llu", cxlmd->cxlds->serial);
> 

Hi Alison,

How should existing keys for high-bit serial numbers be migrated?

This patch changes both the sysfs  ID and kernel key description to unsigned
decimal. ndctl persists the old sysfs string in the key-blob filename and reloads it unchanged.

I wonder how's the old key being migrated ? I don't think the patch rename files under /etc/ndctl/keys , so ndctl may reload the existing secret
while the new kernel searches for the positive new label.

--Richard
 
>  	return cxl_nvd;
>  }
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index c0e5308e4d1b..d683ae5e0f7d 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -503,7 +503,8 @@ struct cxl_nvdimm_bridge {
>  	struct nvdimm_bus_descriptor nd_desc;
>  };
>  
> -#define CXL_DEV_ID_LEN 19
> +/* Holds a u64 serial as a decimal string: up to 20 digits + NUL */
> +#define CXL_DEV_ID_LEN 21
>  
>  enum {
>  	CXL_NVD_F_INVALIDATED = 0,
> diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
> index 261dff7ced9f..a9f50281875d 100644
> --- a/drivers/cxl/pmem.c
> +++ b/drivers/cxl/pmem.c
> @@ -52,7 +52,7 @@ static ssize_t id_show(struct device *dev, struct device_attribute *attr, char *
>  	struct cxl_nvdimm *cxl_nvd = nvdimm_provider_data(nvdimm);
>  	struct cxl_dev_state *cxlds = cxl_nvd->cxlmd->cxlds;
>  
> -	return sysfs_emit(buf, "%lld\n", cxlds->serial);
> +	return sysfs_emit(buf, "%llu\n", cxlds->serial);
>  }
>  static DEVICE_ATTR_RO(id);
>  
> -- 
> 2.37.3
> 
>