Message-ID: <69443f707b025_1cee10022@dwillia2-mobl4.notmuch>
Date: Thu, 18 Dec 2025 09:52:48 -0800
From: <dan.j.williams@...el.com>
To: Borislav Petkov <bp@...en8.de>, Smita Koralahalli
	<Smita.KoralahalliChannabasappa@....com>
CC: <linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<nvdimm@...ts.linux.dev>, <linux-fsdevel@...r.kernel.org>,
	<linux-pm@...r.kernel.org>, Davidlohr Bueso <dave@...olabs.net>, "Jonathan
 Cameron" <jonathan.cameron@...wei.com>, Dave Jiang <dave.jiang@...el.com>,
	Alison Schofield <alison.schofield@...el.com>, Vishal Verma
	<vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>, Dan Williams
	<dan.j.williams@...el.com>, Matthew Wilcox <willy@...radead.org>, Jan Kara
	<jack@...e.cz>, "Rafael J . Wysocki" <rafael@...nel.org>, Len Brown
	<len.brown@...el.com>, Pavel Machek <pavel@...nel.org>, Li Ming
	<ming.li@...omail.com>, Jeff Johnson <jeff.johnson@....qualcomm.com>, "Ying
 Huang" <huang.ying.caritas@...il.com>, Yao Xingtao <yaoxt.fnst@...itsu.com>,
	Peter Zijlstra <peterz@...radead.org>, Greg KH <gregkh@...uxfoundation.org>,
	Nathan Fontenot <nathan.fontenot@....com>, Terry Bowman
	<terry.bowman@....com>, Robert Richter <rrichter@....com>, Benjamin Cheatham
	<benjamin.cheatham@....com>, PradeepVineshReddy Kodamati
	<PradeepVineshReddy.Kodamati@....com>, Zhijian Li <lizhijian@...itsu.com>
Subject: Re: [PATCH 1/6] dax/hmem, e820, resource: Defer Soft Reserved
 registration until hmem is ready

Borislav Petkov wrote:
[..]
> Also, I'd do this resource insertion a bit differently:
> 
> insert_resource_expand_to_fit(struct resource *new)
> {
> 	struct resource *root = &iomem_resource;
> 
> 	if (new->desc == IORES_DESC_SOFT_RESERVED)
> 		root = &soft_reserve_resource;
> 
> 	return __insert_resource_expand_to_fit(root, new);
> }
> 
> and rename the current insert_resource_expand_to_fit() to the __ variant.
> 
> It looks like you want to intercept all callers of
> insert_resource_expand_to_fit() instead of defining a separate set which works
> on the soft-reserve thing.

Finally catching up with this feedback after a few months. In the latest
code from Smita [1] this has become:

void insert_resource_expand_to_fit(struct resource *new)
{
       struct resource *root = &iomem_resource;

#ifdef CONFIG_EFI_SOFT_RESERVE
       if (new->desc == IORES_DESC_SOFT_RESERVED)
               root = &soft_reserve_resource;
#endif

       __insert_resource_expand_to_fit(root, new);
}

[1]: http://lore.kernel.org/20251120031925.87762-1-Smita.KoralahalliChannabasappa@amd.com

I am uncomfortable with insert_resource_expand_to_fit() having
build-conditional semantics. However, I agree with you that my initial
wrapping attempt with insert_resource_late() was not much better.

When someone revisits this code in 10 years I doubt they understand what
this redirect is doing. So, make the deferral explicit, open-coded, and
with some commentary in the only function where the distinction matters,
e820__reserve_resources_late().

Proposed incremental on top of v4 rebased to v6.19-rc1 that also
unconditionally defines 'soft_reserve_resource', drops some redundant
arguments to helper functions, and sprinkles more commentary about this
mechanism.

I will go ahead and pull this into the for-7.0/cxl-init topic branch.
Holler if any of this looks broken.

-- 8< --
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 61bf47553f0e..95662b2fb458 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -232,6 +232,7 @@ struct resource_constraint {
 /* PC/ISA/whatever - the normal PC address spaces: IO and memory */
 extern struct resource ioport_resource;
 extern struct resource iomem_resource;
+extern struct resource soft_reserve_resource;
 
 extern struct resource *request_resource_conflict(struct resource *root, struct resource *new);
 extern int request_resource(struct resource *root, struct resource *new);
@@ -242,8 +243,7 @@ extern void reserve_region_with_split(struct resource *root,
 			     const char *name);
 extern struct resource *insert_resource_conflict(struct resource *parent, struct resource *new);
 extern int insert_resource(struct resource *parent, struct resource *new);
-extern void __insert_resource_expand_to_fit(struct resource *root, struct resource *new);
-extern void insert_resource_expand_to_fit(struct resource *new);
+extern void insert_resource_expand_to_fit(struct resource *root, struct resource *new);
 extern int remove_resource(struct resource *old);
 extern void arch_remove_reservations(struct resource *avail);
 extern int allocate_resource(struct resource *root, struct resource *new,
@@ -419,13 +419,10 @@ walk_system_ram_res_rev(u64 start, u64 end, void *arg,
 extern int
 walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end,
 		    void *arg, int (*func)(struct resource *, void *));
+extern int walk_soft_reserve_res(u64 start, u64 end, void *arg,
+				 int (*func)(struct resource *, void *));
 extern int
-walk_soft_reserve_res_desc(unsigned long desc, unsigned long flags,
-			   u64 start, u64 end, void *arg,
-			   int (*func)(struct resource *, void *));
-extern int
-region_intersects_soft_reserve(resource_size_t start, size_t size,
-			       unsigned long flags, unsigned long desc);
+region_intersects_soft_reserve(resource_size_t start, size_t size);
 
 struct resource *devm_request_free_mem_region(struct device *dev,
 		struct resource *base, unsigned long size);
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 0df7d9bacf82..69c050f50e18 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1151,11 +1151,18 @@ void __init e820__reserve_resources_late(void)
 	int i;
 	struct resource *res;
 
-	res = e820_res;
-	for (i = 0; i < e820_table->nr_entries; i++) {
-		if (!res->parent && res->end)
-			insert_resource_expand_to_fit(res);
-		res++;
+	for (i = 0, res = e820_res; i < e820_table->nr_entries; i++, res++) {
+		/* skip added or uninitialized resources */
+		if (res->parent || !res->end)
+			continue;
+
+		/* set aside soft-reserved resources for driver consideration */
+		if (res->desc == IORES_DESC_SOFT_RESERVED) {
+			insert_resource_expand_to_fit(&soft_reserve_resource, res);
+		} else {
+			/* publish the rest immediately */
+			insert_resource_expand_to_fit(&iomem_resource, res);
+		}
 	}
 
 	/*
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index e4a07fb4f5b2..77ac940e3013 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -856,7 +856,7 @@ static int add_cxl_resources(struct resource *cxl_res)
 		 */
 		cxl_set_public_resource(res, new);
 
-		__insert_resource_expand_to_fit(&iomem_resource, new);
+		insert_resource_expand_to_fit(&iomem_resource, new);
 
 		next = res->sibling;
 		while (next && resource_overlaps(new, next)) {
diff --git a/drivers/dax/hmem/device.c b/drivers/dax/hmem/device.c
index 22732b729017..56e3cbd181b5 100644
--- a/drivers/dax/hmem/device.c
+++ b/drivers/dax/hmem/device.c
@@ -83,8 +83,7 @@ static __init int hmem_register_one(struct resource *res, void *data)
 
 static __init int hmem_init(void)
 {
-	walk_soft_reserve_res_desc(IORES_DESC_SOFT_RESERVED, IORESOURCE_MEM, 0,
-				   -1, NULL, hmem_register_one);
+	walk_soft_reserve_res(0, -1, NULL, hmem_register_one);
 	return 0;
 }
 
diff --git a/drivers/dax/hmem/hmem.c b/drivers/dax/hmem/hmem.c
index 48f4642f4bb8..1cf7c2a0ee1c 100644
--- a/drivers/dax/hmem/hmem.c
+++ b/drivers/dax/hmem/hmem.c
@@ -73,9 +73,7 @@ static int hmem_register_device(struct device *host, int target_nid,
 		return 0;
 	}
 
-	rc = region_intersects_soft_reserve(res->start, resource_size(res),
-					    IORESOURCE_MEM,
-					    IORES_DESC_SOFT_RESERVED);
+	rc = region_intersects_soft_reserve(res->start, resource_size(res));
 	if (rc != REGION_INTERSECTS)
 		return 0;
 
diff --git a/kernel/resource.c b/kernel/resource.c
index 7287919b2380..7f1c252212d0 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -48,6 +48,14 @@ struct resource iomem_resource = {
 };
 EXPORT_SYMBOL(iomem_resource);
 
+struct resource soft_reserve_resource = {
+	.name	= "Soft Reserved",
+	.start	= 0,
+	.end	= -1,
+	.desc	= IORES_DESC_SOFT_RESERVED,
+	.flags	= IORESOURCE_MEM,
+};
+
 static DEFINE_RWLOCK(resource_lock);
 
 /*
@@ -451,24 +459,17 @@ int walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start,
 }
 EXPORT_SYMBOL_GPL(walk_iomem_res_desc);
 
-#ifdef CONFIG_EFI_SOFT_RESERVE
-static struct resource soft_reserve_resource = {
-	.name	= "Soft Reserved",
-	.start	= 0,
-	.end	= -1,
-	.desc	= IORES_DESC_SOFT_RESERVED,
-	.flags	= IORESOURCE_MEM,
-};
-
-int walk_soft_reserve_res_desc(unsigned long desc, unsigned long flags,
-			       u64 start, u64 end, void *arg,
-			       int (*func)(struct resource *, void *))
+/*
+ * In support of device drivers claiming Soft Reserved resources, walk the Soft
+ * Reserved resource deferral tree.
+ */
+int walk_soft_reserve_res(u64 start, u64 end, void *arg,
+			  int (*func)(struct resource *, void *))
 {
-	return walk_res_desc(&soft_reserve_resource, start, end, flags, desc,
-			     arg, func);
+	return walk_res_desc(&soft_reserve_resource, start, end, IORESOURCE_MEM,
+			     IORES_DESC_SOFT_RESERVED, arg, func);
 }
-EXPORT_SYMBOL_GPL(walk_soft_reserve_res_desc);
-#endif
+EXPORT_SYMBOL_GPL(walk_soft_reserve_res);
 
 /*
  * This function calls the @func callback against all memory ranges of type
@@ -692,21 +693,22 @@ int region_intersects(resource_size_t start, size_t size, unsigned long flags,
 }
 EXPORT_SYMBOL_GPL(region_intersects);
 
-#ifdef CONFIG_EFI_SOFT_RESERVE
-int region_intersects_soft_reserve(resource_size_t start, size_t size,
-				   unsigned long flags, unsigned long desc)
+/*
+ * Check if the provided range is registered in the Soft Reserved resource
+ * deferral tree for driver consideration.
+ */
+int region_intersects_soft_reserve(resource_size_t start, size_t size)
 {
 	int ret;
 
 	read_lock(&resource_lock);
-	ret = __region_intersects(&soft_reserve_resource, start, size, flags,
-				  desc);
+	ret = __region_intersects(&soft_reserve_resource, start, size,
+				  IORESOURCE_MEM, IORES_DESC_SOFT_RESERVED);
 	read_unlock(&resource_lock);
 
 	return ret;
 }
 EXPORT_SYMBOL_GPL(region_intersects_soft_reserve);
-#endif
 
 void __weak arch_remove_reservations(struct resource *avail)
 {
@@ -1026,7 +1028,7 @@ EXPORT_SYMBOL_GPL(insert_resource);
  * Insert a resource into the resource tree, possibly expanding it in order
  * to make it encompass any conflicting resources.
  */
-void __insert_resource_expand_to_fit(struct resource *root, struct resource *new)
+void insert_resource_expand_to_fit(struct resource *root, struct resource *new)
 {
 	if (new->parent)
 		return;
@@ -1057,19 +1059,7 @@ void __insert_resource_expand_to_fit(struct resource *root, struct resource *new
  * to use this interface. The former are built-in and only the latter,
  * CXL, is a module.
  */
-EXPORT_SYMBOL_NS_GPL(__insert_resource_expand_to_fit, "CXL");
-
-void insert_resource_expand_to_fit(struct resource *new)
-{
-	struct resource *root = &iomem_resource;
-
-#ifdef CONFIG_EFI_SOFT_RESERVE
-	if (new->desc == IORES_DESC_SOFT_RESERVED)
-		root = &soft_reserve_resource;
-#endif
-
-	__insert_resource_expand_to_fit(root, new);
-}
+EXPORT_SYMBOL_NS_GPL(insert_resource_expand_to_fit, "CXL");
 
 /**
  * remove_resource - Remove a resource in the resource tree

Powered by blists - more mailing lists