[PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[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_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(-)

-- 
2.1.3

[PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[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_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(-)

-- 
2.1.3

[PATCH v2 1/2] arm64: don't make early_*map() calls post paging_init()

[Leif Lindholm]

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.

Signed-off-by: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
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(-)

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index a34fd3b..92f4d44 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -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, ...)						\
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 6fac253..e311066 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -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");
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index b809911..ebf7820 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -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();
 
-- 
2.1.3

[PATCH v2 1/2] arm64: don't make early_*map() calls post paging_init()

[Leif Lindholm]

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.

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
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(-)

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index a34fd3b..92f4d44 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -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, ...)						\
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 6fac253..e311066 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -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");
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index b809911..ebf7820 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -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();
 
-- 
2.1.3

[PATCH v2 2/2] arm64: call early_ioremap_reset() in paging_init()

[Leif Lindholm]

arm64 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.

Signed-off-by: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Fixes: bf4b558eba92 ("arm64: add early_ioremap support")
Acked-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/mm/mmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 6032f3e..506544f 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -348,6 +348,7 @@ void __init paging_init(void)
 {
 	void *zero_page;
 
+	early_ioremap_reset();
 	map_mem();
 
 	/*
-- 
2.1.3

[PATCH v2 2/2] arm64: call early_ioremap_reset() in paging_init()

[Leif Lindholm]

arm64 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.

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
Fixes: bf4b558eba92 ("arm64: add early_ioremap support")
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/mm/mmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 6032f3e..506544f 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -348,6 +348,7 @@ void __init paging_init(void)
 {
 	void *zero_page;
 
+	early_ioremap_reset();
 	map_mem();
 
 	/*
-- 
2.1.3

Re: [PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Will Deacon]

On Tue, Jan 06, 2015 at 01:41:11PM +0000, Leif Lindholm wrote:
> 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.

Looks reasonable to me; do they need a CC stable, or is this not a problem
that matters in practice?

Will

[PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Will Deacon]

On Tue, Jan 06, 2015 at 01:41:11PM +0000, Leif Lindholm wrote:
> 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.

Looks reasonable to me; do they need a CC stable, or is this not a problem
that matters in practice?

Will

Re: [PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Ard Biesheuvel]

On 6 January 2015 at 13:59, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Tue, Jan 06, 2015 at 01:41:11PM +0000, Leif Lindholm wrote:
>> 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.
>
> 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.

-- 
Ard.


> or is this not a problem
> that matters in practice?
>
> Will

[PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Ard Biesheuvel]

On 6 January 2015 at 13:59, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jan 06, 2015 at 01:41:11PM +0000, Leif Lindholm wrote:
>> 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.
>
> 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.

-- 
Ard.


> or is this not a problem
> that matters in practice?
>
> Will

Re: [PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Leif Lindholm]

On Tue, Jan 06, 2015 at 02:04:50PM +0000, Ard Biesheuvel wrote:
> >> 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.
> >
> > 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

[PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Leif Lindholm]

On Tue, Jan 06, 2015 at 02:04:50PM +0000, Ard Biesheuvel wrote:
> >> 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.
> >
> > 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

Re: [PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Ard Biesheuvel]

On 6 January 2015 at 15:52, Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Jan 06, 2015 at 02:04:50PM +0000, Ard Biesheuvel wrote:
>> >> 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.
>> >
>> > 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.

-- 
Ard.

[PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Ard Biesheuvel]

On 6 January 2015 at 15:52, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Jan 06, 2015 at 02:04:50PM +0000, Ard Biesheuvel wrote:
>> >> 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.
>> >
>> > 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.

-- 
Ard.

Re: [PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Will Deacon]

On Tue, Jan 06, 2015 at 03:54:29PM +0000, Ard Biesheuvel wrote:
> On 6 January 2015 at 15:52, Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Tue, Jan 06, 2015 at 02:04:50PM +0000, Ard Biesheuvel wrote:
> >> >> 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.
> >> >
> >> > 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.

I thought Greg didn't take anything until it had hit mainline anyway? I
think it should be fine with just the Cc...

Will

[PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Will Deacon]

On Tue, Jan 06, 2015 at 03:54:29PM +0000, Ard Biesheuvel wrote:
> On 6 January 2015 at 15:52, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Tue, Jan 06, 2015 at 02:04:50PM +0000, Ard Biesheuvel wrote:
> >> >> 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.
> >> >
> >> > 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.

I thought Greg didn't take anything until it had hit mainline anyway? I
think it should be fine with just the Cc...

Will

Re: [PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Ard Biesheuvel]

On 6 January 2015 at 16:05, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Tue, Jan 06, 2015 at 03:54:29PM +0000, Ard Biesheuvel wrote:
>> On 6 January 2015 at 15:52, Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > On Tue, Jan 06, 2015 at 02:04:50PM +0000, Ard Biesheuvel wrote:
>> >> >> 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.
>> >> >
>> >> > 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.
>
> 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.

[PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Ard Biesheuvel]

On 6 January 2015 at 16:05, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jan 06, 2015 at 03:54:29PM +0000, Ard Biesheuvel wrote:
>> On 6 January 2015 at 15:52, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> > On Tue, Jan 06, 2015 at 02:04:50PM +0000, Ard Biesheuvel wrote:
>> >> >> 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.
>> >> >
>> >> > 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.
>
> 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.

Re: [PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Will Deacon]

On Tue, Jan 06, 2015 at 04:23:58PM +0000, Ard Biesheuvel wrote:
> On 6 January 2015 at 16:05, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> > On Tue, Jan 06, 2015 at 03:54:29PM +0000, Ard Biesheuvel wrote:
> >> On 6 January 2015 at 15:52, Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >> > On Tue, Jan 06, 2015 at 02:04:50PM +0000, Ard Biesheuvel wrote:
> >> >> >> 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.
> >> >> >
> >> >> > 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.
> >
> > 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

[PATCH v2 0/2] arm64: don't call early_*map() post paging_init()

[Will Deacon]

On Tue, Jan 06, 2015 at 04:23:58PM +0000, Ard Biesheuvel wrote:
> On 6 January 2015 at 16:05, Will Deacon <will.deacon@arm.com> wrote:
> > On Tue, Jan 06, 2015 at 03:54:29PM +0000, Ard Biesheuvel wrote:
> >> On 6 January 2015 at 15:52, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> > On Tue, Jan 06, 2015 at 02:04:50PM +0000, Ard Biesheuvel wrote:
> >> >> >> 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.
> >> >> >
> >> >> > 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.
> >
> > 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

Re: [PATCH v2 1/2] arm64: don't make early_*map() calls post paging_init()

[Mark Salter]

On Tue, 2015-01-06 at 13:41 +0000, Leif Lindholm wrote:
> 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.

> Signed-off-by: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 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(-)
> 
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index a34fd3b..92f4d44 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -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, ...)						\
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 6fac253..e311066 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -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);

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)? 

> +	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");
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index b809911..ebf7820 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -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()

[Mark Salter]

On Tue, 2015-01-06 at 13:41 +0000, Leif Lindholm wrote:
> 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.

> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> 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(-)
> 
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index a34fd3b..92f4d44 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -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, ...)						\
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 6fac253..e311066 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -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);

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)? 

> +	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");
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index b809911..ebf7820 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -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();
>  

Re: [PATCH v2 1/2] arm64: don't make early_*map() calls post paging_init()

[Will Deacon]

On Tue, Jan 06, 2015 at 08:35:22PM +0000, Mark Salter wrote:
> On Tue, 2015-01-06 at 13:41 +0000, Leif Lindholm wrote:
> > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > index 6fac253..e311066 100644
> > --- a/arch/arm64/kernel/efi.c
> > +++ b/arch/arm64/kernel/efi.c
> > @@ -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);
> 
> 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.

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()

[Will Deacon]

On Tue, Jan 06, 2015 at 08:35:22PM +0000, Mark Salter wrote:
> On Tue, 2015-01-06 at 13:41 +0000, Leif Lindholm wrote:
> > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > index 6fac253..e311066 100644
> > --- a/arch/arm64/kernel/efi.c
> > +++ b/arch/arm64/kernel/efi.c
> > @@ -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);
> 
> 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.

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

Re: [PATCH v2 1/2] arm64: don't make early_*map() calls post paging_init()

[Ard Biesheuvel]

On 7 January 2015 at 10:58, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jan 06, 2015 at 08:35:22PM +0000, Mark Salter wrote:
>> On Tue, 2015-01-06 at 13:41 +0000, Leif Lindholm wrote:
>> > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> > index 6fac253..e311066 100644
>> > --- a/arch/arm64/kernel/efi.c
>> > +++ b/arch/arm64/kernel/efi.c
>> > @@ -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);
>>
>> 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.
>
> 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?
>

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.

-- 
Ard.

[PATCH v2 1/2] arm64: don't make early_*map() calls post paging_init()

[Ard Biesheuvel]

On 7 January 2015 at 10:58, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jan 06, 2015 at 08:35:22PM +0000, Mark Salter wrote:
>> On Tue, 2015-01-06 at 13:41 +0000, Leif Lindholm wrote:
>> > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> > index 6fac253..e311066 100644
>> > --- a/arch/arm64/kernel/efi.c
>> > +++ b/arch/arm64/kernel/efi.c
>> > @@ -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);
>>
>> 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.
>
> 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?
>

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.

-- 
Ard.

Re: [PATCH v2 1/2] arm64: don't make early_*map() calls post paging_init()

[Leif Lindholm]

On Wed, Jan 07, 2015 at 01:13:06PM +0000, Ard Biesheuvel wrote:
> >> > -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);
> >>
> >> 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.
> >
> > 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?
> 
> 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.

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()

[Leif Lindholm]

On Wed, Jan 07, 2015 at 01:13:06PM +0000, Ard Biesheuvel wrote:
> >> > -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);
> >>
> >> 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.
> >
> > 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?
> 
> 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.

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

Re: [PATCH v2 1/2] arm64: don't make early_*map() calls post paging_init()

[Will Deacon]

On Wed, Jan 07, 2015 at 01:31:09PM +0000, Leif Lindholm wrote:
> On Wed, Jan 07, 2015 at 01:13:06PM +0000, Ard Biesheuvel wrote:
> > >> > -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);
> > >>
> > >> 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.
> > >
> > > 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?
> > 
> > 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.
> 
> 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 1/2] arm64: don't make early_*map() calls post paging_init()

[Will Deacon]

On Wed, Jan 07, 2015 at 01:31:09PM +0000, Leif Lindholm wrote:
> On Wed, Jan 07, 2015 at 01:13:06PM +0000, Ard Biesheuvel wrote:
> > >> > -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);
> > >>
> > >> 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.
> > >
> > > 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?
> > 
> > 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.
> 
> 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