I am trying to fix an issue where the kernel panics randomly on my Vim3
board. The problem happens when the ARM Trusted Firmware memory is not
removed from the available ram.
This happens because my u-boot provides /memreserve/, but it doesn't flag
it as nomap. And the kernel scan of /reserved-memory node can't map as
nomap as the region is already reserved.
The idea now is to scan /memreserve/ after /reserved-memory node, and
/memreserve/ will skip regions already reserved by /reserved-memory node.
V1 Patch series: https://lore.kernel.org/all/[email protected]/
Previous Threads:
#regzbot link: https://lore.kernel.org/linux-arm-kernel/[email protected]/#regzbot
#regzbot link: https://lore.kernel.org/all/CAJX_Q+1Tjc+-TjZ6JW9X0NxEdFe=82a9626yL63j7uVD4LpxEA@mail.gmail.com/
Change from V1:
- Instead of allowing to mark nomap a region already reserved, give
precedence to /reserved-memory node scan.
Lucas Tanure (1):
of: fdt: Scan /memreserve/ last
drivers/of/fdt.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
--
2.40.0
Change the order of scanning /memreserve/ and /reserved-memory node.
/reserved-memory node should go first, as it has a more updated
description of the memory regions and it can apply flags, like nomap.
Also, /memreserve/ should avoid reserving regions described in
/reserved-memory node.
Signed-off-by: Lucas Tanure <[email protected]>
---
drivers/of/fdt.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1a68b6d03b3..26e608d8025d 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -635,16 +635,21 @@ void __init early_init_fdt_scan_reserved_mem(void)
if (!initial_boot_params)
return;
+ fdt_scan_reserved_mem();
+ fdt_reserve_elfcorehdr();
+
/* Process header /memreserve/ fields */
for (n = 0; ; n++) {
fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
if (!size)
break;
+ if (memblock_overlaps_region(&memblock.memory, base, size) &&
+ memblock_is_region_reserved(base, size))
+ break;
+
memblock_reserve(base, size);
}
- fdt_scan_reserved_mem();
- fdt_reserve_elfcorehdr();
fdt_init_reserved_mem();
}
--
2.40.0
On Mon, Apr 10, 2023 at 7:00 AM Lucas Tanure <[email protected]> wrote:
>
> Change the order of scanning /memreserve/ and /reserved-memory node.
> /reserved-memory node should go first, as it has a more updated
> description of the memory regions and it can apply flags, like nomap.
> Also, /memreserve/ should avoid reserving regions described in
> /reserved-memory node.
Like I said on v1, I think doing this has a high chance of causing
regressions on other platforms. It should probably not go to stable
for some time after a kernel release.
> Signed-off-by: Lucas Tanure <[email protected]>
> ---
> drivers/of/fdt.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index d1a68b6d03b3..26e608d8025d 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -635,16 +635,21 @@ void __init early_init_fdt_scan_reserved_mem(void)
> if (!initial_boot_params)
> return;
>
> + fdt_scan_reserved_mem();
> + fdt_reserve_elfcorehdr();
> +
> /* Process header /memreserve/ fields */
> for (n = 0; ; n++) {
> fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
> if (!size)
> break;
> + if (memblock_overlaps_region(&memblock.memory, base, size) &&
> + memblock_is_region_reserved(base, size))
Just to make sure, a partial overlap will still get reserved?
> + break;
Shouldn't we continue to the next entry rather than stopping.
> +
> memblock_reserve(base, size);
> }
>
> - fdt_scan_reserved_mem();
> - fdt_reserve_elfcorehdr();
> fdt_init_reserved_mem();
> }
>
> --
> 2.40.0
>
On Mon, Apr 10, 2023 at 8:52 AM Rob Herring <[email protected]> wrote:
>
> On Mon, Apr 10, 2023 at 7:00 AM Lucas Tanure <[email protected]> wrote:
> >
> > Change the order of scanning /memreserve/ and /reserved-memory node.
> > /reserved-memory node should go first, as it has a more updated
> > description of the memory regions and it can apply flags, like nomap.
> > Also, /memreserve/ should avoid reserving regions described in
> > /reserved-memory node.
>
> Like I said on v1, I think doing this has a high chance of causing
> regressions on other platforms. It should probably not go to stable
> for some time after a kernel release.
>
> > Signed-off-by: Lucas Tanure <[email protected]>
> > ---
> > drivers/of/fdt.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index d1a68b6d03b3..26e608d8025d 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -635,16 +635,21 @@ void __init early_init_fdt_scan_reserved_mem(void)
> > if (!initial_boot_params)
> > return;
> >
> > + fdt_scan_reserved_mem();
> > + fdt_reserve_elfcorehdr();
> > +
> > /* Process header /memreserve/ fields */
> > for (n = 0; ; n++) {
> > fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
> > if (!size)
> > break;
> > + if (memblock_overlaps_region(&memblock.memory, base, size) &&
> > + memblock_is_region_reserved(base, size))
>
> Just to make sure, a partial overlap will still get reserved?
A partial overlap will get reserved if not already reserved by the
/reserved-memory node.
>
> > + break;
>
> Shouldn't we continue to the next entry rather than stopping.
Yes, my mistake; I will send v3.
>
> > +
> > memblock_reserve(base, size);
> > }
> >
> > - fdt_scan_reserved_mem();
> > - fdt_reserve_elfcorehdr();
> > fdt_init_reserved_mem();
> > }
> >
> > --
> > 2.40.0
> >
On Mon, Apr 10, 2023 at 08:00:17AM -0400, Lucas Tanure wrote:
> Change the order of scanning /memreserve/ and /reserved-memory node.
> /reserved-memory node should go first, as it has a more updated
> description of the memory regions and it can apply flags, like nomap.
> Also, /memreserve/ should avoid reserving regions described in
> /reserved-memory node.
>
> Signed-off-by: Lucas Tanure <[email protected]>
> ---
> drivers/of/fdt.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index d1a68b6d03b3..26e608d8025d 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -635,16 +635,21 @@ void __init early_init_fdt_scan_reserved_mem(void)
> if (!initial_boot_params)
> return;
>
> + fdt_scan_reserved_mem();
> + fdt_reserve_elfcorehdr();
> +
> /* Process header /memreserve/ fields */
> for (n = 0; ; n++) {
> fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
> if (!size)
> break;
> + if (memblock_overlaps_region(&memblock.memory, base, size) &&
> + memblock_is_region_reserved(base, size))
> + break;
I don't think this is really needed, it's ok to reserve the same ranges
multiple times.
Both checks are not cheap, so it'll be better to just reserve everything
both from performance and simplicity points of view.
> +
> memblock_reserve(base, size);
> }
>
> - fdt_scan_reserved_mem();
> - fdt_reserve_elfcorehdr();
> fdt_init_reserved_mem();
> }
>
> --
> 2.40.0
>
--
Sincerely yours,
Mike.