Re: [PATCH v2] cxl/hdm: Avoid incorrect DVSEC fallback when HDM decoders are enabled

[Koralahalli Channabasappa, Smita]

On 3/16/2026 5:08 PM, Dave Jiang wrote:
> On 3/16/26 1:19 PM, Smita Koralahalli wrote:
> > Check the global CXL_HDM_DECODER_ENABLE bit instead of looping over
> > per-decoder COMMITTED bits to determine whether to fall back to DVSEC
> > range emulation. When the HDM decoder capability is globally enabled,
> > ignore DVSEC range registers regardless of individual decoder commit
> > state.
> >
> > should_emulate_decoders() currently loops over per-decoder COMMITTED
> > bits, which leads to an incorrect DVSEC fallback when those bits are
> > zero. One way to trigger this is to destroy a region and bounce the
> > memdev:
> >
> >    cxl disable-region region0
> >    cxl destroy-region region0
> >    cxl disable-memdev mem0
> >    cxl enable-memdev mem0
> >
> > Region teardown zeroes the HDM decoder registers including the committed
> > bits. The subsequent memdev re-probe finds uncommitted decoders and falls
> > back to DVSEC emulation, even though HDM remains globally enabled.
> >
> > Observed failures:
> >
> >    should_emulate_decoders: cxl_port endpoint6: decoder6.0: committed: 0 base: 0x0_00000000 size: 0x0_00000000
> >    devm_cxl_setup_hdm: cxl_port endpoint6: Fallback map 1 range register
> >    ..
> >    devm_cxl_add_region: cxl_acpi ACPI0017:00: decoder0.0: created region0
> >    __construct_region: cxl_pci 0000:e1:00.0: mem1:decoder6.0:
> >    __construct_region region0 res: [mem 0x850000000-0x284fffffff flags 0x200] iw: 1 ig: 4096
> >    cxl region0: pci0000:e0:port1 cxl_port_setup_targets expected iw: 1 ig: 4096 ..
> >    cxl region0: pci0000:e0:port1 cxl_port_setup_targets got iw: 1 ig: 256 state: disabled ..
> >    cxl_port endpoint6: failed to attach decoder6.0 to region0: -6
> >    ..
> >    devm_cxl_add_region: cxl_acpi ACPI0017:00: decoder0.0: created region4
> >    alloc_hpa: cxl region4: HPA allocation error (-34) ..
> >
> > Fixes: 52cc48ad2a76 ("cxl/hdm: Limit emulation to the number of range registers")
> > Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@xxxxxxx>
> > Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>
> Smita,
>
> Applied to cxl/fixes
> 75cea0776de5 cxl/hdm: Avoid incorrect DVSEC fallback when HDM decoders are enabled
>
> Any chance to add the issue reproduce steps to a regression test script in CXL CLI test please? Thanks!

Thank you! Okay I will add it. That should be in ndctl/test?

Thanks
Smita

> > ---
> > v2:
> >    Reworded commit message to focus on root cause per Dan's feedback.
> >    Dropped downstream details (IG/construct_region/hpa_alloc)
> >    Added error signatures.
> >    Link to v1: https://lore.kernel.org/r/20260212223800.23624-1-Smita.KoralahalliChannabasappa@xxxxxxx
> > ---
> >   drivers/cxl/core/hdm.c | 25 +++++++++----------------
> >   1 file changed, 9 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index c222e98ae736..cb5d5a047a9d 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -94,7 +94,6 @@ static bool should_emulate_decoders(struct cxl_endpoint_dvsec_info *info)
> >   	struct cxl_hdm *cxlhdm;
> >   	void __iomem *hdm;
> >   	u32 ctrl;
> > -	int i;
> >   
> >   	if (!info)
> >   		return false;
> > @@ -113,22 +112,16 @@ static bool should_emulate_decoders(struct cxl_endpoint_dvsec_info *info)
> >   		return false;
> >   
> >   	/*
> > -	 * If any decoders are committed already, there should not be any
> > -	 * emulated DVSEC decoders.
> > +	 * If HDM decoders are globally enabled, do not fall back to DVSEC
> > +	 * range emulation. Zeroed decoder registers after region teardown
> > +	 * do not imply absence of HDM capability.
> > +	 *
> > +	 * Falling back to DVSEC here would treat the decoder as AUTO and
> > +	 * may incorrectly latch default interleave settings.
> >   	 */
> > -	for (i = 0; i < cxlhdm->decoder_count; i++) {
> > -		ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(i));
> > -		dev_dbg(&info->port->dev,
> > -			"decoder%d.%d: committed: %ld base: %#x_%.8x size: %#x_%.8x\n",
> > -			info->port->id, i,
> > -			FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl),
> > -			readl(hdm + CXL_HDM_DECODER0_BASE_HIGH_OFFSET(i)),
> > -			readl(hdm + CXL_HDM_DECODER0_BASE_LOW_OFFSET(i)),
> > -			readl(hdm + CXL_HDM_DECODER0_SIZE_HIGH_OFFSET(i)),
> > -			readl(hdm + CXL_HDM_DECODER0_SIZE_LOW_OFFSET(i)));
> > -		if (FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl))
> > -			return false;
> > -	}
> > +	ctrl = readl(hdm + CXL_HDM_DECODER_CTRL_OFFSET);
> > +	if (ctrl & CXL_HDM_DECODER_ENABLE)
> > +		return false;
> >   
> >   	return true;
> >   }
> >
> > base-commit: 93d0fcdddc9e7be9d4f42acbe57bc90dbb0fe75d