* [PATCH efi-next 0/3] efi: assorted fixes
@ 2020-02-28 10:02 ` Ard Biesheuvel
0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2020-02-28 10:02 UTC (permalink / raw)
To: linux-efi; +Cc: Ard Biesheuvel, linux-arm-kernel
Some fixes for the code that is currentely queued up in efi/next for
v5.7
Patch #3 fixes a boot regression for non-EFI boot in linux-next, so
that will be sent onwards reasonably quickly.
Ard Biesheuvel (3):
efi/arm: clean EFI stub exit code from cache instead of avoiding it
efi/arm64: clean EFI stub exit code from cache instead of avoiding it
efi: mark all EFI runtime services as unsupported on non-EFI boot
arch/arm/boot/compressed/head.S | 18 ++++++++----------
arch/arm64/kernel/efi-entry.S | 18 +++++++++---------
arch/arm64/kernel/image-vars.h | 2 +-
drivers/firmware/efi/efi.c | 6 +++---
4 files changed, 21 insertions(+), 23 deletions(-)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH efi-next 1/3] efi/arm: clean EFI stub exit code from cache instead of avoiding it
2020-02-28 10:02 ` Ard Biesheuvel
@ 2020-02-28 10:02 ` Ard Biesheuvel
-1 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2020-02-28 10:02 UTC (permalink / raw)
To: linux-efi; +Cc: linux-arm-kernel, Ard Biesheuvel
Commit c7225494b ("efi/arm: Work around missing cache maintenance in
decompressor handover") modified the handover code written in assembler
to work around the missing cache maintenance of the piece of code that
is executed after the MMU and caches are turned off. Due to the fact
that this sequence incorporates a subroutine call, cleaning that code
from the cache is not a matter of simply passing the start and end of
the currently running subroutine into cache_clean_flush(), which is why
instead, the code jumps across into the cleaned copy of the image.
However, this assumes that this copy is executable, and this means we
expect EFI_LOADER_DATA regions to be executable as well, which is not
a reasonable assumption to make, even if this is true for most UEFI
implementations today.
So change this back, and add a cache_clean_flush() call to cover the
remaining code in the subroutine, and any code it may execute in the
context of cache_off().
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/boot/compressed/head.S | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 36ffbeecd30b..04f77214f050 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -1452,13 +1452,11 @@ ENTRY(efi_enter_kernel)
@ The PE/COFF loader might not have cleaned the code we are
@ running beyond the PoU, and so calling cache_off below from
- @ inside the PE/COFF loader allocated region is unsafe. Let's
- @ assume our own zImage relocation code did a better job, and
- @ jump into its version of this routine before proceeding.
- ldr r1, .Ljmp
- sub r1, r7, r1
- mov pc, r1 @ no mode switch
-0:
+ @ inside the PE/COFF loader allocated region is unsafe unless
+ @ we explicitly clean it to the PoC.
+ adr r0, call_cache_fn @ region of code we will
+ adr r1, 0f @ run with MMU off
+ bl cache_clean_flush
bl cache_off
@ Set parameters for booting zImage according to boot protocol
@@ -1467,10 +1465,10 @@ ENTRY(efi_enter_kernel)
mov r0, #0
mov r1, #0xFFFFFFFF
mov r2, r4
- b __efi_start
+ add r7, r7, #(__efi_start - start)
+ mov pc, r7 @ no mode switch
ENDPROC(efi_enter_kernel)
- .align 2
-.Ljmp: .long start - 0b
+0:
#endif
.align
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH efi-next 1/3] efi/arm: clean EFI stub exit code from cache instead of avoiding it
@ 2020-02-28 10:02 ` Ard Biesheuvel
0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2020-02-28 10:02 UTC (permalink / raw)
To: linux-efi; +Cc: Ard Biesheuvel, linux-arm-kernel
Commit c7225494b ("efi/arm: Work around missing cache maintenance in
decompressor handover") modified the handover code written in assembler
to work around the missing cache maintenance of the piece of code that
is executed after the MMU and caches are turned off. Due to the fact
that this sequence incorporates a subroutine call, cleaning that code
from the cache is not a matter of simply passing the start and end of
the currently running subroutine into cache_clean_flush(), which is why
instead, the code jumps across into the cleaned copy of the image.
However, this assumes that this copy is executable, and this means we
expect EFI_LOADER_DATA regions to be executable as well, which is not
a reasonable assumption to make, even if this is true for most UEFI
implementations today.
So change this back, and add a cache_clean_flush() call to cover the
remaining code in the subroutine, and any code it may execute in the
context of cache_off().
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/boot/compressed/head.S | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 36ffbeecd30b..04f77214f050 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -1452,13 +1452,11 @@ ENTRY(efi_enter_kernel)
@ The PE/COFF loader might not have cleaned the code we are
@ running beyond the PoU, and so calling cache_off below from
- @ inside the PE/COFF loader allocated region is unsafe. Let's
- @ assume our own zImage relocation code did a better job, and
- @ jump into its version of this routine before proceeding.
- ldr r1, .Ljmp
- sub r1, r7, r1
- mov pc, r1 @ no mode switch
-0:
+ @ inside the PE/COFF loader allocated region is unsafe unless
+ @ we explicitly clean it to the PoC.
+ adr r0, call_cache_fn @ region of code we will
+ adr r1, 0f @ run with MMU off
+ bl cache_clean_flush
bl cache_off
@ Set parameters for booting zImage according to boot protocol
@@ -1467,10 +1465,10 @@ ENTRY(efi_enter_kernel)
mov r0, #0
mov r1, #0xFFFFFFFF
mov r2, r4
- b __efi_start
+ add r7, r7, #(__efi_start - start)
+ mov pc, r7 @ no mode switch
ENDPROC(efi_enter_kernel)
- .align 2
-.Ljmp: .long start - 0b
+0:
#endif
.align
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH efi-next 2/3] efi/arm64: clean EFI stub exit code from cache instead of avoiding it
2020-02-28 10:02 ` Ard Biesheuvel
@ 2020-02-28 10:02 ` Ard Biesheuvel
-1 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2020-02-28 10:02 UTC (permalink / raw)
To: linux-efi; +Cc: linux-arm-kernel, Ard Biesheuvel
Commit 9f9223778 ("efi/libstub/arm: Make efi_entry() an ordinary PE/COFF
entrypoint") modified the handover code written in assembler, and for
maintainability, aligned the logic with the logic used in the 32-bit ARM
version, which is to avoid cache maintenance on the remaining instructions
in the subroutine that will be executed with the MMU and caches off, and
instead, branch into the relocated copy of the kernel image.
However, this assumes that this copy is executable, and this means we
expect EFI_LOADER_DATA regions to be executable as well, which is not
a reasonable assumption to make, even if this is true for most UEFI
implementations today.
So change this back, and add a __flush_dcache_area() call to cover the
remaining code in the subroutine.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm64/kernel/efi-entry.S | 18 +++++++++---------
arch/arm64/kernel/image-vars.h | 2 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
index 4cfd03c35c49..d5dee064975f 100644
--- a/arch/arm64/kernel/efi-entry.S
+++ b/arch/arm64/kernel/efi-entry.S
@@ -19,7 +19,8 @@ ENTRY(efi_enter_kernel)
* point stored in x0. Save those values in registers which are
* callee preserved.
*/
- mov x19, x0 // relocated Image address
+ ldr w2, =stext_offset
+ add x19, x0, x2 // relocated Image entrypoint
mov x20, x1 // DTB address
/*
@@ -29,15 +30,14 @@ ENTRY(efi_enter_kernel)
ldr w1, =kernel_size
bl __flush_dcache_area
ic ialluis
- dsb sy
/*
- * Jump across, into the copy of the image that we just cleaned
- * to the PoC, so that we can safely disable the MMU and caches.
+ * Flush the remainder of this routine to the PoC
+ * so that we can safely disable the MMU and caches.
*/
- ldr w0, .Ljmp
- sub x0, x19, w0, sxtw
- br x0
+ adr x0, 0f
+ ldr w1, 3f
+ bl __flush_dcache_area
0:
/* Turn off Dcache and MMU */
mrs x0, CurrentEL
@@ -63,6 +63,6 @@ ENTRY(efi_enter_kernel)
mov x1, xzr
mov x2, xzr
mov x3, xzr
- b stext
+ br x19
ENDPROC(efi_enter_kernel)
-.Ljmp: .long _text - 0b
+3: .long . - 0b
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 9a7aef0d6f70..28bf98f84adf 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -13,6 +13,7 @@
#ifdef CONFIG_EFI
__efistub_kernel_size = _edata - _text;
+__efistub_stext_offset = stext - _text;
/*
@@ -43,7 +44,6 @@ __efistub___memset = __pi_memset;
#endif
__efistub__text = _text;
-__efistub_stext = stext;
__efistub__end = _end;
__efistub__edata = _edata;
__efistub_screen_info = screen_info;
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH efi-next 2/3] efi/arm64: clean EFI stub exit code from cache instead of avoiding it
@ 2020-02-28 10:02 ` Ard Biesheuvel
0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2020-02-28 10:02 UTC (permalink / raw)
To: linux-efi; +Cc: Ard Biesheuvel, linux-arm-kernel
Commit 9f9223778 ("efi/libstub/arm: Make efi_entry() an ordinary PE/COFF
entrypoint") modified the handover code written in assembler, and for
maintainability, aligned the logic with the logic used in the 32-bit ARM
version, which is to avoid cache maintenance on the remaining instructions
in the subroutine that will be executed with the MMU and caches off, and
instead, branch into the relocated copy of the kernel image.
However, this assumes that this copy is executable, and this means we
expect EFI_LOADER_DATA regions to be executable as well, which is not
a reasonable assumption to make, even if this is true for most UEFI
implementations today.
So change this back, and add a __flush_dcache_area() call to cover the
remaining code in the subroutine.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm64/kernel/efi-entry.S | 18 +++++++++---------
arch/arm64/kernel/image-vars.h | 2 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
index 4cfd03c35c49..d5dee064975f 100644
--- a/arch/arm64/kernel/efi-entry.S
+++ b/arch/arm64/kernel/efi-entry.S
@@ -19,7 +19,8 @@ ENTRY(efi_enter_kernel)
* point stored in x0. Save those values in registers which are
* callee preserved.
*/
- mov x19, x0 // relocated Image address
+ ldr w2, =stext_offset
+ add x19, x0, x2 // relocated Image entrypoint
mov x20, x1 // DTB address
/*
@@ -29,15 +30,14 @@ ENTRY(efi_enter_kernel)
ldr w1, =kernel_size
bl __flush_dcache_area
ic ialluis
- dsb sy
/*
- * Jump across, into the copy of the image that we just cleaned
- * to the PoC, so that we can safely disable the MMU and caches.
+ * Flush the remainder of this routine to the PoC
+ * so that we can safely disable the MMU and caches.
*/
- ldr w0, .Ljmp
- sub x0, x19, w0, sxtw
- br x0
+ adr x0, 0f
+ ldr w1, 3f
+ bl __flush_dcache_area
0:
/* Turn off Dcache and MMU */
mrs x0, CurrentEL
@@ -63,6 +63,6 @@ ENTRY(efi_enter_kernel)
mov x1, xzr
mov x2, xzr
mov x3, xzr
- b stext
+ br x19
ENDPROC(efi_enter_kernel)
-.Ljmp: .long _text - 0b
+3: .long . - 0b
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 9a7aef0d6f70..28bf98f84adf 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -13,6 +13,7 @@
#ifdef CONFIG_EFI
__efistub_kernel_size = _edata - _text;
+__efistub_stext_offset = stext - _text;
/*
@@ -43,7 +44,6 @@ __efistub___memset = __pi_memset;
#endif
__efistub__text = _text;
-__efistub_stext = stext;
__efistub__end = _end;
__efistub__edata = _edata;
__efistub_screen_info = screen_info;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH efi-next 2/3] efi/arm64: clean EFI stub exit code from cache instead of avoiding it
2020-02-28 10:02 ` Ard Biesheuvel
@ 2020-02-28 11:14 ` Mark Rutland
-1 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2020-02-28 11:14 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, linux-arm-kernel
On Fri, Feb 28, 2020 at 11:02:43AM +0100, Ard Biesheuvel wrote:
> Commit 9f9223778 ("efi/libstub/arm: Make efi_entry() an ordinary PE/COFF
> entrypoint") modified the handover code written in assembler, and for
> maintainability, aligned the logic with the logic used in the 32-bit ARM
> version, which is to avoid cache maintenance on the remaining instructions
> in the subroutine that will be executed with the MMU and caches off, and
> instead, branch into the relocated copy of the kernel image.
>
> However, this assumes that this copy is executable, and this means we
> expect EFI_LOADER_DATA regions to be executable as well, which is not
> a reasonable assumption to make, even if this is true for most UEFI
> implementations today.
>
> So change this back, and add a __flush_dcache_area() call to cover the
> remaining code in the subroutine.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/arm64/kernel/efi-entry.S | 18 +++++++++---------
> arch/arm64/kernel/image-vars.h | 2 +-
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
> index 4cfd03c35c49..d5dee064975f 100644
> --- a/arch/arm64/kernel/efi-entry.S
> +++ b/arch/arm64/kernel/efi-entry.S
> @@ -19,7 +19,8 @@ ENTRY(efi_enter_kernel)
> * point stored in x0. Save those values in registers which are
> * callee preserved.
> */
> - mov x19, x0 // relocated Image address
> + ldr w2, =stext_offset
> + add x19, x0, x2 // relocated Image entrypoint
> mov x20, x1 // DTB address
>
> /*
> @@ -29,15 +30,14 @@ ENTRY(efi_enter_kernel)
> ldr w1, =kernel_size
> bl __flush_dcache_area
> ic ialluis
> - dsb sy
>
> /*
> - * Jump across, into the copy of the image that we just cleaned
> - * to the PoC, so that we can safely disable the MMU and caches.
> + * Flush the remainder of this routine to the PoC
Minor nit, but could we please say 'Clean' rather than 'Flush' here?
Even better, we now have __clean_dcache_area_poc(), and can use that
too.
Thanks,
Mark.
> + * so that we can safely disable the MMU and caches.
> */
> - ldr w0, .Ljmp
> - sub x0, x19, w0, sxtw
> - br x0
> + adr x0, 0f
> + ldr w1, 3f
> + bl __flush_dcache_area
> 0:
> /* Turn off Dcache and MMU */
> mrs x0, CurrentEL
> @@ -63,6 +63,6 @@ ENTRY(efi_enter_kernel)
> mov x1, xzr
> mov x2, xzr
> mov x3, xzr
> - b stext
> + br x19
> ENDPROC(efi_enter_kernel)
> -.Ljmp: .long _text - 0b
> +3: .long . - 0b
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index 9a7aef0d6f70..28bf98f84adf 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -13,6 +13,7 @@
> #ifdef CONFIG_EFI
>
> __efistub_kernel_size = _edata - _text;
> +__efistub_stext_offset = stext - _text;
>
>
> /*
> @@ -43,7 +44,6 @@ __efistub___memset = __pi_memset;
> #endif
>
> __efistub__text = _text;
> -__efistub_stext = stext;
> __efistub__end = _end;
> __efistub__edata = _edata;
> __efistub_screen_info = screen_info;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH efi-next 2/3] efi/arm64: clean EFI stub exit code from cache instead of avoiding it
@ 2020-02-28 11:14 ` Mark Rutland
0 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2020-02-28 11:14 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, linux-arm-kernel
On Fri, Feb 28, 2020 at 11:02:43AM +0100, Ard Biesheuvel wrote:
> Commit 9f9223778 ("efi/libstub/arm: Make efi_entry() an ordinary PE/COFF
> entrypoint") modified the handover code written in assembler, and for
> maintainability, aligned the logic with the logic used in the 32-bit ARM
> version, which is to avoid cache maintenance on the remaining instructions
> in the subroutine that will be executed with the MMU and caches off, and
> instead, branch into the relocated copy of the kernel image.
>
> However, this assumes that this copy is executable, and this means we
> expect EFI_LOADER_DATA regions to be executable as well, which is not
> a reasonable assumption to make, even if this is true for most UEFI
> implementations today.
>
> So change this back, and add a __flush_dcache_area() call to cover the
> remaining code in the subroutine.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/arm64/kernel/efi-entry.S | 18 +++++++++---------
> arch/arm64/kernel/image-vars.h | 2 +-
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
> index 4cfd03c35c49..d5dee064975f 100644
> --- a/arch/arm64/kernel/efi-entry.S
> +++ b/arch/arm64/kernel/efi-entry.S
> @@ -19,7 +19,8 @@ ENTRY(efi_enter_kernel)
> * point stored in x0. Save those values in registers which are
> * callee preserved.
> */
> - mov x19, x0 // relocated Image address
> + ldr w2, =stext_offset
> + add x19, x0, x2 // relocated Image entrypoint
> mov x20, x1 // DTB address
>
> /*
> @@ -29,15 +30,14 @@ ENTRY(efi_enter_kernel)
> ldr w1, =kernel_size
> bl __flush_dcache_area
> ic ialluis
> - dsb sy
>
> /*
> - * Jump across, into the copy of the image that we just cleaned
> - * to the PoC, so that we can safely disable the MMU and caches.
> + * Flush the remainder of this routine to the PoC
Minor nit, but could we please say 'Clean' rather than 'Flush' here?
Even better, we now have __clean_dcache_area_poc(), and can use that
too.
Thanks,
Mark.
> + * so that we can safely disable the MMU and caches.
> */
> - ldr w0, .Ljmp
> - sub x0, x19, w0, sxtw
> - br x0
> + adr x0, 0f
> + ldr w1, 3f
> + bl __flush_dcache_area
> 0:
> /* Turn off Dcache and MMU */
> mrs x0, CurrentEL
> @@ -63,6 +63,6 @@ ENTRY(efi_enter_kernel)
> mov x1, xzr
> mov x2, xzr
> mov x3, xzr
> - b stext
> + br x19
> ENDPROC(efi_enter_kernel)
> -.Ljmp: .long _text - 0b
> +3: .long . - 0b
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index 9a7aef0d6f70..28bf98f84adf 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -13,6 +13,7 @@
> #ifdef CONFIG_EFI
>
> __efistub_kernel_size = _edata - _text;
> +__efistub_stext_offset = stext - _text;
>
>
> /*
> @@ -43,7 +44,6 @@ __efistub___memset = __pi_memset;
> #endif
>
> __efistub__text = _text;
> -__efistub_stext = stext;
> __efistub__end = _end;
> __efistub__edata = _edata;
> __efistub_screen_info = screen_info;
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH efi-next 2/3] efi/arm64: clean EFI stub exit code from cache instead of avoiding it
2020-02-28 11:14 ` Mark Rutland
@ 2020-02-28 11:19 ` Mark Rutland
-1 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2020-02-28 11:19 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, linux-arm-kernel
On Fri, Feb 28, 2020 at 11:14:50AM +0000, Mark Rutland wrote:
> On Fri, Feb 28, 2020 at 11:02:43AM +0100, Ard Biesheuvel wrote:
> > Commit 9f9223778 ("efi/libstub/arm: Make efi_entry() an ordinary PE/COFF
> > entrypoint") modified the handover code written in assembler, and for
> > maintainability, aligned the logic with the logic used in the 32-bit ARM
> > version, which is to avoid cache maintenance on the remaining instructions
> > in the subroutine that will be executed with the MMU and caches off, and
> > instead, branch into the relocated copy of the kernel image.
> >
> > However, this assumes that this copy is executable, and this means we
> > expect EFI_LOADER_DATA regions to be executable as well, which is not
> > a reasonable assumption to make, even if this is true for most UEFI
> > implementations today.
> >
> > So change this back, and add a __flush_dcache_area() call to cover the
> > remaining code in the subroutine.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > arch/arm64/kernel/efi-entry.S | 18 +++++++++---------
> > arch/arm64/kernel/image-vars.h | 2 +-
> > 2 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
> > index 4cfd03c35c49..d5dee064975f 100644
> > --- a/arch/arm64/kernel/efi-entry.S
> > +++ b/arch/arm64/kernel/efi-entry.S
> > @@ -19,7 +19,8 @@ ENTRY(efi_enter_kernel)
> > * point stored in x0. Save those values in registers which are
> > * callee preserved.
> > */
> > - mov x19, x0 // relocated Image address
> > + ldr w2, =stext_offset
> > + add x19, x0, x2 // relocated Image entrypoint
> > mov x20, x1 // DTB address
> >
> > /*
> > @@ -29,15 +30,14 @@ ENTRY(efi_enter_kernel)
> > ldr w1, =kernel_size
> > bl __flush_dcache_area
> > ic ialluis
> > - dsb sy
> >
> > /*
> > - * Jump across, into the copy of the image that we just cleaned
> > - * to the PoC, so that we can safely disable the MMU and caches.
> > + * Flush the remainder of this routine to the PoC
>
> Minor nit, but could we please say 'Clean' rather than 'Flush' here?
>
> Even better, we now have __clean_dcache_area_poc(), and can use that
> too.
... or if that's better as a subsequent cleanup for consistency, that'd
also be fine, and needn't block this patch.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH efi-next 2/3] efi/arm64: clean EFI stub exit code from cache instead of avoiding it
@ 2020-02-28 11:19 ` Mark Rutland
0 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2020-02-28 11:19 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, linux-arm-kernel
On Fri, Feb 28, 2020 at 11:14:50AM +0000, Mark Rutland wrote:
> On Fri, Feb 28, 2020 at 11:02:43AM +0100, Ard Biesheuvel wrote:
> > Commit 9f9223778 ("efi/libstub/arm: Make efi_entry() an ordinary PE/COFF
> > entrypoint") modified the handover code written in assembler, and for
> > maintainability, aligned the logic with the logic used in the 32-bit ARM
> > version, which is to avoid cache maintenance on the remaining instructions
> > in the subroutine that will be executed with the MMU and caches off, and
> > instead, branch into the relocated copy of the kernel image.
> >
> > However, this assumes that this copy is executable, and this means we
> > expect EFI_LOADER_DATA regions to be executable as well, which is not
> > a reasonable assumption to make, even if this is true for most UEFI
> > implementations today.
> >
> > So change this back, and add a __flush_dcache_area() call to cover the
> > remaining code in the subroutine.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > arch/arm64/kernel/efi-entry.S | 18 +++++++++---------
> > arch/arm64/kernel/image-vars.h | 2 +-
> > 2 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
> > index 4cfd03c35c49..d5dee064975f 100644
> > --- a/arch/arm64/kernel/efi-entry.S
> > +++ b/arch/arm64/kernel/efi-entry.S
> > @@ -19,7 +19,8 @@ ENTRY(efi_enter_kernel)
> > * point stored in x0. Save those values in registers which are
> > * callee preserved.
> > */
> > - mov x19, x0 // relocated Image address
> > + ldr w2, =stext_offset
> > + add x19, x0, x2 // relocated Image entrypoint
> > mov x20, x1 // DTB address
> >
> > /*
> > @@ -29,15 +30,14 @@ ENTRY(efi_enter_kernel)
> > ldr w1, =kernel_size
> > bl __flush_dcache_area
> > ic ialluis
> > - dsb sy
> >
> > /*
> > - * Jump across, into the copy of the image that we just cleaned
> > - * to the PoC, so that we can safely disable the MMU and caches.
> > + * Flush the remainder of this routine to the PoC
>
> Minor nit, but could we please say 'Clean' rather than 'Flush' here?
>
> Even better, we now have __clean_dcache_area_poc(), and can use that
> too.
... or if that's better as a subsequent cleanup for consistency, that'd
also be fine, and needn't block this patch.
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH efi-next 2/3] efi/arm64: clean EFI stub exit code from cache instead of avoiding it
2020-02-28 11:19 ` Mark Rutland
@ 2020-02-28 11:21 ` Ard Biesheuvel
-1 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2020-02-28 11:21 UTC (permalink / raw)
To: Mark Rutland; +Cc: linux-efi, linux-arm-kernel
On Fri, 28 Feb 2020 at 12:19, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Fri, Feb 28, 2020 at 11:14:50AM +0000, Mark Rutland wrote:
> > On Fri, Feb 28, 2020 at 11:02:43AM +0100, Ard Biesheuvel wrote:
> > > Commit 9f9223778 ("efi/libstub/arm: Make efi_entry() an ordinary PE/COFF
> > > entrypoint") modified the handover code written in assembler, and for
> > > maintainability, aligned the logic with the logic used in the 32-bit ARM
> > > version, which is to avoid cache maintenance on the remaining instructions
> > > in the subroutine that will be executed with the MMU and caches off, and
> > > instead, branch into the relocated copy of the kernel image.
> > >
> > > However, this assumes that this copy is executable, and this means we
> > > expect EFI_LOADER_DATA regions to be executable as well, which is not
> > > a reasonable assumption to make, even if this is true for most UEFI
> > > implementations today.
> > >
> > > So change this back, and add a __flush_dcache_area() call to cover the
> > > remaining code in the subroutine.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > > arch/arm64/kernel/efi-entry.S | 18 +++++++++---------
> > > arch/arm64/kernel/image-vars.h | 2 +-
> > > 2 files changed, 10 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
> > > index 4cfd03c35c49..d5dee064975f 100644
> > > --- a/arch/arm64/kernel/efi-entry.S
> > > +++ b/arch/arm64/kernel/efi-entry.S
> > > @@ -19,7 +19,8 @@ ENTRY(efi_enter_kernel)
> > > * point stored in x0. Save those values in registers which are
> > > * callee preserved.
> > > */
> > > - mov x19, x0 // relocated Image address
> > > + ldr w2, =stext_offset
> > > + add x19, x0, x2 // relocated Image entrypoint
> > > mov x20, x1 // DTB address
> > >
> > > /*
> > > @@ -29,15 +30,14 @@ ENTRY(efi_enter_kernel)
> > > ldr w1, =kernel_size
> > > bl __flush_dcache_area
> > > ic ialluis
> > > - dsb sy
> > >
> > > /*
> > > - * Jump across, into the copy of the image that we just cleaned
> > > - * to the PoC, so that we can safely disable the MMU and caches.
> > > + * Flush the remainder of this routine to the PoC
> >
> > Minor nit, but could we please say 'Clean' rather than 'Flush' here?
> >
> > Even better, we now have __clean_dcache_area_poc(), and can use that
> > too.
>
> ... or if that's better as a subsequent cleanup for consistency, that'd
> also be fine, and needn't block this patch.
>
Thanks Mark. That sounds like a worthwhile improvement to fold in.
I'll need to add the __efistub_ alias for it, but I'm touching
image-vars.h in this patch anyway.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH efi-next 2/3] efi/arm64: clean EFI stub exit code from cache instead of avoiding it
@ 2020-02-28 11:21 ` Ard Biesheuvel
0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2020-02-28 11:21 UTC (permalink / raw)
To: Mark Rutland; +Cc: linux-efi, linux-arm-kernel
On Fri, 28 Feb 2020 at 12:19, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Fri, Feb 28, 2020 at 11:14:50AM +0000, Mark Rutland wrote:
> > On Fri, Feb 28, 2020 at 11:02:43AM +0100, Ard Biesheuvel wrote:
> > > Commit 9f9223778 ("efi/libstub/arm: Make efi_entry() an ordinary PE/COFF
> > > entrypoint") modified the handover code written in assembler, and for
> > > maintainability, aligned the logic with the logic used in the 32-bit ARM
> > > version, which is to avoid cache maintenance on the remaining instructions
> > > in the subroutine that will be executed with the MMU and caches off, and
> > > instead, branch into the relocated copy of the kernel image.
> > >
> > > However, this assumes that this copy is executable, and this means we
> > > expect EFI_LOADER_DATA regions to be executable as well, which is not
> > > a reasonable assumption to make, even if this is true for most UEFI
> > > implementations today.
> > >
> > > So change this back, and add a __flush_dcache_area() call to cover the
> > > remaining code in the subroutine.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > > arch/arm64/kernel/efi-entry.S | 18 +++++++++---------
> > > arch/arm64/kernel/image-vars.h | 2 +-
> > > 2 files changed, 10 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
> > > index 4cfd03c35c49..d5dee064975f 100644
> > > --- a/arch/arm64/kernel/efi-entry.S
> > > +++ b/arch/arm64/kernel/efi-entry.S
> > > @@ -19,7 +19,8 @@ ENTRY(efi_enter_kernel)
> > > * point stored in x0. Save those values in registers which are
> > > * callee preserved.
> > > */
> > > - mov x19, x0 // relocated Image address
> > > + ldr w2, =stext_offset
> > > + add x19, x0, x2 // relocated Image entrypoint
> > > mov x20, x1 // DTB address
> > >
> > > /*
> > > @@ -29,15 +30,14 @@ ENTRY(efi_enter_kernel)
> > > ldr w1, =kernel_size
> > > bl __flush_dcache_area
> > > ic ialluis
> > > - dsb sy
> > >
> > > /*
> > > - * Jump across, into the copy of the image that we just cleaned
> > > - * to the PoC, so that we can safely disable the MMU and caches.
> > > + * Flush the remainder of this routine to the PoC
> >
> > Minor nit, but could we please say 'Clean' rather than 'Flush' here?
> >
> > Even better, we now have __clean_dcache_area_poc(), and can use that
> > too.
>
> ... or if that's better as a subsequent cleanup for consistency, that'd
> also be fine, and needn't block this patch.
>
Thanks Mark. That sounds like a worthwhile improvement to fold in.
I'll need to add the __efistub_ alias for it, but I'm touching
image-vars.h in this patch anyway.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH efi-next 3/3] efi: mark all EFI runtime services as unsupported on non-EFI boot
2020-02-28 10:02 ` Ard Biesheuvel
@ 2020-02-28 10:02 ` Ard Biesheuvel
-1 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2020-02-28 10:02 UTC (permalink / raw)
To: linux-efi; +Cc: linux-arm-kernel, Ard Biesheuvel
Recent changes to the way we deal with EFI runtime services that
are marked as unsupported by the firmware resulted in a regression
for non-EFI boot. The problem is that all EFI runtime services are
marked as available by default, and any non-NULL checks on the EFI
service function pointers (which will be non-NULL even for runtime
services that are unsupported on an EFI boot) were replaced with
checks against the mask stored in efi.runtime_supported_mask.
When doing a non-EFI boot, this check against the mask will return
a false positive, given the fact that all runtime services are
marked as enabled by default. Since we dropped the non-NULL check
of the runtime service function pointer in favor of the mask check,
we will now unconditionally dereference the function pointer, even
if it is NULL, and go boom.
So let's ensure that the mask reflects reality on a non-EFI boot,
which is that all EFI runtime services are unsupported.
Reported-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
drivers/firmware/efi/efi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 41269a95ff85..d1746a579c99 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -300,12 +300,12 @@ static int __init efisubsys_init(void)
{
int error;
- if (!efi_enabled(EFI_BOOT))
- return 0;
-
if (!efi_enabled(EFI_RUNTIME_SERVICES))
efi.runtime_supported_mask = 0;
+ if (!efi_enabled(EFI_BOOT))
+ return 0;
+
if (efi.runtime_supported_mask) {
/*
* Since we process only one efi_runtime_service() at a time, an
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH efi-next 3/3] efi: mark all EFI runtime services as unsupported on non-EFI boot
@ 2020-02-28 10:02 ` Ard Biesheuvel
0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2020-02-28 10:02 UTC (permalink / raw)
To: linux-efi; +Cc: Ard Biesheuvel, linux-arm-kernel
Recent changes to the way we deal with EFI runtime services that
are marked as unsupported by the firmware resulted in a regression
for non-EFI boot. The problem is that all EFI runtime services are
marked as available by default, and any non-NULL checks on the EFI
service function pointers (which will be non-NULL even for runtime
services that are unsupported on an EFI boot) were replaced with
checks against the mask stored in efi.runtime_supported_mask.
When doing a non-EFI boot, this check against the mask will return
a false positive, given the fact that all runtime services are
marked as enabled by default. Since we dropped the non-NULL check
of the runtime service function pointer in favor of the mask check,
we will now unconditionally dereference the function pointer, even
if it is NULL, and go boom.
So let's ensure that the mask reflects reality on a non-EFI boot,
which is that all EFI runtime services are unsupported.
Reported-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
drivers/firmware/efi/efi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 41269a95ff85..d1746a579c99 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -300,12 +300,12 @@ static int __init efisubsys_init(void)
{
int error;
- if (!efi_enabled(EFI_BOOT))
- return 0;
-
if (!efi_enabled(EFI_RUNTIME_SERVICES))
efi.runtime_supported_mask = 0;
+ if (!efi_enabled(EFI_BOOT))
+ return 0;
+
if (efi.runtime_supported_mask) {
/*
* Since we process only one efi_runtime_service() at a time, an
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread