Add a helper to determine whether a given Soft Reserved memory range is
fully contained within the committed CXL region.
This helper provides a primitive for policy decisions in subsequent
patches such as co-ordination with dax_hmem to determine whether CXL has
fully claimed ownership of Soft Reserved memory ranges.
No functional changes are introduced by this patch.
Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
---
drivers/cxl/core/region.c | 29 +++++++++++++++++++++++++++++
drivers/cxl/cxl.h | 5 +++++
2 files changed, 34 insertions(+)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 45ee598daf95..9827a6dd3187 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3875,6 +3875,35 @@ static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
cxl_region_debugfs_poison_clear, "%llx\n");
+static int cxl_region_contains_sr_cb(struct device *dev, void *data)
+{
+ struct resource *res = data;
+ struct cxl_region *cxlr;
+ struct cxl_region_params *p;
+
+ if (!is_cxl_region(dev))
+ return 0;
+
+ cxlr = to_cxl_region(dev);
+ p = &cxlr->params;
+
+ if (p->state != CXL_CONFIG_COMMIT)
+ return 0;
+
+ if (!p->res)
+ return 0;
+
+ return resource_contains(p->res, res) ? 1 : 0;
+}
+
+bool cxl_region_contains_soft_reserve(const struct resource *res)
+{
+ guard(rwsem_read)(&cxl_rwsem.region);
+ return bus_for_each_dev(&cxl_bus_type, NULL, (void *)res,
+ cxl_region_contains_sr_cb) != 0;
+}
+EXPORT_SYMBOL_GPL(cxl_region_contains_soft_reserve);
+
static int cxl_region_can_probe(struct cxl_region *cxlr)
{
struct cxl_region_params *p = &cxlr->params;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index c796c3db36e0..b0ff6b65ea0b 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -906,6 +906,7 @@ struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
int cxl_add_to_region(struct cxl_endpoint_decoder *cxled);
struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
+bool cxl_region_contains_soft_reserve(const struct resource *res);
#else
static inline bool is_cxl_pmem_region(struct device *dev)
{
@@ -928,6 +929,10 @@ static inline u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint,
{
return 0;
}
+static inline bool cxl_region_contains_soft_reserve(const struct resource *res)
+{
+ return false;
+}
#endif
void cxl_endpoint_parse_cdat(struct cxl_port *port);
--
2.17.1
Smita Koralahalli wrote:
> Add a helper to determine whether a given Soft Reserved memory range is
> fully contained within the committed CXL region.
>
> This helper provides a primitive for policy decisions in subsequent
> patches such as co-ordination with dax_hmem to determine whether CXL has
> fully claimed ownership of Soft Reserved memory ranges.
>
> No functional changes are introduced by this patch.
>
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> ---
> drivers/cxl/core/region.c | 29 +++++++++++++++++++++++++++++
> drivers/cxl/cxl.h | 5 +++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 45ee598daf95..9827a6dd3187 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3875,6 +3875,35 @@ static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
> DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
> cxl_region_debugfs_poison_clear, "%llx\n");
>
> +static int cxl_region_contains_sr_cb(struct device *dev, void *data)
> +{
> + struct resource *res = data;
> + struct cxl_region *cxlr;
> + struct cxl_region_params *p;
> +
> + if (!is_cxl_region(dev))
> + return 0;
> +
> + cxlr = to_cxl_region(dev);
> + p = &cxlr->params;
> +
> + if (p->state != CXL_CONFIG_COMMIT)
> + return 0;
> +
> + if (!p->res)
> + return 0;
> +
> + return resource_contains(p->res, res) ? 1 : 0;
I suspect this is too precise and this should instead be
resource_overlaps(). Because, in the case where the platform is taking
liberties with the specification, there is a high likelihood that
driver's view of the region does not neatly contain the range published
in the memory map.
There is also the problem with the fact that firmware does not
necessarily need to split memory map entries on region boundaries. That
gets slightly better if this @res argument is the range boundary that
has been adjusted by HMAT, but still not a guarantee per the spec.
> +}
> +
> +bool cxl_region_contains_soft_reserve(const struct resource *res)
> +{
> + guard(rwsem_read)(&cxl_rwsem.region);
> + return bus_for_each_dev(&cxl_bus_type, NULL, (void *)res,
No, need to cast pointers to 'void *'.
On 1/27/2026 1:59 PM, dan.j.williams@intel.com wrote:
> Smita Koralahalli wrote:
>> Add a helper to determine whether a given Soft Reserved memory range is
>> fully contained within the committed CXL region.
>>
>> This helper provides a primitive for policy decisions in subsequent
>> patches such as co-ordination with dax_hmem to determine whether CXL has
>> fully claimed ownership of Soft Reserved memory ranges.
>>
>> No functional changes are introduced by this patch.
>>
>> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
>> ---
>> drivers/cxl/core/region.c | 29 +++++++++++++++++++++++++++++
>> drivers/cxl/cxl.h | 5 +++++
>> 2 files changed, 34 insertions(+)
>>
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 45ee598daf95..9827a6dd3187 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -3875,6 +3875,35 @@ static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
>> DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
>> cxl_region_debugfs_poison_clear, "%llx\n");
>>
>> +static int cxl_region_contains_sr_cb(struct device *dev, void *data)
>> +{
>> + struct resource *res = data;
>> + struct cxl_region *cxlr;
>> + struct cxl_region_params *p;
>> +
>> + if (!is_cxl_region(dev))
>> + return 0;
>> +
>> + cxlr = to_cxl_region(dev);
>> + p = &cxlr->params;
>> +
>> + if (p->state != CXL_CONFIG_COMMIT)
>> + return 0;
>> +
>> + if (!p->res)
>> + return 0;
>> +
>> + return resource_contains(p->res, res) ? 1 : 0;
>
> I suspect this is too precise and this should instead be
> resource_overlaps(). Because, in the case where the platform is taking
> liberties with the specification, there is a high likelihood that
> driver's view of the region does not neatly contain the range published
> in the memory map.
>
> There is also the problem with the fact that firmware does not
> necessarily need to split memory map entries on region boundaries. That
> gets slightly better if this @res argument is the range boundary that
> has been adjusted by HMAT, but still not a guarantee per the spec.
Hmm, My reading of your earlier guidance was that this logic is meant to
be coarse, and is acceptable to fall back to HMEM if firmware
descriptions don’t line up cleanly.
If firmware takes liberties and publishes ranges that don’t neatly
contain inside a committed region resource, my assumption was that
failing the containment check and falling back is acceptable.
However, given that the SR ranges HMEM walks are already filtered by
ACPI HMAT, and that there is a relatively low likelihood that a single
HMAT range spans multiple committed CXL regions, it would be sufficient
to treat any overlap with a committed region as acceptable?
If so, the description in patch 6 should also be updated accordingly,
the wording around fully contained would need to be relaxed to reflect
overlap rather than strict containment.
>
>> +}
>> +
>> +bool cxl_region_contains_soft_reserve(const struct resource *res)
>> +{
>> + guard(rwsem_read)(&cxl_rwsem.region);
>> + return bus_for_each_dev(&cxl_bus_type, NULL, (void *)res,
>
> No, need to cast pointers to 'void *'.
Okay.
Thanks
Smita
Koralahalli Channabasappa, Smita wrote: [..] > > There is also the problem with the fact that firmware does not > > necessarily need to split memory map entries on region boundaries. That > > gets slightly better if this @res argument is the range boundary that > > has been adjusted by HMAT, but still not a guarantee per the spec. > > Hmm, My reading of your earlier guidance was that this logic is meant to > be coarse, and is acceptable to fall back to HMEM if firmware > descriptions don’t line up cleanly. > > If firmware takes liberties and publishes ranges that don’t neatly > contain inside a committed region resource, my assumption was that > failing the containment check and falling back is acceptable. > > However, given that the SR ranges HMEM walks are already filtered by > ACPI HMAT, and that there is a relatively low likelihood that a single > HMAT range spans multiple committed CXL regions, it would be sufficient > to treat any overlap with a committed region as acceptable? Oh, am I reading the polarity wrong...? /me reads patch 6. Yes, I was missing that cxl_contains_soft_reserve() is doing an "overlap" check and then cxl_region_contains_soft_reserve() is making sure it lines up exactly. So yes, the way you have it matches what I expected and I was confused reading patch 4 in isolation. I think cxl_contains_soft_reserve() might want to be named differently since it is validating that any overlap is precisely contained. Perhaps soft_reserve_has_cxl_match()?
On 1/21/26 9:55 PM, Smita Koralahalli wrote:
> Add a helper to determine whether a given Soft Reserved memory range is
> fully contained within the committed CXL region.
>
> This helper provides a primitive for policy decisions in subsequent
> patches such as co-ordination with dax_hmem to determine whether CXL has
> fully claimed ownership of Soft Reserved memory ranges.
>
> No functional changes are introduced by this patch.
>
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Just a nit below.
> ---
> drivers/cxl/core/region.c | 29 +++++++++++++++++++++++++++++
> drivers/cxl/cxl.h | 5 +++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 45ee598daf95..9827a6dd3187 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3875,6 +3875,35 @@ static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
> DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
> cxl_region_debugfs_poison_clear, "%llx\n");
>
> +static int cxl_region_contains_sr_cb(struct device *dev, void *data)
Since it's a local helper, maybe just call it region_contains_soft_reserve()?
DJ
> +{
> + struct resource *res = data;
> + struct cxl_region *cxlr;
> + struct cxl_region_params *p;
> +
> + if (!is_cxl_region(dev))
> + return 0;
> +
> + cxlr = to_cxl_region(dev);
> + p = &cxlr->params;
> +
> + if (p->state != CXL_CONFIG_COMMIT)
> + return 0;
> +
> + if (!p->res)
> + return 0;
> +
> + return resource_contains(p->res, res) ? 1 : 0;
> +}
> +
> +bool cxl_region_contains_soft_reserve(const struct resource *res)
> +{
> + guard(rwsem_read)(&cxl_rwsem.region);
> + return bus_for_each_dev(&cxl_bus_type, NULL, (void *)res,
> + cxl_region_contains_sr_cb) != 0;
> +}
> +EXPORT_SYMBOL_GPL(cxl_region_contains_soft_reserve);
> +
> static int cxl_region_can_probe(struct cxl_region *cxlr)
> {
> struct cxl_region_params *p = &cxlr->params;
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index c796c3db36e0..b0ff6b65ea0b 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -906,6 +906,7 @@ struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
> int cxl_add_to_region(struct cxl_endpoint_decoder *cxled);
> struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
> u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
> +bool cxl_region_contains_soft_reserve(const struct resource *res);
> #else
> static inline bool is_cxl_pmem_region(struct device *dev)
> {
> @@ -928,6 +929,10 @@ static inline u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint,
> {
> return 0;
> }
> +static inline bool cxl_region_contains_soft_reserve(const struct resource *res)
> +{
> + return false;
> +}
> #endif
>
> void cxl_endpoint_parse_cdat(struct cxl_port *port);
On 1/23/2026 2:19 PM, Dave Jiang wrote:
>
>
> On 1/21/26 9:55 PM, Smita Koralahalli wrote:
>> Add a helper to determine whether a given Soft Reserved memory range is
>> fully contained within the committed CXL region.
>>
>> This helper provides a primitive for policy decisions in subsequent
>> patches such as co-ordination with dax_hmem to determine whether CXL has
>> fully claimed ownership of Soft Reserved memory ranges.
>>
>> No functional changes are introduced by this patch.
>>
>> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>
> Just a nit below.
Thank you Jonathan and Dave for the review.
>
>> ---
>> drivers/cxl/core/region.c | 29 +++++++++++++++++++++++++++++
>> drivers/cxl/cxl.h | 5 +++++
>> 2 files changed, 34 insertions(+)
>>
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index 45ee598daf95..9827a6dd3187 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -3875,6 +3875,35 @@ static int cxl_region_debugfs_poison_clear(void *data, u64 offset)
>> DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
>> cxl_region_debugfs_poison_clear, "%llx\n");
>>
>> +static int cxl_region_contains_sr_cb(struct device *dev, void *data)
>
> Since it's a local helper, maybe just call it region_contains_soft_reserve()?
Okay I will rename in next revision.
Thanks
Smita
>
> DJ
>
>> +{
>> + struct resource *res = data;
>> + struct cxl_region *cxlr;
>> + struct cxl_region_params *p;
>> +
>> + if (!is_cxl_region(dev))
>> + return 0;
>> +
>> + cxlr = to_cxl_region(dev);
>> + p = &cxlr->params;
>> +
>> + if (p->state != CXL_CONFIG_COMMIT)
>> + return 0;
>> +
>> + if (!p->res)
>> + return 0;
>> +
>> + return resource_contains(p->res, res) ? 1 : 0;
>> +}
>> +
>> +bool cxl_region_contains_soft_reserve(const struct resource *res)
>> +{
>> + guard(rwsem_read)(&cxl_rwsem.region);
>> + return bus_for_each_dev(&cxl_bus_type, NULL, (void *)res,
>> + cxl_region_contains_sr_cb) != 0;
>> +}
>> +EXPORT_SYMBOL_GPL(cxl_region_contains_soft_reserve);
>> +
>> static int cxl_region_can_probe(struct cxl_region *cxlr)
>> {
>> struct cxl_region_params *p = &cxlr->params;
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index c796c3db36e0..b0ff6b65ea0b 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -906,6 +906,7 @@ struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
>> int cxl_add_to_region(struct cxl_endpoint_decoder *cxled);
>> struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
>> u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
>> +bool cxl_region_contains_soft_reserve(const struct resource *res);
>> #else
>> static inline bool is_cxl_pmem_region(struct device *dev)
>> {
>> @@ -928,6 +929,10 @@ static inline u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint,
>> {
>> return 0;
>> }
>> +static inline bool cxl_region_contains_soft_reserve(const struct resource *res)
>> +{
>> + return false;
>> +}
>> #endif
>>
>> void cxl_endpoint_parse_cdat(struct cxl_port *port);
>
On Thu, 22 Jan 2026 04:55:40 +0000 Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> wrote: > Add a helper to determine whether a given Soft Reserved memory range is > fully contained within the committed CXL region. > > This helper provides a primitive for policy decisions in subsequent > patches such as co-ordination with dax_hmem to determine whether CXL has > fully claimed ownership of Soft Reserved memory ranges. > > No functional changes are introduced by this patch. Given it's not used yet, I'd not have this line of description. We tend to say things like that on refactor patches. > > Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
On 1/22/2026 8:25 AM, Jonathan Cameron wrote: > On Thu, 22 Jan 2026 04:55:40 +0000 > Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> wrote: > >> Add a helper to determine whether a given Soft Reserved memory range is >> fully contained within the committed CXL region. >> >> This helper provides a primitive for policy decisions in subsequent >> patches such as co-ordination with dax_hmem to determine whether CXL has >> fully claimed ownership of Soft Reserved memory ranges. >> >> No functional changes are introduced by this patch. > > Given it's not used yet, I'd not have this line of description. > We tend to say things like that on refactor patches. Sure, let me fix this Thanks Smita > >> >> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> > Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com> >
© 2016 - 2026 Red Hat, Inc.