Re: [PATCH v4 0/2] Support zero-sized HDM decoders

[Dan Williams (nvidia)]

Richard Cheng wrote:
> Hello,
> 
> This v4 continues Vishal Aslot's "Support zero-sized decoders" series [1]
> and addresses the v3 review of patch 1's port->hdm_end handling [2].
> 
> CXL r3.2 §8.2.4.20.12 and §14.13.10 permit committing an HDM decoder with
> size 0. BIOS commits and LOCKs such decoders to burn the trailing, unused
> slots so the OS cannot program regions through them, e.g. a Type 3 device
> in a Trusted Computing Base (TCB) established via the Trusted Security
> Protocol (TSP). init_hdm_decoder() rejected these with -ENXIO during port
> enumeration and aborted the whole port, so affected systems showed nothing
> under 'cxl list'.
> 
> Patch 1 enumerates the decoder into the topology with its HW-reported LOCK
> state and skips the DPA reservation it does not need.
> 
> On port->hdm_end (the v3 review): v3 advanced the watermark for the
> zero-size decoder. sashiko correctly noted the write was outside
> cxl_rwsem.dpa, and that advancing it without a balanced release strands
> hdm_end -- cxl_dpa_free() returns early on !dpa_res, so it can never be
> decremented past the zero-size id, breaking LIFO teardown of lower
> decoders. v4 therefore does not touch hdm_end at all. The in-order check
> in __cxl_dpa_reserve() is its only consumer and is never legitimately
> reached past such a decoder: the burned slots are trailing, so enumeration
> reserves no committed decoder after one, and the OS must not program a
> region through a locked slot. hdm_end stays at the last sized reservation,
> which is accurate. IMHO, if a non-trailing zero-size layout ever needs
> support, the check should key off commit_end rather than hdm_end,
> out of scope here.

I am not comfortable with this outcome. It assumes that zero-sized
decoders are always committed. I would much rather keep the meaning of
hdm_end as the marker of the last decoder set aside for a reservation.

Consider the case of a zero-sized decoder in the PMEM partition with the
RAM partition not fully allocated. That decoder would have a non-zero
skip and zero sized DPA allocation to arrange for the next decoder to
start at the beginning of the PMEM partition. Otherwise, it would seem
to need more special casing to understand which decoder is responsible
for carrying the skip.

I think the way to solve this is something like below (untested). It
keeps @hdm_end aligned with the available decoders, and tracks the start
of zero allocations relative to their skip. I believe it may also
address the Sashiko report.