| Message ID | 1406570583-22848-1-git-send-email-leif.lindholm@linaro.org |
|---|---|
| State | Accepted |
| Commit | 86c8b27a01cf6c16fc159ade223cb2ccc70dc4b5 |
| Headers | show |
On Mon, Jul 28, 2014 at 07:03:03PM +0100, Leif Lindholm wrote: > UEFI provides its own method for marking regions to reserve, via the > memory map which is also used to initialise memblock. So when using the > UEFI memory map, ignore any memreserve entries present in the DT. It's worth noting that no-one is relying on this at present, and were they it would imply that their UEFI implementation is broken (providing a dodgy memory map). So before people start doing dodgy things, let's, close that hole. If we need a way of modifying the memory map we should come up with a more general plan. > Reported-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> > --- > arch/arm64/kernel/efi.c | 2 ++ > arch/arm64/mm/init.c | 4 +++- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c > index 14db1f6..7ad17b2 100644 > --- a/arch/arm64/kernel/efi.c > +++ b/arch/arm64/kernel/efi.c > @@ -188,6 +188,8 @@ static __init void reserve_regions(void) > if (uefi_debug) > pr_cont("\n"); > } > + > + set_bit(EFI_MEMMAP, &efi.flags); Nothing outside of arch/x86 seems to hang off this, so using it here looks fine. > } > > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > index e90c542..58dbf2e 100644 > --- a/arch/arm64/mm/init.c > +++ b/arch/arm64/mm/init.c > @@ -32,6 +32,7 @@ > #include <linux/of_fdt.h> > #include <linux/dma-mapping.h> > #include <linux/dma-contiguous.h> > +#include <linux/efi.h> > > #include <asm/sections.h> > #include <asm/setup.h> > @@ -151,7 +152,8 @@ void __init arm64_memblock_init(void) > memblock_reserve(__pa(swapper_pg_dir), SWAPPER_DIR_SIZE); > memblock_reserve(__pa(idmap_pg_dir), IDMAP_DIR_SIZE); > > - early_init_fdt_scan_reserved_mem(); > + if (!efi_enabled(EFI_MEMMAP)) > + early_init_fdt_scan_reserved_mem(); And this is a false stub for !EFI, so that looks fine. FWIW: Reviewed-by: Mark Rutland <mark.rutland@arm.com> I wonder if we have any other flat tree uses we need to be careful of when booting via EFI. Thanks Mark. > > /* 4GB maximum for 32-bit only capable devices */ > if (IS_ENABLED(CONFIG_ZONE_DMA)) > -- > 1.7.10.4 > >
On Mon, Jul 28, 2014 at 07:14:49PM +0100, Mark Rutland wrote: > On Mon, Jul 28, 2014 at 07:03:03PM +0100, Leif Lindholm wrote: > > UEFI provides its own method for marking regions to reserve, via the > > memory map which is also used to initialise memblock. So when using the > > UEFI memory map, ignore any memreserve entries present in the DT. > > It's worth noting that no-one is relying on this at present, and were > they it would imply that their UEFI implementation is broken (providing > a dodgy memory map). > > So before people start doing dodgy things, let's, close that hole. If we > need a way of modifying the memory map we should come up with a more > general plan. Catalin - any comments? As it is a bugfix, can it go into 3.17? / Leif > > Reported-by: Mark Rutland <mark.rutland@arm.com> > > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> > > --- > > arch/arm64/kernel/efi.c | 2 ++ > > arch/arm64/mm/init.c | 4 +++- > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c > > index 14db1f6..7ad17b2 100644 > > --- a/arch/arm64/kernel/efi.c > > +++ b/arch/arm64/kernel/efi.c > > @@ -188,6 +188,8 @@ static __init void reserve_regions(void) > > if (uefi_debug) > > pr_cont("\n"); > > } > > + > > + set_bit(EFI_MEMMAP, &efi.flags); > > Nothing outside of arch/x86 seems to hang off this, so using it here > looks fine. > > > } > > > > > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > > index e90c542..58dbf2e 100644 > > --- a/arch/arm64/mm/init.c > > +++ b/arch/arm64/mm/init.c > > @@ -32,6 +32,7 @@ > > #include <linux/of_fdt.h> > > #include <linux/dma-mapping.h> > > #include <linux/dma-contiguous.h> > > +#include <linux/efi.h> > > > > #include <asm/sections.h> > > #include <asm/setup.h> > > @@ -151,7 +152,8 @@ void __init arm64_memblock_init(void) > > memblock_reserve(__pa(swapper_pg_dir), SWAPPER_DIR_SIZE); > > memblock_reserve(__pa(idmap_pg_dir), IDMAP_DIR_SIZE); > > > > - early_init_fdt_scan_reserved_mem(); > > + if (!efi_enabled(EFI_MEMMAP)) > > + early_init_fdt_scan_reserved_mem(); > > And this is a false stub for !EFI, so that looks fine. > > FWIW: > > Reviewed-by: Mark Rutland <mark.rutland@arm.com> > > I wonder if we have any other flat tree uses we need to be careful of > when booting via EFI.
On Wed, Aug 13, 2014 at 01:22:39PM +0100, Leif Lindholm wrote: > On Mon, Jul 28, 2014 at 07:14:49PM +0100, Mark Rutland wrote: > > On Mon, Jul 28, 2014 at 07:03:03PM +0100, Leif Lindholm wrote: > > > UEFI provides its own method for marking regions to reserve, via the > > > memory map which is also used to initialise memblock. So when using the > > > UEFI memory map, ignore any memreserve entries present in the DT. > > > > It's worth noting that no-one is relying on this at present, and were > > they it would imply that their UEFI implementation is broken (providing > > a dodgy memory map). > > > > So before people start doing dodgy things, let's, close that hole. If we > > need a way of modifying the memory map we should come up with a more > > general plan. > > Catalin - any comments? > As it is a bugfix, can it go into 3.17? It looks fine to me: Acked-by: Catalin Marinas <catalin.marinas@arm.com> (cc'ing Will who handles the 3.17 merging) > > > Reported-by: Mark Rutland <mark.rutland@arm.com> > > > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> > > > --- > > > arch/arm64/kernel/efi.c | 2 ++ > > > arch/arm64/mm/init.c | 4 +++- > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c > > > index 14db1f6..7ad17b2 100644 > > > --- a/arch/arm64/kernel/efi.c > > > +++ b/arch/arm64/kernel/efi.c > > > @@ -188,6 +188,8 @@ static __init void reserve_regions(void) > > > if (uefi_debug) > > > pr_cont("\n"); > > > } > > > + > > > + set_bit(EFI_MEMMAP, &efi.flags); > > > } > > > > > > > > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > > > index e90c542..58dbf2e 100644 > > > --- a/arch/arm64/mm/init.c > > > +++ b/arch/arm64/mm/init.c > > > @@ -32,6 +32,7 @@ > > > #include <linux/of_fdt.h> > > > #include <linux/dma-mapping.h> > > > #include <linux/dma-contiguous.h> > > > +#include <linux/efi.h> > > > > > > #include <asm/sections.h> > > > #include <asm/setup.h> > > > @@ -151,7 +152,8 @@ void __init arm64_memblock_init(void) > > > memblock_reserve(__pa(swapper_pg_dir), SWAPPER_DIR_SIZE); > > > memblock_reserve(__pa(idmap_pg_dir), IDMAP_DIR_SIZE); > > > > > > - early_init_fdt_scan_reserved_mem(); > > > + if (!efi_enabled(EFI_MEMMAP)) > > > + early_init_fdt_scan_reserved_mem();
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c index 14db1f6..7ad17b2 100644 --- a/arch/arm64/kernel/efi.c +++ b/arch/arm64/kernel/efi.c @@ -188,6 +188,8 @@ static __init void reserve_regions(void) if (uefi_debug) pr_cont("\n"); } + + set_bit(EFI_MEMMAP, &efi.flags); } diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index e90c542..58dbf2e 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -32,6 +32,7 @@ #include <linux/of_fdt.h> #include <linux/dma-mapping.h> #include <linux/dma-contiguous.h> +#include <linux/efi.h> #include <asm/sections.h> #include <asm/setup.h> @@ -151,7 +152,8 @@ void __init arm64_memblock_init(void) memblock_reserve(__pa(swapper_pg_dir), SWAPPER_DIR_SIZE); memblock_reserve(__pa(idmap_pg_dir), IDMAP_DIR_SIZE); - early_init_fdt_scan_reserved_mem(); + if (!efi_enabled(EFI_MEMMAP)) + early_init_fdt_scan_reserved_mem(); /* 4GB maximum for 32-bit only capable devices */ if (IS_ENABLED(CONFIG_ZONE_DMA))
UEFI provides its own method for marking regions to reserve, via the memory map which is also used to initialise memblock. So when using the UEFI memory map, ignore any memreserve entries present in the DT. Reported-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> --- arch/arm64/kernel/efi.c | 2 ++ arch/arm64/mm/init.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)