Re: [PATCH] cxl/region: Restore passthrough decoder enable on region re-assembly

[Richard Cheng]

On Mon, Jul 27, 2026 at 01:25:48PM +0800, Dave Jiang wrote:
> 
> 
> On 7/27/26 3:37 AM, Richard Cheng wrote:
> > After a CXL endpoint is PCI hot-removed and the bus rescanned, an
> > auto-discovered region fails to rebiuld and is lost, evne though the
> 
> rebuild
> even
>

Hi Dave,
Thanks for the review, I'll fix the typo in v2.
 
> > device's HDM decoder is still committed and decoding.
> 

I think this part is talking about physical remove/insert ? sorry maybe I commit message was too vague about the scenario.
The case here is a SW-only teardown via sysfs, this is what I did.

"""
$ echo 1 > /sys/bus/pci/devices/$BDF/remove
$ echo 1 > /sys/bus/pci/devices/$BDF/rescan
"""

No physical removal, no power cycle, no link-down, no reset. Linux drops and re-enumerates the same device, which never stopped running.
The endpoint HDM decoder is still committed.

The device is byte-identical before and after, the kernel doesn't clear it either, the decoder is locked and cxl_decoder_reset() returns
early for CXL_DECODER_F_LOCK before touching any register. The memory keeps decoding.

I don't think anything needs re-programming here? the only thing lost is kernel-side bookkeeping on the passthrough decoder, which is
freed with port and reallocated with F_ENABLE clear on rescan. The endpoint recovers its state from HW.
The passthrough decoder has no HW to recover from, that asymmetry is the bug.

> Also, a complete different device with possibly different size can be inserted. And the device showed up would be unconfigured. Given
> there's no BIOS to program the device since the OS has taken over, should it be still considered part of the auto-region?

Agreed that would be wrong, and I don't think this patch allows it. A different or freshly-inserted device fails the existing endpoint
checks, its own decoder won't come back COMMITTED with a matching HPA range, so cxl_add_to_region() won't re-assemble the auto-region
relardless of what the passthrough decoder's flag says. This patch guards on the decode config still matching (iw, ig, spa_maps_hap()),
and on !cxld->commit, so it can only ever touch a SW-only stub.

So the coverage here is SW-only, I should state it more clearly in v2.

Btw, the check "!cxld->commit" is doing a lot of implicit work in the code base, I think that's worth fixing regardless of the patch.
"commit == NULL" is not unique to passthrough decoders, cxl_setup_hdm_decoder_from_dvsec() also sets "commit = NULL" for DVSEC-emulated
RCD endpoint decoders, and root decoder never set it. My call happens to be safe, but that isn't visible from the condition.

I would suggest an inline helper like the following, do you think it's reasonable ?

"""
static inline bool cxl_decoder_is_passthrough(struct cxl_decoder *cxld)
{
    if (cxld->commit)
        return false;
    if (!is_switch_decoder(&cxld->dev) || is_root_decoder(&cxld->dev))
        return false;
    return to_cxl_switch_decoder(&cxld->dev)->nr_targets <= 1;
}
"""

There're about 4 call site today, and it distinguishes "passthrough" from "has no commit routine" , which are currently the same check
for different things.

Happy to send it as a separate patch and make this one a patch series if you prefer the shape.

Best regards,
Richard Cheng.

> Any thoughts Jonathan?
> 
> > 
> > A single-dport host bridge/root port has no HDM decoder capability, so
> > its switch decoder is a SW-only passthrough. Its CXL_DECODER_F_ENABLE
> > flag is cleared on region teardown and never restored on rescan, so
> > cxl_port_setup_targets() fails with -ENXIO.
> > 
> > Re-enable the passthrough decoder when its interleave and HPA config
> > still match the region, it holds no HW state.
> > 
> > Signed-off-by: Richard Cheng <icheng@xxxxxxxxxx>
> > ---
> >  drivers/cxl/core/region.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 1e211542b6b6..011ceb5bae79 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -1574,6 +1574,19 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> >  	}
> >  
> >  	if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) {
> > +		/*
> > +		 * A passthrough switch decoder holds no HW decode state.
> > +		 * It's CXL_DECODER_F_ENABLE flag is pure software bookkeeping
> > +		 * that is cleared when the region is torn down. On auto-discovery
> > +		 * re-assembly after a subsequent rescan the decode config still
> > +		 * matches the region, so restore the flag rather than fail to
> > +		 * rebuild a region that HW is in fact still decoding.
> > +		 */
> > +		if (!cxld->commit && cxld->interleave_ways == iw &&
> > +		    (iw <= 1 || cxld->interleave_granularity == ig) &&
> > +		    spa_maps_hpa(p, &cxld->hpa_range))
> > +			cxld->flags |= CXL_DECODER_F_ENABLE;
> > +
> >  		if (cxld->interleave_ways != iw ||
> >  		    (iw > 1 && cxld->interleave_granularity != ig) ||
> >  		    !spa_maps_hpa(p, &cxld->hpa_range) ||
> > 
> > base-commit: 4539944e515183668109bdf4d0c3d7d228383d88
>