Hi,

Sorry for the delay here. I was on vacation. Responses inline.

On 12/2/2025 2:19 PM, [email protected] wrote:
Smita Koralahalli wrote:
From: Dan Williams <[email protected]>

Insert Soft Reserved memory into a dedicated soft_reserve_resource tree
instead of the iomem_resource tree at boot. Delay publishing these ranges
into the iomem hierarchy until ownership is resolved and the HMEM path
is ready to consume them.

Publishing Soft Reserved ranges into iomem too early conflicts with CXL
hotplug and prevents region assembly when those ranges overlap CXL
windows.

Follow up patches will reinsert Soft Reserved ranges into iomem after CXL
window publication is complete and HMEM is ready to claim the memory. This
provides a cleaner handoff between EFI-defined memory ranges and CXL
resource management without trimming or deleting resources later.

Please, when you modify a patch from an original, add your
Co-developed-by: and clarify what you changed.

Thanks Dan. Yeah, this was a bit of a gray area for me. I had the
impression or remember reading somewhere that Co-developed-by tags are
typically added only when the modifications are substantial, so I didn’t
include it initially. I will add the Co-developed-by: line.



Signed-off-by: Dan Williams <[email protected]>
Signed-off-by: Smita Koralahalli <[email protected]>
---
  arch/x86/kernel/e820.c    |  2 +-
  drivers/cxl/acpi.c        |  2 +-
  drivers/dax/hmem/device.c |  4 +-
  drivers/dax/hmem/hmem.c   |  7 ++-
  include/linux/ioport.h    | 13 +++++-
  kernel/resource.c         | 92 +++++++++++++++++++++++++++++++++------
  6 files changed, 100 insertions(+), 20 deletions(-)

[..]
@@ -426,6 +443,26 @@ 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
+struct resource soft_reserve_resource = {
+       .name   = "Soft Reserved",
+       .start  = 0,
+       .end    = -1,
+       .desc   = IORES_DESC_SOFT_RESERVED,
+       .flags  = IORESOURCE_MEM,
+};
+EXPORT_SYMBOL_GPL(soft_reserve_resource);

It looks like one of the things you changed from my RFC was the addition
of walk_soft_reserve_res_desc() and region_intersects_soft_reserve().
With those APIs not only does this symbol not need to be exported, but
it also can be static / private to resource.c.

I remember these helpers were introduced in your RFC but I think they
weren't yet defined. With them in place, agreed there’s no need to
export soft_reserve_resource. Will fix this in the next revision.


+
+int walk_soft_reserve_res_desc(unsigned long desc, unsigned long flags,
+                              u64 start, u64 end, void *arg,
+                              int (*func)(struct resource *, void *))
+{
+       return walk_res_desc(&soft_reserve_resource, start, end, flags, desc,
+                            arg, func);
+}
+EXPORT_SYMBOL_GPL(walk_soft_reserve_res_desc);
+#endif
+
  /*
   * This function calls the @func callback against all memory ranges of type
   * System RAM which are marked as IORESOURCE_SYSTEM_RAM and IORESOUCE_BUSY.
@@ -648,6 +685,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)
+{
+       int ret;
+
+       read_lock(&resource_lock);
+       ret = __region_intersects(&soft_reserve_resource, start, size, flags,
+                                 desc);
+       read_unlock(&resource_lock);
+
+       return ret;
+}
+EXPORT_SYMBOL_GPL(region_intersects_soft_reserve);
+#endif
+
  void __weak arch_remove_reservations(struct resource *avail)
  {
  }
@@ -966,7 +1019,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;
@@ -997,7 +1050,20 @@ 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");
+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

I can not say I am entirely happy with this change, I would prefer to
avoid ifdef in C, and I would prefer not to break the legacy semantics
of this function, but it meets the spirit of the original RFC without
introducing a new insert_resource_late(). I assume review feedback
requested this?

Yeah here, https://lore.kernel.org/all/20250909161210.GBaMBR2rN8h6eT9JHe@fat_crate.local/


+       __insert_resource_expand_to_fit(root, new);
+}
+EXPORT_SYMBOL_GPL(insert_resource_expand_to_fit);

There are no consumers for this export, so it can be dropped.

Okay.

Thanks
Smita


Reply via email to