Re: [PATCH 01/11] of: reserved_mem: fix region count for nodes with multiple reg entries

Re: [PATCH 01/11] of: reserved_mem: fix region count for nodes with multiple reg entries

[Rob Herring]

On Wed, Apr 29, 2026 at 02:58:21PM +0800, Chen Wandun wrote:
> When a reserved-memory node contains multiple reg entries (e.g.,
> reg = <base1 size1>, <base2 size2>), the count used for
> total_reserved_mem_cnt is wrong in two places:
> 
> 1) __reserved_mem_reserve_reg() returns 0 on success regardless of how
>    many regions it reserved in memblock. The caller in
>    fdt_scan_reserved_mem() then increments count by just 1.

Just to make sure, more than 1 worked before the referenced commits? It 
would be easier to just define we only expect/support 1 entry.

> 
> 2) fdt_scan_reserved_mem_late() uses of_flat_dt_get_addr_size() which
>    only reads the first reg entry. Subsequent entries are never
>    initialized via fdt_init_reserved_mem_node(), so their metadata is
>    lost.
> 
> Fix both issues:
>  - Make __reserved_mem_reserve_reg() return the actual number of
>    regions successfully reserved. Update the caller to accumulate
>    the returned count.
>  - Rewrite fdt_scan_reserved_mem_late() to use
>    of_flat_dt_get_addr_size_prop() and iterate all reg entries,
>    initializing each one via fdt_init_reserved_mem_node().
> 
> Fixes: 8a6e02d0c00e ("of: reserved_mem: Restructure how the reserved memory regions are processed")
> Fixes: 00c9a452a235 ("of: reserved_mem: Add code to dynamically allocate reserved_mem array")

Re: [PATCH 01/11] of: reserved_mem: fix region count for nodes with multiple reg entries

[Wandun]

在 2026/5/6 09:47, Rob Herring 写道:
> On Wed, Apr 29, 2026 at 02:58:21PM +0800, Chen Wandun wrote:
>> When a reserved-memory node contains multiple reg entries (e.g.,
>> reg = <base1 size1>, <base2 size2>), the count used for
>> total_reserved_mem_cnt is wrong in two places:
>>
>> 1) __reserved_mem_reserve_reg() returns 0 on success regardless of how
>>     many regions it reserved in memblock. The caller in
>>     fdt_scan_reserved_mem() then increments count by just 1.
> Just to make sure, more than 1 worked before the referenced commits? It
> would be easier to just define we only expect/support 1 entry.
Looking at the pre-8a6e02d0c00e code, __reserved_mem_reserve_reg()
reserved memblock memory for all reg entries, but only called
fdt_reserved_mem_save_node() for the first one (guarded by an 'if 
(first)' flag).

So multiple reg entries were never fully supported: subsequent entries
got their memory reserved in memblock, but their metadata was lost
and driver-specific init callbacks were never invoked for them.

The referenced commits made this worse by also breaking the count
tracking, but the root limitation predates them.

I support documenting "only 1 entry supported" based on the
following reasons:

   - of_reserved_mem_lookup() does a name-based linear scan and returns
     the *first* matching entry. A node with N reg entries would create
     N entries with identical names; entries [1..N-1] are permanently
     unreachable via lookup.

   - Drivers like CMA and DMA coherent are designed to initialize a
     single contiguous pool per node_init call. Calling node_init
     multiple times with the same FDT node is not a supported usage.


>
>> 2) fdt_scan_reserved_mem_late() uses of_flat_dt_get_addr_size() which
>>     only reads the first reg entry. Subsequent entries are never
>>     initialized via fdt_init_reserved_mem_node(), so their metadata is
>>     lost.
>>
>> Fix both issues:
>>   - Make __reserved_mem_reserve_reg() return the actual number of
>>     regions successfully reserved. Update the caller to accumulate
>>     the returned count.
>>   - Rewrite fdt_scan_reserved_mem_late() to use
>>     of_flat_dt_get_addr_size_prop() and iterate all reg entries,
>>     initializing each one via fdt_init_reserved_mem_node().
>>
>> Fixes: 8a6e02d0c00e ("of: reserved_mem: Restructure how the reserved memory regions are processed")
>> Fixes: 00c9a452a235 ("of: reserved_mem: Add code to dynamically allocate reserved_mem array")

Re: [PATCH 02/11] of: reserved_mem: reject reserved memory outside physical address range

[Rob Herring]

On Wed, Apr 29, 2026 at 02:58:22PM +0800, Chen Wandun wrote:
> early_init_dt_reserve_memory() does not validate whether the region
> falls within physical memory. If a device tree incorrectly specifies a
> reserved memory region outside the physical address range:
> 
>  - For the non-nomap path, memblock_reserve() blindly adds the region
>    to memblock.reserved, creating a stale entry that refers to
>    non-existent memory.
> 
>  - For the nomap path, memblock_mark_nomap() silently fails to match
>    any region in memblock.memory, but still returns success.
> 
> Add a memblock_overlaps_region() check at the entry of
> early_init_dt_reserve_memory() to reject such regions before any
> memblock operation takes place. This also simplifies the existing nomap
> guard: the original "overlaps && is_reserved" condition reduces to just
> "is_reserved", since the overlap with physical memory is already
> guaranteed by the new check.

While I agree, I suspect we already have cases abusing reserved-memory 
like this.

> 
> Signed-off-by: Chen Wandun <chenwandun@lixiang.com>
> Tested-by: Zhao Meijing <zhaomeijing@lixiang.com>
> ---
>  drivers/of/of_reserved_mem.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 9d1b0193864c..03c676052dab 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -112,14 +112,21 @@ static int fdt_fixup_reserved_mem_node(unsigned long node,
>  static int __init early_init_dt_reserve_memory(phys_addr_t base,
>  					       phys_addr_t size, bool nomap)
>  {
> +	if (!memblock_overlaps_region(&memblock.memory, base, size)) {
> +		phys_addr_t end = base + size - 1;
> +
> +		pr_warn("Reserved memory region %pa..%pa is outside of physical memory\n",
> +			&base, &end);
> +		return -EINVAL;
> +	}
> +
>  	if (nomap) {
>  		/*
>  		 * If the memory is already reserved (by another region), we
> -		 * should not allow it to be marked nomap, but don't worry
> -		 * if the region isn't memory as it won't be mapped.
> +		 * should not allow it to be marked nomap. The region being
> +		 * physical memory is guaranteed by the overlap check above.
>  		 */
> -		if (memblock_overlaps_region(&memblock.memory, base, size) &&
> -		    memblock_is_region_reserved(base, size))
> +		if (memblock_is_region_reserved(base, size))
>  			return -EBUSY;
>  
>  		return memblock_mark_nomap(base, size);
> -- 
> 2.43.0
> 

Re: [PATCH 02/11] of: reserved_mem: reject reserved memory outside physical address range

[Wandun]

On 5/6/26 09:51, Rob Herring wrote:
> On Wed, Apr 29, 2026 at 02:58:22PM +0800, Chen Wandun wrote:
>> early_init_dt_reserve_memory() does not validate whether the region
>> falls within physical memory. If a device tree incorrectly specifies a
>> reserved memory region outside the physical address range:
>>
>>   - For the non-nomap path, memblock_reserve() blindly adds the region
>>     to memblock.reserved, creating a stale entry that refers to
>>     non-existent memory.
>>
>>   - For the nomap path, memblock_mark_nomap() silently fails to match
>>     any region in memblock.memory, but still returns success.
>>
>> Add a memblock_overlaps_region() check at the entry of
>> early_init_dt_reserve_memory() to reject such regions before any
>> memblock operation takes place. This also simplifies the existing nomap
>> guard: the original "overlaps && is_reserved" condition reduces to just
>> "is_reserved", since the overlap with physical memory is already
>> guaranteed by the new check.
> While I agree, I suspect we already have cases abusing reserved-memory
> like this.
Sashiko reviewed this patch and told me:
"Historically, the reserved-memory binding is often used to describe 
hardware
SRAM, DSP memory, or IOMEM carveouts that reside outside of system RAM."
IIUC, nowdays using mmio-sram DT binding is more appropriate for SRAM or 
IOMEM carveouts.
Should I drop this patch or keep it ?
Thanks.
>
>> Signed-off-by: Chen Wandun <chenwandun@lixiang.com>
>> Tested-by: Zhao Meijing <zhaomeijing@lixiang.com>
>> ---
>>   drivers/of/of_reserved_mem.c | 15 +++++++++++----
>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
>> index 9d1b0193864c..03c676052dab 100644
>> --- a/drivers/of/of_reserved_mem.c
>> +++ b/drivers/of/of_reserved_mem.c
>> @@ -112,14 +112,21 @@ static int fdt_fixup_reserved_mem_node(unsigned long node,
>>   static int __init early_init_dt_reserve_memory(phys_addr_t base,
>>   					       phys_addr_t size, bool nomap)
>>   {
>> +	if (!memblock_overlaps_region(&memblock.memory, base, size)) {
>> +		phys_addr_t end = base + size - 1;
>> +
>> +		pr_warn("Reserved memory region %pa..%pa is outside of physical memory\n",
>> +			&base, &end);
>> +		return -EINVAL;
>> +	}
>> +
>>   	if (nomap) {
>>   		/*
>>   		 * If the memory is already reserved (by another region), we
>> -		 * should not allow it to be marked nomap, but don't worry
>> -		 * if the region isn't memory as it won't be mapped.
>> +		 * should not allow it to be marked nomap. The region being
>> +		 * physical memory is guaranteed by the overlap check above.
>>   		 */
>> -		if (memblock_overlaps_region(&memblock.memory, base, size) &&
>> -		    memblock_is_region_reserved(base, size))
>> +		if (memblock_is_region_reserved(base, size))
>>   			return -EBUSY;
>>   
>>   		return memblock_mark_nomap(base, size);
>> -- 
>> 2.43.0
>>

Re: [PATCH 05/11] of: reserved_mem: add linux,no-dump property support for reserved memory regions

[Rob Herring]

On Wed, Apr 29, 2026 at 02:58:25PM +0800, Chen Wandun wrote:
> Add a 'no_dump' field to struct reserved_mem and parse the
> 'linux,no-dump' device tree property during reserved memory node
> initialization. This property allows device tree authors to mark
> specific reserved memory regions that should be excluded from kdump
> vmcore dumps.
> 
> Reserved memory regions used by device firmware (e.g., GPU, DSP, modem)
> typically contain data that is not useful for kernel crash analysis and
> can significantly increase vmcore size. The 'linux,no-dump' property
> provides a declarative way to indicate these regions should be filtered
> out when constructing the elfcorehdr for kdump.
> 
> The property is named with a 'linux,' prefix because kdump/vmcore is
> Linux-specific and the property is an OS hint rather than a hardware
> description, matching existing properties such as 'linux,cma-default'
> and 'linux,usable-memory-range'.
> 
> The 'linux,no-dump' property is only effective when the region:
>  - Does not have 'no-map': these regions are already excluded from
>    vmcore since they are removed from the linear mapping (MEMBLOCK_NOMAP).
>  - Does not have 'reusable': CMA reusable regions are actively used by
>    the kernel for movable page allocations, and their contents are
>    valuable for crash analysis.
> 
> The no-dump status is also printed in the boot log alongside the
> existing nomap and reusable flags for diagnostic purposes.

I think this property is the wrong way around and probably not needed. 
The default should be exclude the regions, but if Linux is using the 
regions (like CMA) then it can decide on its own to include them.

With the restructuring that went into 7.1, it should be possible for the 
CMA code (and code for any other regions) to set some flag for the 
region.

Rob

Re: [PATCH 05/11] of: reserved_mem: add linux,no-dump property support for reserved memory regions

[Wandun]

On 5/6/26 22:45, Rob Herring wrote:
> On Wed, Apr 29, 2026 at 02:58:25PM +0800, Chen Wandun wrote:
>> Add a 'no_dump' field to struct reserved_mem and parse the
>> 'linux,no-dump' device tree property during reserved memory node
>> initialization. This property allows device tree authors to mark
>> specific reserved memory regions that should be excluded from kdump
>> vmcore dumps.
>>
>> Reserved memory regions used by device firmware (e.g., GPU, DSP, modem)
>> typically contain data that is not useful for kernel crash analysis and
>> can significantly increase vmcore size. The 'linux,no-dump' property
>> provides a declarative way to indicate these regions should be filtered
>> out when constructing the elfcorehdr for kdump.
>>
>> The property is named with a 'linux,' prefix because kdump/vmcore is
>> Linux-specific and the property is an OS hint rather than a hardware
>> description, matching existing properties such as 'linux,cma-default'
>> and 'linux,usable-memory-range'.
>>
>> The 'linux,no-dump' property is only effective when the region:
>>   - Does not have 'no-map': these regions are already excluded from
>>     vmcore since they are removed from the linear mapping (MEMBLOCK_NOMAP).
>>   - Does not have 'reusable': CMA reusable regions are actively used by
>>     the kernel for movable page allocations, and their contents are
>>     valuable for crash analysis.
>>
>> The no-dump status is also printed in the boot log alongside the
>> existing nomap and reusable flags for diagnostic purposes.
> I think this property is the wrong way around and probably not needed.
> The default should be exclude the regions, but if Linux is using the
> regions (like CMA) then it can decide on its own to include them.
>
> With the restructuring that went into 7.1, it should be possible for the
> CMA code (and code for any other regions) to set some flag for the
> region.
Agree that the property direction is wrong. Rework in v2:
   - Default: exclude reserved regions from vmcore
   - CMA (reusable) setup path marks regions as kernel-managed general 
memory → include
   - No DT binding needed; linux,no-dump proposal withdrawn

Thanks.
>
> Rob

Re: [PATCH 07/11] of: reserved_mem: add no-dump crash_mem exclusion helpers

[Rob Herring]

On Wed, Apr 29, 2026 at 02:58:27PM +0800, Chen Wandun wrote:
> Provide two kdump-oriented helpers so that arch kexec_file code does
> not have to open-code the no-dump filtering loop:
> 
>  - of_reserved_mem_no_dump_nr_ranges() returns the number of reserved
>    regions flagged with linux,no-dump. Each exclusion may split one
>    existing crash_mem range into two, so callers use this count to
>    pre-size their crash_mem allocation.
> 
>  - of_reserved_mem_exclude_no_dump() walks the reserved_mem[] array
>    and calls crash_exclude_mem_range() for each no-dump region.
> 
> Both helpers are guarded by CONFIG_KEXEC_FILE; empty inline stubs are
> provided for the !KEXEC_FILE case so architecture code can call them
> unconditionally.
> 
> The consumers are added in the following arm64, riscv and loongarch
> patches in this series.
> 
> Signed-off-by: Chen Wandun <chenwandun@lixiang.com>
> Tested-by: Zhao Meijing <zhaomeijing@lixiang.com>
> ---
>  drivers/of/of_reserved_mem.c    | 54 +++++++++++++++++++++++++++++++++
>  include/linux/of_reserved_mem.h | 15 +++++++++
>  2 files changed, 69 insertions(+)
> 
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 4b80420da2d2..038056a6408a 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -27,6 +27,10 @@
>  
>  #include "of_private.h"
>  
> +#ifdef CONFIG_KEXEC_FILE
> +#include <linux/crash_core.h>
> +#endif

You shouldn't need ifdef around includes.

> +
>  static struct reserved_mem reserved_mem_array[MAX_RESERVED_REGIONS] __initdata;
>  static struct reserved_mem *reserved_mem __refdata = reserved_mem_array;
>  static int total_reserved_mem_cnt = MAX_RESERVED_REGIONS;
> @@ -916,6 +920,56 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
>  }
>  EXPORT_SYMBOL_GPL(of_reserved_mem_lookup);
>  
> +#ifdef CONFIG_KEXEC_FILE

Use 'if (IS_ENABLED())' within the function.

> +/**
> + * of_reserved_mem_no_dump_nr_ranges() - count reserved regions flagged
> + * with the linux,no-dump property.
> + *
> + * Each such region may split an existing crash_mem range into two when
> + * it is excluded, so callers can use this count to pre-size their
> + * crash_mem allocation.
> + */
> +unsigned int of_reserved_mem_no_dump_nr_ranges(void)
> +{
> +	unsigned int i, n = 0;
> +
> +	for (i = 0; i < reserved_mem_count; i++)
> +		if (reserved_mem[i].no_dump)
> +			n++;
> +	return n;
> +}
> +
> +/**
> + * of_reserved_mem_exclude_no_dump() - exclude no-dump reserved regions
> + * from a crash_mem list.
> + * @cmem: crash memory list to modify
> + *
> + * Walks the reserved_mem[] array and calls crash_exclude_mem_range() for
> + * every region with no_dump set. Intended to be called from arch kdump
> + * code when constructing the elfcorehdr.
> + *
> + * Returns 0 on success, or a negative error returned by
> + * crash_exclude_mem_range() on the first failure.
> + */
> +int of_reserved_mem_exclude_no_dump(struct crash_mem *cmem)
> +{
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < reserved_mem_count; i++) {
> +		struct reserved_mem *r = &reserved_mem[i];
> +
> +		if (!r->no_dump || !r->size)
> +			continue;
> +		ret = crash_exclude_mem_range(cmem, r->base,
> +					      r->base + r->size - 1);
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
> +}
> +#endif /* CONFIG_KEXEC_FILE */

Re: [PATCH 07/11] of: reserved_mem: add no-dump crash_mem exclusion helpers

[Wandun]

On 5/6/26 22:50, Rob Herring wrote:
> On Wed, Apr 29, 2026 at 02:58:27PM +0800, Chen Wandun wrote:
>> Provide two kdump-oriented helpers so that arch kexec_file code does
>> not have to open-code the no-dump filtering loop:
>>
>>   - of_reserved_mem_no_dump_nr_ranges() returns the number of reserved
>>     regions flagged with linux,no-dump. Each exclusion may split one
>>     existing crash_mem range into two, so callers use this count to
>>     pre-size their crash_mem allocation.
>>
>>   - of_reserved_mem_exclude_no_dump() walks the reserved_mem[] array
>>     and calls crash_exclude_mem_range() for each no-dump region.
>>
>> Both helpers are guarded by CONFIG_KEXEC_FILE; empty inline stubs are
>> provided for the !KEXEC_FILE case so architecture code can call them
>> unconditionally.
>>
>> The consumers are added in the following arm64, riscv and loongarch
>> patches in this series.
>>
>> Signed-off-by: Chen Wandun <chenwandun@lixiang.com>
>> Tested-by: Zhao Meijing <zhaomeijing@lixiang.com>
>> ---
>>   drivers/of/of_reserved_mem.c    | 54 +++++++++++++++++++++++++++++++++
>>   include/linux/of_reserved_mem.h | 15 +++++++++
>>   2 files changed, 69 insertions(+)
>>
>> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
>> index 4b80420da2d2..038056a6408a 100644
>> --- a/drivers/of/of_reserved_mem.c
>> +++ b/drivers/of/of_reserved_mem.c
>> @@ -27,6 +27,10 @@
>>   
>>   #include "of_private.h"
>>   
>> +#ifdef CONFIG_KEXEC_FILE
>> +#include <linux/crash_core.h>
>> +#endif
> You shouldn't need ifdef around includes.
>
>> +
>>   static struct reserved_mem reserved_mem_array[MAX_RESERVED_REGIONS] __initdata;
>>   static struct reserved_mem *reserved_mem __refdata = reserved_mem_array;
>>   static int total_reserved_mem_cnt = MAX_RESERVED_REGIONS;
>> @@ -916,6 +920,56 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
>>   }
>>   EXPORT_SYMBOL_GPL(of_reserved_mem_lookup);
>>   
>> +#ifdef CONFIG_KEXEC_FILE
> Use 'if (IS_ENABLED())' within the function.
Get it, thanks.
>
>> +/**
>> + * of_reserved_mem_no_dump_nr_ranges() - count reserved regions flagged
>> + * with the linux,no-dump property.
>> + *
>> + * Each such region may split an existing crash_mem range into two when
>> + * it is excluded, so callers can use this count to pre-size their
>> + * crash_mem allocation.
>> + */
>> +unsigned int of_reserved_mem_no_dump_nr_ranges(void)
>> +{
>> +	unsigned int i, n = 0;
>> +
>> +	for (i = 0; i < reserved_mem_count; i++)
>> +		if (reserved_mem[i].no_dump)
>> +			n++;
>> +	return n;
>> +}
>> +
>> +/**
>> + * of_reserved_mem_exclude_no_dump() - exclude no-dump reserved regions
>> + * from a crash_mem list.
>> + * @cmem: crash memory list to modify
>> + *
>> + * Walks the reserved_mem[] array and calls crash_exclude_mem_range() for
>> + * every region with no_dump set. Intended to be called from arch kdump
>> + * code when constructing the elfcorehdr.
>> + *
>> + * Returns 0 on success, or a negative error returned by
>> + * crash_exclude_mem_range() on the first failure.
>> + */
>> +int of_reserved_mem_exclude_no_dump(struct crash_mem *cmem)
>> +{
>> +	unsigned int i;
>> +	int ret;
>> +
>> +	for (i = 0; i < reserved_mem_count; i++) {
>> +		struct reserved_mem *r = &reserved_mem[i];
>> +
>> +		if (!r->no_dump || !r->size)
>> +			continue;
>> +		ret = crash_exclude_mem_range(cmem, r->base,
>> +					      r->base + r->size - 1);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +	return 0;
>> +}
>> +#endif /* CONFIG_KEXEC_FILE */