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

["Koralahalli Channabasappa, Smita" <skoralah@amd.com>]

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