Vikram Sethi wrote: [..] > > It could certainly be done that way, but also seems like common > functionality, so wouldn't it be better to handle that in the > "core/bus" driver, rather than each endpoint driver to be bit banging > standard registers for standardized resets? Perhaps minimally some > exported functions that could be called by endpoint drivers. Right, when I say "endpoint driver reponsible" for something common like this it implies a common core library function that drivers can use in their reset handler. > Another thing I've been thinking about recently is what the > responsibilities of the CXL core/bus driver are around the equivalent > of PCIe Bus mastering enable (BME) and shutdown/kexec paths for > CXL.cache. It's been a while since I looked at that code, but IIRC for > PCIe, Root Port BME gets cleared as part of shutdown/kexec paths. BME does not clear on typical shutdown, but it does clear on kexec-reboot to make sure the new kernel is not surprised by in-flight DMA. Otherwise it assumes device reset will quiet DMA. > This can prevent crashes due to errant DMA in shutdown/kexec flows, > even if the endpoint driver didn't disable its own BME in its shutdown > callback. A CXL host bridge would need to disable both BME for CXL.IO, > and also CXL.cache for .cache capable devices. Unfortunately, the > naming and control of the ".cache disable" is a bit convoluted on the > CXL host bridge side and doesn't match the endpoint register naming. > The CXL "Root Port n security policy" register in the CXL Extended > Security capability structure allows for setting the Device trust > Level =2 which results in CXL.cache requests being aborted by the > host, which is roughly equivalent to RP BME disable on the PCIe/CXL.IO > side. > Do you agree this is something the core/bus driver must do since it is > controlling the host bridge/RP registers and the host must protect > itself against errant DMA from devices? I agree some self protection might be needed in the kexec case where there is no transfer of control back to the BIOS to perform the reset. > There may be other similar usecases. Just thought I'd bring it up, > that one can't purely think of .cache as an endpoint driver thing with > no services provided by the core. I can certainly see the point that > endpoint drivers must orchestrate their own standard controls by > calling common exported services provided by a common layer in their > own callbacks, which could include device side .cache disable and BME > disable as part of both shutdown and reset_prepare callbacks. Agree, there may be some cases where the core should manage common features that would be messy to distribute per-driver.
