mbox series

[RFC,0/3] Dynamic allocation of reserved_mem array.

Message ID 20231019184825.9712-1-quic_obabatun@quicinc.com
Headers show
Series Dynamic allocation of reserved_mem array. | expand

Message

Oreoluwa Babatunde Oct. 19, 2023, 6:48 p.m. UTC
The reserved_mem array is used to store the data of the different
reserved memory regions specified in the DT of a device.
The array stores information such as the name, node, starting address,
and size of a reserved memory region.

The array is currently statically allocated with a size of
MAX_RESERVED_REGIONS(64). This means that any system that specifies a
number of reserved memory regions greater than MAX_RESERVED_REGIONS(64)
will not have enough space to store the information for all the regions.

Therefore, this series changes the reserved_mem array from a static
array with a fixed size of MAX_RESERVED_REGIONS(64), to a dynamically
allocated array, using memblock_alloc(), based on the number of reserved
memory regions specified in the DT.

Memory gotten from memblock_alloc() is only writable after paging_init()
is called, but the reserved memory regions need to be reserved before
then so that the system does not create page table mappings for them.

It is possible to call memblock_resverve() and memblock_mark_nomap() on
the reserved memory regions and not need to save them to the array until
we dynamically allocate the array after paging_init(), but this becomes
an issue with reserved memory regions that do not have their starting
address specified in the DT.
i.e. regions specified with the "alloc_ranges" and "size" properties.

Therefore, this series achieves the allocation and population of the
reserved_mem array in two steps:

1. Before paging_init()
   Before paging_init() is called, iterate through the reserved_mem
   nodes in the DT and do the following:
   - Allocate memory for dynamically placed reserved memory regions and
     store their starting address in the static allocated reserved_mem
     array.
   - Call memblock_reserve() and memblock_mark_nomap() on all the
     reserved memory regions.
   - Count the total number of reserved_mem nodes in the DT.

2. After paging_init()
   After paging_init() is called:
   - Allocate new memory for the reserved_mem array based on the number
     of reserved memory nodes in the DT.
   - Transfer all the information that was stored in the static array
     into the new array.
   - Store the rest of the reserved_mem regions in the new array.
     i.e. reserved memory nodes specified with the "reg" property.

The static array is no longer needed after this point, but there is
currently no obvious way to free the memory. Therefore, the size of the
initial static array is now defined using a config option.
Since its size would vary depending on the user, scaling it can help get
some memory savings.

Oreoluwa Babatunde (3):
  of: reserved_mem: Change the order that reserved_mem regions are
    stored
  of: reserved_mem: Add code to dynamically allocate reserved_mem array
  of: reserved_mem: Make MAX_RESERVED_REGIONS a config option

 arch/arm64/kernel/setup.c       |   4 ++
 drivers/of/Kconfig              |  13 ++++
 drivers/of/fdt.c                |  63 +++++++++++++++----
 drivers/of/of_private.h         |   3 +-
 drivers/of/of_reserved_mem.c    | 108 ++++++++++++++++++++++----------
 include/linux/of_fdt.h          |   1 +
 include/linux/of_reserved_mem.h |   9 +++
 7 files changed, 154 insertions(+), 47 deletions(-)

Comments

Rob Herring Oct. 19, 2023, 7:46 p.m. UTC | #1
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?

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.

Rob
Rob Herring Oct. 24, 2023, 7:15 p.m. UTC | #2
On Thu, Oct 19, 2023 at 03:45:37PM -0700, Oreoluwa Babatunde wrote:
> 
> 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.


> >> --- 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?

In general, I want fewer calls between arch code and DT core and for the 
DT core to be more in control of the ordering that things happen. Your 
series does the opposite.

Rob