Koralahalli Channabasappa, Smita wrote: [..] > > As I learned from Keith's recent CXL_PMEM dependency fix for CXL_ACPI > > [1], this wants to be: > > > > depends on DEV_DAX_HMEM || !DEV_DAX_HMEM > > depends on CXL_ACPI || !CXL_ACPI > > depends on CXL_PCI || !CXL_PCI > > > > ...to make sure that DEV_DAX_CXL can never be built-in unless all of its > > dependencies are built-in. > > > > [1]: http://lore.kernel.org/[email protected] > > > > At this point I am wondering if all of the feedback I have for this > > series should just be incremental fixes. I also want to have a canned > > unit test that verifies the base expectations. That can also be > > something I reply incrementally. > > Two things on the Kconfig change: > > When DEV_DAX_HMEM = y and CXL_ACPI = m and CXL_PCI = m
Right, this should not be possible. The patch I am testing moves the optional CXL dependencies to DEV_DAX_HMEM where they belong. I mistakenly showed them against DEV_DAX_CXL in my comment. > 1. Regarding switching from >= to || ! pattern: > > The >= pattern disabled DEV_DAX_CXL entirely when DEV_DAX_HMEM = y and > CXL_ACPI/CXL_PCI = m. So, HMEM unconditionally owned all ranges - the > CXL deferral path is never entered. That is one of the broken configurations to fix. It should never be possible to set DEV_DAX_HMEM=y unless CXL_ACPI and CXL_PCI are both disabled or both built-in. > When DEV_DAX_HMEM = y and CXL core is built as a module hmem.c calls > cxl_region_contains_resource() which lives in cxl_core.ko causing an > undefined reference at link time. Yes, I hit this as well and requires another CXL_BUS dependency.
