On 10/15/24 17:37, Dan Williams wrote:
Alejandro Lucero Palau wrote:
[..]
Then it makes sense that change. I'll do it if not already taken. I'll
send v4 without the PROBE_FORCE_SYNCHRONOUS flag and without the
previous loop with delays implemented in v3.
So I think EPROBE_DEFER can stay out of the CXL core because it is up to
the accelerator driver to decide whether CXL availability is fatal to
init or not.
It needs support from the cxl core though. If the cxl root is not there
yet, the driver needs to know, and that is what you did in your original
patch and I'm keeping as well.
So there are two ways, check if a registered @memdev has
@memdev->dev.driver set, assuming you know that the cxl_mem driver is
available, or call devm_cxl_enumerate_ports() yourself and skip the
cxl_mem driver indirection.
Setting @memdev->endpoint to ERR_PTR(-EPROBE_DEFER), as I originally
had, is an even more indirect way to convey a similar result and is
starting to feel a bit "mid-layer-y".
I was a bit confused with this answer until I read again the patch
commit from your original work.
The confusion came from my assumption about if the root device is not
there, it is due to the hardware root initialization requiring more
time. But I realize now you specifically said "the root driver has not
attached yet" what turns it into this problem of kernel modules not
loaded yet.
If so, I think I can solve this within the type2 driver code and
kconfig. Kconfig will force the driver being compiled as a module if the
cxl core is a module, and with MODULE_SOFTDEP("pre: cxl_core cxl_port
cxl_acpi cxl-mem) the type2 driver modprobe will trigger loading of
dependencies beforehand. This makes unnecessary EPROBE_DEFER.
What do you think?
Additionally, I am less and less convinced that Type-2 drivers should be
forced to depend on the cxl_mem driver to attach vs just arranging for
those Type-2 drivers to call devm_cxl_enumerate_ports() and
devm_cxl_add_endpoint() directly. In other words I am starting to worry
that the generic cxl_mem driver design pattern is a midlayer mistake.
You know better than me but in my view, a Type2 should follow what a
Type3 does with some small changes for dealing with the differences,
mainly the way it is going to be used and those optional capabilities
for Type2. This makes more sense to me for Type1.
If an accelerator driver *wants* to depend on cxl_mem, it can, but all
the cxl_core functions that cxl_mem uses are also exported for
accelerator drivers to use directly to avoid needing to guess about when
/ if cxl_mem is going to attach.
So, if it makes it easier for sfc, I would be open to exploring a direct
scheme for endpoint attachment, and not requiring the generic cxl_mem
driver as an intermediary.
V4 is ready (just having problems when testing with 6.12-rcX) so I would
like to keep it and explore this once we have something working and
accepted. Type2 and Type1 with CXL cache will bring new challenges and I
bet we will need refactoring in the code and potentially new design for
generic code (Type3 and Type2, Type2 and Type1).
Yeah, no need to switch horses mid-race if you already have a cxl_mem
dependent approach nearly ready, but a potential simplification to
explore going forward.
