* [PATCH 0/2] fdt: A couple of no-map fixes @ 2021-01-15 11:45 Quentin Perret 2021-01-15 11:45 ` [PATCH 1/2] fdt: Properly handle "no-map" field in the memory region Quentin Perret 2021-01-15 11:45 ` [PATCH 2/2] of/fdt: Make sure no-map does not remove already reserved regions Quentin Perret 0 siblings, 2 replies; 6+ messages in thread From: Quentin Perret @ 2021-01-15 11:45 UTC (permalink / raw) To: robh+dt, frowand.list Cc: devicetree, linux-kernel, drinkcat, swboyd, f.fainelli, karahmed, kernel-team In the context of a KVM/arm64 thread [1], it became apparent that the difference in behaviour of EFI and DT with respect to no-map regions was something worth fixing. Rob pointed out in that thread that a couple of patches in this area had been sent but still needed to be integrated together, which is basically what this series attempts to do. - Patch 01 is a generic fix that should make sense on its own; - Patch 02 allows the kernel to have a saner behaviour with a broken DT. This one had a small rebase conflict (caused by patch 01). I tested it in qemu and everything is looking good, the error message shows up in dmesg when it should. Thanks, Quentin [1] https://lore.kernel.org/kvmarm/20210108121524.656872-1-qperret@google.com/ KarimAllah Ahmed (1): fdt: Properly handle "no-map" field in the memory region Nicolas Boichat (1): of/fdt: Make sure no-map does not remove already reserved regions drivers/of/fdt.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) -- 2.30.0.284.gd98b1dd5eaa7-goog ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] fdt: Properly handle "no-map" field in the memory region 2021-01-15 11:45 [PATCH 0/2] fdt: A couple of no-map fixes Quentin Perret @ 2021-01-15 11:45 ` Quentin Perret 2021-01-15 17:29 ` Rob Herring 2021-01-15 11:45 ` [PATCH 2/2] of/fdt: Make sure no-map does not remove already reserved regions Quentin Perret 1 sibling, 1 reply; 6+ messages in thread From: Quentin Perret @ 2021-01-15 11:45 UTC (permalink / raw) To: robh+dt, frowand.list Cc: devicetree, linux-kernel, drinkcat, swboyd, f.fainelli, karahmed, kernel-team From: KarimAllah Ahmed <karahmed@amazon.de> Mark the memory region with NOMAP flag instead of completely removing it from the memory blocks. That makes the FDT handling consistent with the EFI memory map handling. Cc: Rob Herring <robh+dt@kernel.org> Cc: Frank Rowand <frowand.list@gmail.com> Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de> Signed-off-by: Quentin Perret <qperret@google.com> --- drivers/of/fdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index feb0f2d67fc5..427b534d60d2 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -1147,7 +1147,7 @@ int __init __weak early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size, bool nomap) { if (nomap) - return memblock_remove(base, size); + return memblock_mark_nomap(base, size); return memblock_reserve(base, size); } -- 2.30.0.284.gd98b1dd5eaa7-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] fdt: Properly handle "no-map" field in the memory region 2021-01-15 11:45 ` [PATCH 1/2] fdt: Properly handle "no-map" field in the memory region Quentin Perret @ 2021-01-15 17:29 ` Rob Herring 0 siblings, 0 replies; 6+ messages in thread From: Rob Herring @ 2021-01-15 17:29 UTC (permalink / raw) To: Quentin Perret Cc: robh+dt, kernel-team, linux-kernel, frowand.list, drinkcat, swboyd, karahmed, f.fainelli, devicetree On Fri, 15 Jan 2021 11:45:43 +0000, Quentin Perret wrote: > From: KarimAllah Ahmed <karahmed@amazon.de> > > Mark the memory region with NOMAP flag instead of completely removing it > from the memory blocks. That makes the FDT handling consistent with the EFI > memory map handling. > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de> > Signed-off-by: Quentin Perret <qperret@google.com> > --- > drivers/of/fdt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Applied, thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] of/fdt: Make sure no-map does not remove already reserved regions 2021-01-15 11:45 [PATCH 0/2] fdt: A couple of no-map fixes Quentin Perret 2021-01-15 11:45 ` [PATCH 1/2] fdt: Properly handle "no-map" field in the memory region Quentin Perret @ 2021-01-15 11:45 ` Quentin Perret 2021-01-15 17:30 ` Rob Herring 1 sibling, 1 reply; 6+ messages in thread From: Quentin Perret @ 2021-01-15 11:45 UTC (permalink / raw) To: robh+dt, frowand.list Cc: devicetree, linux-kernel, drinkcat, swboyd, f.fainelli, karahmed, kernel-team From: Nicolas Boichat <drinkcat@chromium.org> If the device tree is incorrectly configured, and attempts to define a "no-map" reserved memory that overlaps with the kernel data/code, the kernel would crash quickly after boot, with no obvious clue about the nature of the issue. For example, this would happen if we have the kernel mapped at these addresses (from /proc/iomem): 40000000-41ffffff : System RAM 40080000-40dfffff : Kernel code 40e00000-411fffff : reserved 41200000-413e0fff : Kernel data And we declare a no-map shared-dma-pool region at a fixed address within that range: mem_reserved: mem_region { compatible = "shared-dma-pool"; reg = <0 0x40000000 0 0x01A00000>; no-map; }; To fix this, when removing memory regions at early boot (which is what "no-map" regions do), we need to make sure that the memory is not already reserved. If we do, __reserved_mem_reserve_reg will throw an error: [ 0.000000] OF: fdt: Reserved memory: failed to reserve memory for node 'mem_region': base 0x0000000040000000, size 26 MiB and the code that will try to use the region should also fail, later on. We do not do anything for non-"no-map" regions, as memblock explicitly allows reserved regions to overlap, and the commit that this fixes removed the check for that precise reason. [ qperret: fixed conflicts caused by the usage of memblock_mark_nomap ] Fixes: 094cb98179f19b7 ("of/fdt: memblock_reserve /memreserve/ regions in the case of partial overlap") Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Reviewed-by: Stephen Boyd <swboyd@chromium.org> Signed-off-by: Quentin Perret <qperret@google.com> --- drivers/of/fdt.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 427b534d60d2..dcc1dd96911a 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -1146,8 +1146,16 @@ int __init __weak early_init_dt_mark_hotplug_memory_arch(u64 base, u64 size) int __init __weak early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t size, bool nomap) { - if (nomap) + if (nomap) { + /* + * If the memory is already reserved (by another region), we + * should not allow it to be marked nomap. + */ + if (memblock_is_region_reserved(base, size)) + return -EBUSY; + return memblock_mark_nomap(base, size); + } return memblock_reserve(base, size); } -- 2.30.0.284.gd98b1dd5eaa7-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] of/fdt: Make sure no-map does not remove already reserved regions 2021-01-15 11:45 ` [PATCH 2/2] of/fdt: Make sure no-map does not remove already reserved regions Quentin Perret @ 2021-01-15 17:30 ` Rob Herring 2021-01-16 0:30 ` Nicolas Boichat 0 siblings, 1 reply; 6+ messages in thread From: Rob Herring @ 2021-01-15 17:30 UTC (permalink / raw) To: Quentin Perret Cc: robh+dt, drinkcat, devicetree, f.fainelli, swboyd, karahmed, kernel-team, frowand.list, linux-kernel On Fri, 15 Jan 2021 11:45:44 +0000, Quentin Perret wrote: > From: Nicolas Boichat <drinkcat@chromium.org> > > If the device tree is incorrectly configured, and attempts to > define a "no-map" reserved memory that overlaps with the kernel > data/code, the kernel would crash quickly after boot, with no > obvious clue about the nature of the issue. > > For example, this would happen if we have the kernel mapped at > these addresses (from /proc/iomem): > 40000000-41ffffff : System RAM > 40080000-40dfffff : Kernel code > 40e00000-411fffff : reserved > 41200000-413e0fff : Kernel data > > And we declare a no-map shared-dma-pool region at a fixed address > within that range: > mem_reserved: mem_region { > compatible = "shared-dma-pool"; > reg = <0 0x40000000 0 0x01A00000>; > no-map; > }; > > To fix this, when removing memory regions at early boot (which is > what "no-map" regions do), we need to make sure that the memory > is not already reserved. If we do, __reserved_mem_reserve_reg > will throw an error: > [ 0.000000] OF: fdt: Reserved memory: failed to reserve memory > for node 'mem_region': base 0x0000000040000000, size 26 MiB > and the code that will try to use the region should also fail, > later on. > > We do not do anything for non-"no-map" regions, as memblock > explicitly allows reserved regions to overlap, and the commit > that this fixes removed the check for that precise reason. > > [ qperret: fixed conflicts caused by the usage of memblock_mark_nomap ] > > Fixes: 094cb98179f19b7 ("of/fdt: memblock_reserve /memreserve/ regions in the case of partial overlap") > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > Reviewed-by: Stephen Boyd <swboyd@chromium.org> > Signed-off-by: Quentin Perret <qperret@google.com> > --- > drivers/of/fdt.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > Applied, thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] of/fdt: Make sure no-map does not remove already reserved regions 2021-01-15 17:30 ` Rob Herring @ 2021-01-16 0:30 ` Nicolas Boichat 0 siblings, 0 replies; 6+ messages in thread From: Nicolas Boichat @ 2021-01-16 0:30 UTC (permalink / raw) To: Rob Herring Cc: Quentin Perret, Rob Herring, Devicetree List, Florian Fainelli, Stephen Boyd, KarimAllah Ahmed, kernel-team, Frank Rowand, lkml On Sat, Jan 16, 2021 at 1:30 AM Rob Herring <robh@kernel.org> wrote: > > On Fri, 15 Jan 2021 11:45:44 +0000, Quentin Perret wrote: > > From: Nicolas Boichat <drinkcat@chromium.org> > > > > If the device tree is incorrectly configured, and attempts to > > define a "no-map" reserved memory that overlaps with the kernel > > data/code, the kernel would crash quickly after boot, with no > > obvious clue about the nature of the issue. > > > > For example, this would happen if we have the kernel mapped at > > these addresses (from /proc/iomem): > > 40000000-41ffffff : System RAM > > 40080000-40dfffff : Kernel code > > 40e00000-411fffff : reserved > > 41200000-413e0fff : Kernel data > > > > And we declare a no-map shared-dma-pool region at a fixed address > > within that range: > > mem_reserved: mem_region { > > compatible = "shared-dma-pool"; > > reg = <0 0x40000000 0 0x01A00000>; > > no-map; > > }; > > > > To fix this, when removing memory regions at early boot (which is > > what "no-map" regions do), we need to make sure that the memory > > is not already reserved. If we do, __reserved_mem_reserve_reg > > will throw an error: > > [ 0.000000] OF: fdt: Reserved memory: failed to reserve memory > > for node 'mem_region': base 0x0000000040000000, size 26 MiB > > and the code that will try to use the region should also fail, > > later on. > > > > We do not do anything for non-"no-map" regions, as memblock > > explicitly allows reserved regions to overlap, and the commit > > that this fixes removed the check for that precise reason. > > > > [ qperret: fixed conflicts caused by the usage of memblock_mark_nomap ] > > > > Fixes: 094cb98179f19b7 ("of/fdt: memblock_reserve /memreserve/ regions in the case of partial overlap") > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > Reviewed-by: Stephen Boyd <swboyd@chromium.org> > > Signed-off-by: Quentin Perret <qperret@google.com> > > --- > > drivers/of/fdt.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > Thanks for picking up this old patch Quentin ,-) > Applied, thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-16 0:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-15 11:45 [PATCH 0/2] fdt: A couple of no-map fixes Quentin Perret 2021-01-15 11:45 ` [PATCH 1/2] fdt: Properly handle "no-map" field in the memory region Quentin Perret 2021-01-15 17:29 ` Rob Herring 2021-01-15 11:45 ` [PATCH 2/2] of/fdt: Make sure no-map does not remove already reserved regions Quentin Perret 2021-01-15 17:30 ` Rob Herring 2021-01-16 0:30 ` Nicolas Boichat
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).