Re: [PATCH v6 3/9] cxl/region: Skip decoder reset on detach for autodiscovered regions

[Alejandro Lucero Palau <alucerop@amd.com>]

On 3/18/26 21:27, Alison Schofield wrote:

<snip>

> As a step in the direction you suggest, AND  aiming to address Type2
> need, here is what I'd like a direction check on:
>
> Start separating decode-reset policy rom CXL_REGION_F_AUTO:
> - keep CXL_REGION_F_AUTO as origin / assembly semantics
> - introduce CXL_REGION_F_PRESERVE_DECODE as a region-scoped policy
> - initialize that policy from auto-assembly
> - clear it on explicit decommit in commit_store()
> - use it to gate cxl_region_decode_reset() in __cxl_decoder_detach()
>
> The decode-reset decision is factored through a small helper,
> cxl_region_preserve_decode(), so the policy can be extended independent
> of the detach mechanics. Maybe overkill in this simple case, but I
> wanted to acknowledge the 'policy' direction.


I like this approach which separates AUTO flag from this need.


>
> Compiled but not yet tested, pending a direction check:


I have tested it using the Type2 v24 and adding some debug lines for 
seeing the proper flag check works when decoder detach.


Maybe there are some other aspects of this approach I can not envision, 
but I'm happy with this change for current Type2 needs. Hopefully this 
plus v24 can go through before the next kernel window closes.


Thank you,

Alejandro


>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 42874948b589..f99e4aca72f0 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -432,6 +432,12 @@ static ssize_t commit_store(struct device *dev, struct device_attribute *attr,
>          if (rc)
>                  return rc;
>
> +       /*
> +        * Explicit decommit is destructive. Clear preserve bit before
> +        * unbinding so detach paths do not skip decoder reset.
> +        */
> +       clear_bit(CXL_REGION_F_PRESERVE_DECODE, &cxlr->flags);
> +
>          /*
>           * Unmap the region and depend the reset-pending state to ensure
>           * it does not go active again until post reset
> @@ -2153,6 +2159,12 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>          return 0;
>   }
>
> +/* Region-scoped policy for preserving decoder programming across detach */
> +static bool cxl_region_preserve_decode(struct cxl_region *cxlr)
> +{
> +       return test_bit(CXL_REGION_F_PRESERVE_DECODE, &cxlr->flags);
> +}
> +
>   static struct cxl_region *
>   __cxl_decoder_detach(struct cxl_region *cxlr,
>                       struct cxl_endpoint_decoder *cxled, int pos,
> @@ -2185,7 +2197,8 @@ __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 (!cxl_region_preserve_decode(cxlr))
> +                       cxl_region_decode_reset(cxlr, p->interleave_ways);
>                  p->state = CXL_CONFIG_ACTIVE;
>          }
>
> @@ -3833,6 +3846,7 @@ static int __construct_region(struct cxl_region *cxlr,
>          }
>
>          set_bit(CXL_REGION_F_AUTO, &cxlr->flags);
> +       set_bit(CXL_REGION_F_PRESERVE_DECODE, &cxlr->flags);
>          cxlr->hpa_range = *hpa_range;
>
>          res = kmalloc_obj(*res);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 9b947286eb9b..e6fbbee37252 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -532,6 +532,16 @@ enum cxl_partition_mode {
>    */
>   #define CXL_REGION_F_NORMALIZED_ADDRESSING 3
>
> +/*
> + * Indicate that decoder programming should be preserved when endpoint
> + * decoders detach from this region. This allows region decode state to
> + * survive endpoint removal and be recovered by subsequent enumeration.
> + * Automatic assembly may set this flag, and future userspace control
> + * may allow it to be set explicitly. Explicit region decommit should
> + * clear this flag before destructive cleanup.
> + */
> +#define CXL_REGION_F_PRESERVE_DECODE 4
> +
>   /**
>    * struct cxl_region - CXL region
>    * @dev: This region's device
>
>
>