Re: [PATCH v7 01/10] cxl/region: Add decoder check to check_commit_order()

[<dan.j.williams@intel.com>]

dan.j.williams@ wrote:
> Dave Jiang wrote:
> > check_commit_order() attempts to convert a device to a decoder without
> > making sure the device is a decoder. So far this has been working due
> > to pure luck
> 
> s/pure luck/cxl_port_attach_region() only handling switch port decoder
> assignment/
> 
> >. Issue discovered while doing deferred dport probing when
> > child ports are now in the midst of decoders due to ordering change
> 
> s/midst/mix/?
> 
> So I went looking to see if this change actually makes a difference in
> the end, or if this was just the result of debugging an interim state of
> the code that ended up passing non-switch decoders through this path.
> 
> The disinction matters for understanding code-flow and history here.
> Like something is wrong if this expectation is violated in some future
> refactoring and may even be worth putting a dev_WARN_ONCE() in this
> path.
> 
> In fact I put a dev_WARN_ONCE in this path with all the patches applied
> and it never triggers with a cxl test suite run, so I think it is safe
> to do something like the below.
> 
> ...but before going there, can you confirm the expectation that the
> safety is not strictly needed?

Ok, so I finally remembered the previous discussion here and switch
ports have both endpoint-port child devices and decoder child devices.

However, endpoint-ports are always added *after* decoders. That is
enforced by the fact that decoders are added in cxl_switch_port_probe()
and endpoint-ports serialize against that before adding new ports.

The device_for_each_child_reverse_from() staring from a decoder makes
sure to only see decoder children. So check_commit_order() seeing
endpoint ports is a bug not something that should be silently allowed.