[PATCH v5 4/7] cxl/region: Add helper to check Soft Reserved containment by CXL regions

Smita Koralahalli posted 7 patches 2 weeks, 4 days ago
[PATCH v5 4/7] cxl/region: Add helper to check Soft Reserved containment by CXL regions
Posted by Smita Koralahalli 2 weeks, 4 days ago
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
Re: [PATCH v5 4/7] cxl/region: Add helper to check Soft Reserved containment by CXL regions
Posted by dan.j.williams@intel.com 1 week, 6 days ago
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 *'.
Re: [PATCH v5 4/7] cxl/region: Add helper to check Soft Reserved containment by CXL regions
Posted by Koralahalli Channabasappa, Smita 1 week, 5 days ago
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

Re: [PATCH v5 4/7] cxl/region: Add helper to check Soft Reserved containment by CXL regions
Posted by dan.j.williams@intel.com 1 week, 5 days ago
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()?
Re: [PATCH v5 4/7] cxl/region: Add helper to check Soft Reserved containment by CXL regions
Posted by Dave Jiang 2 weeks, 3 days ago

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);
Re: [PATCH v5 4/7] cxl/region: Add helper to check Soft Reserved containment by CXL regions
Posted by Koralahalli Channabasappa, Smita 2 weeks, 1 day ago
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);
>
Re: [PATCH v5 4/7] cxl/region: Add helper to check Soft Reserved containment by CXL regions
Posted by Jonathan Cameron 2 weeks, 4 days ago
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>
Re: [PATCH v5 4/7] cxl/region: Add helper to check Soft Reserved containment by CXL regions
Posted by Koralahalli Channabasappa, Smita 1 week, 6 days ago
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>
>