devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
Cc: saravanak@google.com, klarasmodin@gmail.com,
	aisheng.dong@nxp.com,  hch@lst.de, m.szyprowski@samsung.com,
	robin.murphy@arm.com,  devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,  iommu@lists.linux.dev,
	will@kernel.org, catalin.marinas@arm.com,  kernel@quicinc.com
Subject: Re: [PATCH v7 1/2] of: reserved_mem: Restruture how the reserved memory regions are processed
Date: Mon, 19 Aug 2024 17:04:32 -0500	[thread overview]
Message-ID: <CAL_JsqL=Pc7FJJevMskvYYOoYZYCKF+db9C2Y7_cm7DZNyTYPw@mail.gmail.com> (raw)
In-Reply-To: <20240809184814.2703050-2-quic_obabatun@quicinc.com>

On Fri, Aug 9, 2024 at 1:48 PM Oreoluwa Babatunde
<quic_obabatun@quicinc.com> wrote:
>
> Reserved memory regions defined in the devicetree can be broken up into
> two groups:
> i) Statically-placed reserved memory regions
> i.e. regions defined with a static start address and size using the
>      "reg" property.
> ii) Dynamically-placed reserved memory regions.
> i.e. regions defined by specifying an address range where they can be
>      placed in memory using the "alloc_ranges" and "size" properties.
>
> These regions are processed and set aside at boot time.
> This is done in two stages as seen below:
>
> Stage 1:
> At this stage, fdt_scan_reserved_mem() scans through the child nodes of
> the reserved_memory node using the flattened devicetree and does the
> following:
>
> 1) If the node represents a statically-placed reserved memory region,
>    i.e. if it is defined using the "reg" property:
>    - Call memblock_reserve() or memblock_mark_nomap() as needed.
>    - Add the information for that region into the reserved_mem array
>      using fdt_reserved_mem_save_node().
>      i.e. fdt_reserved_mem_save_node(node, name, base, size).
>
> 2) If the node represents a dynamically-placed reserved memory region,
>    i.e. if it is defined using "alloc-ranges" and "size" properties:
>    - Add the information for that region to the reserved_mem array with
>      the starting address and size set to 0.
>      i.e. fdt_reserved_mem_save_node(node, name, 0, 0).
>    Note: This region is saved to the array with a starting address of 0
>    because a starting address is not yet allocated for it.
>
> Stage 2:
> After iterating through all the reserved memory nodes and storing their
> relevant information in the reserved_mem array,fdt_init_reserved_mem() is
> called and does the following:
>
> 1) For statically-placed reserved memory regions:
>    - Call the region specific init function using
>      __reserved_mem_init_node().
> 2) For dynamically-placed reserved memory regions:
>    - Call __reserved_mem_alloc_size() which is used to allocate memory
>      for each of these regions, and mark them as nomap if they have the
>      nomap property specified in the DT.
>    - Call the region specific init function.
>
> The current size of the resvered_mem array is 64 as is defined by
> MAX_RESERVED_REGIONS. This means that there is a limitation of 64 for
> how many reserved memory regions can be specified on a system.
> As systems continue to grow more and more complex, the number of
> reserved memory regions needed are also growing and are starting to hit
> this 64 count limit, hence the need to make the reserved_mem array
> dynamically sized (i.e. dynamically allocating memory for the
> reserved_mem array using membock_alloc_*).
>
> On architectures such as arm64, memory allocated using memblock is
> writable only after the page tables have been setup. This means that if
> the reserved_mem array is going to be dynamically allocated, it needs to
> happen after the page tables have been setup, not before.
>
> Since the reserved memory regions are currently being processed and
> added to the array before the page tables are setup, there is a need to
> change the order in which some of the processing is done to allow for
> the reserved_mem array to be dynamically sized.
>
> It is possible to process the statically-placed reserved memory regions
> without needing to store them in the reserved_mem array until after the
> page tables have been setup because all the information stored in the
> array is readily available in the devicetree and can be referenced at
> any time.
> Dynamically-placed reserved memory regions on the other hand get
> assigned a start address only at runtime, and hence need a place to be
> stored once they are allocated since there is no other referrence to the
> start address for these regions.
>
> Hence this patch changes the processing order of the reserved memory
> regions in the following ways:
>
> Step 1:
> fdt_scan_reserved_mem() scans through the child nodes of
> the reserved_memory node using the flattened devicetree and does the
> following:
>
> 1) If the node represents a statically-placed reserved memory region,
>    i.e. if it is defined using the "reg" property:
>    - Call memblock_reserve() or memblock_mark_nomap() as needed.
>    - Call the region specific initialization function for the region
>      using fdt_init_reserved_mem_node().
>
> 2) If the node represents a dynamically-placed reserved memory region,
>    i.e. if it is defined using "alloc-ranges" and "size" properties:
>    - Call __reserved_mem_alloc_size() which will:
>      i) Allocate memory for the reserved region and call
>      memblock_mark_nomap() as needed.
>      ii) Call the region specific initialization function using
>      fdt_init_reserved_mem_node().
>      iii) Save the region information in the reserved_mem array using
>      fdt_reserved_mem_save_node().
>
> Step 2:
> 1) This stage of the reserved memory processing is now only used to add
>    the statically-placed reserved memory regions into the reserved_mem
>    array using fdt_scan_reserved_mem_reg_nodes().
>
> 2) This step is also moved to be after the page tables have been
>    setup. Moving this will allow us to replace the reserved_mem
>    array with a dynamically sized array before storing the rest of
>    these regions.
>
> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
> ---
>  drivers/of/fdt.c             |   5 +-
>  drivers/of/of_private.h      |   3 +-
>  drivers/of/of_reserved_mem.c | 172 +++++++++++++++++++++++++----------
>  3 files changed, 131 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 68103ad230ee..d4b7aaa70e31 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -511,8 +511,6 @@ void __init early_init_fdt_scan_reserved_mem(void)
>                         break;
>                 memblock_reserve(base, size);
>         }
> -
> -       fdt_init_reserved_mem();
>  }
>
>  /**
> @@ -1239,6 +1237,9 @@ void __init unflatten_device_tree(void)
>         of_alias_scan(early_init_dt_alloc_memory_arch);
>
>         unittest_unflatten_overlay_base();
> +
> +       /* Save the statically-placed regions in the reserved_mem array */
> +       fdt_scan_reserved_mem_reg_nodes();

I'm still not understanding why the unflatttened API doesn't work
here? It was just used in of_alias_scan() above here.

The problem reported is this function uses initial_boot_params, but
that's NULL for x86.

Rob

  parent reply	other threads:[~2024-08-19 22:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-09 18:48 [PATCH v7 0/2] Dynamic Allocation of the reserved_mem array Oreoluwa Babatunde
2024-08-09 18:48 ` [PATCH v7 1/2] of: reserved_mem: Restruture how the reserved memory regions are processed Oreoluwa Babatunde
2024-08-16 21:07   ` Rob Herring (Arm)
2024-08-19 22:04   ` Rob Herring [this message]
2024-08-21 23:41     ` Oreoluwa Babatunde
2024-08-09 18:48 ` [PATCH v7 2/2] of: reserved_mem: Add code to dynamically allocate reserved_mem array Oreoluwa Babatunde
2024-08-16 21:08   ` Rob Herring (Arm)
2024-08-09 20:09 ` [PATCH v7 0/2] Dynamic Allocation of the " Klara Modin
2024-08-19 17:23 ` Andy Shevchenko
2024-08-19 21:55   ` Rob Herring
2024-08-19 22:33     ` Andy Shevchenko
2024-08-30 16:35   ` Oreoluwa Babatunde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAL_JsqL=Pc7FJJevMskvYYOoYZYCKF+db9C2Y7_cm7DZNyTYPw@mail.gmail.com' \
    --to=robh@kernel.org \
    --cc=aisheng.dong@nxp.com \
    --cc=catalin.marinas@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=hch@lst.de \
    --cc=iommu@lists.linux.dev \
    --cc=kernel@quicinc.com \
    --cc=klarasmodin@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=quic_obabatun@quicinc.com \
    --cc=robin.murphy@arm.com \
    --cc=saravanak@google.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).