linux-kernel - Re: [PATCH v1 05/20] cxl/region: Separate cxl_region_alloc() from devm_cxl_add

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250721141653.89805-1-joshua.hahnjy@gmail.com>
Date: Mon, 21 Jul 2025 07:16:52 -0700
From: Joshua Hahn <joshua.hahnjy@...il.com>
To: Robert Richter <rrichter@....com>
Cc: Alison Schofield <alison.schofield@...el.com>,
	Vishal Verma <vishal.l.verma@...el.com>,
	Ira Weiny <ira.weiny@...el.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	Dave Jiang <dave.jiang@...el.com>,
	Davidlohr Bueso <dave@...olabs.net>,
	linux-cxl@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Gregory Price <gourry@...rry.net>,
	"Fabio M. De Francesco" <fabio.m.de.francesco@...ux.intel.com>,
	Terry Bowman <terry.bowman@....com>
Subject: Re: [PATCH v1 05/20] cxl/region: Separate cxl_region_alloc() from devm_cxl_add_region()

On Tue, 15 Jul 2025 21:11:28 +0200 Robert Richter <rrichter@....com> wrote:

> In interleaving configs multiple endpoint decoders connect to the same
> region. The region's parameters must be the same for all endpoint
> decoders that share the interleaving setup. During initialization, the
> region's parameters are determined for each endpoint decoder.
> If a region for the same hpa range already exists, no new region is
> created and the existing one is reused.
> 
> To simplify region setup and the collection of the region parameters,
> separate region allocation from its registration. This allows it to
> allocate and setup a region before checking the parameters with
> existing other regions and adding it to the cxl tree or releasing it
> and instead reusing an existing region.
> 
> Here, only separate cxl_region_alloc() from devm_cxl_add_region().
> 
> Signed-off-by: Robert Richter <rrichter@....com>
> ---

Hello Robert,

Thank you for this great patch! I have one small nit:

>  /**
> - * devm_cxl_add_region - Adds a region to a decoder
> - * @cxlrd: root decoder
> - * @id: memregion id to create, or memregion_free() on failure
> - * @mode: mode for the endpoint decoders of this region
> - * @type: select whether this is an expander or accelerator (type-2 or type-3)
> + * devm_cxl_add_region - Adds a region to the CXL hierarchy.
> + * @cxlr: region to be added
> + * @id: memregion id to create must match current @port_id of the
> + *      region's @cxlrd
>   *
>   * This is the second step of region initialization. Regions exist within an
>   * address space which is mapped by a @cxlrd.
>   *
> - * Return: 0 if the region was added to the @cxlrd, else returns negative error
> - * code. The region will be named "regionZ" where Z is the unique region number.
> + * Return: Pointer to the region if the region could be registered
> + * (for use in a tail call). The region will be named "regionZ" where
> + * Z is the unique region number. On errors, devm_cxl_add_region()
> + * returns an encoded negative error code and releases or unregisters
> + * @cxlr.
>   */

It seems like the changes that this new return description corresponds to
were actually made in the previous patch. Would it make sense to move this
new description to the previous patch?

> -static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
> -					      int id,
> -					      enum cxl_partition_mode mode,
> -					      enum cxl_decoder_type type)
> +static struct cxl_region *devm_cxl_add_region(struct cxl_region *cxlr, int id)

Otherwise, LGTM!

Reviewed-by: Joshua Hahn <joshua.hahnjy@...il.com>

Sent using hkml (https://github.com/sjp38/hackermail)