arm64: don't call early_*map() post paging_init()
v2 cover arm64: don't call early_*map() post paging_init()
This (tiny) series resolves a fairly serious problem with early_ioremap/iounmap/memremap/memunmap on arm64. These functions cannot safely be called after paging_init(), but the sanity check was not triggering.
As a result, a fixmap entry was incorrectly cleared during early_initcalls on arm64 UEFI systems.
1/2 reworks the arm64 UEFI support code to not attempt these calls and
2/2 enables the sanity check
Changes since v1:
- Rebased to v3.19-rc3
- Added 'Fixes:' tags
- Reworked 1/2 to restore call to efi_setup_idmap() to the original location in the boot process.
Leif Lindholm (2):
arm64: don't make early_*map() calls post paging_init()
arm64: call early_ioremap_reset() in paging_init()
arch/arm64/include/asm/efi.h | 4 ++-- arch/arm64/kernel/efi.c | 27 ++++++++++++++------------- arch/arm64/kernel/setup.c | 2 +- arch/arm64/mm/mmu.c | 1 + 4 files changed, 18 insertions(+), 16 deletions(-)
Signature · 1 line
2.1.3
PATCH v2 1/2 arm64: don't make early_*map() calls post paging_init()
arm64 early_ioremap/iounmap/memremap/memunmap are not supported beyond the call to paging_init(), but arm64_enter_virtual_mode() (an early initcall) makes one call to unmap the UEFI memory map.
Rearrange the code to unmap this region before paging_init(), and then pull back the remapping of the EFI memory map to the second part of UEFI initialisation - efi_idmap_init() - renaming that function as efi_memmap_init(), which better describes what it now does.
--- arch/arm64/include/asm/efi.h | 4 ++-- arch/arm64/kernel/efi.c | 27 ++++++++++++++------------- arch/arm64/kernel/setup.c | 2 +- 3 files changed, 17 insertions(+), 16 deletions(-)
arch/arm64/include/asm/efi.h +2 -2
@@ -6,10 +6,10 @@#ifdef CONFIG_EFI extern void efi_init (void ); -extern void efi_idmap_init (void ); +extern void efi_memmap_init (void );#else #define efi_init () -#define efi_idmap_init () +#define efi_memmap_init ()#endif #define efi_call_virt (f, ...) \
arch/arm64/kernel/efi.c +14 -13
@@ -313,17 +313,26 @@ void __init efi_init(void) memmap.desc_size = params.desc_size; memmap.desc_version = params.desc_ver; -if (uefi_init () <0 ) -return ; +if (uefi_init () >=0 ) +reserve_regions (); -reserve_regions (); +early_memunmap (memmap.map, params.mmap_size); } -void __init efi_idmap_init (void ) +void __init efi_memmap_init (void ) { +u64 mapsize; +if (!efi_enabled (EFI_BOOT ))return ; +/* replace early memmap mapping with permanent mapping */ + mapsize = memmap.map_end - memmap.map; + memmap.map = (__force void *)ioremap_cache ((phys_addr_t )memmap.phys_map, + mapsize); + memmap.map_end = memmap.map + mapsize; + efi.memmap = &memmap; +/* boot time idmap_pg_dir is incomplete, so fill in missing parts */ efi_setup_idmap (); }
@@ -379,23 +388,15 @@ static int __init arm64_enter_virtual_mode(void)return -1 ; } - mapsize = memmap.map_end - memmap.map; -early_memunmap (memmap.map, mapsize); -if (efi_runtime_disabled ()) {pr_info ("EFI runtime services will be disabled.\n" );return -1 ; }pr_info ("Remapping and enabling EFI services.\n" ); -/* replace early memmap mapping with permanent mapping */ - memmap.map = (__force void *)ioremap_cache ((phys_addr_t )memmap.phys_map, - mapsize); - memmap.map_end = memmap.map + mapsize; - - efi.memmap = &memmap;/* Map the runtime regions */ + mapsize = memmap.map_end - memmap.map; virtmap =kmalloc (mapsize,GFP_KERNEL );if (!virtmap) {pr_err ("Failed to allocate EFI virtual memmap\n" );
arch/arm64/kernel/setup.c +1 -1
@@ -401,7 +401,7 @@ void __init setup_arch(char **cmdline_p)paging_init ();request_standard_resources (); -efi_idmap_init (); +efi_memmap_init ();unflatten_device_tree ();
Signature · 1 line
2.1.3
PATCH v2 1/2 arm64: don't make early_*map() calls post paging_init()
Quoted 53 lines · Leif Lindholm
arm64 early_ioremap/iounmap/memremap/memunmap are not supported beyond the call to
paging_init(), butarm64_enter_virtual_mode()(an early initcall) makes one call to unmap the UEFI memory map.Rearrange the code to unmap this region before
paging_init(), and then pull back the remapping of the EFI memory map to the second part of UEFI initialisation -efi_idmap_init()- renaming that function asefi_memmap_init(), which better describes what it now does.Signed-off-by Leif Lindholm <[email protected]>Fixes f84d02755f5a ("arm64: add EFI runtime services")--- arch/arm64/include/asm/efi.h | 4 ++-- arch/arm64/kernel/efi.c | 27 ++++++++++++++------------- arch/arm64/kernel/setup.c | 2 +- 3 files changed, 17 insertions(+), 16 deletions(-)arch/arm64/include/asm/efi.h +2
-2@@ -6,10 +6,10 @@#ifdef CONFIG_EFI extern void efi_init (void ); -extern void efi_idmap_init (void ); +extern void efi_memmap_init (void );#else #define efi_init () -#define efi_idmap_init () +#define efi_memmap_init ()#endif #define efi_call_virt (f, ...) \arch/arm64/kernel/efi.c +4
-4@@ -313,17 +313,26 @@ void __init efi_init(void) memmap.desc_size = params.desc_size; memmap.desc_version = params.desc_ver; -if (uefi_init () <0 ) -return ; +if (uefi_init () >=0 ) +reserve_regions (); -reserve_regions (); +early_memunmap (memmap.map, params.mmap_size); } -void __init efi_idmap_init (void ) +void __init efi_memmap_init (void )
arch/arm64/kernel/efi.c void __init efi_init(void) · line 332
Quoted 10 lines
arch/arm64/kernel/efi.c +6
-0{ +u64 mapsize; +if (!efi_enabled (EFI_BOOT ))return ; +/* replace early memmap mapping with permanent mapping */ + mapsize = memmap.map_end - memmap.map; + memmap.map = (__force void *)ioremap_cache ((phys_addr_t )memmap.phys_map, + mapsize);
ioremap_cache() could potententially fail here if the phys_map address doesn't have a valid pfn (not in the kernel linear ram mapping) because some of the underlying vm support hasn't been initialized yet.
Since this is a mapping which we need during early boot and beyond, why not just create a fixed mapping for it similar to what we do for the early console (see FIX_EARLYCON_MEM_BASE)?
Quoted 44 lines
arch/arm64/kernel/efi.c +4
-9+ memmap.map_end = memmap.map + mapsize; + efi.memmap = &memmap; +/* boot time idmap_pg_dir is incomplete, so fill in missing parts */ efi_setup_idmap (); }@@ -379,23 +388,15 @@ static int __init arm64_enter_virtual_mode(void)return -1 ; } - mapsize = memmap.map_end - memmap.map; -early_memunmap (memmap.map, mapsize); -if (efi_runtime_disabled ()) {pr_info ("EFI runtime services will be disabled.\n" );return -1 ; }pr_info ("Remapping and enabling EFI services.\n" ); -/* replace early memmap mapping with permanent mapping */ - memmap.map = (__force void *)ioremap_cache ((phys_addr_t )memmap.phys_map, - mapsize); - memmap.map_end = memmap.map + mapsize; - - efi.memmap = &memmap;/* Map the runtime regions */ + mapsize = memmap.map_end - memmap.map; virtmap =kmalloc (mapsize,GFP_KERNEL );if (!virtmap) {pr_err ("Failed to allocate EFI virtual memmap\n" );arch/arm64/kernel/setup.c +1
-1@@ -401,7 +401,7 @@ void __init setup_arch(char **cmdline_p)paging_init ();request_standard_resources (); -efi_idmap_init (); +efi_memmap_init ();unflatten_device_tree ();
PATCH v2 1/2 arm64: don't make early_*map() calls post paging_init()
Quoted 26 lines · Mark Salter
Quoted 25 lines · Leif Lindholm
arch/arm64/kernel/efi.c +6
-4@@ -313,17 +313,26 @@ void __init efi_init(void) memmap.desc_size = params.desc_size; memmap.desc_version = params.desc_ver; -if (uefi_init () <0 ) -return ; +if (uefi_init () >=0 ) +reserve_regions (); -reserve_regions (); +early_memunmap (memmap.map, params.mmap_size); } -void __init efi_idmap_init (void ) +void __init efi_memmap_init (void ) { +u64 mapsize; +if (!efi_enabled (EFI_BOOT ))return ;
Quoted 8 lines
arch/arm64/kernel/efi.c void __init efi_init(void) · line 332
Quoted 4 lines
arch/arm64/kernel/efi.c +4
-0+/* replace early memmap mapping with permanent mapping */ + mapsize = memmap.map_end - memmap.map; + memmap.map = (__force void *)ioremap_cache ((phys_addr_t )memmap.phys_map, + mapsize);
ioremap_cache()could potententially fail here if thephys_mapaddress doesn't have a valid pfn (not in the kernel linear ram mapping) because some of the underlying vm support hasn't been initialized yet.
Can you be more specific about the case you have in mind, please? pfn_valid uses the memblocks on arm64, and that should all have been sorted out in paging_init(). What's the issue that you're anticipating?
Will
PATCH v2 1/2 arm64: don't make early_*map() calls post paging_init()
Quoted 36 lines · Will Deacon
Quoted 26 lines · Mark Salter
Quoted 25 lines · Leif Lindholm
arch/arm64/kernel/efi.c +6
-4@@ -313,17 +313,26 @@ void __init efi_init(void) memmap.desc_size = params.desc_size; memmap.desc_version = params.desc_ver; -if (uefi_init () <0 ) -return ; +if (uefi_init () >=0 ) +reserve_regions (); -reserve_regions (); +early_memunmap (memmap.map, params.mmap_size); } -void __init efi_idmap_init (void ) +void __init efi_memmap_init (void ) { +u64 mapsize; +if (!efi_enabled (EFI_BOOT ))return ;Quoted 8 lines
arch/arm64/kernel/efi.c void __init efi_init(void) · line 332
Quoted 4 lines
arch/arm64/kernel/efi.c +4
-0+/* replace early memmap mapping with permanent mapping */ + mapsize = memmap.map_end - memmap.map; + memmap.map = (__force void *)ioremap_cache ((phys_addr_t )memmap.phys_map, + mapsize);
ioremap_cache()could potententially fail here if thephys_mapaddress doesn't have a valid pfn (not in the kernel linear ram mapping) because some of the underlying vm support hasn't been initialized yet.
Quoted 4 lines
Can you be more specific about the case you have in mind, please?
pfn_validuses the memblocks on arm64, and that should all have been sorted out inpaging_init(). What's the issue that you're anticipating?
I think Mark's concern is that it is too early to call __get_free_page(), which is what happens if ioremap_cache() finds that the requested address is not covered by the existing linear mapping. Currently, UEFI reserved RAM regions are covered by the linear mapping, but that is something we intend to change in the future.
Signature · 1 line
Ard.
PATCH v2 1/2 arm64: don't make early_*map() calls post paging_init()
Quoted 21 lines · Ard Biesheuvel
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
arch/arm64/kernel/efi.c +3
-1-void __init efi_idmap_init (void ) +void __init efi_memmap_init (void ) { +u64 mapsize; +if (!efi_enabled (EFI_BOOT ))return ;Quoted 8 lines
arch/arm64/kernel/efi.c void __init efi_init(void) · line 332
Quoted 4 lines
arch/arm64/kernel/efi.c +4
-0+/* replace early memmap mapping with permanent mapping */ + mapsize = memmap.map_end - memmap.map; + memmap.map = (__force void *)ioremap_cache ((phys_addr_t )memmap.phys_map, + mapsize);
ioremap_cache()could potententially fail here if thephys_mapaddress doesn't have a valid pfn (not in the kernel linear ram mapping) because some of the underlying vm support hasn't been initialized yet.Quoted 3 lines
Can you be more specific about the case you have in mind, please?
pfn_validuses the memblocks on arm64, and that should all have been sorted out inpaging_init(). What's the issue that you're anticipating?
Quoted 5 lines
I think Mark's concern is that it is too early to call
__get_free_page(), which is what happens ifioremap_cache()finds that the requested address is not covered by the existing linear mapping. Currently, UEFI reserved RAM regions are covered by the linear mapping, but that is something we intend to change in the future.
Which shouldn't be a problem, right? Since this function will be going away with your "stable mappings" set, and the remap call bumped down to an early initcall in arm64_enter_virtual_mode() (or potential future name for that function).
/
Leif
PATCH v2 1/2 arm64: don't make early_*map() calls post paging_init()
Quoted 28 lines · Leif Lindholm
Quoted 21 lines · Ard Biesheuvel
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
arch/arm64/kernel/efi.c +3
-1-void __init efi_idmap_init (void ) +void __init efi_memmap_init (void ) { +u64 mapsize; +if (!efi_enabled (EFI_BOOT ))return ;Quoted 8 lines
arch/arm64/kernel/efi.c void __init efi_init(void) · line 332
Quoted 4 lines
arch/arm64/kernel/efi.c +4
-0+/* replace early memmap mapping with permanent mapping */ + mapsize = memmap.map_end - memmap.map; + memmap.map = (__force void *)ioremap_cache ((phys_addr_t )memmap.phys_map, + mapsize);
ioremap_cache()could potententially fail here if thephys_mapaddress doesn't have a valid pfn (not in the kernel linear ram mapping) because some of the underlying vm support hasn't been initialized yet.Quoted 3 lines
Can you be more specific about the case you have in mind, please?
pfn_validuses the memblocks on arm64, and that should all have been sorted out inpaging_init(). What's the issue that you're anticipating?Quoted 5 lines
I think Mark's concern is that it is too early to call
__get_free_page(), which is what happens ifioremap_cache()finds that the requested address is not covered by the existing linear mapping. Currently, UEFI reserved RAM regions are covered by the linear mapping, but that is something we intend to change in the future.
Quoted 4 lines
Which shouldn't be a problem, right? Since this function will be going away with your "stable mappings" set, and the remap call bumped down to an early initcall in
arm64_enter_virtual_mode()(or potential future name for that function).
Sounds reasonable to me... Ard?
However, I'd certainly like something in the commit log and/or a comment in the code with our reasoning.
Will
PATCH v2 2/2 arm64: call early_ioremap_reset() in paging_init()
In reply to Leif Lindholmarm64 does not support early_memremap/memunmap/ioremap/iounmap after paging_init() has been called. The core early_*remap code handles this via the after_paging_init variable, which is set by a call to early_ioremap_reset().
However, arm64 currently does not call early_ioremap_reset(), which has made it possible to poke around in the fixmap region after kmap is enabled. Add the required call.
--- arch/arm64/mm/mmu.c | 1 + 1 file changed, 1 insertion(+)
arch/arm64/mm/mmu.c +1 -0
@@ -348,6 +348,7 @@ void __init paging_init(void) {void *zero_page; +early_ioremap_reset ();map_mem ();/*
Signature · 1 line
2.1.3
v2 cover arm64: don't call early_*map() post paging_init()
In reply to Leif LindholmQuoted 12 lines · Leif Lindholm
This (tiny) series resolves a fairly serious problem with early_ioremap/iounmap/memremap/memunmap on arm64. These functions cannot safely be called after
paging_init(), but the sanity check was not triggering.As a result, a fixmap entry was incorrectly cleared during
early_initcallson arm64 UEFI systems.1/2 reworks the arm64 UEFI support code to not attempt these calls and
2/2 enables the sanity check
Quoted 5 lines
Changes since v1:
- Rebased to v3.19-rc3
- Added 'Fixes:' tags
- Reworked 1/2 to restore call toefi_setup_idmap()to the original location in the boot process.
Looks reasonable to me; do they need a CC stable, or is this not a problem that matters in practice?
Will
v2 cover arm64: don't call early_*map() post paging_init()
Quoted 13 lines · Will Deacon
Quoted 12 lines · Leif Lindholm
This (tiny) series resolves a fairly serious problem with early_ioremap/iounmap/memremap/memunmap on arm64. These functions cannot safely be called after
paging_init(), but the sanity check was not triggering.As a result, a fixmap entry was incorrectly cleared during
early_initcallson arm64 UEFI systems.1/2 reworks the arm64 UEFI support code to not attempt these calls and
2/2 enables the sanity check
Quoted 7 lines
Quoted 5 lines
Changes since v1:
- Rebased to v3.19-rc3
- Added 'Fixes:' tags
- Reworked 1/2 to restore call toefi_setup_idmap()to the original location in the boot process.Looks reasonable to me; do they need a CC stable,
Yes, but with a note that both patches should be taken and the order is retained, or we break bisect.
Signature · 1 line
Ard.
Quoted 4 lines
or is this not a problem
that matters in practice?Will
v2 cover arm64: don't call early_*map() post paging_init()
Quoted 10 lines · Ard Biesheuvel
Quoted 7 lines
Quoted 5 lines
Changes since v1:
- Rebased to v3.19-rc3
- Added 'Fixes:' tags
- Reworked 1/2 to restore call toefi_setup_idmap()to the original location in the boot process.Looks reasonable to me; do they need a CC stable,
Yes, but with a note that both patches should be taken and the order is retained, or we break bisect.
Where would one add such a note?
/
Leif
v2 cover arm64: don't call early_*map() post paging_init()
Quoted 7 lines · Leif Lindholm
Quoted 6 lines · Ard Biesheuvel
Quoted 6 lines
Quoted 5 lines
Changes since v1:
- Rebased to v3.19-rc3
- Added 'Fixes:' tags
- Reworked 1/2 to restore call toefi_setup_idmap()to the original location in the boot process.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Looks reasonable to me; do they need a CC stable,
Yes, but with a note that both patches should be taken and the order is retained, or we break bisect.
Where would one add such a note?
I guess the best way would be to not add a 'Cc: stable' tag, but send an email to Greg once the patches have been merged by Linus.
Signature · 1 line
Ard.
v2 cover arm64: don't call early_*map() post paging_init()
Quoted 10 lines · Ard Biesheuvel
Quoted 9 lines · Leif Lindholm
Quoted 8 lines · Ard Biesheuvel
Quoted 7 lines
Quoted 5 lines
Changes since v1:
- Rebased to v3.19-rc3
- Added 'Fixes:' tags
- Reworked 1/2 to restore call toefi_setup_idmap()to the original location in the boot process.Looks reasonable to me; do they need a CC stable,
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Yes, but with a note that both patches should be taken and the order is retained, or we break bisect.
Where would one add such a note?
I guess the best way would be to not add a 'Cc: stable' tag, but send an email to Greg once the patches have been merged by Linus.
I thought Greg didn't take anything until it had hit mainline anyway? I think it should be fine with just the Cc...
Will
v2 cover arm64: don't call early_*map() post paging_init()
Quoted 14 lines · Will Deacon
Quoted 13 lines · Ard Biesheuvel
Quoted 12 lines · Leif Lindholm
Quoted 10 lines · Ard Biesheuvel
Quoted 7 lines
Quoted 5 lines
Changes since v1:
- Rebased to v3.19-rc3
- Added 'Fixes:' tags
- Reworked 1/2 to restore call toefi_setup_idmap()to the original location in the boot process.Looks reasonable to me; do they need a CC stable,
Yes, but with a note that both patches should be taken and the order is retained, or we break bisect.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
Where would one add such a note?
I guess the best way would be to not add a 'Cc: stable' tag, but send an email to Greg once the patches have been merged by Linus.
I thought Greg didn't take anything until it had hit mainline anyway? I think it should be fine with just the Cc...
I know he doesn't, but do you know for a fact that he always takes patches in the same order they appear in mainline? It would seem like the reasonable thing to do, I guess, but we just have to make sure.
v2 cover arm64: don't call early_*map() post paging_init()
Quoted 21 lines · Ard Biesheuvel
Quoted 20 lines · Will Deacon
Quoted 18 lines · Ard Biesheuvel
Quoted 7 lines · Leif Lindholm
Quoted 6 lines · Ard Biesheuvel
Quoted 6 lines
Quoted 5 lines
Changes since v1:
- Rebased to v3.19-rc3
- Added 'Fixes:' tags
- Reworked 1/2 to restore call toefi_setup_idmap()to the original location in the boot process.Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Looks reasonable to me; do they need a CC stable,
Yes, but with a note that both patches should be taken and the order is retained, or we break bisect.
Where would one add such a note?
I guess the best way would be to not add a 'Cc: stable' tag, but send an email to Greg once the patches have been merged by Linus.
Quoted 7 lines
Quoted 3 lines
I thought Greg didn't take anything until it had hit mainline anyway? I think it should be fine with just the Cc...
I know he doesn't, but do you know for a fact that he always takes patches in the same order they appear in mainline? It would seem like the reasonable thing to do, I guess, but we just have to make sure.
If we really want to make sure we could either fold the two patches into one or add a dependency without a corresponding sha. I don't like waiting until they've hit mainline, because I'm sure that I'll forget!
Will