[PATCH v5 3/7] cxl/region: Skip decoder reset on detach for autodiscovered regions

Smita Koralahalli posted 7 patches 2 weeks, 4 days ago
[PATCH v5 3/7] cxl/region: Skip decoder reset on detach for autodiscovered regions
Posted by Smita Koralahalli 2 weeks, 4 days ago
__cxl_decoder_detach() currently resets decoder programming whenever a
region is detached if cxl_config_state is beyond CXL_CONFIG_ACTIVE. For
autodiscovered regions, this can incorrectly tear down decoder state
that may be relied upon by other consumers or by subsequent ownership
decisions.

Skip cxl_region_decode_reset() during detach when CXL_REGION_F_AUTO is
set.

Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
---
 drivers/cxl/core/region.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index ae899f68551f..45ee598daf95 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2178,7 +2178,9 @@ __cxl_decoder_detach(struct cxl_region *cxlr,
 		cxled->part = -1;
 
 	if (p->state > CXL_CONFIG_ACTIVE) {
-		cxl_region_decode_reset(cxlr, p->interleave_ways);
+		if (!test_bit(CXL_REGION_F_AUTO, &cxlr->flags))
+			cxl_region_decode_reset(cxlr, p->interleave_ways);
+
 		p->state = CXL_CONFIG_ACTIVE;
 	}
 
-- 
2.17.1
Re: [PATCH v5 3/7] cxl/region: Skip decoder reset on detach for autodiscovered regions
Posted by Dave Jiang 2 weeks, 3 days ago

On 1/21/26 9:55 PM, Smita Koralahalli wrote:
> __cxl_decoder_detach() currently resets decoder programming whenever a
> region is detached if cxl_config_state is beyond CXL_CONFIG_ACTIVE. For
> autodiscovered regions, this can incorrectly tear down decoder state
> that may be relied upon by other consumers or by subsequent ownership
> decisions.
> 
> Skip cxl_region_decode_reset() during detach when CXL_REGION_F_AUTO is
> set.
> 
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>


> ---
>  drivers/cxl/core/region.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index ae899f68551f..45ee598daf95 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2178,7 +2178,9 @@ __cxl_decoder_detach(struct cxl_region *cxlr,
>  		cxled->part = -1;
>  
>  	if (p->state > CXL_CONFIG_ACTIVE) {
> -		cxl_region_decode_reset(cxlr, p->interleave_ways);
> +		if (!test_bit(CXL_REGION_F_AUTO, &cxlr->flags))
> +			cxl_region_decode_reset(cxlr, p->interleave_ways);
> +
>  		p->state = CXL_CONFIG_ACTIVE;
>  	}
>
Re: [PATCH v5 3/7] cxl/region: Skip decoder reset on detach for autodiscovered regions
Posted by Alejandro Lucero Palau 2 weeks, 3 days ago
On 1/22/26 04:55, Smita Koralahalli wrote:
> __cxl_decoder_detach() currently resets decoder programming whenever a
> region is detached if cxl_config_state is beyond CXL_CONFIG_ACTIVE. For
> autodiscovered regions, this can incorrectly tear down decoder state
> that may be relied upon by other consumers or by subsequent ownership
> decisions.
>
> Skip cxl_region_decode_reset() during detach when CXL_REGION_F_AUTO is
> set.
>
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> ---
>   drivers/cxl/core/region.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index ae899f68551f..45ee598daf95 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2178,7 +2178,9 @@ __cxl_decoder_detach(struct cxl_region *cxlr,
>   		cxled->part = -1;
>   
>   	if (p->state > CXL_CONFIG_ACTIVE) {
> -		cxl_region_decode_reset(cxlr, p->interleave_ways);
> +		if (!test_bit(CXL_REGION_F_AUTO, &cxlr->flags))
> +			cxl_region_decode_reset(cxlr, p->interleave_ways);
> +
>   		p->state = CXL_CONFIG_ACTIVE;
>   	}
>   


I think for some Type2 drivers this should not be the case, and some 
kind of leverage should be implemented, but I guess this is enough with 
the current supported cases, so;


Reviewed-by: Alejandro Lucero <alucerop@amd.com>
Re: [PATCH v5 3/7] cxl/region: Skip decoder reset on detach for autodiscovered regions
Posted by Jonathan Cameron 2 weeks, 4 days ago
On Thu, 22 Jan 2026 04:55:39 +0000
Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> wrote:

> __cxl_decoder_detach() currently resets decoder programming whenever a
> region is detached if cxl_config_state is beyond CXL_CONFIG_ACTIVE. For
> autodiscovered regions, this can incorrectly tear down decoder state
> that may be relied upon by other consumers or by subsequent ownership
> decisions.
> 
> Skip cxl_region_decode_reset() during detach when CXL_REGION_F_AUTO is
> set.
> 
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
purely on basis we should probably not undo things we didn't do in the
first place.

J
Re: [PATCH v5 3/7] cxl/region: Skip decoder reset on detach for autodiscovered regions
Posted by Koralahalli Channabasappa, Smita 2 weeks ago
Hi Jonathan,

On 1/22/2026 8:18 AM, Jonathan Cameron wrote:
> On Thu, 22 Jan 2026 04:55:39 +0000
> Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> wrote:
> 
>> __cxl_decoder_detach() currently resets decoder programming whenever a
>> region is detached if cxl_config_state is beyond CXL_CONFIG_ACTIVE. For
>> autodiscovered regions, this can incorrectly tear down decoder state
>> that may be relied upon by other consumers or by subsequent ownership
>> decisions.
>>
>> Skip cxl_region_decode_reset() during detach when CXL_REGION_F_AUTO is
>> set.
>>
>> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> purely on basis we should probably not undo things we didn't do in the
> first place.
> 
> J

Thank you!

I’m re reading Dan’s note here:
https://lore.kernel.org/all/6930dacd6510f_198110020@dwillia2-mobl4.notmuch/

Specifically this part:
"If the administrator actually wants to destroy and reclaim that
physical address space then they need to forcefully de-commit that
auto-assembled region via the @commit sysfs attribute. So that means
commit_store() needs to clear CXL_REGION_F_AUTO to get the decoder reset
to happen."

Today the sysfs commit=0 path inside commit_store() resets decoders 
without the AUTO check whereas the detach path now skips the reset when 
CXL_REGION_F_AUTO is set.

I think the same rationale should apply to the sysfs de-commit path as 
well? I’m trying to understand the implications of not guarding the 
reset with AUTO in commit_store().

Thanks
Smita

Re: [PATCH v5 3/7] cxl/region: Skip decoder reset on detach for autodiscovered regions
Posted by dan.j.williams@intel.com 1 week, 6 days ago
Koralahalli Channabasappa, Smita wrote:
[..]
> I’m re reading Dan’s note here:
> https://lore.kernel.org/all/6930dacd6510f_198110020@dwillia2-mobl4.notmuch/
> 
> Specifically this part:
> "If the administrator actually wants to destroy and reclaim that
> physical address space then they need to forcefully de-commit that
> auto-assembled region via the @commit sysfs attribute. So that means
> commit_store() needs to clear CXL_REGION_F_AUTO to get the decoder reset
> to happen."
> 
> Today the sysfs commit=0 path inside commit_store() resets decoders 
> without the AUTO check whereas the detach path now skips the reset when 
> CXL_REGION_F_AUTO is set.
> 
> I think the same rationale should apply to the sysfs de-commit path as 
> well? I’m trying to understand the implications of not guarding the 
> reset with AUTO in commit_store().

Linux tends to give the administrator the ability to know better than the
kernel. So if the root forcefully decommits the region, root gets to
keep the pieces.
Re: [PATCH v5 3/7] cxl/region: Skip decoder reset on detach for autodiscovered regions
Posted by Alejandro Lucero Palau 1 week, 5 days ago
On 1/27/26 23:37, dan.j.williams@intel.com wrote:
> Koralahalli Channabasappa, Smita wrote:
> [..]
>> I’m re reading Dan’s note here:
>> https://lore.kernel.org/all/6930dacd6510f_198110020@dwillia2-mobl4.notmuch/
>>
>> Specifically this part:
>> "If the administrator actually wants to destroy and reclaim that
>> physical address space then they need to forcefully de-commit that
>> auto-assembled region via the @commit sysfs attribute. So that means
>> commit_store() needs to clear CXL_REGION_F_AUTO to get the decoder reset
>> to happen."
>>
>> Today the sysfs commit=0 path inside commit_store() resets decoders
>> without the AUTO check whereas the detach path now skips the reset when
>> CXL_REGION_F_AUTO is set.
>>
>> I think the same rationale should apply to the sysfs de-commit path as
>> well? I’m trying to understand the implications of not guarding the
>> reset with AUTO in commit_store().
> Linux tends to give the administrator the ability to know better than the
> kernel. So if the root forcefully decommits the region, root gets to
> keep the pieces.


I have been trying to figure out how to preserve the decoders for Type2 
auto discover regions since, I think, this was demanded after v22 sent 
upstream. This patch/change is what I was looking for, and although I 
did implement it in another way requiring "consensus", this one seems 
good enough and already discussed and approved, so all good.


However, I think it would be also interesting to give the Type2 driver 
the option of resetting decoders as well, what I have been using for v22 
and successfully tested. But this change will preclude that other 
possibility, so, what about an option for clearing CXL_REGION_F_AUTO by 
Type2 drivers? If you want this only to be done by admin/root, I guess a 
module param would do it.


Re: [PATCH v5 3/7] cxl/region: Skip decoder reset on detach for autodiscovered regions
Posted by dan.j.williams@intel.com 1 week, 5 days ago
Alejandro Lucero Palau wrote:
[..]
> I have been trying to figure out how to preserve the decoders for Type2 
> auto discover regions since, I think, this was demanded after v22 sent 
> upstream. This patch/change is what I was looking for, and although I 
> did implement it in another way requiring "consensus", this one seems 
> good enough and already discussed and approved, so all good.
> 
> 
> However, I think it would be also interesting to give the Type2 driver 
> the option of resetting decoders as well, what I have been using for v22 
> and successfully tested. But this change will preclude that other 
> possibility, so, what about an option for clearing CXL_REGION_F_AUTO by 
> Type2 drivers? If you want this only to be done by admin/root, I guess a 
> module param would do it.

The expecation is that as long as "Fixed Device Configuration" is not
set then reconfiguration is possible. The best case is BIOS does not
create the ambiguity and leaves accelerators alone. The hard part is
that HPA space going back into a general CXL pool for other regions when
it is really earmarked for a singular use case.

So yes, a future helper for accelerator drivers to reclaim decoder
ownership from the platform seems reasonable, but the first hope is to
just have a BIOS switch / change to stop mapping accelerators and let
the OS handle it.