[PATCH v4] cxl/pci: Skip reset detection for DVSEC emulated decoders

[PATCH v4] cxl/pci: Skip reset detection for DVSEC emulated decoders

[Guixin Liu]

After an FLR or SBR, __cxl_endpoint_decoder_reset_detected() samples the
Committed bit at cxlhdm->regs.hdm_decoder for every decoder that has
CXL_DECODER_F_ENABLE set. Decoders emulated from the CXL DVSEC range
registers carry that flag too, but their state does not live in the HDM
decoder registers. When the component registers expose no HDM decoder
capability, regs.hdm_decoder is NULL and the readl() oopses in the reset
completion path. When the capability is present but firmware left
Mem_Enable set with the global HDM decoder enable clear, the Committed bit
reads zero and cxl_reset_done() reports a reset that never happened: it
taints the kernel and strips CXL_DECODER_F_ENABLE and CXL_DECODER_F_LOCK
from every endpoint decoder, even though the driver still cannot reprogram
the DVSEC ranges.

Skip the check when cxld->commit is NULL. Only
cxl_setup_hdm_decoder_from_dvsec() leaves that callback unset, so that one
test covers both emulation paths, and a DVSEC-described range has no
Committed bit for a reset to clear.

Fixes: 934edcd436dc ("cxl: Add post-reset warning if reset results in loss of previously committed HDM decoders")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
This was patch 4/8 of the "cxl: Assorted fixes" series [1], resent
individually per review feedback.

The two synchronisation concerns the review bot raised on v2 - the missing
exclusion against a cxl_port unbind freeing the devm allocated cxl_hdm, and
the plain read-modify-write of cxld->flags in
cxl_endpoint_decoder_clear_reset_flags() while the region paths update the
same word under cxl_rwsem.region - are still untouched here. Both are
about the reset handler's locking rather than about which registers it
reads, and neither fix is local; happy to follow up with separate patches.

v1->v2:
- rebase onto cxl/next
- rewrite the commit message to describe the behaviour rather than narrate
  the code change (Alison Schofield)

v2->v3:
- test cxld->commit instead of cxlhdm->regs.hdm_decoder, so that decoders
  emulated from the DVSEC ranges are also skipped when the HDM decoder
  registers exist but are globally disabled (Richard Cheng)
- update the subject and the commit message for the widened scope

v3->v4:
- shorten the commit message (Dave Jiang)

[1] https://lore.kernel.org/linux-cxl/20260811113608.2815625-1-kanie@linux.alibaba.com/

 drivers/cxl/core/pci.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 9d807c1a002c..d8b07f86bab0 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -683,6 +683,13 @@ static int __cxl_endpoint_decoder_reset_detected(struct device *dev, void *data)
 	if ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)
 		return 0;
 
+	/*
+	 * Decoders emulated from the DVSEC range registers have no commit
+	 * callback and no HDM decoder registers to consult.
+	 */
+	if (!cxld->commit)
+		return 0;
+
 	cxlhdm = dev_get_drvdata(&port->dev);
 	hdm = cxlhdm->regs.hdm_decoder;
 	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));

base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
-- 
2.43.7

Re: [PATCH v4] cxl/pci: Skip reset detection for DVSEC emulated decoders

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing exclusion against `cxl_port` unbind in `__cxl_endpoint_decoder_reset_detected()` can lead to a Use-After-Free or NULL pointer dereference of `cxlhdm`.
- [High] Unsynchronized read-modify-write of `cxld->flags` in `cxl_endpoint_decoder_clear_reset_flags()` races with concurrent updates.
--

--- Patch [1]: [PATCH v4] cxl/pci: Skip reset detection for DVSEC emulated decoders ---
commit b6f7c04ddbcccfb809033bdab75f996fc7c78db7
Author: Guixin Liu <kanie@linux.alibaba.com>
cxl/pci: Skip reset detection for DVSEC emulated decoders

This commit skips the reset detection check for decoders emulated from the
CXL DVSEC range registers, as they lack HDM decoder registers and commit
callbacks. This prevents a kernel oops when checking for a reset on these
emulated decoders.

Link: https://lore.kernel.org/linux-cxl/20260811113608.2815625-1-kanie@linux.alibaba.com/

> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 9d807c1a002ca..d8b07f86bab09 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -683,6 +683,13 @@ static int __cxl_endpoint_decoder_reset_detected(struct device *dev, void *data)
[ ... ]
> +	/*
> +	 * Decoders emulated from the DVSEC range registers have no commit
> +	 * callback and no HDM decoder registers to consult.
> +	 */
> +	if (!cxld->commit)
> +		return 0;
> +
>  	cxlhdm = dev_get_drvdata(&port->dev);
>  	hdm = cxlhdm->regs.hdm_decoder;

[Severity: High]
This is a pre-existing issue, but could this missing exclusion against a
cxl_port unbind lead to a Use-After-Free or NULL pointer dereference of
cxlhdm?

If a concurrent sysfs unbind of the cxl_port driver occurs while a PCI
reset handler iterates over the endpoint's children in cxl_reset_done(),
the devm-allocated memory for cxlhdm could be freed and driver data cleared
before it is accessed here.

>  	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
>  
>  	return !FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl);
>  }

For the pre-existing read-modify-write synchronization issue in
cxl_endpoint_decoder_clear_reset_flags():

drivers/cxl/pci.c:cxl_endpoint_decoder_clear_reset_flags() {
	...
	cxled = to_cxl_endpoint_decoder(dev);
	cxled->cxld.flags &= ~CXL_DECODER_F_RESET_MASK;
	...
}

[Severity: High]
This is a pre-existing issue, but does this function have an
unsynchronized read-modify-write of cxld->flags?

If a concurrent region setup or teardown operation occurs simultaneously with
a PCI reset, the plain bitwise update on cxld->flags in the asynchronous PCI
reset path ignores the cxl_rwsem.region semaphore used by region paths.

Could this race with concurrent updates and lead to state corruption due to
lost flag updates, potentially leaving decoders in an invalid state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821021029.2550584-1-kanie@linux.alibaba.com?part=1

Re: [PATCH v4] cxl/pci: Skip reset detection for DVSEC emulated decoders

[Jonathan Cameron]

On Fri, 21 Aug 2026 10:10:29 +0800
Guixin Liu <kanie@linux.alibaba.com> wrote:

> After an FLR or SBR, __cxl_endpoint_decoder_reset_detected() samples the
> Committed bit at cxlhdm->regs.hdm_decoder for every decoder that has
> CXL_DECODER_F_ENABLE set. Decoders emulated from the CXL DVSEC range
> registers carry that flag too, but their state does not live in the HDM
> decoder registers. When the component registers expose no HDM decoder
> capability, regs.hdm_decoder is NULL and the readl() oopses in the reset
> completion path. When the capability is present but firmware left
> Mem_Enable set with the global HDM decoder enable clear, the Committed bit
> reads zero and cxl_reset_done() reports a reset that never happened: it
> taints the kernel and strips CXL_DECODER_F_ENABLE and CXL_DECODER_F_LOCK
> from every endpoint decoder, even though the driver still cannot reprogram
> the DVSEC ranges.
> 
> Skip the check when cxld->commit is NULL. Only
> cxl_setup_hdm_decoder_from_dvsec() leaves that callback unset, so that one
> test covers both emulation paths, and a DVSEC-described range has no
> Committed bit for a reset to clear.
> 
This is still a lot of text to read - I'll have stab an what I think would be sufficient.
Take this perhaps as inspiration, not a precise suggestion!

When HDM decoders are emulated from DVSEC range registers either (a) there are
no HDM decoder registers present or (b) the DVSEC registers were in use at driver
load.

After FLR or SBR, __cxl_endpoint_decoder_reset_detected() checks the HDM
decoder committed bit for any previously committed decoders. This includes
emulated decoders: (a) results in a NULL pointer dereference, (b) in a false
detection of reset when they are present and not in use as the committed bit
was never set.

Use absence of cxld->commit to elide the reset check for emulated decoders.

> Fixes: 934edcd436dc ("cxl: Add post-reset warning if reset results in loss of previously committed HDM decoders")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>

This does make me wonder if we should be doing something similar to check
the dvsec based decoding reset.  The memory_base_high should reset to 0
for example.

Can we actually do anything with such devices?  Not sure we can today.
That is not a reason to crash however so this fix still makes sense.

Jonathan

> ---
> This was patch 4/8 of the "cxl: Assorted fixes" series [1], resent
> individually per review feedback.
> 

> The two synchronisation concerns the review bot raised on v2 - the missing
> exclusion against a cxl_port unbind freeing the devm allocated cxl_hdm, and
> the plain read-modify-write of cxld->flags in
> cxl_endpoint_decoder_clear_reset_flags() while the region paths update the
> same word under cxl_rwsem.region - are still untouched here. Both are
> about the reset handler's locking rather than about which registers it
> reads, and neither fix is local; happy to follow up with separate patches.
> 
> v1->v2:
> - rebase onto cxl/next
> - rewrite the commit message to describe the behaviour rather than narrate
>   the code change (Alison Schofield)
> 
> v2->v3:
> - test cxld->commit instead of cxlhdm->regs.hdm_decoder, so that decoders
>   emulated from the DVSEC ranges are also skipped when the HDM decoder
>   registers exist but are globally disabled (Richard Cheng)
> - update the subject and the commit message for the widened scope
> 
> v3->v4:
> - shorten the commit message (Dave Jiang)
> 
> [1] https://lore.kernel.org/linux-cxl/20260811113608.2815625-1-kanie@linux.alibaba.com/
> 
>  drivers/cxl/core/pci.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 9d807c1a002c..d8b07f86bab0 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -683,6 +683,13 @@ static int __cxl_endpoint_decoder_reset_detected(struct device *dev, void *data)
>  	if ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)
>  		return 0;
>  
> +	/*
> +	 * Decoders emulated from the DVSEC range registers have no commit
> +	 * callback and no HDM decoder registers to consult.
> +	 */
> +	if (!cxld->commit)
> +		return 0;
> +
>  	cxlhdm = dev_get_drvdata(&port->dev);
>  	hdm = cxlhdm->regs.hdm_decoder;
>  	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
> 
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07

Re: [PATCH v4] cxl/pci: Skip reset detection for DVSEC emulated decoders

[Jonathan Cameron]

On Fri, 21 Aug 2026 23:33:05 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 21 Aug 2026 10:10:29 +0800
> Guixin Liu <kanie@linux.alibaba.com> wrote:
> 
> > After an FLR or SBR, __cxl_endpoint_decoder_reset_detected() samples the
> > Committed bit at cxlhdm->regs.hdm_decoder for every decoder that has
> > CXL_DECODER_F_ENABLE set. Decoders emulated from the CXL DVSEC range
> > registers carry that flag too, but their state does not live in the HDM
> > decoder registers. When the component registers expose no HDM decoder
> > capability, regs.hdm_decoder is NULL and the readl() oopses in the reset
> > completion path. When the capability is present but firmware left
> > Mem_Enable set with the global HDM decoder enable clear, the Committed bit
> > reads zero and cxl_reset_done() reports a reset that never happened: it
> > taints the kernel and strips CXL_DECODER_F_ENABLE and CXL_DECODER_F_LOCK
> > from every endpoint decoder, even though the driver still cannot reprogram
> > the DVSEC ranges.
> > 
> > Skip the check when cxld->commit is NULL. Only
> > cxl_setup_hdm_decoder_from_dvsec() leaves that callback unset, so that one
> > test covers both emulation paths, and a DVSEC-described range has no
> > Committed bit for a reset to clear.
> >   
> This is still a lot of text to read - I'll have stab an what I think would be sufficient.
> Take this perhaps as inspiration, not a precise suggestion!
Ah I see this was the cut down text Dave suggested.
That's fine (though I think he could have gone further!)

Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

I think it is still worth thinking about the question on what it means
to try and recover below.  Though looking at how this is used, it
is just to complain that something crazy happened - so maybe none of this 
matters!

Jonathan


> 
> When HDM decoders are emulated from DVSEC range registers either (a) there are
> no HDM decoder registers present or (b) the DVSEC registers were in use at driver
> load.
> 
> After FLR or SBR, __cxl_endpoint_decoder_reset_detected() checks the HDM
> decoder committed bit for any previously committed decoders. This includes
> emulated decoders: (a) results in a NULL pointer dereference, (b) in a false
> detection of reset when they are present and not in use as the committed bit
> was never set.
> 
> Use absence of cxld->commit to elide the reset check for emulated decoders.
> 
> > Fixes: 934edcd436dc ("cxl: Add post-reset warning if reset results in loss of previously committed HDM decoders")
> > Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>  
> 
> This does make me wonder if we should be doing something similar to check
> the dvsec based decoding reset.  The memory_base_high should reset to 0
> for example.
> 
> Can we actually do anything with such devices?  Not sure we can today.
> That is not a reason to crash however so this fix still makes sense.
> 
> Jonathan
> 
> > ---
> > This was patch 4/8 of the "cxl: Assorted fixes" series [1], resent
> > individually per review feedback.
> >   
> 
> > The two synchronisation concerns the review bot raised on v2 - the missing
> > exclusion against a cxl_port unbind freeing the devm allocated cxl_hdm, and
> > the plain read-modify-write of cxld->flags in
> > cxl_endpoint_decoder_clear_reset_flags() while the region paths update the
> > same word under cxl_rwsem.region - are still untouched here. Both are
> > about the reset handler's locking rather than about which registers it
> > reads, and neither fix is local; happy to follow up with separate patches.
> > 
> > v1->v2:
> > - rebase onto cxl/next
> > - rewrite the commit message to describe the behaviour rather than narrate
> >   the code change (Alison Schofield)
> > 
> > v2->v3:
> > - test cxld->commit instead of cxlhdm->regs.hdm_decoder, so that decoders
> >   emulated from the DVSEC ranges are also skipped when the HDM decoder
> >   registers exist but are globally disabled (Richard Cheng)
> > - update the subject and the commit message for the widened scope
> > 
> > v3->v4:
> > - shorten the commit message (Dave Jiang)
> > 
> > [1] https://lore.kernel.org/linux-cxl/20260811113608.2815625-1-kanie@linux.alibaba.com/
> > 
> >  drivers/cxl/core/pci.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> > index 9d807c1a002c..d8b07f86bab0 100644
> > --- a/drivers/cxl/core/pci.c
> > +++ b/drivers/cxl/core/pci.c
> > @@ -683,6 +683,13 @@ static int __cxl_endpoint_decoder_reset_detected(struct device *dev, void *data)
> >  	if ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)
> >  		return 0;
> >  
> > +	/*
> > +	 * Decoders emulated from the DVSEC range registers have no commit
> > +	 * callback and no HDM decoder registers to consult.
> > +	 */
> > +	if (!cxld->commit)
> > +		return 0;
> > +
> >  	cxlhdm = dev_get_drvdata(&port->dev);
> >  	hdm = cxlhdm->regs.hdm_decoder;
> >  	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
> > 
> > base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07  
> 
>