dan.j.williams@ wrote:
> Smita Koralahalli wrote:
> > The current probe time ownership check for Soft Reserved memory based
> > solely on CXL window intersection is insufficient. dax_hmem probing is not
> > always guaranteed to run after CXL enumeration and region assembly, which
> > can lead to incorrect ownership decisions before the CXL stack has
> > finished publishing windows and assembling committed regions.
> >
> > Introduce deferred ownership handling for Soft Reserved ranges that
> > intersect CXL windows at probe time by scheduling deferred work from
> > dax_hmem and waiting for the CXL stack to complete enumeration and region
> > assembly before deciding ownership.
> >
> > Evaluate ownership of Soft Reserved ranges based on CXL region
> > containment.
> >
> > - If all Soft Reserved ranges are fully contained within committed CXL
> > regions, DROP handling Soft Reserved ranges from dax_hmem and allow
> > dax_cxl to bind.
> >
> > - If any Soft Reserved range is not fully claimed by committed CXL
> > region, tear down all CXL regions and REGISTER the Soft Reserved
> > ranges with dax_hmem instead.
> >
> > While ownership resolution is pending, gate dax_cxl probing to avoid
> > binding prematurely.
> >
> > This enforces a strict ownership. Either CXL fully claims the Soft
> > Reserved ranges or it relinquishes it entirely.
> >
> > Co-developed-by: Dan Williams <[email protected]>
> > Signed-off-by: Dan Williams <[email protected]>
> > Signed-off-by: Smita Koralahalli <[email protected]>
> > ---
> > drivers/cxl/core/region.c | 25 ++++++++++++
> > drivers/cxl/cxl.h | 2 +
> > drivers/dax/cxl.c | 9 +++++
> > drivers/dax/hmem/hmem.c | 81 ++++++++++++++++++++++++++++++++++++++-
> > 4 files changed, 115 insertions(+), 2 deletions(-)
> >
[..]
> > diff --git a/drivers/dax/cxl.c b/drivers/dax/cxl.c
> > index 13cd94d32ff7..b7e90d6dd888 100644
> > --- a/drivers/dax/cxl.c
> > +++ b/drivers/dax/cxl.c
> > @@ -14,6 +14,15 @@ static int cxl_dax_region_probe(struct device *dev)
> > struct dax_region *dax_region;
> > struct dev_dax_data data;
> >
> > + switch (dax_cxl_mode) {
> > + case DAX_CXL_MODE_DEFER:
> > + return -EPROBE_DEFER;
>
> So, I think this causes a mess because now you have 2 workqueues (driver
> core defer-queue and hmem work) competing to disposition this device.
> What this seems to want is to only run in the post "soft reserve
> dispositioned" world. Something like (untested!)
>
> diff --git a/drivers/dax/cxl.c b/drivers/dax/cxl.c
> index 13cd94d32ff7..1162495eb317 100644
> --- a/drivers/dax/cxl.c
> +++ b/drivers/dax/cxl.c
> @@ -14,6 +14,9 @@ static int cxl_dax_region_probe(struct device *dev)
> struct dax_region *dax_region;
> struct dev_dax_data data;
>
> + /* Make sure that dax_cxl_mode is stable, only runs once at boot */
> + flush_hmem_work();
> +
It occurs to me that this likely insta-hangs because
wait_for_device_probe() waits forever for itself to flush. So it may
need to be a scheme where the cxl_dax_region_driver registration does
something like this (untested!):
diff --git a/drivers/dax/cxl.c b/drivers/dax/cxl.c
index 13cd94d32ff7..6a1a38b4f64b 100644
--- a/drivers/dax/cxl.c
+++ b/drivers/dax/cxl.c
@@ -41,7 +41,32 @@ static struct cxl_driver cxl_dax_region_driver = {
},
};
-module_cxl_driver(cxl_dax_region_driver);
+static void cxl_dax_region_driver_register(struct work_struct *work)
+{
+ flush_hmem_work();
+ cxl_driver_register(&cxl_dax_region_driver);
+}
+
+static DECLARE_WORK(cxl_dax_region_driver_work,
cxl_dax_region_driver_register);
+
+static int __init cxl_dax_region_init(void)
+{
+ /*
+ * Need to resolve a race with dax_hmem wanting to drive regions
+ * instead of CXL
+ */
+ queue_work(system_long_wq, &cxl_dax_region_driver_work);
+ return 0;
+}
+module_init(cxl_dax_region_init);
+
+static void __exit cxl_dax_region_exit(void)
+{
+ flush_work(&cxl_dax_region_driver_work);
+ cxl_driver_unregister(&cxl_dax_region_driver);
+}
+module_exit(cxl_dax_region_exit);
+
MODULE_ALIAS_CXL(CXL_DEVICE_DAX_REGION);
MODULE_DESCRIPTION("CXL DAX: direct access to CXL regions");
MODULE_LICENSE("GPL");
