Re: [PATCH] cxl/hdm: Avoid DVSEC fallback after region teardown

[Koralahalli Channabasappa, Smita]

Hi Dan,

On 3/10/2026 8:22 PM, Dan Williams wrote:
> Smita Koralahalli wrote:
> > After destroy-region, cxl_region_decode_reset() clears the HDM decoder
> > registers (base/size/commit). If the memdev is subsequently bounced
> > (disable/enable), port probe re-evaluates decoder capability via
> > should_emulate_decoders().
>
> I do not think this bug is limited to "after destroy region". Simply, if
> the driver sees that the HDM capability is enabled before the driver
> loads it indicates that whomever left that configuration also intended
> for DVSEC range registers to be ignored.
>
> > The existing logic checks each decoder's COMMITTED bit. Since those bits
> > are cleared by region teardown, should_emulate_decoders() incorrectly
> > falls back to DVSEC range emulation, even though HDM capability is still
> > present.
>
> This is 2 separate bugs, right? Bug 1 destroying the register
> configuration of auto-assembled regions, fixed by your pending patch.
> Bug 2 destroying auto-assembled regions does not clean up DVSEC range
> registers making it look like those are set when HDM decode capability
> is disabled. That unintentionally / falsely mimics CXL 1.1 platform
> firmware behavior triggering should_emulate_decoders().
>
> > DVSEC fallback marks the endpoint decoder as AUTO, which triggers
> > cxl_add_to_region() -> construct_region(). That path copies the default
> > interleave_granularity (4096) into the region parameters. The resulting
> > spurious autodiscovered region consumes the CFMWS HPA space and causes a
> > subsequent create-region to fail in hpa_alloc().
>
> I do not think this part is relevant to the fix, right? Once DVSEC is
> being falsely used the rest is just knock-on-effects.
>
> > Use the global CXL_HDM_DECODER_ENABLE bit instead of per-decoder COMMITTED
> > bits to detect HDM capability. If the HDM decoder block is enabled,
>
> ...if the HDM block is enabled the state of the register is irrelevant.
>
> > registers indicate teardown, not absence of HDM support. This prevents the
> > unintended DVSEC fallback and subsequent region creation failure.
>
> I think this wants 2 patches. This one you have here to always trust HDM
> decoder on setup, and another patch to teardown non-auto
> should_emulate_decoders() DVSEC configurations. Those combined with your
> "do not teardown auto regions" should squash this crop of setup bugs.

Looking at this more closely for patch 2, if I'm not wrong, I think the 
problem is that cxl_setup_hdm_decoder_from_dvsec() is not setting up the 
interleave_granularity on the decoder. It inherits the default PAGE_SIZE 
(4096) from cxl_decoder_init(), and construct_region() then copies that 
stale value into the region params.

So even in a DVSEC emulation case, the IG would be whatever 
cxl_decoder_init() defaulted to, not what the actual configuration is right?

Is the second patch about fixing cxl_setup_hdm_decoder_from_dvsec() to 
properly set the interleave granularity?

Please correct me if my understanding is wrong here.

Thanks,
Smita