To construct a region, the region parameters such as address range and
interleaving config need to be determined. This is done while
constructing the region by inspecting the endpoint decoder
configuration. The endpoint decoder is passed as a function argument.
With address translation the endpoint decoder data is no longer
sufficient to extract the region parameters as some of the information
is obtained using other methods such as using firmware calls.
In a first step, separate code to determine and setup the region
parameters from the region construction. Temporarily store all the
data to create the region in the new struct cxl_region_context. Add a
new function setup_region_parameters() to fill that struct and later
use it to construct the region. This simplifies the extension of the
function to support other methods needed, esp. to support address
translation.
Patch is a prerequisite to implement address translation.
Signed-off-by: Robert Richter <rrichter@amd.com>
---
drivers/cxl/core/region.c | 50 +++++++++++++++++++++++++++++----------
1 file changed, 38 insertions(+), 12 deletions(-)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 106692f1e310..57697504410b 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3414,6 +3414,26 @@ static int match_region_by_range(struct device *dev, const void *data)
return 0;
}
+struct cxl_region_context {
+ struct cxl_endpoint_decoder *cxled;
+ struct cxl_memdev *cxlmd;
+ struct range hpa_range;
+ int interleave_ways;
+ int interleave_granularity;
+};
+
+static int setup_region_params(struct cxl_endpoint_decoder *cxled,
+ struct cxl_region_context *ctx)
+{
+ ctx->cxled = cxled;
+ ctx->cxlmd = cxled_to_memdev(cxled);
+ ctx->hpa_range = cxled->cxld.hpa_range;
+ ctx->interleave_ways = cxled->cxld.interleave_ways;
+ ctx->interleave_granularity = cxled->cxld.interleave_granularity;
+
+ return 0;
+}
+
static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr,
struct resource *res)
{
@@ -3453,11 +3473,12 @@ static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr,
}
static int __construct_region(struct cxl_region *cxlr,
- struct cxl_endpoint_decoder *cxled)
+ struct cxl_region_context *ctx)
{
+ struct cxl_endpoint_decoder *cxled = ctx->cxled;
struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
- struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
- struct range *range = &cxled->cxld.hpa_range;
+ struct cxl_memdev *cxlmd = ctx->cxlmd;
+ struct range *range = &ctx->hpa_range;
struct cxl_region_params *p;
struct resource *res;
int rc;
@@ -3506,8 +3527,8 @@ static int __construct_region(struct cxl_region *cxlr,
}
p->res = res;
- p->interleave_ways = cxled->cxld.interleave_ways;
- p->interleave_granularity = cxled->cxld.interleave_granularity;
+ p->interleave_ways = ctx->interleave_ways;
+ p->interleave_granularity = ctx->interleave_granularity;
p->state = CXL_CONFIG_INTERLEAVE_ACTIVE;
rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_target_group());
@@ -3527,9 +3548,10 @@ static int __construct_region(struct cxl_region *cxlr,
/* Establish an empty region covering the given HPA range */
static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
- struct cxl_endpoint_decoder *cxled)
+ struct cxl_region_context *ctx)
{
- struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+ struct cxl_endpoint_decoder *cxled = ctx->cxled;
+ struct cxl_memdev *cxlmd = ctx->cxlmd;
struct cxl_port *port = cxlrd_to_port(cxlrd);
struct cxl_dev_state *cxlds = cxlmd->cxlds;
int rc, part = READ_ONCE(cxled->part);
@@ -3548,7 +3570,7 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
return cxlr;
}
- rc = __construct_region(cxlr, cxled);
+ rc = __construct_region(cxlr, ctx);
if (rc) {
devm_release_action(port->uport_dev, unregister_region, cxlr);
return ERR_PTR(rc);
@@ -3572,13 +3594,17 @@ cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *range)
int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
{
- struct range *range = &cxled->cxld.hpa_range;
+ struct cxl_region_context ctx;
struct cxl_region_params *p;
bool attach = false;
int rc;
+ rc = setup_region_params(cxled, &ctx);
+ if (rc)
+ return rc;
+
struct cxl_root_decoder *cxlrd __free(put_cxl_root_decoder) =
- cxl_find_root_decoder(cxled, range);
+ cxl_find_root_decoder(cxled, &ctx.hpa_range);
if (!cxlrd)
return -ENXIO;
@@ -3589,9 +3615,9 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
*/
mutex_lock(&cxlrd->range_lock);
struct cxl_region *cxlr __free(put_cxl_region) =
- cxl_find_region_by_range(cxlrd, range);
+ cxl_find_region_by_range(cxlrd, &ctx.hpa_range);
if (!cxlr)
- cxlr = construct_region(cxlrd, cxled);
+ cxlr = construct_region(cxlrd, &ctx);
mutex_unlock(&cxlrd->range_lock);
rc = PTR_ERR_OR_ZERO(cxlr);
--
2.39.5On Fri, Sep 12, 2025 at 04:45:08PM +0200, Robert Richter wrote: > To construct a region, the region parameters such as address range and > interleaving config need to be determined. This is done while > constructing the region by inspecting the endpoint decoder > configuration. The endpoint decoder is passed as a function argument. > > With address translation the endpoint decoder data is no longer > sufficient to extract the region parameters as some of the information > is obtained using other methods such as using firmware calls. > > In a first step, separate code to determine and setup the region > parameters from the region construction. Temporarily store all the > data to create the region in the new struct cxl_region_context. Add a > new function setup_region_parameters() to fill that struct and later > use it to construct the region. This simplifies the extension of the > function to support other methods needed, esp. to support address > translation. > > Patch is a prerequisite to implement address translation. > > Signed-off-by: Robert Richter <rrichter@amd.com> Dave already got the init code, so with that change feel free to add Reviewed-by: Gregory Price <gourry@gourry.net>
On 9/12/25 7:45 AM, Robert Richter wrote:
> To construct a region, the region parameters such as address range and
> interleaving config need to be determined. This is done while
> constructing the region by inspecting the endpoint decoder
> configuration. The endpoint decoder is passed as a function argument.
>
> With address translation the endpoint decoder data is no longer
> sufficient to extract the region parameters as some of the information
> is obtained using other methods such as using firmware calls.
>
> In a first step, separate code to determine and setup the region
> parameters from the region construction. Temporarily store all the
> data to create the region in the new struct cxl_region_context. Add a
> new function setup_region_parameters() to fill that struct and later
> use it to construct the region. This simplifies the extension of the
> function to support other methods needed, esp. to support address
> translation.
>
> Patch is a prerequisite to implement address translation.
>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
> drivers/cxl/core/region.c | 50 +++++++++++++++++++++++++++++----------
> 1 file changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 106692f1e310..57697504410b 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3414,6 +3414,26 @@ static int match_region_by_range(struct device *dev, const void *data)
> return 0;
> }
>
> +struct cxl_region_context {
> + struct cxl_endpoint_decoder *cxled;
> + struct cxl_memdev *cxlmd;
> + struct range hpa_range;
> + int interleave_ways;
> + int interleave_granularity;
> +};
> +
> +static int setup_region_params(struct cxl_endpoint_decoder *cxled,
> + struct cxl_region_context *ctx)
> +{
> + ctx->cxled = cxled;
> + ctx->cxlmd = cxled_to_memdev(cxled);
> + ctx->hpa_range = cxled->cxld.hpa_range;
> + ctx->interleave_ways = cxled->cxld.interleave_ways;
> + ctx->interleave_granularity = cxled->cxld.interleave_granularity;
You can init like this:
*ctx = (struct cxl_region_context) {
.cxled = cxled,
.cxlmd = cxled_to_memdev(cxled),
.hpa_range = cxled->cxld.hpa_range,
.interleave_ways = cxled->cxld.interleave_ways,
.interleave_granularity = cxled->cxld.interleave_granularity,
};
> +
> + return 0;
Can probably make this function void if no expected errors and only assignments.
DJ
> +}
> +
> static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr,
> struct resource *res)
> {
> @@ -3453,11 +3473,12 @@ static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr,
> }
>
> static int __construct_region(struct cxl_region *cxlr,
> - struct cxl_endpoint_decoder *cxled)
> + struct cxl_region_context *ctx)
> {
> + struct cxl_endpoint_decoder *cxled = ctx->cxled;
> struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
> - struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> - struct range *range = &cxled->cxld.hpa_range;
> + struct cxl_memdev *cxlmd = ctx->cxlmd;
> + struct range *range = &ctx->hpa_range;
> struct cxl_region_params *p;
> struct resource *res;
> int rc;
> @@ -3506,8 +3527,8 @@ static int __construct_region(struct cxl_region *cxlr,
> }
>
> p->res = res;
> - p->interleave_ways = cxled->cxld.interleave_ways;
> - p->interleave_granularity = cxled->cxld.interleave_granularity;
> + p->interleave_ways = ctx->interleave_ways;
> + p->interleave_granularity = ctx->interleave_granularity;
> p->state = CXL_CONFIG_INTERLEAVE_ACTIVE;
>
> rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_target_group());
> @@ -3527,9 +3548,10 @@ static int __construct_region(struct cxl_region *cxlr,
>
> /* Establish an empty region covering the given HPA range */
> static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
> - struct cxl_endpoint_decoder *cxled)
> + struct cxl_region_context *ctx)
> {
> - struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> + struct cxl_endpoint_decoder *cxled = ctx->cxled;
> + struct cxl_memdev *cxlmd = ctx->cxlmd;
> struct cxl_port *port = cxlrd_to_port(cxlrd);
> struct cxl_dev_state *cxlds = cxlmd->cxlds;
> int rc, part = READ_ONCE(cxled->part);
> @@ -3548,7 +3570,7 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
> return cxlr;
> }
>
> - rc = __construct_region(cxlr, cxled);
> + rc = __construct_region(cxlr, ctx);
> if (rc) {
> devm_release_action(port->uport_dev, unregister_region, cxlr);
> return ERR_PTR(rc);
> @@ -3572,13 +3594,17 @@ cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *range)
>
> int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
> {
> - struct range *range = &cxled->cxld.hpa_range;
> + struct cxl_region_context ctx;
> struct cxl_region_params *p;
> bool attach = false;
> int rc;
>
> + rc = setup_region_params(cxled, &ctx);
> + if (rc)
> + return rc;
> +
> struct cxl_root_decoder *cxlrd __free(put_cxl_root_decoder) =
> - cxl_find_root_decoder(cxled, range);
> + cxl_find_root_decoder(cxled, &ctx.hpa_range);
> if (!cxlrd)
> return -ENXIO;
>
> @@ -3589,9 +3615,9 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
> */
> mutex_lock(&cxlrd->range_lock);
> struct cxl_region *cxlr __free(put_cxl_region) =
> - cxl_find_region_by_range(cxlrd, range);
> + cxl_find_region_by_range(cxlrd, &ctx.hpa_range);
> if (!cxlr)
> - cxlr = construct_region(cxlrd, cxled);
> + cxlr = construct_region(cxlrd, &ctx);
> mutex_unlock(&cxlrd->range_lock);
>
> rc = PTR_ERR_OR_ZERO(cxlr);
On 12.09.25 14:10:06, Dave Jiang wrote:
>
>
> On 9/12/25 7:45 AM, Robert Richter wrote:
> > To construct a region, the region parameters such as address range and
> > interleaving config need to be determined. This is done while
> > constructing the region by inspecting the endpoint decoder
> > configuration. The endpoint decoder is passed as a function argument.
> >
> > With address translation the endpoint decoder data is no longer
> > sufficient to extract the region parameters as some of the information
> > is obtained using other methods such as using firmware calls.
> >
> > In a first step, separate code to determine and setup the region
> > parameters from the region construction. Temporarily store all the
> > data to create the region in the new struct cxl_region_context. Add a
> > new function setup_region_parameters() to fill that struct and later
> > use it to construct the region. This simplifies the extension of the
> > function to support other methods needed, esp. to support address
> > translation.
> >
> > Patch is a prerequisite to implement address translation.
> >
> > Signed-off-by: Robert Richter <rrichter@amd.com>
> > ---
> > drivers/cxl/core/region.c | 50 +++++++++++++++++++++++++++++----------
> > 1 file changed, 38 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 106692f1e310..57697504410b 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -3414,6 +3414,26 @@ static int match_region_by_range(struct device *dev, const void *data)
> > return 0;
> > }
> >
> > +struct cxl_region_context {
> > + struct cxl_endpoint_decoder *cxled;
> > + struct cxl_memdev *cxlmd;
> > + struct range hpa_range;
> > + int interleave_ways;
> > + int interleave_granularity;
> > +};
> > +
> > +static int setup_region_params(struct cxl_endpoint_decoder *cxled,
> > + struct cxl_region_context *ctx)
> > +{
> > + ctx->cxled = cxled;
> > + ctx->cxlmd = cxled_to_memdev(cxled);
> > + ctx->hpa_range = cxled->cxld.hpa_range;
> > + ctx->interleave_ways = cxled->cxld.interleave_ways;
> > + ctx->interleave_granularity = cxled->cxld.interleave_granularity;
>
> You can init like this:
>
> *ctx = (struct cxl_region_context) {
> .cxled = cxled,
> .cxlmd = cxled_to_memdev(cxled),
> .hpa_range = cxled->cxld.hpa_range,
> .interleave_ways = cxled->cxld.interleave_ways,
> .interleave_granularity = cxled->cxld.interleave_granularity,
> };
Will change that for readability and to zero-init possibly missing
members.
>
>
> > +
> > + return 0;
>
> Can probably make this function void if no expected errors and only assignments.
A later extension to the code may return an error code, so I prepared
the function interface already for this.
-Robert
>
> DJ
On 9/15/25 12:31 AM, Robert Richter wrote:
> On 12.09.25 14:10:06, Dave Jiang wrote:
>>
>>
>> On 9/12/25 7:45 AM, Robert Richter wrote:
>>> To construct a region, the region parameters such as address range and
>>> interleaving config need to be determined. This is done while
>>> constructing the region by inspecting the endpoint decoder
>>> configuration. The endpoint decoder is passed as a function argument.
>>>
>>> With address translation the endpoint decoder data is no longer
>>> sufficient to extract the region parameters as some of the information
>>> is obtained using other methods such as using firmware calls.
>>>
>>> In a first step, separate code to determine and setup the region
>>> parameters from the region construction. Temporarily store all the
>>> data to create the region in the new struct cxl_region_context. Add a
>>> new function setup_region_parameters() to fill that struct and later
>>> use it to construct the region. This simplifies the extension of the
>>> function to support other methods needed, esp. to support address
>>> translation.
>>>
>>> Patch is a prerequisite to implement address translation.
>>>
>>> Signed-off-by: Robert Richter <rrichter@amd.com>
>>> ---
>>> drivers/cxl/core/region.c | 50 +++++++++++++++++++++++++++++----------
>>> 1 file changed, 38 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>>> index 106692f1e310..57697504410b 100644
>>> --- a/drivers/cxl/core/region.c
>>> +++ b/drivers/cxl/core/region.c
>>> @@ -3414,6 +3414,26 @@ static int match_region_by_range(struct device *dev, const void *data)
>>> return 0;
>>> }
>>>
>>> +struct cxl_region_context {
>>> + struct cxl_endpoint_decoder *cxled;
>>> + struct cxl_memdev *cxlmd;
>>> + struct range hpa_range;
>>> + int interleave_ways;
>>> + int interleave_granularity;
>>> +};
>>> +
>>> +static int setup_region_params(struct cxl_endpoint_decoder *cxled,
>>> + struct cxl_region_context *ctx)
>>> +{
>>> + ctx->cxled = cxled;
>>> + ctx->cxlmd = cxled_to_memdev(cxled);
>>> + ctx->hpa_range = cxled->cxld.hpa_range;
>>> + ctx->interleave_ways = cxled->cxld.interleave_ways;
>>> + ctx->interleave_granularity = cxled->cxld.interleave_granularity;
>>
>> You can init like this:
>>
>> *ctx = (struct cxl_region_context) {
>> .cxled = cxled,
>> .cxlmd = cxled_to_memdev(cxled),
>> .hpa_range = cxled->cxld.hpa_range,
>> .interleave_ways = cxled->cxld.interleave_ways,
>> .interleave_granularity = cxled->cxld.interleave_granularity,
>> };
>
> Will change that for readability and to zero-init possibly missing
> members.
>
>>
>>
>>> +
>>> + return 0;
>>
>> Can probably make this function void if no expected errors and only assignments.
>
> A later extension to the code may return an error code, so I prepared
> the function interface already for this.
I realized that when I saw it in the later patch. So please ignore the comment.
>
> -Robert
>
>>
>> DJ
© 2016 - 2025 Red Hat, Inc.