[PATCH v3 0/4] Add managed SOFT RESERVE resource handling

[PATCH v3 0/4] Add managed SOFT RESERVE resource handling

[Terry Bowman]

Add the ability to manage SOFT RESERVE iomem resources prior to them being
added to the iomem resource tree. This allows drivers, such as CXL, to
remove any pieces of the SOFT RESERVE resource that intersect with created
CXL regions.

The current approach of leaving the SOFT RESERVE resources as is can cause
failures during hotplug of devices, such as CXL, because the resource is
not available for reuse after teardown of the device.

The approach is to add SOFT RESERVE resources to a separate tree during
boot. This allows any drivers to update the SOFT RESERVE resources before
they are merged into the iomem resource tree. In addition a notifier chain
is added so that drivers can be notified when these SOFT RESERVE resources
are added to the ioeme resource tree.

The CXL driver is modified to use a worker thread that waits for the CXL
PCI and CXL mem drivers to be loaded and for their probe routine to
complete. Then the driver walks through any created CXL regions to trim any
intersections with SOFT RESERVE resources in the iomem tree.

The dax driver uses the new soft reserve notifier chain so it can consume
any remaining SOFT RESERVES once they're added to the iomem tree.

V3 updates:
 - Remove srmem resource tree from kernel/resource.c, this is no longer
   needed in the current implementation. All SOFT RESERVE resources now
   put on the iomem resource tree.
 - Remove the no longer needed SOFT_RESERVED_MANAGED kernel config option.
 - Add the 'nid' parameter back to hmem_register_resource();
 - Remove the no longer used soft reserve notification chain (introduced
   in v2). The dax driver is now notified of SOFT RESERVED resources by
   the CXL driver.

v2 updates:
 - Add config option SOFT_RESERVE_MANAGED to control use of the
   separate srmem resource tree at boot.
 - Only add SOFT RESERVE resources to the soft reserve tree during
   boot, they go to the iomem resource tree after boot.
 - Remove the resource trimming code in the previous patch to re-use
   the existing code in kernel/resource.c
 - Add functionality for the cxl acpi driver to wait for the cxl PCI
   and me drivers to load.

Nathan Fontenot (4):
  kernel/resource: Provide mem region release for SOFT RESERVES
  cxl: Update Soft Reserved resources upon region creation
  dax/mum: Save the dax mum platform device pointer
  cxl/dax: Delay consumption of SOFT RESERVE resources

 drivers/cxl/Kconfig        |  4 ---
 drivers/cxl/acpi.c         | 28 +++++++++++++++++++
 drivers/cxl/core/Makefile  |  2 +-
 drivers/cxl/core/region.c  | 34 ++++++++++++++++++++++-
 drivers/cxl/core/suspend.c | 41 ++++++++++++++++++++++++++++
 drivers/cxl/cxl.h          |  3 +++
 drivers/cxl/cxlmem.h       |  9 -------
 drivers/cxl/cxlpci.h       |  1 +
 drivers/cxl/pci.c          |  2 ++
 drivers/dax/hmem/device.c  | 47 ++++++++++++++++----------------
 drivers/dax/hmem/hmem.c    | 10 ++++---
 include/linux/dax.h        | 11 +++++---
 include/linux/ioport.h     |  3 +++
 include/linux/pm.h         |  7 -----
 kernel/resource.c          | 55 +++++++++++++++++++++++++++++++++++---
 15 files changed, 202 insertions(+), 55 deletions(-)


base-commit: aae0594a7053c60b82621136257c8b648c67b512
-- 
2.34.1

[PATCH v3 1/4] kernel/resource: Provide mem region release for SOFT RESERVES

[Terry Bowman]

From: Nathan Fontenot <nathan.fontenot@amd.com>

Add a release_Sam_region_adjustable() interface to allow for
removing SOFT RESERVE memory resources. This extracts out the code
to remove a mem region into a common __release_mem_region_adjustable()
routine, this routine takes additional parameters of an IORES
descriptor type to add checks for IORES_DESC_* and a flag to check
for IORESOURCE_BUSY to control it's behavior.

The existing release_mem_region_adjustable() is a front end to the
common code and a new release_srmem_region_adjustable() is added to
release SOFT RESERVE resources.

Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
---
 include/linux/ioport.h |  3 +++
 kernel/resource.c      | 55 +++++++++++++++++++++++++++++++++++++++---
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 5385349f0b8a..718360c9c724 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -357,6 +357,9 @@ extern void __release_region(struct resource *, resource_size_t,
 #ifdef CONFIG_MEMORY_HOTREMOVE
 extern void release_mem_region_adjustable(resource_size_t, resource_size_t);
 #endif
+#ifdef CONFIG_CXL_REGION
+extern void release_srmem_region_adjustable(resource_size_t, resource_size_t);
+#endif
 #ifdef CONFIG_MEMORY_HOTPLUG
 extern void merge_system_ram_resource(struct resource *res);
 #endif
diff --git a/kernel/resource.c b/kernel/resource.c
index 12004452d999..0195b31064b0 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1387,7 +1387,7 @@ void __release_region(struct resource *parent, resource_size_t start,
 }
 EXPORT_SYMBOL(__release_region);
 
-#ifdef CONFIG_MEMORY_HOTREMOVE
+#if defined(CONFIG_MEMORY_HOTREMOVE) || defined(CONFIG_CXL_REGION)
 /**
  * release_mem_region_adjustable - release a previously reserved memory region
  * @start: resource start address
@@ -1407,7 +1407,10 @@ EXPORT_SYMBOL(__release_region);
  *   assumes that all children remain in the lower address entry for
  *   simplicity.  Enhance this logic when necessary.
  */
-void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
+static void __release_mem_region_adjustable(resource_size_t start,
+					    resource_size_t size,
+					    bool busy_check,
+					    int res_desc)
 {
 	struct resource *parent = &iomem_resource;
 	struct resource *new_res = NULL;
@@ -1446,7 +1449,12 @@ void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
 		if (!(res->flags & IORESOURCE_MEM))
 			break;
 
-		if (!(res->flags & IORESOURCE_BUSY)) {
+		if (busy_check && !(res->flags & IORESOURCE_BUSY)) {
+			p = &res->child;
+			continue;
+		}
+
+		if (res_desc != IORES_DESC_NONE && res->desc != res_desc) {
 			p = &res->child;
 			continue;
 		}
@@ -1496,7 +1504,46 @@ void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
 	write_unlock(&resource_lock);
 	free_resource(new_res);
 }
-#endif	/* CONFIG_MEMORY_HOTREMOVE */
+#endif
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+/**
+ * release_mem_region_adjustable - release a previously reserved memory region
+ * @start: resource start address
+ * @size: resource region size
+ *
+ * This interface is intended for memory hot-delete.  The requested region
+ * is released from a currently busy memory resource.  The requested region
+ * must either match exactly or fit into a single busy resource entry.  In
+ * the latter case, the remaining resource is adjusted accordingly.
+ * Existing children of the busy memory resource must be immutable in the
+ * request.
+ *
+ * Note:
+ * - Additional release conditions, such as overlapping region, can be
+ *   supported after they are confirmed as valid cases.
+ * - When a busy memory resource gets split into two entries, the code
+ *   assumes that all children remain in the lower address entry for
+ *   simplicity.  Enhance this logic when necessary.
+ */
+void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
+{
+	return __release_mem_region_adjustable(start, size,
+					       true, IORES_DESC_NONE);
+}
+EXPORT_SYMBOL(release_mem_region_adjustable);
+#endif
+
+#ifdef CONFIG_CXL_REGION
+void release_srmem_region_adjustable(resource_size_t start,
+				     resource_size_t size)
+{
+	return __release_mem_region_adjustable(start, size,
+					       false, IORES_DESC_SOFT_RESERVED);
+}
+EXPORT_SYMBOL(release_srmem_region_adjustable);
+#endif
+
 
 #ifdef CONFIG_MEMORY_HOTPLUG
 static bool system_ram_resources_mergeable(struct resource *r1,
-- 
2.34.1

[PATCH v3 2/4] cxl: Update Soft Reserved resources upon region creation

[Terry Bowman]

From: Nathan Fontenot <nathan.fontenot@amd.com>

Update handling of SOFT RESERVE iomem resources that intersect with
CXL region resources to remove intersections from the SOFT RESERVE
resources. The current approach of leaving SOFT RESERVE resources as
is can cause failures during hotplug replace of CXL devices because
the resource is not available for reuse after teardown of the CXL device.

To accomplish this the cxl acpi driver creates a worker thread at the
end of cxl_acpi_probe(). This worker thread first waits for the CXL PCI
CXL mem drivers have loaded. The cxl core/suspend.c code is updated to
add a pci_loaded variable, in addition to the mem_active variable, that
is updated when the pci driver loads. Remove CONFIG_CXL_SUSPEND Kconfig as
it is no longer needed. A new cxl_wait_for_pci_mem() routine uses a
waitqueue for both these driver to be loaded. The need to add this
additional waitqueue is ensure the CXL PCI and CXL mem drivers have loaded
before we wait for their probe, without it the cxl acpi probe worker thread
calls wait_for_device_probe() before these drivers are loaded.

After the CXL PCI and CXL mem drivers load the cxl acpi worker thread
uses wait_for_device_probe() to ensure device probe routines have
completed.

Once probe completes and regions have been created, find all cxl
regions that have been created and trim any SOFT RESERVE resources
that intersect with the region.

Update cxl_acpi_exit() to cancel pending waitqueue work.

Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
---
 drivers/cxl/Kconfig        |  4 ----
 drivers/cxl/acpi.c         | 28 ++++++++++++++++++++++++++
 drivers/cxl/core/Makefile  |  2 +-
 drivers/cxl/core/region.c  | 24 +++++++++++++++++++++-
 drivers/cxl/core/suspend.c | 41 ++++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxl.h          |  3 +++
 drivers/cxl/cxlmem.h       |  9 ---------
 drivers/cxl/cxlpci.h       |  1 +
 drivers/cxl/pci.c          |  2 ++
 include/linux/pm.h         |  7 -------
 10 files changed, 99 insertions(+), 22 deletions(-)

diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index 205547e5543a..c7377956c1d5 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -117,10 +117,6 @@ config CXL_PORT
 	default CXL_BUS
 	tristate
 
-config CXL_SUSPEND
-	def_bool y
-	depends on SUSPEND && CXL_MEM
-
 config CXL_REGION
 	bool "CXL: Region Support"
 	default CXL_BUS
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index cb14829bb9be..94f2d649bb30 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -7,6 +7,8 @@
 #include <linux/acpi.h>
 #include <linux/pci.h>
 #include <linux/node.h>
+#include <linux/pm.h>
+#include <linux/workqueue.h>
 #include <asm/div64.h>
 #include "cxlpci.h"
 #include "cxl.h"
@@ -813,6 +815,27 @@ static int pair_cxl_resource(struct device *dev, void *data)
 	return 0;
 }
 
+static void cxl_srmem_work_fn(struct work_struct *work)
+{
+	/* Wait for CXL PCI and mem drivers to load */
+	cxl_wait_for_pci_mem();
+
+	/*
+	 * Once the CXL PCI and mem drivers have loaded wait
+	 * for the driver probe routines to complete.
+	 */
+	wait_for_device_probe();
+
+	cxl_region_srmem_update();
+}
+
+DECLARE_WORK(cxl_sr_work, cxl_srmem_work_fn);
+
+static void cxl_srmem_update(void)
+{
+	schedule_work(&cxl_sr_work);
+}
+
 static int cxl_acpi_probe(struct platform_device *pdev)
 {
 	int rc;
@@ -887,6 +910,10 @@ static int cxl_acpi_probe(struct platform_device *pdev)
 
 	/* In case PCI is scanned before ACPI re-trigger memdev attach */
 	cxl_bus_rescan();
+
+	/* Update SOFT RESERVED resources that intersect with CXL regions */
+	cxl_srmem_update();
+
 	return 0;
 }
 
@@ -918,6 +945,7 @@ static int __init cxl_acpi_init(void)
 
 static void __exit cxl_acpi_exit(void)
 {
+	cancel_work_sync(&cxl_sr_work);
 	platform_driver_unregister(&cxl_acpi_driver);
 	cxl_bus_drain();
 }
diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index 086df97a0fcf..035864db8a32 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_CXL_BUS) += cxl_core.o
-obj-$(CONFIG_CXL_SUSPEND) += suspend.o
+obj-y += suspend.o
 
 ccflags-y += -I$(srctree)/drivers/cxl
 CFLAGS_trace.o = -DTRACE_INCLUDE_PATH=. -I$(src)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index c3f4dc244df7..25d70175f204 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -10,6 +10,7 @@
 #include <linux/sort.h>
 #include <linux/idr.h>
 #include <linux/memory-tiers.h>
+#include <linux/ioport.h>
 #include <cxlmem.h>
 #include <cxl.h>
 #include "core.h"
@@ -2333,7 +2334,7 @@ const struct device_type cxl_region_type = {
 
 bool is_cxl_region(struct device *dev)
 {
-	return dev->type == &cxl_region_type;
+	return dev && dev->type == &cxl_region_type;
 }
 EXPORT_SYMBOL_NS_GPL(is_cxl_region, "CXL");
 
@@ -3443,6 +3444,27 @@ int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_add_to_region, "CXL");
 
+int cxl_region_srmem_update(void)
+{
+	struct device *dev = NULL;
+	struct cxl_region *cxlr;
+	struct resource *res;
+
+	do {
+		dev = bus_find_next_device(&cxl_bus_type, dev);
+		if (is_cxl_region(dev)) {
+			cxlr = to_cxl_region(dev);
+			res = cxlr->params.res;
+			release_srmem_region_adjustable(res->start,
+							resource_size(res));
+		}
+		put_device(dev);
+	} while (dev);
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_region_srmem_update, "CXL");
+
 u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa)
 {
 	struct cxl_region_ref *iter;
diff --git a/drivers/cxl/core/suspend.c b/drivers/cxl/core/suspend.c
index 29aa5cc5e565..4813641e1b7b 100644
--- a/drivers/cxl/core/suspend.c
+++ b/drivers/cxl/core/suspend.c
@@ -2,9 +2,14 @@
 /* Copyright(c) 2022 Intel Corporation. All rights reserved. */
 #include <linux/atomic.h>
 #include <linux/export.h>
+#include <linux/wait.h>
 #include "cxlmem.h"
+#include "cxlpci.h"
 
 static atomic_t mem_active;
+static atomic_t pci_loaded;
+
+static DECLARE_WAIT_QUEUE_HEAD(cxl_wait_queue);
 
 bool cxl_mem_active(void)
 {
@@ -14,6 +19,7 @@ bool cxl_mem_active(void)
 void cxl_mem_active_inc(void)
 {
 	atomic_inc(&mem_active);
+	wake_up(&cxl_wait_queue);
 }
 EXPORT_SYMBOL_NS_GPL(cxl_mem_active_inc, "CXL");
 
@@ -22,3 +28,38 @@ void cxl_mem_active_dec(void)
 	atomic_dec(&mem_active);
 }
 EXPORT_SYMBOL_NS_GPL(cxl_mem_active_dec, "CXL");
+
+void mark_cxl_pci_loaded(void)
+{
+	atomic_inc(&pci_loaded);
+	wake_up(&cxl_wait_queue);
+}
+EXPORT_SYMBOL_NS_GPL(mark_cxl_pci_loaded, "CXL");
+
+static bool cxl_pci_loaded(void)
+{
+	if (IS_ENABLED(CONFIG_CXL_PCI))
+		return atomic_read(&pci_loaded) != 0;
+
+	return true;
+}
+
+static bool cxl_mem_probed(void)
+{
+	if (IS_ENABLED(CONFIG_CXL_MEM))
+		return atomic_read(&mem_active) != 0;
+
+	return true;
+}
+
+void cxl_wait_for_pci_mem(void)
+{
+	if (IS_ENABLED(CONFIG_CXL_PCI) || IS_ENABLED(CONFIG_CXL_MEM))
+		if (wait_event_timeout(cxl_wait_queue,
+				       cxl_pci_loaded() && cxl_mem_probed(),
+				       30 * HZ)) {
+			pr_debug("Timeout waiting for CXL PCI or CXL Memory probing");
+		}
+
+}
+EXPORT_SYMBOL_NS_GPL(cxl_wait_for_pci_mem, "CXL");
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index be8a7dc77719..40835ec692c8 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -858,6 +858,7 @@ bool is_cxl_pmem_region(struct device *dev);
 struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
 int cxl_add_to_region(struct cxl_port *root,
 		      struct cxl_endpoint_decoder *cxled);
+int cxl_region_srmem_update(void);
 struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
 u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
 #else
@@ -902,6 +903,8 @@ void cxl_coordinates_combine(struct access_coordinate *out,
 
 bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
 
+void cxl_wait_for_pci_mem(void);
+
 /*
  * Unit test builds overrides this to __weak, find the 'strong' version
  * of these symbols in tools/testing/cxl/.
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 3ec6b906371b..1bd1e88c4cc0 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -853,17 +853,8 @@ int cxl_trigger_poison_list(struct cxl_memdev *cxlmd);
 int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa);
 int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa);
 
-#ifdef CONFIG_CXL_SUSPEND
 void cxl_mem_active_inc(void);
 void cxl_mem_active_dec(void);
-#else
-static inline void cxl_mem_active_inc(void)
-{
-}
-static inline void cxl_mem_active_dec(void)
-{
-}
-#endif
 
 int cxl_mem_sanitize(struct cxl_memdev *cxlmd, u16 cmd);
 
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 54e219b0049e..5a811ac63fcf 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -135,4 +135,5 @@ void read_cdat_data(struct cxl_port *port);
 void cxl_cor_error_detected(struct pci_dev *pdev);
 pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
 				    pci_channel_state_t state);
+void mark_cxl_pci_loaded(void);
 #endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 4288f4814cc5..b784008489b3 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1185,6 +1185,8 @@ static int __init cxl_pci_driver_init(void)
 	if (rc)
 		pci_unregister_driver(&cxl_pci_driver);
 
+	mark_cxl_pci_loaded();
+
 	return rc;
 }
 
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 78855d794342..11ff485c9722 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -35,14 +35,7 @@ static inline void pm_vt_switch_unregister(struct device *dev)
 }
 #endif /* CONFIG_VT_CONSOLE_SLEEP */
 
-#ifdef CONFIG_CXL_SUSPEND
 bool cxl_mem_active(void);
-#else
-static inline bool cxl_mem_active(void)
-{
-	return false;
-}
-#endif
 
 /*
  * Device power management
-- 
2.34.1

[PATCH v3 3/4] dax/mum: Save the dax mum platform device pointer

[Terry Bowman]

From: Nathan Fontenot <nathan.fontenot@amd.com>

In order to handle registering hmem devices for SOFT RESERVE
resources after the dax hmem device initialization occurs
we need to save a reference to the dax hmem platform device
that will be used in a following patch.

Saving the platform device pointer also allows us to clean
up the walk_hmem_resources() routine to no require the
struct device argument.

There should be no functional changes.

Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
---
 drivers/dax/hmem/device.c | 4 ++--
 drivers/dax/hmem/hmem.c   | 9 ++++++---
 include/linux/dax.h       | 5 ++---
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/dax/hmem/device.c b/drivers/dax/hmem/device.c
index f9e1a76a04a9..59ad44761191 100644
--- a/drivers/dax/hmem/device.c
+++ b/drivers/dax/hmem/device.c
@@ -17,14 +17,14 @@ static struct resource hmem_active = {
 	.flags = IORESOURCE_MEM,
 };
 
-int walk_hmem_resources(struct device *host, walk_hmem_fn fn)
+int walk_hmem_resources(walk_hmem_fn fn)
 {
 	struct resource *res;
 	int rc = 0;
 
 	mutex_lock(&hmem_resource_lock);
 	for (res = hmem_active.child; res; res = res->sibling) {
-		rc = fn(host, (int) res->desc, res);
+		rc = fn((int) res->desc, res);
 		if (rc)
 			break;
 	}
diff --git a/drivers/dax/hmem/hmem.c b/drivers/dax/hmem/hmem.c
index 5e7c53f18491..3aedef5f1be1 100644
--- a/drivers/dax/hmem/hmem.c
+++ b/drivers/dax/hmem/hmem.c
@@ -9,6 +9,8 @@
 static bool region_idle;
 module_param_named(region_idle, region_idle, bool, 0644);
 
+static struct platform_device *dax_hmem_pdev;
+
 static int dax_hmem_probe(struct platform_device *pdev)
 {
 	unsigned long flags = IORESOURCE_DAX_KMEM;
@@ -59,9 +61,9 @@ static void release_hmem(void *pdev)
 	platform_device_unregister(pdev);
 }
 
-static int hmem_register_device(struct device *host, int target_nid,
-				const struct resource *res)
+static int hmem_register_device(int target_nid, const struct resource *res)
 {
+	struct device *host = &dax_hmem_pdev->dev;
 	struct platform_device *pdev;
 	struct memregion_info info;
 	long id;
@@ -125,7 +127,8 @@ static int hmem_register_device(struct device *host, int target_nid,
 
 static int dax_hmem_platform_probe(struct platform_device *pdev)
 {
-	return walk_hmem_resources(&pdev->dev, hmem_register_device);
+	dax_hmem_pdev = pdev;
+	return walk_hmem_resources(hmem_register_device);
 }
 
 static struct platform_driver dax_hmem_platform_driver = {
diff --git a/include/linux/dax.h b/include/linux/dax.h
index df41a0017b31..4b4d16f94898 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -277,7 +277,6 @@ static inline void hmem_register_resource(int target_nid, struct resource *r)
 }
 #endif
 
-typedef int (*walk_hmem_fn)(struct device *dev, int target_nid,
-			    const struct resource *res);
-int walk_hmem_resources(struct device *dev, walk_hmem_fn fn);
+typedef int (*walk_hmem_fn)(int target_nid, const struct resource *res);
+int walk_hmem_resources(walk_hmem_fn fn);
 #endif
-- 
2.34.1

[PATCH v3 4/4] cxl/dax: Delay consumption of SOFT RESERVE resources

[Terry Bowman]

From: Nathan Fontenot <nathan.fontenot@amd.com>

The dax hmem device initialization will consume any iomem
SOFT RESERVE resources prior to CXL region creation. To allow
for the CXL driver to complete region creation and trim any
SOFT RESERVE resources before the dax driver consumes them
we need to delay the dax driver's search for SOFT RESERVEs.

To do this the dax driver hmem device initialization code
skips the walk of the iomem resource tree if the CXL ACPI
driver is enabled. This allows the CXL driver to complete
region creation and trim any SOFT RESERVES. Once the CXL
driver completes this, the CXL driver then registers any
remaining SOFT RESERVE resources with the dax hmem driver.

Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
---
 drivers/cxl/core/region.c | 10 +++++++++
 drivers/dax/hmem/device.c | 43 ++++++++++++++++++++-------------------
 drivers/dax/hmem/hmem.c   |  3 ++-
 include/linux/dax.h       |  6 ++++++
 4 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 25d70175f204..bf4a4371d98b 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -11,6 +11,7 @@
 #include <linux/idr.h>
 #include <linux/memory-tiers.h>
 #include <linux/ioport.h>
+#include <linux/dax.h>
 #include <cxlmem.h>
 #include <cxl.h>
 #include "core.h"
@@ -3444,6 +3445,11 @@ int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_add_to_region, "CXL");
 
+static int cxl_srmem_register(struct resource *res, void *unused)
+{
+	return hmem_register_device(phys_to_target_node(res->start), res);
+}
+
 int cxl_region_srmem_update(void)
 {
 	struct device *dev = NULL;
@@ -3461,6 +3467,10 @@ int cxl_region_srmem_update(void)
 		put_device(dev);
 	} while (dev);
 
+	/* Now register any remaining SOFT RESERVES with dax */
+	walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED, IORESOURCE_MEM,
+			    0, -1, NULL, cxl_srmem_register);
+
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_region_srmem_update, "CXL");
diff --git a/drivers/dax/hmem/device.c b/drivers/dax/hmem/device.c
index 59ad44761191..cc1ed7bbdb1a 100644
--- a/drivers/dax/hmem/device.c
+++ b/drivers/dax/hmem/device.c
@@ -8,7 +8,6 @@
 static bool nohmem;
 module_param_named(disable, nohmem, bool, 0444);
 
-static bool platform_initialized;
 static DEFINE_MUTEX(hmem_resource_lock);
 static struct resource hmem_active = {
 	.name = "HMEM devices",
@@ -35,9 +34,7 @@ EXPORT_SYMBOL_GPL(walk_hmem_resources);
 
 static void __hmem_register_resource(int target_nid, struct resource *res)
 {
-	struct platform_device *pdev;
 	struct resource *new;
-	int rc;
 
 	new = __request_region(&hmem_active, res->start, resource_size(res), "",
 			       0);
@@ -47,21 +44,6 @@ static void __hmem_register_resource(int target_nid, struct resource *res)
 	}
 
 	new->desc = target_nid;
-
-	if (platform_initialized)
-		return;
-
-	pdev = platform_device_alloc("hmem_platform", 0);
-	if (!pdev) {
-		pr_err_once("failed to register device-dax hmem_platform device\n");
-		return;
-	}
-
-	rc = platform_device_add(pdev);
-	if (rc)
-		platform_device_put(pdev);
-	else
-		platform_initialized = true;
 }
 
 void hmem_register_resource(int target_nid, struct resource *res)
@@ -83,9 +65,28 @@ static __init int hmem_register_one(struct resource *res, void *data)
 
 static __init int hmem_init(void)
 {
-	walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED,
-			IORESOURCE_MEM, 0, -1, NULL, hmem_register_one);
-	return 0;
+	struct platform_device *pdev;
+	int rc;
+
+	if (!IS_ENABLED(CONFIG_CXL_ACPI)) {
+		walk_iomem_res_desc(IORES_DESC_SOFT_RESERVED,
+				    IORESOURCE_MEM, 0, -1, NULL,
+				    hmem_register_one);
+	}
+
+	pdev = platform_device_alloc("hmem_platform", 0);
+	if (!pdev) {
+		pr_err("failed to register device-dax hmem_platform device\n");
+		return -1;
+	}
+
+	rc = platform_device_add(pdev);
+	if (rc) {
+		pr_err("failed to add device-dax hmem_platform device\n");
+		platform_device_put(pdev);
+	}
+
+	return rc;
 }
 
 /*
diff --git a/drivers/dax/hmem/hmem.c b/drivers/dax/hmem/hmem.c
index 3aedef5f1be1..a206b9b383e4 100644
--- a/drivers/dax/hmem/hmem.c
+++ b/drivers/dax/hmem/hmem.c
@@ -61,7 +61,7 @@ static void release_hmem(void *pdev)
 	platform_device_unregister(pdev);
 }
 
-static int hmem_register_device(int target_nid, const struct resource *res)
+int hmem_register_device(int target_nid, const struct resource *res)
 {
 	struct device *host = &dax_hmem_pdev->dev;
 	struct platform_device *pdev;
@@ -124,6 +124,7 @@ static int hmem_register_device(int target_nid, const struct resource *res)
 	platform_device_put(pdev);
 	return rc;
 }
+EXPORT_SYMBOL_GPL(hmem_register_device);
 
 static int dax_hmem_platform_probe(struct platform_device *pdev)
 {
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 4b4d16f94898..a1a75ade9ea7 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -271,10 +271,16 @@ static inline int dax_mem2blk_err(int err)
 
 #ifdef CONFIG_DEV_DAX_HMEM_DEVICES
 void hmem_register_resource(int target_nid, struct resource *r);
+int hmem_register_device(int target_nid, const struct resource *res);
 #else
 static inline void hmem_register_resource(int target_nid, struct resource *r)
 {
 }
+
+static inline int hmem_register_device(int target_nid, const struct resource *res)
+{
+	return 0;
+}
 #endif
 
 typedef int (*walk_hmem_fn)(int target_nid, const struct resource *res);
-- 
2.34.1

Re: [PATCH v3 1/4] kernel/resource: Provide mem region release for SOFT RESERVES

[Andy Shevchenko]

On Thu, Apr 03, 2025 at 01:33:12PM -0500, Terry Bowman wrote:
> From: Nathan Fontenot <nathan.fontenot@amd.com>
> 
> Add a release_Sam_region_adjustable() interface to allow for
> removing SOFT RESERVE memory resources. This extracts out the code
> to remove a mem region into a common __release_mem_region_adjustable()
> routine, this routine takes additional parameters of an IORES
> descriptor type to add checks for IORES_DESC_* and a flag to check
> for IORESOURCE_BUSY to control it's behavior.
> 
> The existing release_mem_region_adjustable() is a front end to the
> common code and a new release_srmem_region_adjustable() is added to
> release SOFT RESERVE resources.

...

> +void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
> +{
> +	return __release_mem_region_adjustable(start, size,

You have still room on the previous line for the parameters.

> +					       true, IORES_DESC_NONE);

Return on void?! Interesting... What do you want to do here?

> +}
> +EXPORT_SYMBOL(release_mem_region_adjustable);
> +#endif
> +
> +#ifdef CONFIG_CXL_REGION
> +void release_srmem_region_adjustable(resource_size_t start,
> +				     resource_size_t size)

This can be put on a single line.

> +{
> +	return __release_mem_region_adjustable(start, size,
> +					       false, IORES_DESC_SOFT_RESERVED);

Same comments as per above function.

> +}
> +EXPORT_SYMBOL(release_srmem_region_adjustable);
> +#endif

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/4] kernel/resource: Provide mem region release for SOFT RESERVES

[Bowman, Terry]

Hi Andy,

Thanks for reviewing this. I'll make updates for all you pointed out below.

Regards,
Terry

On 4/3/2025 1:40 PM, Andy Shevchenko wrote:
> On Thu, Apr 03, 2025 at 01:33:12PM -0500, Terry Bowman wrote:
>> From: Nathan Fontenot <nathan.fontenot@amd.com>
>>
>> Add a release_Sam_region_adjustable() interface to allow for
>> removing SOFT RESERVE memory resources. This extracts out the code
>> to remove a mem region into a common __release_mem_region_adjustable()
>> routine, this routine takes additional parameters of an IORES
>> descriptor type to add checks for IORES_DESC_* and a flag to check
>> for IORESOURCE_BUSY to control it's behavior.
>>
>> The existing release_mem_region_adjustable() is a front end to the
>> common code and a new release_srmem_region_adjustable() is added to
>> release SOFT RESERVE resources.
> ...
>
>> +void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
>> +{
>> +	return __release_mem_region_adjustable(start, size,
> You have still room on the previous line for the parameters.
>
>> +					       true, IORES_DESC_NONE);
> Return on void?! Interesting... What do you want to do here?
>
>> +}
>> +EXPORT_SYMBOL(release_mem_region_adjustable);
>> +#endif
>> +
>> +#ifdef CONFIG_CXL_REGION
>> +void release_srmem_region_adjustable(resource_size_t start,
>> +				     resource_size_t size)
> This can be put on a single line.
>
>> +{
>> +	return __release_mem_region_adjustable(start, size,
>> +					       false, IORES_DESC_SOFT_RESERVED);
> Same comments as per above function.
>
>> +}
>> +EXPORT_SYMBOL(release_srmem_region_adjustable);
>> +#endif

Re: [PATCH v3 1/4] kernel/resource: Provide mem region release for SOFT RESERVES

[kernel test robot]

Hi Terry,

kernel test robot noticed the following build warnings:

[auto build test WARNING on aae0594a7053c60b82621136257c8b648c67b512]

url:    https://github.com/intel-lab-lkp/linux/commits/Terry-Bowman/kernel-resource-Provide-mem-region-release-for-SOFT-RESERVES/20250404-023601
base:   aae0594a7053c60b82621136257c8b648c67b512
patch link:    https://lore.kernel.org/r/20250403183315.286710-2-terry.bowman%40amd.com
patch subject: [PATCH v3 1/4] kernel/resource: Provide mem region release for SOFT RESERVES
config: i386-buildonly-randconfig-003-20250404 (https://download.01.org/0day-ci/archive/20250404/202504042030.Rs5G4dWd-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250404/202504042030.Rs5G4dWd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504042030.Rs5G4dWd-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> kernel/resource.c:1414: warning: Function parameter or struct member 'busy_check' not described in '__release_mem_region_adjustable'
>> kernel/resource.c:1414: warning: Function parameter or struct member 'res_desc' not described in '__release_mem_region_adjustable'
>> kernel/resource.c:1414: warning: expecting prototype for release_mem_region_adjustable(). Prototype was for __release_mem_region_adjustable() instead


vim +1414 kernel/resource.c

^1da177e4c3f41 Linus Torvalds    2005-04-16  1389  
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1390  #if defined(CONFIG_MEMORY_HOTREMOVE) || defined(CONFIG_CXL_REGION)
825f787bb49676 Toshi Kani        2013-04-29  1391  /**
825f787bb49676 Toshi Kani        2013-04-29  1392   * release_mem_region_adjustable - release a previously reserved memory region
825f787bb49676 Toshi Kani        2013-04-29  1393   * @start: resource start address
825f787bb49676 Toshi Kani        2013-04-29  1394   * @size: resource region size
825f787bb49676 Toshi Kani        2013-04-29  1395   *
825f787bb49676 Toshi Kani        2013-04-29  1396   * This interface is intended for memory hot-delete.  The requested region
825f787bb49676 Toshi Kani        2013-04-29  1397   * is released from a currently busy memory resource.  The requested region
825f787bb49676 Toshi Kani        2013-04-29  1398   * must either match exactly or fit into a single busy resource entry.  In
825f787bb49676 Toshi Kani        2013-04-29  1399   * the latter case, the remaining resource is adjusted accordingly.
825f787bb49676 Toshi Kani        2013-04-29  1400   * Existing children of the busy memory resource must be immutable in the
825f787bb49676 Toshi Kani        2013-04-29  1401   * request.
825f787bb49676 Toshi Kani        2013-04-29  1402   *
825f787bb49676 Toshi Kani        2013-04-29  1403   * Note:
825f787bb49676 Toshi Kani        2013-04-29  1404   * - Additional release conditions, such as overlapping region, can be
825f787bb49676 Toshi Kani        2013-04-29  1405   *   supported after they are confirmed as valid cases.
825f787bb49676 Toshi Kani        2013-04-29  1406   * - When a busy memory resource gets split into two entries, the code
825f787bb49676 Toshi Kani        2013-04-29  1407   *   assumes that all children remain in the lower address entry for
825f787bb49676 Toshi Kani        2013-04-29  1408   *   simplicity.  Enhance this logic when necessary.
825f787bb49676 Toshi Kani        2013-04-29  1409   */
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1410  static void __release_mem_region_adjustable(resource_size_t start,
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1411  					    resource_size_t size,
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1412  					    bool busy_check,
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1413  					    int res_desc)
825f787bb49676 Toshi Kani        2013-04-29 @1414  {
cb8e3c8b4f45e4 David Hildenbrand 2020-10-15  1415  	struct resource *parent = &iomem_resource;
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1416  	struct resource *new_res = NULL;
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1417  	bool alloc_nofail = false;
825f787bb49676 Toshi Kani        2013-04-29  1418  	struct resource **p;
825f787bb49676 Toshi Kani        2013-04-29  1419  	struct resource *res;
825f787bb49676 Toshi Kani        2013-04-29  1420  	resource_size_t end;
825f787bb49676 Toshi Kani        2013-04-29  1421  
825f787bb49676 Toshi Kani        2013-04-29  1422  	end = start + size - 1;
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1423  	if (WARN_ON_ONCE((start < parent->start) || (end > parent->end)))
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1424  		return;
825f787bb49676 Toshi Kani        2013-04-29  1425  
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1426  	/*
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1427  	 * We free up quite a lot of memory on memory hotunplug (esp., memap),
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1428  	 * just before releasing the region. This is highly unlikely to
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1429  	 * fail - let's play save and make it never fail as the caller cannot
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1430  	 * perform any error handling (e.g., trying to re-add memory will fail
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1431  	 * similarly).
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1432  	 */
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1433  retry:
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1434  	new_res = alloc_resource(GFP_KERNEL | (alloc_nofail ? __GFP_NOFAIL : 0));
825f787bb49676 Toshi Kani        2013-04-29  1435  
825f787bb49676 Toshi Kani        2013-04-29  1436  	p = &parent->child;
825f787bb49676 Toshi Kani        2013-04-29  1437  	write_lock(&resource_lock);
825f787bb49676 Toshi Kani        2013-04-29  1438  
825f787bb49676 Toshi Kani        2013-04-29  1439  	while ((res = *p)) {
825f787bb49676 Toshi Kani        2013-04-29  1440  		if (res->start >= end)
825f787bb49676 Toshi Kani        2013-04-29  1441  			break;
825f787bb49676 Toshi Kani        2013-04-29  1442  
825f787bb49676 Toshi Kani        2013-04-29  1443  		/* look for the next resource if it does not fit into */
825f787bb49676 Toshi Kani        2013-04-29  1444  		if (res->start > start || res->end < end) {
825f787bb49676 Toshi Kani        2013-04-29  1445  			p = &res->sibling;
825f787bb49676 Toshi Kani        2013-04-29  1446  			continue;
825f787bb49676 Toshi Kani        2013-04-29  1447  		}
825f787bb49676 Toshi Kani        2013-04-29  1448  
825f787bb49676 Toshi Kani        2013-04-29  1449  		if (!(res->flags & IORESOURCE_MEM))
825f787bb49676 Toshi Kani        2013-04-29  1450  			break;
825f787bb49676 Toshi Kani        2013-04-29  1451  
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1452  		if (busy_check && !(res->flags & IORESOURCE_BUSY)) {
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1453  			p = &res->child;
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1454  			continue;
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1455  		}
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1456  
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1457  		if (res_desc != IORES_DESC_NONE && res->desc != res_desc) {
825f787bb49676 Toshi Kani        2013-04-29  1458  			p = &res->child;
825f787bb49676 Toshi Kani        2013-04-29  1459  			continue;
825f787bb49676 Toshi Kani        2013-04-29  1460  		}
825f787bb49676 Toshi Kani        2013-04-29  1461  
825f787bb49676 Toshi Kani        2013-04-29  1462  		/* found the target resource; let's adjust accordingly */
825f787bb49676 Toshi Kani        2013-04-29  1463  		if (res->start == start && res->end == end) {
825f787bb49676 Toshi Kani        2013-04-29  1464  			/* free the whole entry */
825f787bb49676 Toshi Kani        2013-04-29  1465  			*p = res->sibling;
ebff7d8f270d04 Yasuaki Ishimatsu 2013-04-29  1466  			free_resource(res);
825f787bb49676 Toshi Kani        2013-04-29  1467  		} else if (res->start == start && res->end != end) {
825f787bb49676 Toshi Kani        2013-04-29  1468  			/* adjust the start */
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1469  			WARN_ON_ONCE(__adjust_resource(res, end + 1,
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1470  						       res->end - end));
825f787bb49676 Toshi Kani        2013-04-29  1471  		} else if (res->start != start && res->end == end) {
825f787bb49676 Toshi Kani        2013-04-29  1472  			/* adjust the end */
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1473  			WARN_ON_ONCE(__adjust_resource(res, res->start,
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1474  						       start - res->start));
825f787bb49676 Toshi Kani        2013-04-29  1475  		} else {
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1476  			/* split into two entries - we need a new resource */
825f787bb49676 Toshi Kani        2013-04-29  1477  			if (!new_res) {
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1478  				new_res = alloc_resource(GFP_ATOMIC);
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1479  				if (!new_res) {
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1480  					alloc_nofail = true;
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1481  					write_unlock(&resource_lock);
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1482  					goto retry;
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1483  				}
825f787bb49676 Toshi Kani        2013-04-29  1484  			}
825f787bb49676 Toshi Kani        2013-04-29  1485  			new_res->name = res->name;
825f787bb49676 Toshi Kani        2013-04-29  1486  			new_res->start = end + 1;
825f787bb49676 Toshi Kani        2013-04-29  1487  			new_res->end = res->end;
825f787bb49676 Toshi Kani        2013-04-29  1488  			new_res->flags = res->flags;
43ee493bde78da Toshi Kani        2016-01-26  1489  			new_res->desc = res->desc;
825f787bb49676 Toshi Kani        2013-04-29  1490  			new_res->parent = res->parent;
825f787bb49676 Toshi Kani        2013-04-29  1491  			new_res->sibling = res->sibling;
825f787bb49676 Toshi Kani        2013-04-29  1492  			new_res->child = NULL;
825f787bb49676 Toshi Kani        2013-04-29  1493  
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1494  			if (WARN_ON_ONCE(__adjust_resource(res, res->start,
ec62d04e3fdc4b David Hildenbrand 2020-10-15  1495  							   start - res->start)))
825f787bb49676 Toshi Kani        2013-04-29  1496  				break;
825f787bb49676 Toshi Kani        2013-04-29  1497  			res->sibling = new_res;
825f787bb49676 Toshi Kani        2013-04-29  1498  			new_res = NULL;
825f787bb49676 Toshi Kani        2013-04-29  1499  		}
825f787bb49676 Toshi Kani        2013-04-29  1500  
825f787bb49676 Toshi Kani        2013-04-29  1501  		break;
825f787bb49676 Toshi Kani        2013-04-29  1502  	}
825f787bb49676 Toshi Kani        2013-04-29  1503  
825f787bb49676 Toshi Kani        2013-04-29  1504  	write_unlock(&resource_lock);
ebff7d8f270d04 Yasuaki Ishimatsu 2013-04-29  1505  	free_resource(new_res);
825f787bb49676 Toshi Kani        2013-04-29  1506  }
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1507  #endif
e4ebc182a59bbb Nathan Fontenot   2025-04-03  1508  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 1/4] kernel/resource: Provide mem region release for SOFT RESERVES

[Jonathan Cameron]

On Thu, 3 Apr 2025 13:33:12 -0500
Terry Bowman <terry.bowman@amd.com> wrote:

> From: Nathan Fontenot <nathan.fontenot@amd.com>
> 
> Add a release_Sam_region_adjustable() interface to allow for

Who is Sam?  (typo)

> removing SOFT RESERVE memory resources. This extracts out the code
> to remove a mem region into a common __release_mem_region_adjustable()
> routine, this routine takes additional parameters of an IORES
> descriptor type to add checks for IORES_DESC_* and a flag to check
> for IORESOURCE_BUSY to control it's behavior.
> 
> The existing release_mem_region_adjustable() is a front end to the
> common code and a new release_srmem_region_adjustable() is added to
> release SOFT RESERVE resources.
> 
> Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> ---
>  include/linux/ioport.h |  3 +++
>  kernel/resource.c      | 55 +++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 54 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 5385349f0b8a..718360c9c724 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -357,6 +357,9 @@ extern void __release_region(struct resource *, resource_size_t,
>  #ifdef CONFIG_MEMORY_HOTREMOVE
>  extern void release_mem_region_adjustable(resource_size_t, resource_size_t);
>  #endif
> +#ifdef CONFIG_CXL_REGION
> +extern void release_srmem_region_adjustable(resource_size_t, resource_size_t);
I'm not sure the srmem is obvious enough.  Maybe it's worth the long
name to spell it out some more.. e.g. something like

extern void release_softresv_mem_region_adjustable() ?
> +#endif
>  #ifdef CONFIG_MEMORY_HOTPLUG
>  extern void merge_system_ram_resource(struct resource *res);
>  #endif
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 12004452d999..0195b31064b0 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -1387,7 +1387,7 @@ void __release_region(struct resource *parent, resource_size_t start,
>  }
>  EXPORT_SYMBOL(__release_region);
>  
> -#ifdef CONFIG_MEMORY_HOTREMOVE
> +#if defined(CONFIG_MEMORY_HOTREMOVE) || defined(CONFIG_CXL_REGION)
>  /**
>   * release_mem_region_adjustable - release a previously reserved memory region

Looks like you left the old docs which I'm guessing is not the intent.

>   * @start: resource start address
> @@ -1407,7 +1407,10 @@ EXPORT_SYMBOL(__release_region);
>   *   assumes that all children remain in the lower address entry for
>   *   simplicity.  Enhance this logic when necessary.
>   */
> -void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
> +static void __release_mem_region_adjustable(resource_size_t start,
> +					    resource_size_t size,
> +					    bool busy_check,
> +					    int res_desc)
>  {
>  	struct resource *parent = &iomem_resource;
>  	struct resource *new_res = NULL;
> @@ -1446,7 +1449,12 @@ void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
>  		if (!(res->flags & IORESOURCE_MEM))
>  			break;
>  
> -		if (!(res->flags & IORESOURCE_BUSY)) {
> +		if (busy_check && !(res->flags & IORESOURCE_BUSY)) {
> +			p = &res->child;
> +			continue;
> +		}
> +
> +		if (res_desc != IORES_DESC_NONE && res->desc != res_desc) {
>  			p = &res->child;
>  			continue;
>  		}
> @@ -1496,7 +1504,46 @@ void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
>  	write_unlock(&resource_lock);
>  	free_resource(new_res);
>  }
> -#endif	/* CONFIG_MEMORY_HOTREMOVE */
> +#endif
> +
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +/**
> + * release_mem_region_adjustable - release a previously reserved memory region
As above. I was surprised to see new docs in here for an existing function.
I think you forgot to delete the now wrongly placed ones above.

Jonathan

Re: [PATCH v3 1/4] kernel/resource: Provide mem region release for SOFT RESERVES

[Andy Shevchenko]

On Fri, Apr 04, 2025 at 02:16:39PM +0100, Jonathan Cameron wrote:
> On Thu, 3 Apr 2025 13:33:12 -0500 Terry Bowman <terry.bowman@amd.com> wrote:

> > Add a release_Sam_region_adjustable() interface to allow for
> 
> Who is Sam?  (typo)

Somebody's uncle?

...

> >  #ifdef CONFIG_MEMORY_HOTREMOVE
> >  extern void release_mem_region_adjustable(resource_size_t, resource_size_t);
> >  #endif
> > +#ifdef CONFIG_CXL_REGION
> > +extern void release_srmem_region_adjustable(resource_size_t, resource_size_t);
> I'm not sure the srmem is obvious enough.  Maybe it's worth the long
> name to spell it out some more.. e.g. something like

And perhaps drop 'extern' as it's not needed.

> extern void release_softresv_mem_region_adjustable() ?

> >  #ifdef CONFIG_MEMORY_HOTPLUG
> >  extern void merge_system_ram_resource(struct resource *res);
> >  #endif

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/4] cxl: Update Soft Reserved resources upon region creation

[Jonathan Cameron]

On Thu, 3 Apr 2025 13:33:13 -0500
Terry Bowman <terry.bowman@amd.com> wrote:

> From: Nathan Fontenot <nathan.fontenot@amd.com>
> 
> Update handling of SOFT RESERVE iomem resources that intersect with
> CXL region resources to remove intersections from the SOFT RESERVE
> resources. The current approach of leaving SOFT RESERVE resources as
> is can cause failures during hotplug replace of CXL devices because
> the resource is not available for reuse after teardown of the CXL device.
> 
> To accomplish this the cxl acpi driver creates a worker thread at the

Inconsistent in capitalization. I'd just use CXL ACPI here given you used CXL PCI
below.

> end of cxl_acpi_probe(). This worker thread first waits for the CXL PCI
> CXL mem drivers have loaded. The cxl core/suspend.c code is updated to
> add a pci_loaded variable, in addition to the mem_active variable, that
> is updated when the pci driver loads. Remove CONFIG_CXL_SUSPEND Kconfig as
> it is no longer needed. A new cxl_wait_for_pci_mem() routine uses a
> waitqueue for both these driver to be loaded. The need to add this
> additional waitqueue is ensure the CXL PCI and CXL mem drivers have loaded
> before we wait for their probe, without it the cxl acpi probe worker thread
> calls wait_for_device_probe() before these drivers are loaded.
> 
> After the CXL PCI and CXL mem drivers load the cxl acpi worker thread
CXL ACPI

> uses wait_for_device_probe() to ensure device probe routines have
> completed.

Does it matter if these drivers go away again?  Everything seems
to be one way at the moment.

> 
> Once probe completes and regions have been created, find all cxl

CXL

> regions that have been created and trim any SOFT RESERVE resources
> that intersect with the region.
> 
> Update cxl_acpi_exit() to cancel pending waitqueue work.
> 
> Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>


> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index be8a7dc77719..40835ec692c8 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -858,6 +858,7 @@ bool is_cxl_pmem_region(struct device *dev);
>  struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
>  int cxl_add_to_region(struct cxl_port *root,
>  		      struct cxl_endpoint_decoder *cxled);
> +int cxl_region_srmem_update(void);

As before: srmem is a bit obscure. Maybe spell it out more.

>  struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
>  u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
>  #else
> @@ -902,6 +903,8 @@ void cxl_coordinates_combine(struct access_coordinate *out,
>  
>  bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
>  
> +void cxl_wait_for_pci_mem(void);

Re: [PATCH v3 3/4] dax/mum: Save the dax mum platform device pointer

[Jonathan Cameron]

On Thu, 3 Apr 2025 13:33:14 -0500
Terry Bowman <terry.bowman@amd.com> wrote:

> From: Nathan Fontenot <nathan.fontenot@amd.com>

mum?

> 
> In order to handle registering hmem devices for SOFT RESERVE
> resources after the dax hmem device initialization occurs
> we need to save a reference to the dax hmem platform device
> that will be used in a following patch.
> 
> Saving the platform device pointer also allows us to clean
> up the walk_hmem_resources() routine to no require the
> struct device argument.
> 
> There should be no functional changes.
> 
> Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>

Re: [PATCH v3 4/4] cxl/dax: Delay consumption of SOFT RESERVE resources

[Jonathan Cameron]

On Thu, 3 Apr 2025 13:33:15 -0500
Terry Bowman <terry.bowman@amd.com> wrote:

> From: Nathan Fontenot <nathan.fontenot@amd.com>
> 
> The dax hmem device initialization will consume any iomem
> SOFT RESERVE resources prior to CXL region creation. To allow
> for the CXL driver to complete region creation and trim any
> SOFT RESERVE resources before the dax driver consumes them
> we need to delay the dax driver's search for SOFT RESERVEs.
> 
> To do this the dax driver hmem device initialization code
> skips the walk of the iomem resource tree if the CXL ACPI
> driver is enabled. This allows the CXL driver to complete
> region creation and trim any SOFT RESERVES. Once the CXL
> driver completes this, the CXL driver then registers any
> remaining SOFT RESERVE resources with the dax hmem driver.
> 
> Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Looks fine to me, but I'm not feeling confident enough of
this area of the kernel to give a tag.

Jonathan

Re: [PATCH v3 2/4] cxl: Update Soft Reserved resources upon region creation

[kernel test robot]

Hi Terry,

kernel test robot noticed the following build errors:

[auto build test ERROR on aae0594a7053c60b82621136257c8b648c67b512]

url:    https://github.com/intel-lab-lkp/linux/commits/Terry-Bowman/kernel-resource-Provide-mem-region-release-for-SOFT-RESERVES/20250404-023601
base:   aae0594a7053c60b82621136257c8b648c67b512
patch link:    https://lore.kernel.org/r/20250403183315.286710-3-terry.bowman%40amd.com
patch subject: [PATCH v3 2/4] cxl: Update Soft Reserved resources upon region creation
config: hexagon-randconfig-001-20250404 (https://download.01.org/0day-ci/archive/20250404/202504042103.wFCRBR7K-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250404/202504042103.wFCRBR7K-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504042103.wFCRBR7K-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/cxl/core/suspend.c:7:
>> drivers/cxl/cxlpci.h:126:2: error: call to undeclared function 'pcie_capability_read_word'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
           pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &lnksta2);
           ^
   1 error generated.


vim +/pcie_capability_read_word +126 drivers/cxl/cxlpci.h

e0c818e00443ce Robert Richter 2024-02-16  116  
4d07a05397c8c1 Dave Jiang     2023-12-21  117  /*
4d07a05397c8c1 Dave Jiang     2023-12-21  118   * CXL v3.0 6.2.3 Table 6-4
4d07a05397c8c1 Dave Jiang     2023-12-21  119   * The table indicates that if PCIe Flit Mode is set, then CXL is in 256B flits
4d07a05397c8c1 Dave Jiang     2023-12-21  120   * mode, otherwise it's 68B flits mode.
4d07a05397c8c1 Dave Jiang     2023-12-21  121   */
4d07a05397c8c1 Dave Jiang     2023-12-21  122  static inline bool cxl_pci_flit_256(struct pci_dev *pdev)
4d07a05397c8c1 Dave Jiang     2023-12-21  123  {
4d07a05397c8c1 Dave Jiang     2023-12-21  124  	u16 lnksta2;
4d07a05397c8c1 Dave Jiang     2023-12-21  125  
4d07a05397c8c1 Dave Jiang     2023-12-21 @126  	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &lnksta2);
4d07a05397c8c1 Dave Jiang     2023-12-21  127  	return lnksta2 & PCI_EXP_LNKSTA2_FLIT;
4d07a05397c8c1 Dave Jiang     2023-12-21  128  }
4d07a05397c8c1 Dave Jiang     2023-12-21  129  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 0/4] Add managed SOFT RESERVE resource handling

[Zhijian Li (Fujitsu)]

Hi Terry,

If I understand correctly, this patch set has only considered the situation where the
soft reserved area and the region are exactly the same, as in pattern 1.

However, I believe we also need to consider situations where these two are not equal,
which are outlined in pattern 2 and 3 below. Let me explain them:

===========================================
Pattern 1:
- region0 will be created during OS booting due to programed hdm decoder
- After OS booted, region0 can be re-created again after destroy it
┌────────────────────┐
│       CFMW         │
└────────────────────┘
┌────────────────────┐
│    reserved0       │
└────────────────────┘
┌────────────────────┐
│       mem0         │
└────────────────────┘
┌────────────────────┐
│      region0       │
└────────────────────┘


Pattern 2:
The HDM decoder is not in a committed state, so during the kernel boot process,
egion0 will not be created automatically. In this case, the soft reserved area will
not be removed from the iomem tree. After the OS starts,
users cannot create a region (cxl create-region) either, as there should
be an intersection between the soft reserved area and the region.
                              
┌────────────────────┐
│       CFMW         │
└────────────────────┘
┌────────────────────┐
│    reserved0       │
└────────────────────┘
┌────────────────────┐
│       mem0*        │
└────────────────────┘
┌────────────────────┐
│      N/A           │ region0
└────────────────────┘
*HDM decoder in mem0 is not committed.
                                       
               
Pattern 3:
Region0 is a child of the soft reserved area. In this case, the soft reserved area will
not be removed from the iomem tree, resulting in being unable to be recreated later after destroy.
┌────────────────────┐
│       CFMW         │
└────────────────────┘
┌────────────────────┐
│   reserved         │
└────────────────────┘
┌────────────────────┐
│ mem0    | mem1*    │
└────────────────────┘
┌────────────────────┐
│region0  |  N/A     │ region1
└────────────────────┘
*HDM decoder in mem1 is not committed.


Thanks
Zhijian



On 04/04/2025 02:33, Terry Bowman wrote:
> Add the ability to manage SOFT RESERVE iomem resources prior to them being
> added to the iomem resource tree. This allows drivers, such as CXL, to
> remove any pieces of the SOFT RESERVE resource that intersect with created
> CXL regions.
> 
> The current approach of leaving the SOFT RESERVE resources as is can cause
> failures during hotplug of devices, such as CXL, because the resource is
> not available for reuse after teardown of the device.
> 
> The approach is to add SOFT RESERVE resources to a separate tree during
> boot. This allows any drivers to update the SOFT RESERVE resources before
> they are merged into the iomem resource tree. In addition a notifier chain
> is added so that drivers can be notified when these SOFT RESERVE resources
> are added to the ioeme resource tree.
> 
> The CXL driver is modified to use a worker thread that waits for the CXL
> PCI and CXL mem drivers to be loaded and for their probe routine to
> complete. Then the driver walks through any created CXL regions to trim any
> intersections with SOFT RESERVE resources in the iomem tree.
> 
> The dax driver uses the new soft reserve notifier chain so it can consume
> any remaining SOFT RESERVES once they're added to the iomem tree.
> 
> V3 updates:
>   - Remove srmem resource tree from kernel/resource.c, this is no longer
>     needed in the current implementation. All SOFT RESERVE resources now
>     put on the iomem resource tree.
>   - Remove the no longer needed SOFT_RESERVED_MANAGED kernel config option.
>   - Add the 'nid' parameter back to hmem_register_resource();
>   - Remove the no longer used soft reserve notification chain (introduced
>     in v2). The dax driver is now notified of SOFT RESERVED resources by
>     the CXL driver.
> 
> v2 updates:
>   - Add config option SOFT_RESERVE_MANAGED to control use of the
>     separate srmem resource tree at boot.
>   - Only add SOFT RESERVE resources to the soft reserve tree during
>     boot, they go to the iomem resource tree after boot.
>   - Remove the resource trimming code in the previous patch to re-use
>     the existing code in kernel/resource.c
>   - Add functionality for the cxl acpi driver to wait for the cxl PCI
>     and me drivers to load.
> 
> Nathan Fontenot (4):
>    kernel/resource: Provide mem region release for SOFT RESERVES
>    cxl: Update Soft Reserved resources upon region creation
>    dax/mum: Save the dax mum platform device pointer
>    cxl/dax: Delay consumption of SOFT RESERVE resources
> 
>   drivers/cxl/Kconfig        |  4 ---
>   drivers/cxl/acpi.c         | 28 +++++++++++++++++++
>   drivers/cxl/core/Makefile  |  2 +-
>   drivers/cxl/core/region.c  | 34 ++++++++++++++++++++++-
>   drivers/cxl/core/suspend.c | 41 ++++++++++++++++++++++++++++
>   drivers/cxl/cxl.h          |  3 +++
>   drivers/cxl/cxlmem.h       |  9 -------
>   drivers/cxl/cxlpci.h       |  1 +
>   drivers/cxl/pci.c          |  2 ++
>   drivers/dax/hmem/device.c  | 47 ++++++++++++++++----------------
>   drivers/dax/hmem/hmem.c    | 10 ++++---
>   include/linux/dax.h        | 11 +++++---
>   include/linux/ioport.h     |  3 +++
>   include/linux/pm.h         |  7 -----
>   kernel/resource.c          | 55 +++++++++++++++++++++++++++++++++++---
>   15 files changed, 202 insertions(+), 55 deletions(-)
> 
> 
> base-commit: aae0594a7053c60b82621136257c8b648c67b512

Re: [PATCH v3 0/4] Add managed SOFT RESERVE resource handling

[Bowman, Terry]

On 4/7/2025 2:31 AM, Zhijian Li (Fujitsu) wrote:
> Hi Terry,
>
> If I understand correctly, this patch set has only considered the situation where the
> soft reserved area and the region are exactly the same, as in pattern 1.

Hi Zhijian,

I'm working on example test case(s) for your questions. I'll respond here.

Regards,
Terry


> However, I believe we also need to consider situations where these two are not equal,
> which are outlined in pattern 2 and 3 below. Let me explain them:
>
> ===========================================
> Pattern 1:
> - region0 will be created during OS booting due to programed hdm decoder
> - After OS booted, region0 can be re-created again after destroy it
> ┌────────────────────┐
> │       CFMW         │
> └────────────────────┘
> ┌────────────────────┐
> │    reserved0       │
> └────────────────────┘
> ┌────────────────────┐
> │       mem0         │
> └────────────────────┘
> ┌────────────────────┐
> │      region0       │
> └────────────────────┘
>
>
> Pattern 2:
> The HDM decoder is not in a committed state, so during the kernel boot process,
> egion0 will not be created automatically. In this case, the soft reserved area will
> not be removed from the iomem tree. After the OS starts,
> users cannot create a region (cxl create-region) either, as there should
> be an intersection between the soft reserved area and the region.
>                               
> ┌────────────────────┐
> │       CFMW         │
> └────────────────────┘
> ┌────────────────────┐
> │    reserved0       │
> └────────────────────┘
> ┌────────────────────┐
> │       mem0*        │
> └────────────────────┘
> ┌────────────────────┐
> │      N/A           │ region0
> └────────────────────┘
> *HDM decoder in mem0 is not committed.
>                                        
>                
> Pattern 3:
> Region0 is a child of the soft reserved area. In this case, the soft reserved area will
> not be removed from the iomem tree, resulting in being unable to be recreated later after destroy.
> ┌────────────────────┐
> │       CFMW         │
> └────────────────────┘
> ┌────────────────────┐
> │   reserved         │
> └────────────────────┘
> ┌────────────────────┐
> │ mem0    | mem1*    │
> └────────────────────┘
> ┌────────────────────┐
> │region0  |  N/A     │ region1
> └────────────────────┘
> *HDM decoder in mem1 is not committed.
>
>
> Thanks
> Zhijian
>
>
>
> On 04/04/2025 02:33, Terry Bowman wrote:
>> Add the ability to manage SOFT RESERVE iomem resources prior to them being
>> added to the iomem resource tree. This allows drivers, such as CXL, to
>> remove any pieces of the SOFT RESERVE resource that intersect with created
>> CXL regions.
>>
>> The current approach of leaving the SOFT RESERVE resources as is can cause
>> failures during hotplug of devices, such as CXL, because the resource is
>> not available for reuse after teardown of the device.
>>
>> The approach is to add SOFT RESERVE resources to a separate tree during
>> boot. This allows any drivers to update the SOFT RESERVE resources before
>> they are merged into the iomem resource tree. In addition a notifier chain
>> is added so that drivers can be notified when these SOFT RESERVE resources
>> are added to the ioeme resource tree.
>>
>> The CXL driver is modified to use a worker thread that waits for the CXL
>> PCI and CXL mem drivers to be loaded and for their probe routine to
>> complete. Then the driver walks through any created CXL regions to trim any
>> intersections with SOFT RESERVE resources in the iomem tree.
>>
>> The dax driver uses the new soft reserve notifier chain so it can consume
>> any remaining SOFT RESERVES once they're added to the iomem tree.
>>
>> V3 updates:
>>   - Remove srmem resource tree from kernel/resource.c, this is no longer
>>     needed in the current implementation. All SOFT RESERVE resources now
>>     put on the iomem resource tree.
>>   - Remove the no longer needed SOFT_RESERVED_MANAGED kernel config option.
>>   - Add the 'nid' parameter back to hmem_register_resource();
>>   - Remove the no longer used soft reserve notification chain (introduced
>>     in v2). The dax driver is now notified of SOFT RESERVED resources by
>>     the CXL driver.
>>
>> v2 updates:
>>   - Add config option SOFT_RESERVE_MANAGED to control use of the
>>     separate srmem resource tree at boot.
>>   - Only add SOFT RESERVE resources to the soft reserve tree during
>>     boot, they go to the iomem resource tree after boot.
>>   - Remove the resource trimming code in the previous patch to re-use
>>     the existing code in kernel/resource.c
>>   - Add functionality for the cxl acpi driver to wait for the cxl PCI
>>     and me drivers to load.
>>
>> Nathan Fontenot (4):
>>    kernel/resource: Provide mem region release for SOFT RESERVES
>>    cxl: Update Soft Reserved resources upon region creation
>>    dax/mum: Save the dax mum platform device pointer
>>    cxl/dax: Delay consumption of SOFT RESERVE resources
>>
>>   drivers/cxl/Kconfig        |  4 ---
>>   drivers/cxl/acpi.c         | 28 +++++++++++++++++++
>>   drivers/cxl/core/Makefile  |  2 +-
>>   drivers/cxl/core/region.c  | 34 ++++++++++++++++++++++-
>>   drivers/cxl/core/suspend.c | 41 ++++++++++++++++++++++++++++
>>   drivers/cxl/cxl.h          |  3 +++
>>   drivers/cxl/cxlmem.h       |  9 -------
>>   drivers/cxl/cxlpci.h       |  1 +
>>   drivers/cxl/pci.c          |  2 ++
>>   drivers/dax/hmem/device.c  | 47 ++++++++++++++++----------------
>>   drivers/dax/hmem/hmem.c    | 10 ++++---
>>   include/linux/dax.h        | 11 +++++---
>>   include/linux/ioport.h     |  3 +++
>>   include/linux/pm.h         |  7 -----
>>   kernel/resource.c          | 55 +++++++++++++++++++++++++++++++++++---
>>   15 files changed, 202 insertions(+), 55 deletions(-)
>>
>>
>> base-commit: aae0594a7053c60b82621136257c8b648c67b512

Re: [PATCH v3 1/4] kernel/resource: Provide mem region release for SOFT RESERVES

[Bowman, Terry]

On 4/4/2025 8:16 AM, Jonathan Cameron wrote:
> On Thu, 3 Apr 2025 13:33:12 -0500
> Terry Bowman <terry.bowman@amd.com> wrote:
> 
>> From: Nathan Fontenot <nathan.fontenot@amd.com>
>>
>> Add a release_Sam_region_adjustable() interface to allow for
> 
> Who is Sam?  (typo)
> 

Hi Jonathan,

It is a typo. I will fix.

>> removing SOFT RESERVE memory resources. This extracts out the code
>> to remove a mem region into a common __release_mem_region_adjustable()
>> routine, this routine takes additional parameters of an IORES
>> descriptor type to add checks for IORES_DESC_* and a flag to check
>> for IORESOURCE_BUSY to control it's behavior.
>>
>> The existing release_mem_region_adjustable() is a front end to the
>> common code and a new release_srmem_region_adjustable() is added to
>> release SOFT RESERVE resources.
>>
>> Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> ---
>>  include/linux/ioport.h |  3 +++
>>  kernel/resource.c      | 55 +++++++++++++++++++++++++++++++++++++++---
>>  2 files changed, 54 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
>> index 5385349f0b8a..718360c9c724 100644
>> --- a/include/linux/ioport.h
>> +++ b/include/linux/ioport.h
>> @@ -357,6 +357,9 @@ extern void __release_region(struct resource *, resource_size_t,
>>  #ifdef CONFIG_MEMORY_HOTREMOVE
>>  extern void release_mem_region_adjustable(resource_size_t, resource_size_t);
>>  #endif
>> +#ifdef CONFIG_CXL_REGION
>> +extern void release_srmem_region_adjustable(resource_size_t, resource_size_t);
> I'm not sure the srmem is obvious enough.  Maybe it's worth the long
> name to spell it out some more.. e.g. something like
> 

Yes, I'll update with a re name.

> extern void release_softresv_mem_region_adjustable() ?
>> +#endif
>>  #ifdef CONFIG_MEMORY_HOTPLUG
>>  extern void merge_system_ram_resource(struct resource *res);
>>  #endif
>> diff --git a/kernel/resource.c b/kernel/resource.c
>> index 12004452d999..0195b31064b0 100644
>> --- a/kernel/resource.c
>> +++ b/kernel/resource.c
>> @@ -1387,7 +1387,7 @@ void __release_region(struct resource *parent, resource_size_t start,
>>  }
>>  EXPORT_SYMBOL(__release_region);
>>  
>> -#ifdef CONFIG_MEMORY_HOTREMOVE
>> +#if defined(CONFIG_MEMORY_HOTREMOVE) || defined(CONFIG_CXL_REGION)
>>  /**
>>   * release_mem_region_adjustable - release a previously reserved memory region
> 
> Looks like you left the old docs which I'm guessing is not the intent.
> 

Correct, this is not intended. Ill remove.

>>   * @start: resource start address
>> @@ -1407,7 +1407,10 @@ EXPORT_SYMBOL(__release_region);
>>   *   assumes that all children remain in the lower address entry for
>>   *   simplicity.  Enhance this logic when necessary.
>>   */
>> -void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
>> +static void __release_mem_region_adjustable(resource_size_t start,
>> +					    resource_size_t size,
>> +					    bool busy_check,
>> +					    int res_desc)
>>  {
>>  	struct resource *parent = &iomem_resource;
>>  	struct resource *new_res = NULL;
>> @@ -1446,7 +1449,12 @@ void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
>>  		if (!(res->flags & IORESOURCE_MEM))
>>  			break;
>>  
>> -		if (!(res->flags & IORESOURCE_BUSY)) {
>> +		if (busy_check && !(res->flags & IORESOURCE_BUSY)) {
>> +			p = &res->child;
>> +			continue;
>> +		}
>> +
>> +		if (res_desc != IORES_DESC_NONE && res->desc != res_desc) {
>>  			p = &res->child;
>>  			continue;
>>  		}
>> @@ -1496,7 +1504,46 @@ void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
>>  	write_unlock(&resource_lock);
>>  	free_resource(new_res);
>>  }
>> -#endif	/* CONFIG_MEMORY_HOTREMOVE */
>> +#endif
>> +
>> +#ifdef CONFIG_MEMORY_HOTREMOVE
>> +/**
>> + * release_mem_region_adjustable - release a previously reserved memory region
> As above. I was surprised to see new docs in here for an existing function.
> I think you forgot to delete the now wrongly placed ones above.
> 
> Jonathan
> 
Right, I'll remove the now old function comment.

-Terry

Re: [PATCH v3 1/4] kernel/resource: Provide mem region release for SOFT RESERVES

[Bowman, Terry]

On 4/4/2025 8:25 AM, Andy Shevchenko wrote:
> On Fri, Apr 04, 2025 at 02:16:39PM +0100, Jonathan Cameron wrote:
>> On Thu, 3 Apr 2025 13:33:12 -0500 Terry Bowman <terry.bowman@amd.com> wrote:
> 
>>> Add a release_Sam_region_adjustable() interface to allow for
>>
>> Who is Sam?  (typo)
> 
> Somebody's uncle?
> 
> ...
> 
>>>  #ifdef CONFIG_MEMORY_HOTREMOVE
>>>  extern void release_mem_region_adjustable(resource_size_t, resource_size_t);
>>>  #endif
>>> +#ifdef CONFIG_CXL_REGION
>>> +extern void release_srmem_region_adjustable(resource_size_t, resource_size_t);
>> I'm not sure the srmem is obvious enough.  Maybe it's worth the long
>> name to spell it out some more.. e.g. something like
> 
> And perhaps drop 'extern' as it's not needed.
> 

Got it.

-Terry

>> extern void release_softresv_mem_region_adjustable() ?
> 
>>>  #ifdef CONFIG_MEMORY_HOTPLUG
>>>  extern void merge_system_ram_resource(struct resource *res);
>>>  #endif
> 

Re: [PATCH v3 2/4] cxl: Update Soft Reserved resources upon region creation

[Bowman, Terry]

On 4/4/2025 8:32 AM, Jonathan Cameron wrote:
> On Thu, 3 Apr 2025 13:33:13 -0500
> Terry Bowman <terry.bowman@amd.com> wrote:
> 
>> From: Nathan Fontenot <nathan.fontenot@amd.com>
>>
>> Update handling of SOFT RESERVE iomem resources that intersect with
>> CXL region resources to remove intersections from the SOFT RESERVE
>> resources. The current approach of leaving SOFT RESERVE resources as
>> is can cause failures during hotplug replace of CXL devices because
>> the resource is not available for reuse after teardown of the CXL device.
>>
>> To accomplish this the cxl acpi driver creates a worker thread at the
> 
> Inconsistent in capitalization. I'd just use CXL ACPI here given you used CXL PCI
> below.
> 

Thanks. I will update.

>> end of cxl_acpi_probe(). This worker thread first waits for the CXL PCI
>> CXL mem drivers have loaded. The cxl core/suspend.c code is updated to
>> add a pci_loaded variable, in addition to the mem_active variable, that
>> is updated when the pci driver loads. Remove CONFIG_CXL_SUSPEND Kconfig as
>> it is no longer needed. A new cxl_wait_for_pci_mem() routine uses a
>> waitqueue for both these driver to be loaded. The need to add this
>> additional waitqueue is ensure the CXL PCI and CXL mem drivers have loaded
>> before we wait for their probe, without it the cxl acpi probe worker thread
>> calls wait_for_device_probe() before these drivers are loaded.
>>
>> After the CXL PCI and CXL mem drivers load the cxl acpi worker thread
> CXL ACPI
> 
>> uses wait_for_device_probe() to ensure device probe routines have
>> completed.
> 
> Does it matter if these drivers go away again?  Everything seems
> to be one way at the moment.
> 

There is a maximum timeout wait period. I'll add these details to the 
commit message here.

>>
>> Once probe completes and regions have been created, find all cxl
> 
> CXL
> 
>> regions that have been created and trim any SOFT RESERVE resources
>> that intersect with the region.
>>
>> Update cxl_acpi_exit() to cancel pending waitqueue work.
>>
>> Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> 
> 
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index be8a7dc77719..40835ec692c8 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -858,6 +858,7 @@ bool is_cxl_pmem_region(struct device *dev);
>>  struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
>>  int cxl_add_to_region(struct cxl_port *root,
>>  		      struct cxl_endpoint_decoder *cxled);
>> +int cxl_region_srmem_update(void);
> 
> As before: srmem is a bit obscure. Maybe spell it out more.
> 

Yes, will update.

-Terry

>>  struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
>>  u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
>>  #else
>> @@ -902,6 +903,8 @@ void cxl_coordinates_combine(struct access_coordinate *out,
>>  
>>  bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
>>  
>> +void cxl_wait_for_pci_mem(void);
> 

Re: [PATCH v3 3/4] dax/mum: Save the dax mum platform device pointer

[Bowman, Terry]

On 4/4/2025 8:34 AM, Jonathan Cameron wrote:
> On Thu, 3 Apr 2025 13:33:14 -0500
> Terry Bowman <terry.bowman@amd.com> wrote:
> 
>> From: Nathan Fontenot <nathan.fontenot@amd.com>
> 
> mum?
> 
>>
>> In order to handle registering hmem devices for SOFT RESERVE
>> resources after the dax hmem device initialization occurs
>> we need to save a reference to the dax hmem platform device
>> that will be used in a following patch.
>>
>> Saving the platform device pointer also allows us to clean
>> up the walk_hmem_resources() routine to no require the
>> struct device argument.
>>
>> There should be no functional changes.
>>
>> Signed-off-by: Nathan Fontenot <nathan.fontenot@amd.com>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> 

Thanks. Updated to be hmem. 

-Terry

Re: [PATCH v3 0/4] Add managed SOFT RESERVE resource handling

[Bowman, Terry]

Hi Zhijian,

We recreated the failure for the cases you mentioned below. We will be 
adding the fix into v4 I am working on now.

Regards,
Terry



On 4/7/2025 2:31 AM, Zhijian Li (Fujitsu) wrote:
> Hi Terry,
>
> If I understand correctly, this patch set has only considered the situation where the
> soft reserved area and the region are exactly the same, as in pattern 1.
>
> However, I believe we also need to consider situations where these two are not equal,
> which are outlined in pattern 2 and 3 below. Let me explain them:
>
> ===========================================
> Pattern 1:
> - region0 will be created during OS booting due to programed hdm decoder
> - After OS booted, region0 can be re-created again after destroy it
> ┌────────────────────┐
> │       CFMW         │
> └────────────────────┘
> ┌────────────────────┐
> │    reserved0       │
> └────────────────────┘
> ┌────────────────────┐
> │       mem0         │
> └────────────────────┘
> ┌────────────────────┐
> │      region0       │
> └────────────────────┘
>
>
> Pattern 2:
> The HDM decoder is not in a committed state, so during the kernel boot process,
> egion0 will not be created automatically. In this case, the soft reserved area will
> not be removed from the iomem tree. After the OS starts,
> users cannot create a region (cxl create-region) either, as there should
> be an intersection between the soft reserved area and the region.
>                               
> ┌────────────────────┐
> │       CFMW         │
> └────────────────────┘
> ┌────────────────────┐
> │    reserved0       │
> └────────────────────┘
> ┌────────────────────┐
> │       mem0*        │
> └────────────────────┘
> ┌────────────────────┐
> │      N/A           │ region0
> └────────────────────┘
> *HDM decoder in mem0 is not committed.
>                                        
>                
> Pattern 3:
> Region0 is a child of the soft reserved area. In this case, the soft reserved area will
> not be removed from the iomem tree, resulting in being unable to be recreated later after destroy.
> ┌────────────────────┐
> │       CFMW         │
> └────────────────────┘
> ┌────────────────────┐
> │   reserved         │
> └────────────────────┘
> ┌────────────────────┐
> │ mem0    | mem1*    │
> └────────────────────┘
> ┌────────────────────┐
> │region0  |  N/A     │ region1
> └────────────────────┘
> *HDM decoder in mem1 is not committed.
>
>
> Thanks
> Zhijian
>
>
>
> On 04/04/2025 02:33, Terry Bowman wrote:
>> Add the ability to manage SOFT RESERVE iomem resources prior to them being
>> added to the iomem resource tree. This allows drivers, such as CXL, to
>> remove any pieces of the SOFT RESERVE resource that intersect with created
>> CXL regions.
>>
>> The current approach of leaving the SOFT RESERVE resources as is can cause
>> failures during hotplug of devices, such as CXL, because the resource is
>> not available for reuse after teardown of the device.
>>
>> The approach is to add SOFT RESERVE resources to a separate tree during
>> boot. This allows any drivers to update the SOFT RESERVE resources before
>> they are merged into the iomem resource tree. In addition a notifier chain
>> is added so that drivers can be notified when these SOFT RESERVE resources
>> are added to the ioeme resource tree.
>>
>> The CXL driver is modified to use a worker thread that waits for the CXL
>> PCI and CXL mem drivers to be loaded and for their probe routine to
>> complete. Then the driver walks through any created CXL regions to trim any
>> intersections with SOFT RESERVE resources in the iomem tree.
>>
>> The dax driver uses the new soft reserve notifier chain so it can consume
>> any remaining SOFT RESERVES once they're added to the iomem tree.
>>
>> V3 updates:
>>   - Remove srmem resource tree from kernel/resource.c, this is no longer
>>     needed in the current implementation. All SOFT RESERVE resources now
>>     put on the iomem resource tree.
>>   - Remove the no longer needed SOFT_RESERVED_MANAGED kernel config option.
>>   - Add the 'nid' parameter back to hmem_register_resource();
>>   - Remove the no longer used soft reserve notification chain (introduced
>>     in v2). The dax driver is now notified of SOFT RESERVED resources by
>>     the CXL driver.
>>
>> v2 updates:
>>   - Add config option SOFT_RESERVE_MANAGED to control use of the
>>     separate srmem resource tree at boot.
>>   - Only add SOFT RESERVE resources to the soft reserve tree during
>>     boot, they go to the iomem resource tree after boot.
>>   - Remove the resource trimming code in the previous patch to re-use
>>     the existing code in kernel/resource.c
>>   - Add functionality for the cxl acpi driver to wait for the cxl PCI
>>     and me drivers to load.
>>
>> Nathan Fontenot (4):
>>    kernel/resource: Provide mem region release for SOFT RESERVES
>>    cxl: Update Soft Reserved resources upon region creation
>>    dax/mum: Save the dax mum platform device pointer
>>    cxl/dax: Delay consumption of SOFT RESERVE resources
>>
>>   drivers/cxl/Kconfig        |  4 ---
>>   drivers/cxl/acpi.c         | 28 +++++++++++++++++++
>>   drivers/cxl/core/Makefile  |  2 +-
>>   drivers/cxl/core/region.c  | 34 ++++++++++++++++++++++-
>>   drivers/cxl/core/suspend.c | 41 ++++++++++++++++++++++++++++
>>   drivers/cxl/cxl.h          |  3 +++
>>   drivers/cxl/cxlmem.h       |  9 -------
>>   drivers/cxl/cxlpci.h       |  1 +
>>   drivers/cxl/pci.c          |  2 ++
>>   drivers/dax/hmem/device.c  | 47 ++++++++++++++++----------------
>>   drivers/dax/hmem/hmem.c    | 10 ++++---
>>   include/linux/dax.h        | 11 +++++---
>>   include/linux/ioport.h     |  3 +++
>>   include/linux/pm.h         |  7 -----
>>   kernel/resource.c          | 55 +++++++++++++++++++++++++++++++++++---
>>   15 files changed, 202 insertions(+), 55 deletions(-)
>>
>>
>> base-commit: aae0594a7053c60b82621136257c8b648c67b512