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/[email protected]
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
