Re: [PATCH 1/3] memblock: define functions to set the usable memory range

2022-01-24 Thread Frank van der Linden
Meanwhile, it seems that this issue was already addressed in:

https://lore.kernel.org/all/20211215021348.8766-1-kernelf...@gmail.com/

..which has now been pulled in, and sent to stable@ for 5.15. I
somehow missed that message, and sent my change in a few weeks
later.

The fix to just reserve the ranges does seem a bit cleaner overall,
but this will do fine.

Thanks!

- Frank

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH 1/3] memblock: define functions to set the usable memory range

2022-01-14 Thread Frank van der Linden
On Thu, Jan 13, 2022 at 07:33:11PM +0200, Mike Rapoport wrote:
> On Tue, Jan 11, 2022 at 08:44:41PM +0000, Frank van der Linden wrote:
> > On Tue, Jan 11, 2022 at 12:31:58PM +0200, Mike Rapoport wrote:
> > > > --- a/include/linux/memblock.h
> > > > +++ b/include/linux/memblock.h
> > > > @@ -481,6 +481,8 @@ phys_addr_t memblock_reserved_size(void);
> > > >  phys_addr_t memblock_start_of_DRAM(void);
> > > >  phys_addr_t memblock_end_of_DRAM(void);
> > > >  void memblock_enforce_memory_limit(phys_addr_t memory_limit);
> > > > +void memblock_set_usable_range(phys_addr_t base, phys_addr_t size);
> > > > +void memblock_enforce_usable_range(void);
> > > >  void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
> > > >  void memblock_mem_limit_remove_map(phys_addr_t limit);
> > >
> > > We already have 3 very similar interfaces that deal with memory capping.
> > > Now you suggest to add fourth that will "generically" solve a single use
> > > case of DT, EFI and kdump interaction on arm64.
> > >
> > > Looks like a workaround for a fundamental issue of incompatibility between
> > > DT and EFI wrt memory registration.
> >
> > Yep, I figured this would be the main argument against this - arm64
> > already added several other more-or-less special cased interfaces over
> > time.
> >
> > I'm more than happy to solve this in a different way.
> >
> > What would you suggest:
> >
> > 1) Try to merge the similar interfaces in to one.
> > 2) Just deal with it at a lower (arm64) level?
> > 3) Some other way?
> 
> We've discussed this with Ard on IRC, and our conclusion was that on arm64
> kdump kernel should have memblock.memory exactly the same as the normal
> kernel. Then, the memory outside usable-memory-range should be reserved so
> that kdump kernel won't step over it.
> 
> With that, simple (untested) patch below could be what we need:
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index bdca35284ceb..371418dffaf1 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1275,7 +1275,8 @@ void __init early_init_dt_scan_nodes(void)
> of_scan_flat_dt(early_init_dt_scan_memory, NULL);
> 
> /* Handle linux,usable-memory-range property */
> -   memblock_cap_memory_range(cap_mem_addr, cap_mem_size);
> +   memblock_reserve(0, cap_mem_addr);
> +   memblock_reserve(cap_mem_addr + cap_mem_size, PHYS_ADDR_MAX);
>  }
> 
>  bool __init early_init_dt_scan(void *params)

Ok, tested this on 5.17-rc, and it's working OK there. Main kernel has
32G, crash kernel gets 512M:

Main kernel:

[0.00] Zone ranges:
[0.00]   DMA  [mem 0x4000-0x]
[0.00]   DMA32empty
[0.00]   Normal   [mem 0x0001-0x000b96ff]
[0.00] Movable zone start for each node
[0.00] Early memory node ranges
[0.00]   node   0: [mem 0x4000-0x786e]
[0.00]   node   0: [mem 0x786f-0x7872]
[0.00]   node   0: [mem 0x7873-0x7bbf]
[0.00]   node   0: [mem 0x7bc0-0x7bfd]
[0.00]   node   0: [mem 0x7bfe-0x7fff]
[0.00]   node   0: [mem 0x0004-0x000b96ff]
[0.00] Initmem setup node 0 [mem 0x4000-0x000b96ff]
[0.00] On node 0, zone Normal: 4096 pages in unavailable ranges
[0.00] cma: Reserved 64 MiB at 0x7c00
[0.00] crashkernel reserved: 0x5440 - 0x7440 
(512 MB)


Crash kernel:

[0.00] Zone ranges:
[0.00]   DMA  [mem 0x5440-0x7bfd]
[0.00]   DMA32empty
[0.00]   Normal   empty
[0.00] Movable zone start for each node
[0.00] Early memory node ranges
[0.00]   node   0: [mem 0x5440-0x743f]
[0.00]   node   0: [mem 0x786f-0x7872]
[0.00]   node   0: [mem 0x7bc0-0x7bfd]
[0.00] Initmem setup node 0 [mem 0x5440-0x7bfd]
[0.00] On node 0, zone DMA: 17408 pages in unavailable ranges
[0.00] On node 0, zone DMA: 17136 pages in unavailable ranges
[0.00] On node 0, zone DMA: 13520 pages in unavailable ranges
[0.00] On node 0, zone DMA: 16416 pages in unavailable ranges

Not sure why I had trouble with the same on 5.15, I'll have to look
at that again. But this seems fine for 5.16+

Thanks,

- Frank

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH 1/3] memblock: define functions to set the usable memory range

2022-01-13 Thread Frank van der Linden
On Fri, Jan 14, 2022 at 12:10:46AM +, Frank van der Linden wrote:
> Thanks for discussing this further! I tried the above change, although
> I wrapped it in an if (cap_mem_size != 0), so that a normal kernel
> doesn't get its entire memory marked as reserved.

Just to clarify: I had to do that because the code is slightly different
on 5.15, where I tried it. It wasn't a bug in the proposed patch.

Anyway, will try 5.17-rc tomorrow, I'll let you know, thanks again.

- Frank

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH 1/3] memblock: define functions to set the usable memory range

2022-01-13 Thread Frank van der Linden
On Thu, Jan 13, 2022 at 07:33:11PM +0200, Mike Rapoport wrote:
> On Tue, Jan 11, 2022 at 08:44:41PM +0000, Frank van der Linden wrote:
> > On Tue, Jan 11, 2022 at 12:31:58PM +0200, Mike Rapoport wrote:
> > > > --- a/include/linux/memblock.h
> > > > +++ b/include/linux/memblock.h
> > > > @@ -481,6 +481,8 @@ phys_addr_t memblock_reserved_size(void);
> > > >  phys_addr_t memblock_start_of_DRAM(void);
> > > >  phys_addr_t memblock_end_of_DRAM(void);
> > > >  void memblock_enforce_memory_limit(phys_addr_t memory_limit);
> > > > +void memblock_set_usable_range(phys_addr_t base, phys_addr_t size);
> > > > +void memblock_enforce_usable_range(void);
> > > >  void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
> > > >  void memblock_mem_limit_remove_map(phys_addr_t limit);
> > >
> > > We already have 3 very similar interfaces that deal with memory capping.
> > > Now you suggest to add fourth that will "generically" solve a single use
> > > case of DT, EFI and kdump interaction on arm64.
> > >
> > > Looks like a workaround for a fundamental issue of incompatibility between
> > > DT and EFI wrt memory registration.
> >
> > Yep, I figured this would be the main argument against this - arm64
> > already added several other more-or-less special cased interfaces over
> > time.
> >
> > I'm more than happy to solve this in a different way.
> >
> > What would you suggest:
> >
> > 1) Try to merge the similar interfaces in to one.
> > 2) Just deal with it at a lower (arm64) level?
> > 3) Some other way?
> 
> We've discussed this with Ard on IRC, and our conclusion was that on arm64
> kdump kernel should have memblock.memory exactly the same as the normal
> kernel. Then, the memory outside usable-memory-range should be reserved so
> that kdump kernel won't step over it.
> 
> With that, simple (untested) patch below could be what we need:
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index bdca35284ceb..371418dffaf1 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1275,7 +1275,8 @@ void __init early_init_dt_scan_nodes(void)
> of_scan_flat_dt(early_init_dt_scan_memory, NULL);
> 
> /* Handle linux,usable-memory-range property */
> -   memblock_cap_memory_range(cap_mem_addr, cap_mem_size);
> +   memblock_reserve(0, cap_mem_addr);
> +   memblock_reserve(cap_mem_addr + cap_mem_size, PHYS_ADDR_MAX);
>  }
> 
>  bool __init early_init_dt_scan(void *params)

Thanks for discussing this further! I tried the above change, although
I wrapped it in an if (cap_mem_size != 0), so that a normal kernel
doesn't get its entire memory marked as reserved.

The crash kernel hung without producing output - I haven't chased that
down. This particular kernel was a bit older (5.15), so I'll try again
with 5.17-rc to make sure.

- Frank

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH 1/3] memblock: define functions to set the usable memory range

2022-01-11 Thread Frank van der Linden
On Tue, Jan 11, 2022 at 12:31:58PM +0200, Mike Rapoport wrote:
> > --- a/include/linux/memblock.h
> > +++ b/include/linux/memblock.h
> > @@ -481,6 +481,8 @@ phys_addr_t memblock_reserved_size(void);
> >  phys_addr_t memblock_start_of_DRAM(void);
> >  phys_addr_t memblock_end_of_DRAM(void);
> >  void memblock_enforce_memory_limit(phys_addr_t memory_limit);
> > +void memblock_set_usable_range(phys_addr_t base, phys_addr_t size);
> > +void memblock_enforce_usable_range(void);
> >  void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
> >  void memblock_mem_limit_remove_map(phys_addr_t limit);
> 
> We already have 3 very similar interfaces that deal with memory capping.
> Now you suggest to add fourth that will "generically" solve a single use
> case of DT, EFI and kdump interaction on arm64.
> 
> Looks like a workaround for a fundamental issue of incompatibility between
> DT and EFI wrt memory registration.

Yep, I figured this would be the main argument against this - arm64
already added several other more-or-less special cased interfaces over
time.

I'm more than happy to solve this in a different way.

What would you suggest:

1) Try to merge the similar interfaces in to one.
2) Just deal with it at a lower (arm64) level?
3) Some other way?

Thanks,

- Frank

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[PATCH 3/3] efi: enforce usable memory range after reserving regions

2022-01-10 Thread Frank van der Linden
The usable memory range may be restricted through parameters that
did not come from EFI, like the FDT "linux,usable-memory-range"
property.

Enforce this range after the EFI memory map regions have been
processed.

Signed-off-by: Frank van der Linden 
---
 drivers/firmware/efi/efi-init.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/firmware/efi/efi-init.c b/drivers/firmware/efi/efi-init.c
index b19ce1a83f91..280e9178c7df 100644
--- a/drivers/firmware/efi/efi-init.c
+++ b/drivers/firmware/efi/efi-init.c
@@ -204,6 +204,13 @@ static __init void reserve_regions(void)
memblock_reserve(paddr, size);
}
}
+
+   /*
+* Done, filter !nomap memory we just added so that the
+* usable range is enforced. This is normally only set
+* for crash kernels on some architectures.
+*/
+   memblock_enforce_usable_range();
 }
 
 void __init efi_init(void)
-- 
2.32.0


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[PATCH 1/3] memblock: define functions to set the usable memory range

2022-01-10 Thread Frank van der Linden
Some architectures might limit the usable memory range based
on a firmware property, like "linux,usable-memory-range"
for ARM crash kernels. This limit needs to be enforced after
firmware memory map processing has been done, which might be
e.g. FDT or EFI, or both.

Define an interface for it that is firmware type agnostic.

Signed-off-by: Frank van der Linden 
---
 include/linux/memblock.h |  2 ++
 mm/memblock.c| 37 +
 2 files changed, 39 insertions(+)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 34de69b3b8ba..6128efa50d33 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -481,6 +481,8 @@ phys_addr_t memblock_reserved_size(void);
 phys_addr_t memblock_start_of_DRAM(void);
 phys_addr_t memblock_end_of_DRAM(void);
 void memblock_enforce_memory_limit(phys_addr_t memory_limit);
+void memblock_set_usable_range(phys_addr_t base, phys_addr_t size);
+void memblock_enforce_usable_range(void);
 void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size);
 void memblock_mem_limit_remove_map(phys_addr_t limit);
 bool memblock_is_memory(phys_addr_t addr);
diff --git a/mm/memblock.c b/mm/memblock.c
index 5096500b2647..cb961965f3ad 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -101,6 +101,7 @@ unsigned long max_low_pfn;
 unsigned long min_low_pfn;
 unsigned long max_pfn;
 unsigned long long max_possible_pfn;
+phys_addr_t usable_start, usable_size;
 
 static struct memblock_region 
memblock_memory_init_regions[INIT_MEMBLOCK_REGIONS] __initdata_memblock;
 static struct memblock_region 
memblock_reserved_init_regions[INIT_MEMBLOCK_RESERVED_REGIONS] 
__initdata_memblock;
@@ -1715,6 +1716,42 @@ void __init memblock_cap_memory_range(phys_addr_t base, 
phys_addr_t size)
base + size, PHYS_ADDR_MAX);
 }
 
+/**
+ * memblock_set_usable_range - set usable memory range
+ * @base: physical address that is the start of the range
+ * @size: size of the range.
+ *
+ * Used when a firmware property limits the range of usable
+ * memory, like for the linux,usable-memory-range property
+ * used by ARM crash kernels.
+ */
+void __init memblock_set_usable_range(phys_addr_t base, phys_addr_t size)
+{
+   usable_start = base;
+   usable_size = size;
+}
+
+/**
+ * memblock_enforce_usable_range - cap memory ranges to usable range
+ *
+ * Some architectures call this during boot after firmware memory ranges
+ * have been scanned, to make sure they fall within the usable range
+ * set by memblock_set_usable_range.
+ *
+ * This may be called more than once if there are multiple firmware sources
+ * for memory ranges.
+ *
+ * Avoid "no memory registered" warning - the warning itself is
+ * useful, but we know this can be called with no registered
+ * memory (e.g. when the synthetic DT for the crash kernel has
+ * been parsed on EFI arm64 systems).
+ */
+void __init memblock_enforce_usable_range(void)
+{
+   if (memblock_memory->total_size)
+   memblock_cap_memory_range(usable_start, usable_size);
+}
+
 void __init memblock_mem_limit_remove_map(phys_addr_t limit)
 {
phys_addr_t max_addr;
-- 
2.32.0


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[PATCH 0/3] usable memory range fixes (arm64/fdt/efi)

2022-01-10 Thread Frank van der Linden
b261dba2fdb2 ("arm64: kdump: Remove custom linux,usable-memory-range handling")
moved capping memory ranges using the FDT-specified linux,usable-memory-range
to the FDT code. This property is used to specify the memory range that
a crash kernel runs in.

While this correctly filters any memory ranges that come from the DT,
this breaks crash kernels on arm64 EFI systems. In these cases, DT is used
for the usable-memory-range property, but the actual memory ranges
come from EFI. Since the call to filter them was moved to the FDT
code, which runs before the EFI init code, the EFI ranges are not
filtered anymore, leading to the crash kernel using memory that
it shouldn't.

This set fixes the the issue by having the EFI code cap its
memory ranges too, and defining a common interface for both the
DT and EFI code to use.

These changes stick to the "firmware code should cap its own memory ranges"
idea, using a common memblock interface. An alternative would be to
use an FDT-specific interface as before, called from arm64_memblock_init,
but having things a little more generalized seemed like a good idea.

This is only a functional change on architectures that have both
DT and EFI, and a usable-memory-range property (which is just arm64).
On any other architecture, usable_size will not be set, leading to a
memblock_cap_memory_range call with 0 size, which is a no-op.

Frank van der Linden (3):
  memblock: define functions to set the usable memory range
  of: fdt: use memblock usable range interface
  efi: enforce usable memory range after reserving regions

 drivers/firmware/efi/efi-init.c |  7 +++
 drivers/of/fdt.c|  3 ++-
 include/linux/memblock.h|  2 ++
 mm/memblock.c   | 37 +
 4 files changed, 48 insertions(+), 1 deletion(-)

-- 
2.32.0


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[PATCH 2/3] of: fdt: use memblock usable range interface

2022-01-10 Thread Frank van der Linden
Use the memblock usable range interface to set and enforce the
usable memory range (if any).

Signed-off-by: Frank van der Linden 
---
 drivers/of/fdt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 4546572af24b..b3c2a4124518 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1279,7 +1279,8 @@ void __init early_init_dt_scan_nodes(void)
of_scan_flat_dt(early_init_dt_scan_memory, NULL);
 
/* Handle linux,usable-memory-range property */
-   memblock_cap_memory_range(cap_mem_addr, cap_mem_size);
+   memblock_set_usable_range(cap_mem_addr, cap_mem_size);
+   memblock_enforce_usable_range();
 }
 
 bool __init early_init_dt_scan(void *params)
-- 
2.32.0


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec