Re: [RFC PATCH 1/3] of: reserved_mem: Change the order that reserved_mem regions are stored

[Oreoluwa Babatunde <quic_obabatun@quicinc.com>]

On 10/19/2023 12:46 PM, Rob Herring wrote:
> On Thu, Oct 19, 2023 at 1:49 PM Oreoluwa Babatunde
> <quic_obabatun@quicinc.com> wrote:
>> The dynamic allocation of the reserved_mem array needs to be done after
>> paging_init() is called because memory allocated using memblock_alloc()
>> is not writeable before that.
>>
>> Nodes that already have their starting address specified in the DT
>> (i.e. nodes that are defined using the "reg" property) can wait until
>> after paging_init() to be stored in the array.
>> But nodes that are dynamically placed need to be reserved and saved in
>> the array before paging_init() so that page table entries are not
>> created for these regions.
>>
>> Hence, change the code to:
>> 1. Before paging_init(), allocate and store information for the
>>    dynamically placed reserved memory regions.
>> 2. After paging_init(), store the rest of the reserved memory regions
>>    which are defined with the "reg" property.
>>
>> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
>> ---
>>  arch/arm64/kernel/setup.c       |  4 +++
>>  drivers/of/fdt.c                | 56 ++++++++++++++++++++++++++-------
>>  drivers/of/of_private.h         |  1 -
>>  drivers/of/of_reserved_mem.c    | 54 ++++++++++++++-----------------
>>  include/linux/of_fdt.h          |  1 +
>>  include/linux/of_reserved_mem.h |  9 ++++++
>>  6 files changed, 83 insertions(+), 42 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
>> index 417a8a86b2db..6002d3ad0b19 100644
>> --- a/arch/arm64/kernel/setup.c
>> +++ b/arch/arm64/kernel/setup.c
>> @@ -27,6 +27,8 @@
>>  #include <linux/proc_fs.h>
>>  #include <linux/memblock.h>
>>  #include <linux/of_fdt.h>
>> +#include <linux/of_reserved_mem.h>
>> +
>>  #include <linux/efi.h>
>>  #include <linux/psci.h>
>>  #include <linux/sched/task.h>
>> @@ -346,6 +348,8 @@ void __init __no_sanitize_address setup_arch(char **cmdline_p)
>>
>>         paging_init();
>>
>> +       fdt_init_reserved_mem();
>> +
> You removed this call from the common code and add it to arm64 arch
> code, doesn't that break every other arch?
Yes, the same changes will be needed for every other arch. I was hoping to
get some feedback on the RFC before implementing this on other archs which
is why the change is currently only in arm64.
> The very next thing done here is unflattening the DT. So another call
> from the arch code to the DT code isn't needed either.
Yes, I see that unflatten_device_tree() is being called right after here.
Just to clarify, are you suggesting to move fdt_init_reserved_mem() into the
unflatten_device_tree() call?


Thanks, Oreoluwa