linux-kernel - Re: [RFC PATCH 19/20] cxl/pmem_region: Prep patch to accommodate pmem

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250623105337.000028c6@huawei.com>
Date: Mon, 23 Jun 2025 10:53:37 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Neeraj Kumar <s.neeraj@...sung.com>
CC: <dan.j.williams@...el.com>, <dave@...olabs.net>, <dave.jiang@...el.com>,
	<alison.schofield@...el.com>, <vishal.l.verma@...el.com>,
	<ira.weiny@...el.com>, <a.manzanares@...sung.com>, <nifan.cxl@...il.com>,
	<anisa.su@...sung.com>, <vishak.g@...sung.com>, <krish.reddy@...sung.com>,
	<arun.george@...sung.com>, <alok.rathore@...sung.com>,
	<neeraj.kernel@...il.com>, <linux-kernel@...r.kernel.org>,
	<linux-cxl@...r.kernel.org>, <nvdimm@...ts.linux.dev>,
	<gost.dev@...sung.com>, <cpgs@...sung.com>
Subject: Re: [RFC PATCH 19/20] cxl/pmem_region: Prep patch to accommodate
 pmem_region attributes

On Tue, 17 Jun 2025 18:09:43 +0530
Neeraj Kumar <s.neeraj@...sung.com> wrote:

> Created a separate file core/pmem_region.c along with CONFIG_PMEM_REGION
> Moved pmem_region related code from core/region.c to core/pmem_region.c
> For region label update, need to create device attribute, which calls
> nvdimm exported function thus making pmem_region dependent on libnvdimm.
> Because of this dependency of pmem region on libnvdimm, segregated pmem

segregate

> region related code from core/region.c
> 
> Signed-off-by: Neeraj Kumar <s.neeraj@...sung.com>
> ---
>  drivers/cxl/Kconfig            |  12 ++
>  drivers/cxl/core/Makefile      |   1 +
>  drivers/cxl/core/core.h        |   8 +-
>  drivers/cxl/core/pmem_region.c | 222 +++++++++++++++++++++++++++++++++
>  drivers/cxl/core/port.c        |   2 +-
>  drivers/cxl/core/region.c      | 217 ++------------------------------
>  drivers/cxl/cxl.h              |  42 +++++--
>  tools/testing/cxl/Kbuild       |   1 +
>  8 files changed, 283 insertions(+), 222 deletions(-)
>  create mode 100644 drivers/cxl/core/pmem_region.c
> 
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 876469e23f7a..f0cbb096bfe7 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -128,6 +128,18 @@ config CXL_REGION
>  
>  	  If unsure say 'y'
>  
> +config CXL_PMEM_REGION
> +	bool "CXL: Pmem Region Support"
> +	default CXL_BUS
> +	depends on CXL_REGION
> +	select LIBNVDIMM if CXL_BUS = y

This is in the block covered by if CXL_BUS so I think you can simplify this check.

> +	help
> +	  Enable the CXL core to enumerate and provision CXL pmem regions.
> +	  A CXL pmem region need to update region label into LSA. For LSA
> +	  updation/deletion libnvdimm is required.
> +
> +	  If unsure say 'y'
> +
>  config CXL_REGION_INVALIDATION_TEST
>  	bool "CXL: Region Cache Management Bypass (TEST)"
>  	depends on CXL_REGION

> diff --git a/drivers/cxl/core/pmem_region.c b/drivers/cxl/core/pmem_region.c
> new file mode 100644
> index 000000000000..a29526c27d40
> --- /dev/null
> +++ b/drivers/cxl/core/pmem_region.c
> @@ -0,0 +1,222 @@

> @@ -3273,92 +3155,6 @@ static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
>  	return cxlr_dax;
>  }

>  static void cxlr_dax_unregister(void *_cxlr_dax)
>  {
>  	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
> @@ -3646,7 +3442,10 @@ static int cxl_region_probe(struct device *dev)
>  
>  	switch (cxlr->mode) {
>  	case CXL_DECODER_PMEM:
> -		return devm_cxl_add_pmem_region(cxlr);
> +		if (IS_ENABLED(CONFIG_CXL_PMEM_REGION))
> +			return devm_cxl_add_pmem_region(cxlr);
> +		else
> +			return -EINVAL;
		if (!IS_ENABLED(CONFIG_CXL_PMEM_REGION))
			return -EINVAL;

		return devm_cxl_add_pmem_region()

Where ever possible keep the error conditions as the out of line ones.
That generally improves code readability and is common practice in the kernel.

>  	case CXL_DECODER_RAM:
>  		/*
>  		 * The region can not be manged by CXL if any portion of