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

[Dan Williams]

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.