* [PATCH] arm64: entry: simplify trampoline data page
@ 2022-04-27 10:22 Ard Biesheuvel
2022-06-22 14:41 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2022-04-27 10:22 UTC (permalink / raw)
To: linux-arm-kernel
Cc: will, catalin.marinas, mark.rutland, Ard Biesheuvel, James Morse
Get rid of some clunky open coded arithmetic on section addresses, by
emitting the trampoline data variables into a separate, dedicated r/o
data section, and putting it at the next page boundary. This way, we can
access the literals via single LDR instruction.
While at it, get rid of other, implicit literals, and use ADRP/ADD or
MOVZ/MOVK sequences, as appropriate. Note that the latter are only
supported for CONFIG_RELOCATABLE=n (which is usually the case if
CONFIG_RANDOMIZE_BASE=n), so update the CPP conditionals to reflect
this.
Cc: James Morse <james.morse@arm.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm64/include/asm/fixmap.h | 4 +-
arch/arm64/kernel/entry.S | 45 ++++++--------------
arch/arm64/kernel/vmlinux.lds.S | 3 +-
arch/arm64/mm/mmu.c | 10 ++---
4 files changed, 22 insertions(+), 40 deletions(-)
diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
index daff882883f9..71ed5fdf718b 100644
--- a/arch/arm64/include/asm/fixmap.h
+++ b/arch/arm64/include/asm/fixmap.h
@@ -62,10 +62,12 @@ enum fixed_addresses {
#endif /* CONFIG_ACPI_APEI_GHES */
#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
+#ifdef CONFIG_RELOCATABLE
+ FIX_ENTRY_TRAMP_TEXT4, /* one extra slot for the data page */
+#endif
FIX_ENTRY_TRAMP_TEXT3,
FIX_ENTRY_TRAMP_TEXT2,
FIX_ENTRY_TRAMP_TEXT1,
- FIX_ENTRY_TRAMP_DATA,
#define TRAMP_VALIAS (__fix_to_virt(FIX_ENTRY_TRAMP_TEXT1))
#endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
__end_of_permanent_fixed_addresses,
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index ede028dee81b..aed2b41e05aa 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -636,18 +636,20 @@ alternative_else_nop_endif
*/
.endm
- .macro tramp_data_page dst
- adr_l \dst, .entry.tramp.text
- sub \dst, \dst, PAGE_SIZE
- .endm
-
- .macro tramp_data_read_var dst, var
-#ifdef CONFIG_RANDOMIZE_BASE
- tramp_data_page \dst
- add \dst, \dst, #:lo12:__entry_tramp_data_\var
- ldr \dst, [\dst]
+ .macro tramp_data_read_var dst, var
+#ifdef CONFIG_RELOCATABLE
+ ldr \dst, .L__tramp_data_\var
+ .ifndef .L__tramp_data_\var
+ .pushsection ".entry.tramp.rodata", "a", %progbits
+ .align 3
+.L__tramp_data_\var:
+ .quad \var
+ .popsection
+ .endif
#else
- ldr \dst, =\var
+ movz \dst, :abs_g2_s:\var
+ movk \dst, :abs_g1_nc:\var
+ movk \dst, :abs_g0_nc:\var
#endif
.endm
@@ -695,7 +697,7 @@ alternative_else_nop_endif
msr vbar_el1, x30
isb
.else
- ldr x30, =vectors
+ adr_l x30, vectors
.endif // \kpti == 1
.if \bhb == BHB_MITIGATION_FW
@@ -764,24 +766,7 @@ SYM_CODE_END(tramp_exit_native)
SYM_CODE_START(tramp_exit_compat)
tramp_exit 32
SYM_CODE_END(tramp_exit_compat)
-
- .ltorg
.popsection // .entry.tramp.text
-#ifdef CONFIG_RANDOMIZE_BASE
- .pushsection ".rodata", "a"
- .align PAGE_SHIFT
-SYM_DATA_START(__entry_tramp_data_start)
-__entry_tramp_data_vectors:
- .quad vectors
-#ifdef CONFIG_ARM_SDE_INTERFACE
-__entry_tramp_data___sdei_asm_handler:
- .quad __sdei_asm_handler
-#endif /* CONFIG_ARM_SDE_INTERFACE */
-__entry_tramp_data_this_cpu_vector:
- .quad this_cpu_vector
-SYM_DATA_END(__entry_tramp_data_start)
- .popsection // .rodata
-#endif /* CONFIG_RANDOMIZE_BASE */
#endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
/*
@@ -932,7 +917,6 @@ NOKPROBE(call_on_irq_stack)
* This clobbers x4, __sdei_handler() will restore this from firmware's
* copy.
*/
-.ltorg
.pushsection ".entry.tramp.text", "ax"
SYM_CODE_START(__sdei_asm_entry_trampoline)
mrs x4, ttbr1_el1
@@ -967,7 +951,6 @@ SYM_CODE_START(__sdei_asm_exit_trampoline)
1: sdei_handler_exit exit_mode=x2
SYM_CODE_END(__sdei_asm_exit_trampoline)
NOKPROBE(__sdei_asm_exit_trampoline)
- .ltorg
.popsection // .entry.tramp.text
#endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index edaf0faf766f..17e554be9198 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -117,7 +117,8 @@ jiffies = jiffies_64;
__entry_tramp_text_start = .; \
*(.entry.tramp.text) \
. = ALIGN(PAGE_SIZE); \
- __entry_tramp_text_end = .;
+ __entry_tramp_text_end = .; \
+ *(.entry.tramp.rodata)
#else
#define TRAMP_TEXT
#endif
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 626ec32873c6..be4d6c3f5692 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -665,13 +665,9 @@ static int __init map_entry_trampoline(void)
__set_fixmap(FIX_ENTRY_TRAMP_TEXT1 - i,
pa_start + i * PAGE_SIZE, prot);
- if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
- extern char __entry_tramp_data_start[];
-
- __set_fixmap(FIX_ENTRY_TRAMP_DATA,
- __pa_symbol(__entry_tramp_data_start),
- PAGE_KERNEL_RO);
- }
+ if (IS_ENABLED(CONFIG_RELOCATABLE))
+ __set_fixmap(FIX_ENTRY_TRAMP_TEXT1 - i,
+ pa_start + i * PAGE_SIZE, PAGE_KERNEL_RO);
return 0;
}
--
2.30.2
_______________________________________________
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] 6+ messages in thread
* Re: [PATCH] arm64: entry: simplify trampoline data page
2022-04-27 10:22 [PATCH] arm64: entry: simplify trampoline data page Ard Biesheuvel
@ 2022-06-22 14:41 ` Ard Biesheuvel
2022-06-22 15:04 ` Mark Rutland
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2022-06-22 14:41 UTC (permalink / raw)
To: Linux ARM; +Cc: Will Deacon, Catalin Marinas, Mark Rutland, James Morse
On Wed, 27 Apr 2022 at 12:23, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Get rid of some clunky open coded arithmetic on section addresses, by
> emitting the trampoline data variables into a separate, dedicated r/o
> data section, and putting it at the next page boundary. This way, we can
> access the literals via single LDR instruction.
>
> While at it, get rid of other, implicit literals, and use ADRP/ADD or
> MOVZ/MOVK sequences, as appropriate. Note that the latter are only
> supported for CONFIG_RELOCATABLE=n (which is usually the case if
> CONFIG_RANDOMIZE_BASE=n), so update the CPP conditionals to reflect
> this.
>
> Cc: James Morse <james.morse@arm.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Ping, in case this one slipped behind the desk.
> ---
> arch/arm64/include/asm/fixmap.h | 4 +-
> arch/arm64/kernel/entry.S | 45 ++++++--------------
> arch/arm64/kernel/vmlinux.lds.S | 3 +-
> arch/arm64/mm/mmu.c | 10 ++---
> 4 files changed, 22 insertions(+), 40 deletions(-)
>
> diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
> index daff882883f9..71ed5fdf718b 100644
> --- a/arch/arm64/include/asm/fixmap.h
> +++ b/arch/arm64/include/asm/fixmap.h
> @@ -62,10 +62,12 @@ enum fixed_addresses {
> #endif /* CONFIG_ACPI_APEI_GHES */
>
> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> +#ifdef CONFIG_RELOCATABLE
> + FIX_ENTRY_TRAMP_TEXT4, /* one extra slot for the data page */
> +#endif
> FIX_ENTRY_TRAMP_TEXT3,
> FIX_ENTRY_TRAMP_TEXT2,
> FIX_ENTRY_TRAMP_TEXT1,
> - FIX_ENTRY_TRAMP_DATA,
> #define TRAMP_VALIAS (__fix_to_virt(FIX_ENTRY_TRAMP_TEXT1))
> #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> __end_of_permanent_fixed_addresses,
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index ede028dee81b..aed2b41e05aa 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -636,18 +636,20 @@ alternative_else_nop_endif
> */
> .endm
>
> - .macro tramp_data_page dst
> - adr_l \dst, .entry.tramp.text
> - sub \dst, \dst, PAGE_SIZE
> - .endm
> -
> - .macro tramp_data_read_var dst, var
> -#ifdef CONFIG_RANDOMIZE_BASE
> - tramp_data_page \dst
> - add \dst, \dst, #:lo12:__entry_tramp_data_\var
> - ldr \dst, [\dst]
> + .macro tramp_data_read_var dst, var
> +#ifdef CONFIG_RELOCATABLE
> + ldr \dst, .L__tramp_data_\var
> + .ifndef .L__tramp_data_\var
> + .pushsection ".entry.tramp.rodata", "a", %progbits
> + .align 3
> +.L__tramp_data_\var:
> + .quad \var
> + .popsection
> + .endif
> #else
> - ldr \dst, =\var
> + movz \dst, :abs_g2_s:\var
> + movk \dst, :abs_g1_nc:\var
> + movk \dst, :abs_g0_nc:\var
> #endif
> .endm
>
> @@ -695,7 +697,7 @@ alternative_else_nop_endif
> msr vbar_el1, x30
> isb
> .else
> - ldr x30, =vectors
> + adr_l x30, vectors
> .endif // \kpti == 1
>
> .if \bhb == BHB_MITIGATION_FW
> @@ -764,24 +766,7 @@ SYM_CODE_END(tramp_exit_native)
> SYM_CODE_START(tramp_exit_compat)
> tramp_exit 32
> SYM_CODE_END(tramp_exit_compat)
> -
> - .ltorg
> .popsection // .entry.tramp.text
> -#ifdef CONFIG_RANDOMIZE_BASE
> - .pushsection ".rodata", "a"
> - .align PAGE_SHIFT
> -SYM_DATA_START(__entry_tramp_data_start)
> -__entry_tramp_data_vectors:
> - .quad vectors
> -#ifdef CONFIG_ARM_SDE_INTERFACE
> -__entry_tramp_data___sdei_asm_handler:
> - .quad __sdei_asm_handler
> -#endif /* CONFIG_ARM_SDE_INTERFACE */
> -__entry_tramp_data_this_cpu_vector:
> - .quad this_cpu_vector
> -SYM_DATA_END(__entry_tramp_data_start)
> - .popsection // .rodata
> -#endif /* CONFIG_RANDOMIZE_BASE */
> #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
>
> /*
> @@ -932,7 +917,6 @@ NOKPROBE(call_on_irq_stack)
> * This clobbers x4, __sdei_handler() will restore this from firmware's
> * copy.
> */
> -.ltorg
> .pushsection ".entry.tramp.text", "ax"
> SYM_CODE_START(__sdei_asm_entry_trampoline)
> mrs x4, ttbr1_el1
> @@ -967,7 +951,6 @@ SYM_CODE_START(__sdei_asm_exit_trampoline)
> 1: sdei_handler_exit exit_mode=x2
> SYM_CODE_END(__sdei_asm_exit_trampoline)
> NOKPROBE(__sdei_asm_exit_trampoline)
> - .ltorg
> .popsection // .entry.tramp.text
> #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
>
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index edaf0faf766f..17e554be9198 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -117,7 +117,8 @@ jiffies = jiffies_64;
> __entry_tramp_text_start = .; \
> *(.entry.tramp.text) \
> . = ALIGN(PAGE_SIZE); \
> - __entry_tramp_text_end = .;
> + __entry_tramp_text_end = .; \
> + *(.entry.tramp.rodata)
> #else
> #define TRAMP_TEXT
> #endif
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 626ec32873c6..be4d6c3f5692 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -665,13 +665,9 @@ static int __init map_entry_trampoline(void)
> __set_fixmap(FIX_ENTRY_TRAMP_TEXT1 - i,
> pa_start + i * PAGE_SIZE, prot);
>
> - if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> - extern char __entry_tramp_data_start[];
> -
> - __set_fixmap(FIX_ENTRY_TRAMP_DATA,
> - __pa_symbol(__entry_tramp_data_start),
> - PAGE_KERNEL_RO);
> - }
> + if (IS_ENABLED(CONFIG_RELOCATABLE))
> + __set_fixmap(FIX_ENTRY_TRAMP_TEXT1 - i,
> + pa_start + i * PAGE_SIZE, PAGE_KERNEL_RO);
>
> return 0;
> }
> --
> 2.30.2
>
_______________________________________________
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] 6+ messages in thread
* Re: [PATCH] arm64: entry: simplify trampoline data page
2022-06-22 14:41 ` Ard Biesheuvel
@ 2022-06-22 15:04 ` Mark Rutland
2022-06-22 15:27 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2022-06-22 15:04 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Linux ARM, Will Deacon, Catalin Marinas, James Morse
On Wed, Jun 22, 2022 at 04:41:41PM +0200, Ard Biesheuvel wrote:
> On Wed, 27 Apr 2022 at 12:23, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > Get rid of some clunky open coded arithmetic on section addresses, by
> > emitting the trampoline data variables into a separate, dedicated r/o
> > data section, and putting it at the next page boundary. This way, we can
> > access the literals via single LDR instruction.
> >
> > While at it, get rid of other, implicit literals, and use ADRP/ADD or
> > MOVZ/MOVK sequences, as appropriate. Note that the latter are only
> > supported for CONFIG_RELOCATABLE=n (which is usually the case if
> > CONFIG_RANDOMIZE_BASE=n), so update the CPP conditionals to reflect
> > this.
> >
> > Cc: James Morse <james.morse@arm.com>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Ping, in case this one slipped behind the desk.
Sorry for the delay. THis has been on my queue of things to look at with a
bunch of other stuff, and I've had some difficulty prioritizing all that.
>
> > ---
> > arch/arm64/include/asm/fixmap.h | 4 +-
> > arch/arm64/kernel/entry.S | 45 ++++++--------------
> > arch/arm64/kernel/vmlinux.lds.S | 3 +-
> > arch/arm64/mm/mmu.c | 10 ++---
> > 4 files changed, 22 insertions(+), 40 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
> > index daff882883f9..71ed5fdf718b 100644
> > --- a/arch/arm64/include/asm/fixmap.h
> > +++ b/arch/arm64/include/asm/fixmap.h
> > @@ -62,10 +62,12 @@ enum fixed_addresses {
> > #endif /* CONFIG_ACPI_APEI_GHES */
> >
> > #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> > +#ifdef CONFIG_RELOCATABLE
> > + FIX_ENTRY_TRAMP_TEXT4, /* one extra slot for the data page */
> > +#endif
> > FIX_ENTRY_TRAMP_TEXT3,
> > FIX_ENTRY_TRAMP_TEXT2,
> > FIX_ENTRY_TRAMP_TEXT1,
> > - FIX_ENTRY_TRAMP_DATA,
> > #define TRAMP_VALIAS (__fix_to_virt(FIX_ENTRY_TRAMP_TEXT1))
> > #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> > __end_of_permanent_fixed_addresses,
> > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> > index ede028dee81b..aed2b41e05aa 100644
> > --- a/arch/arm64/kernel/entry.S
> > +++ b/arch/arm64/kernel/entry.S
> > @@ -636,18 +636,20 @@ alternative_else_nop_endif
> > */
> > .endm
> >
> > - .macro tramp_data_page dst
> > - adr_l \dst, .entry.tramp.text
> > - sub \dst, \dst, PAGE_SIZE
> > - .endm
> > -
> > - .macro tramp_data_read_var dst, var
> > -#ifdef CONFIG_RANDOMIZE_BASE
> > - tramp_data_page \dst
> > - add \dst, \dst, #:lo12:__entry_tramp_data_\var
> > - ldr \dst, [\dst]
> > + .macro tramp_data_read_var dst, var
> > +#ifdef CONFIG_RELOCATABLE
> > + ldr \dst, .L__tramp_data_\var
> > + .ifndef .L__tramp_data_\var
> > + .pushsection ".entry.tramp.rodata", "a", %progbits
> > + .align 3
> > +.L__tramp_data_\var:
> > + .quad \var
> > + .popsection
> > + .endif
> > #else
> > - ldr \dst, =\var
> > + movz \dst, :abs_g2_s:\var
> > + movk \dst, :abs_g1_nc:\var
> > + movk \dst, :abs_g0_nc:\var
> > #endif
> > .endm
Given the lack of a g3 reloc, I assumme `var` is always an address, and we're
assuming it's always in the upper 48-bits? I think it'd be worth a comment as
to why this is safe, or just use a g3 reloc since then it's always good per
inspection.
I'm a bit confused that we've put the var into the literal; I thought the idea
here was that it was secret and needed to be placed in a page not mapped during
userspace. Is the assumption there that it's pointless for !RELOCATABLE kernels
since it can be known anyway, have I misunderstood, or something else?
Otherwise this all looks good superficially; I just haven't had the time to
page it all in.
Mark.
> >
> > @@ -695,7 +697,7 @@ alternative_else_nop_endif
> > msr vbar_el1, x30
> > isb
> > .else
> > - ldr x30, =vectors
> > + adr_l x30, vectors
> > .endif // \kpti == 1
> >
> > .if \bhb == BHB_MITIGATION_FW
> > @@ -764,24 +766,7 @@ SYM_CODE_END(tramp_exit_native)
> > SYM_CODE_START(tramp_exit_compat)
> > tramp_exit 32
> > SYM_CODE_END(tramp_exit_compat)
> > -
> > - .ltorg
> > .popsection // .entry.tramp.text
> > -#ifdef CONFIG_RANDOMIZE_BASE
> > - .pushsection ".rodata", "a"
> > - .align PAGE_SHIFT
> > -SYM_DATA_START(__entry_tramp_data_start)
> > -__entry_tramp_data_vectors:
> > - .quad vectors
> > -#ifdef CONFIG_ARM_SDE_INTERFACE
> > -__entry_tramp_data___sdei_asm_handler:
> > - .quad __sdei_asm_handler
> > -#endif /* CONFIG_ARM_SDE_INTERFACE */
> > -__entry_tramp_data_this_cpu_vector:
> > - .quad this_cpu_vector
> > -SYM_DATA_END(__entry_tramp_data_start)
> > - .popsection // .rodata
> > -#endif /* CONFIG_RANDOMIZE_BASE */
> > #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> >
> > /*
> > @@ -932,7 +917,6 @@ NOKPROBE(call_on_irq_stack)
> > * This clobbers x4, __sdei_handler() will restore this from firmware's
> > * copy.
> > */
> > -.ltorg
> > .pushsection ".entry.tramp.text", "ax"
> > SYM_CODE_START(__sdei_asm_entry_trampoline)
> > mrs x4, ttbr1_el1
> > @@ -967,7 +951,6 @@ SYM_CODE_START(__sdei_asm_exit_trampoline)
> > 1: sdei_handler_exit exit_mode=x2
> > SYM_CODE_END(__sdei_asm_exit_trampoline)
> > NOKPROBE(__sdei_asm_exit_trampoline)
> > - .ltorg
> > .popsection // .entry.tramp.text
> > #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> >
> > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> > index edaf0faf766f..17e554be9198 100644
> > --- a/arch/arm64/kernel/vmlinux.lds.S
> > +++ b/arch/arm64/kernel/vmlinux.lds.S
> > @@ -117,7 +117,8 @@ jiffies = jiffies_64;
> > __entry_tramp_text_start = .; \
> > *(.entry.tramp.text) \
> > . = ALIGN(PAGE_SIZE); \
> > - __entry_tramp_text_end = .;
> > + __entry_tramp_text_end = .; \
> > + *(.entry.tramp.rodata)
> > #else
> > #define TRAMP_TEXT
> > #endif
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index 626ec32873c6..be4d6c3f5692 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -665,13 +665,9 @@ static int __init map_entry_trampoline(void)
> > __set_fixmap(FIX_ENTRY_TRAMP_TEXT1 - i,
> > pa_start + i * PAGE_SIZE, prot);
> >
> > - if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> > - extern char __entry_tramp_data_start[];
> > -
> > - __set_fixmap(FIX_ENTRY_TRAMP_DATA,
> > - __pa_symbol(__entry_tramp_data_start),
> > - PAGE_KERNEL_RO);
> > - }
> > + if (IS_ENABLED(CONFIG_RELOCATABLE))
> > + __set_fixmap(FIX_ENTRY_TRAMP_TEXT1 - i,
> > + pa_start + i * PAGE_SIZE, PAGE_KERNEL_RO);
> >
> > return 0;
> > }
> > --
> > 2.30.2
> >
_______________________________________________
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] 6+ messages in thread
* Re: [PATCH] arm64: entry: simplify trampoline data page
2022-06-22 15:04 ` Mark Rutland
@ 2022-06-22 15:27 ` Ard Biesheuvel
2022-06-22 15:40 ` Mark Rutland
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2022-06-22 15:27 UTC (permalink / raw)
To: Mark Rutland; +Cc: Linux ARM, Will Deacon, Catalin Marinas, James Morse
On Wed, 22 Jun 2022 at 17:05, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Wed, Jun 22, 2022 at 04:41:41PM +0200, Ard Biesheuvel wrote:
> > On Wed, 27 Apr 2022 at 12:23, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > Get rid of some clunky open coded arithmetic on section addresses, by
> > > emitting the trampoline data variables into a separate, dedicated r/o
> > > data section, and putting it at the next page boundary. This way, we can
> > > access the literals via single LDR instruction.
> > >
> > > While at it, get rid of other, implicit literals, and use ADRP/ADD or
> > > MOVZ/MOVK sequences, as appropriate. Note that the latter are only
> > > supported for CONFIG_RELOCATABLE=n (which is usually the case if
> > > CONFIG_RANDOMIZE_BASE=n), so update the CPP conditionals to reflect
> > > this.
> > >
> > > Cc: James Morse <james.morse@arm.com>
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > Ping, in case this one slipped behind the desk.
>
> Sorry for the delay. THis has been on my queue of things to look at with a
> bunch of other stuff, and I've had some difficulty prioritizing all that.
>
Yeah, no worries.
> >
> > > ---
> > > arch/arm64/include/asm/fixmap.h | 4 +-
> > > arch/arm64/kernel/entry.S | 45 ++++++--------------
> > > arch/arm64/kernel/vmlinux.lds.S | 3 +-
> > > arch/arm64/mm/mmu.c | 10 ++---
> > > 4 files changed, 22 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
> > > index daff882883f9..71ed5fdf718b 100644
> > > --- a/arch/arm64/include/asm/fixmap.h
> > > +++ b/arch/arm64/include/asm/fixmap.h
> > > @@ -62,10 +62,12 @@ enum fixed_addresses {
> > > #endif /* CONFIG_ACPI_APEI_GHES */
> > >
> > > #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> > > +#ifdef CONFIG_RELOCATABLE
> > > + FIX_ENTRY_TRAMP_TEXT4, /* one extra slot for the data page */
> > > +#endif
> > > FIX_ENTRY_TRAMP_TEXT3,
> > > FIX_ENTRY_TRAMP_TEXT2,
> > > FIX_ENTRY_TRAMP_TEXT1,
> > > - FIX_ENTRY_TRAMP_DATA,
> > > #define TRAMP_VALIAS (__fix_to_virt(FIX_ENTRY_TRAMP_TEXT1))
> > > #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> > > __end_of_permanent_fixed_addresses,
> > > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> > > index ede028dee81b..aed2b41e05aa 100644
> > > --- a/arch/arm64/kernel/entry.S
> > > +++ b/arch/arm64/kernel/entry.S
> > > @@ -636,18 +636,20 @@ alternative_else_nop_endif
> > > */
> > > .endm
> > >
> > > - .macro tramp_data_page dst
> > > - adr_l \dst, .entry.tramp.text
> > > - sub \dst, \dst, PAGE_SIZE
> > > - .endm
> > > -
> > > - .macro tramp_data_read_var dst, var
> > > -#ifdef CONFIG_RANDOMIZE_BASE
> > > - tramp_data_page \dst
> > > - add \dst, \dst, #:lo12:__entry_tramp_data_\var
> > > - ldr \dst, [\dst]
> > > + .macro tramp_data_read_var dst, var
> > > +#ifdef CONFIG_RELOCATABLE
> > > + ldr \dst, .L__tramp_data_\var
> > > + .ifndef .L__tramp_data_\var
> > > + .pushsection ".entry.tramp.rodata", "a", %progbits
> > > + .align 3
> > > +.L__tramp_data_\var:
> > > + .quad \var
> > > + .popsection
> > > + .endif
> > > #else
> > > - ldr \dst, =\var
> > > + movz \dst, :abs_g2_s:\var
> > > + movk \dst, :abs_g1_nc:\var
> > > + movk \dst, :abs_g0_nc:\var
> > > #endif
> > > .endm
>
> Given the lack of a g3 reloc, I assumme `var` is always an address, and we're
> assuming it's always in the upper 48-bits? I think it'd be worth a comment as
> to why this is safe, or just use a g3 reloc since then it's always good per
> inspection.
>
Upper 47 bits, yes. This is because since the 52-bit VA address space
overhaul, the kernel, fixmap and anything else we may want to address
statically here will always be in the upper 47-bit addressable part of
the address space. The abs_g2_s relocation sign extends that into the
bits above.
I opted for as few instructions as required, as these sequences are
emitted into the vector table.
> I'm a bit confused that we've put the var into the literal; I thought the idea
> here was that it was secret and needed to be placed in a page not mapped during
> userspace. Is the assumption there that it's pointless for !RELOCATABLE kernels
> since it can be known anyway, have I misunderstood, or something else?
>
Basically, yes. !RELOCATABLE implies !RANDOMIZE_BASE, and so the
kernel will be running from a known address anyway. So if you are
using KPTI without KASLR, there is no need to use a literal load here.
> Otherwise this all looks good superficially; I just haven't had the time to
> page it all in.
>
> Mark.
>
> > >
> > > @@ -695,7 +697,7 @@ alternative_else_nop_endif
> > > msr vbar_el1, x30
> > > isb
> > > .else
> > > - ldr x30, =vectors
> > > + adr_l x30, vectors
> > > .endif // \kpti == 1
> > >
> > > .if \bhb == BHB_MITIGATION_FW
> > > @@ -764,24 +766,7 @@ SYM_CODE_END(tramp_exit_native)
> > > SYM_CODE_START(tramp_exit_compat)
> > > tramp_exit 32
> > > SYM_CODE_END(tramp_exit_compat)
> > > -
> > > - .ltorg
> > > .popsection // .entry.tramp.text
> > > -#ifdef CONFIG_RANDOMIZE_BASE
> > > - .pushsection ".rodata", "a"
> > > - .align PAGE_SHIFT
> > > -SYM_DATA_START(__entry_tramp_data_start)
> > > -__entry_tramp_data_vectors:
> > > - .quad vectors
> > > -#ifdef CONFIG_ARM_SDE_INTERFACE
> > > -__entry_tramp_data___sdei_asm_handler:
> > > - .quad __sdei_asm_handler
> > > -#endif /* CONFIG_ARM_SDE_INTERFACE */
> > > -__entry_tramp_data_this_cpu_vector:
> > > - .quad this_cpu_vector
> > > -SYM_DATA_END(__entry_tramp_data_start)
> > > - .popsection // .rodata
> > > -#endif /* CONFIG_RANDOMIZE_BASE */
> > > #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> > >
> > > /*
> > > @@ -932,7 +917,6 @@ NOKPROBE(call_on_irq_stack)
> > > * This clobbers x4, __sdei_handler() will restore this from firmware's
> > > * copy.
> > > */
> > > -.ltorg
> > > .pushsection ".entry.tramp.text", "ax"
> > > SYM_CODE_START(__sdei_asm_entry_trampoline)
> > > mrs x4, ttbr1_el1
> > > @@ -967,7 +951,6 @@ SYM_CODE_START(__sdei_asm_exit_trampoline)
> > > 1: sdei_handler_exit exit_mode=x2
> > > SYM_CODE_END(__sdei_asm_exit_trampoline)
> > > NOKPROBE(__sdei_asm_exit_trampoline)
> > > - .ltorg
> > > .popsection // .entry.tramp.text
> > > #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> > >
> > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> > > index edaf0faf766f..17e554be9198 100644
> > > --- a/arch/arm64/kernel/vmlinux.lds.S
> > > +++ b/arch/arm64/kernel/vmlinux.lds.S
> > > @@ -117,7 +117,8 @@ jiffies = jiffies_64;
> > > __entry_tramp_text_start = .; \
> > > *(.entry.tramp.text) \
> > > . = ALIGN(PAGE_SIZE); \
> > > - __entry_tramp_text_end = .;
> > > + __entry_tramp_text_end = .; \
> > > + *(.entry.tramp.rodata)
> > > #else
> > > #define TRAMP_TEXT
> > > #endif
> > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > > index 626ec32873c6..be4d6c3f5692 100644
> > > --- a/arch/arm64/mm/mmu.c
> > > +++ b/arch/arm64/mm/mmu.c
> > > @@ -665,13 +665,9 @@ static int __init map_entry_trampoline(void)
> > > __set_fixmap(FIX_ENTRY_TRAMP_TEXT1 - i,
> > > pa_start + i * PAGE_SIZE, prot);
> > >
> > > - if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> > > - extern char __entry_tramp_data_start[];
> > > -
> > > - __set_fixmap(FIX_ENTRY_TRAMP_DATA,
> > > - __pa_symbol(__entry_tramp_data_start),
> > > - PAGE_KERNEL_RO);
> > > - }
> > > + if (IS_ENABLED(CONFIG_RELOCATABLE))
> > > + __set_fixmap(FIX_ENTRY_TRAMP_TEXT1 - i,
> > > + pa_start + i * PAGE_SIZE, PAGE_KERNEL_RO);
> > >
> > > return 0;
> > > }
> > > --
> > > 2.30.2
> > >
_______________________________________________
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] 6+ messages in thread
* Re: [PATCH] arm64: entry: simplify trampoline data page
2022-06-22 15:27 ` Ard Biesheuvel
@ 2022-06-22 15:40 ` Mark Rutland
2022-06-22 16:07 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2022-06-22 15:40 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Linux ARM, Will Deacon, Catalin Marinas, James Morse
On Wed, Jun 22, 2022 at 05:27:35PM +0200, Ard Biesheuvel wrote:
> On Wed, 22 Jun 2022 at 17:05, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Wed, Jun 22, 2022 at 04:41:41PM +0200, Ard Biesheuvel wrote:
> > > On Wed, 27 Apr 2022 at 12:23, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > Get rid of some clunky open coded arithmetic on section addresses, by
> > > > emitting the trampoline data variables into a separate, dedicated r/o
> > > > data section, and putting it at the next page boundary. This way, we can
> > > > access the literals via single LDR instruction.
> > > >
> > > > While at it, get rid of other, implicit literals, and use ADRP/ADD or
> > > > MOVZ/MOVK sequences, as appropriate. Note that the latter are only
> > > > supported for CONFIG_RELOCATABLE=n (which is usually the case if
> > > > CONFIG_RANDOMIZE_BASE=n), so update the CPP conditionals to reflect
> > > > this.
> > > >
> > > > Cc: James Morse <james.morse@arm.com>
> > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > Ping, in case this one slipped behind the desk.
> >
> > Sorry for the delay. THis has been on my queue of things to look at with a
> > bunch of other stuff, and I've had some difficulty prioritizing all that.
> >
>
> Yeah, no worries.
>
>
> > >
> > > > ---
> > > > arch/arm64/include/asm/fixmap.h | 4 +-
> > > > arch/arm64/kernel/entry.S | 45 ++++++--------------
> > > > arch/arm64/kernel/vmlinux.lds.S | 3 +-
> > > > arch/arm64/mm/mmu.c | 10 ++---
> > > > 4 files changed, 22 insertions(+), 40 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
> > > > index daff882883f9..71ed5fdf718b 100644
> > > > --- a/arch/arm64/include/asm/fixmap.h
> > > > +++ b/arch/arm64/include/asm/fixmap.h
> > > > @@ -62,10 +62,12 @@ enum fixed_addresses {
> > > > #endif /* CONFIG_ACPI_APEI_GHES */
> > > >
> > > > #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> > > > +#ifdef CONFIG_RELOCATABLE
> > > > + FIX_ENTRY_TRAMP_TEXT4, /* one extra slot for the data page */
> > > > +#endif
> > > > FIX_ENTRY_TRAMP_TEXT3,
> > > > FIX_ENTRY_TRAMP_TEXT2,
> > > > FIX_ENTRY_TRAMP_TEXT1,
> > > > - FIX_ENTRY_TRAMP_DATA,
> > > > #define TRAMP_VALIAS (__fix_to_virt(FIX_ENTRY_TRAMP_TEXT1))
> > > > #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> > > > __end_of_permanent_fixed_addresses,
> > > > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> > > > index ede028dee81b..aed2b41e05aa 100644
> > > > --- a/arch/arm64/kernel/entry.S
> > > > +++ b/arch/arm64/kernel/entry.S
> > > > @@ -636,18 +636,20 @@ alternative_else_nop_endif
> > > > */
> > > > .endm
> > > >
> > > > - .macro tramp_data_page dst
> > > > - adr_l \dst, .entry.tramp.text
> > > > - sub \dst, \dst, PAGE_SIZE
> > > > - .endm
> > > > -
> > > > - .macro tramp_data_read_var dst, var
> > > > -#ifdef CONFIG_RANDOMIZE_BASE
> > > > - tramp_data_page \dst
> > > > - add \dst, \dst, #:lo12:__entry_tramp_data_\var
> > > > - ldr \dst, [\dst]
> > > > + .macro tramp_data_read_var dst, var
> > > > +#ifdef CONFIG_RELOCATABLE
> > > > + ldr \dst, .L__tramp_data_\var
> > > > + .ifndef .L__tramp_data_\var
> > > > + .pushsection ".entry.tramp.rodata", "a", %progbits
> > > > + .align 3
> > > > +.L__tramp_data_\var:
> > > > + .quad \var
> > > > + .popsection
> > > > + .endif
> > > > #else
> > > > - ldr \dst, =\var
> > > > + movz \dst, :abs_g2_s:\var
> > > > + movk \dst, :abs_g1_nc:\var
> > > > + movk \dst, :abs_g0_nc:\var
> > > > #endif
> > > > .endm
> >
> > Given the lack of a g3 reloc, I assumme `var` is always an address, and we're
> > assuming it's always in the upper 48-bits? I think it'd be worth a comment as
> > to why this is safe, or just use a g3 reloc since then it's always good per
> > inspection.
> >
>
> Upper 47 bits, yes. This is because since the 52-bit VA address space
> overhaul, the kernel, fixmap and anything else we may want to address
> statically here will always be in the upper 47-bit addressable part of
> the address space. The abs_g2_s relocation sign extends that into the
> bits above.
>
> I opted for as few instructions as required, as these sequences are
> emitted into the vector table.
>
> > I'm a bit confused that we've put the var into the literal; I thought the idea
> > here was that it was secret and needed to be placed in a page not mapped during
> > userspace. Is the assumption there that it's pointless for !RELOCATABLE kernels
> > since it can be known anyway, have I misunderstood, or something else?
> >
>
> Basically, yes. !RELOCATABLE implies !RANDOMIZE_BASE, and so the
> kernel will be running from a known address anyway. So if you are
> using KPTI without KASLR, there is no need to use a literal load here.
Fair enough; that all makes sense to me.
Could we have a comment to that effect, e.g. something like:
/*
* As !RELOCATABLE implies !RANDOMIZE_BASE the address is always a
* compile time constant (and hence not secret and not worth hiding).
*
* As !RELOCATABLE kernels always live in the top 47 bits of the address
* space we can sign-extend bit 47 and avoid an instruction to load the
* upper 16 bits (which must be 0xFFFF).
*/
With something like that:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
>
> > Otherwise this all looks good superficially; I just haven't had the time to
> > page it all in.
> >
> > Mark.
> >
> > > >
> > > > @@ -695,7 +697,7 @@ alternative_else_nop_endif
> > > > msr vbar_el1, x30
> > > > isb
> > > > .else
> > > > - ldr x30, =vectors
> > > > + adr_l x30, vectors
> > > > .endif // \kpti == 1
> > > >
> > > > .if \bhb == BHB_MITIGATION_FW
> > > > @@ -764,24 +766,7 @@ SYM_CODE_END(tramp_exit_native)
> > > > SYM_CODE_START(tramp_exit_compat)
> > > > tramp_exit 32
> > > > SYM_CODE_END(tramp_exit_compat)
> > > > -
> > > > - .ltorg
> > > > .popsection // .entry.tramp.text
> > > > -#ifdef CONFIG_RANDOMIZE_BASE
> > > > - .pushsection ".rodata", "a"
> > > > - .align PAGE_SHIFT
> > > > -SYM_DATA_START(__entry_tramp_data_start)
> > > > -__entry_tramp_data_vectors:
> > > > - .quad vectors
> > > > -#ifdef CONFIG_ARM_SDE_INTERFACE
> > > > -__entry_tramp_data___sdei_asm_handler:
> > > > - .quad __sdei_asm_handler
> > > > -#endif /* CONFIG_ARM_SDE_INTERFACE */
> > > > -__entry_tramp_data_this_cpu_vector:
> > > > - .quad this_cpu_vector
> > > > -SYM_DATA_END(__entry_tramp_data_start)
> > > > - .popsection // .rodata
> > > > -#endif /* CONFIG_RANDOMIZE_BASE */
> > > > #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> > > >
> > > > /*
> > > > @@ -932,7 +917,6 @@ NOKPROBE(call_on_irq_stack)
> > > > * This clobbers x4, __sdei_handler() will restore this from firmware's
> > > > * copy.
> > > > */
> > > > -.ltorg
> > > > .pushsection ".entry.tramp.text", "ax"
> > > > SYM_CODE_START(__sdei_asm_entry_trampoline)
> > > > mrs x4, ttbr1_el1
> > > > @@ -967,7 +951,6 @@ SYM_CODE_START(__sdei_asm_exit_trampoline)
> > > > 1: sdei_handler_exit exit_mode=x2
> > > > SYM_CODE_END(__sdei_asm_exit_trampoline)
> > > > NOKPROBE(__sdei_asm_exit_trampoline)
> > > > - .ltorg
> > > > .popsection // .entry.tramp.text
> > > > #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> > > >
> > > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> > > > index edaf0faf766f..17e554be9198 100644
> > > > --- a/arch/arm64/kernel/vmlinux.lds.S
> > > > +++ b/arch/arm64/kernel/vmlinux.lds.S
> > > > @@ -117,7 +117,8 @@ jiffies = jiffies_64;
> > > > __entry_tramp_text_start = .; \
> > > > *(.entry.tramp.text) \
> > > > . = ALIGN(PAGE_SIZE); \
> > > > - __entry_tramp_text_end = .;
> > > > + __entry_tramp_text_end = .; \
> > > > + *(.entry.tramp.rodata)
> > > > #else
> > > > #define TRAMP_TEXT
> > > > #endif
> > > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > > > index 626ec32873c6..be4d6c3f5692 100644
> > > > --- a/arch/arm64/mm/mmu.c
> > > > +++ b/arch/arm64/mm/mmu.c
> > > > @@ -665,13 +665,9 @@ static int __init map_entry_trampoline(void)
> > > > __set_fixmap(FIX_ENTRY_TRAMP_TEXT1 - i,
> > > > pa_start + i * PAGE_SIZE, prot);
> > > >
> > > > - if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> > > > - extern char __entry_tramp_data_start[];
> > > > -
> > > > - __set_fixmap(FIX_ENTRY_TRAMP_DATA,
> > > > - __pa_symbol(__entry_tramp_data_start),
> > > > - PAGE_KERNEL_RO);
> > > > - }
> > > > + if (IS_ENABLED(CONFIG_RELOCATABLE))
> > > > + __set_fixmap(FIX_ENTRY_TRAMP_TEXT1 - i,
> > > > + pa_start + i * PAGE_SIZE, PAGE_KERNEL_RO);
> > > >
> > > > return 0;
> > > > }
> > > > --
> > > > 2.30.2
> > > >
_______________________________________________
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] 6+ messages in thread
* Re: [PATCH] arm64: entry: simplify trampoline data page
2022-06-22 15:40 ` Mark Rutland
@ 2022-06-22 16:07 ` Ard Biesheuvel
0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2022-06-22 16:07 UTC (permalink / raw)
To: Mark Rutland; +Cc: Linux ARM, Will Deacon, Catalin Marinas, James Morse
On Wed, 22 Jun 2022 at 17:40, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Wed, Jun 22, 2022 at 05:27:35PM +0200, Ard Biesheuvel wrote:
> > On Wed, 22 Jun 2022 at 17:05, Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > On Wed, Jun 22, 2022 at 04:41:41PM +0200, Ard Biesheuvel wrote:
> > > > On Wed, 27 Apr 2022 at 12:23, Ard Biesheuvel <ardb@kernel.org> wrote:
...
> > > > > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> > > > > index ede028dee81b..aed2b41e05aa 100644
> > > > > --- a/arch/arm64/kernel/entry.S
> > > > > +++ b/arch/arm64/kernel/entry.S
> > > > > @@ -636,18 +636,20 @@ alternative_else_nop_endif
> > > > > */
> > > > > .endm
> > > > >
> > > > > - .macro tramp_data_page dst
> > > > > - adr_l \dst, .entry.tramp.text
> > > > > - sub \dst, \dst, PAGE_SIZE
> > > > > - .endm
> > > > > -
> > > > > - .macro tramp_data_read_var dst, var
> > > > > -#ifdef CONFIG_RANDOMIZE_BASE
> > > > > - tramp_data_page \dst
> > > > > - add \dst, \dst, #:lo12:__entry_tramp_data_\var
> > > > > - ldr \dst, [\dst]
> > > > > + .macro tramp_data_read_var dst, var
> > > > > +#ifdef CONFIG_RELOCATABLE
> > > > > + ldr \dst, .L__tramp_data_\var
> > > > > + .ifndef .L__tramp_data_\var
> > > > > + .pushsection ".entry.tramp.rodata", "a", %progbits
> > > > > + .align 3
> > > > > +.L__tramp_data_\var:
> > > > > + .quad \var
> > > > > + .popsection
> > > > > + .endif
> > > > > #else
> > > > > - ldr \dst, =\var
> > > > > + movz \dst, :abs_g2_s:\var
> > > > > + movk \dst, :abs_g1_nc:\var
> > > > > + movk \dst, :abs_g0_nc:\var
> > > > > #endif
> > > > > .endm
> > >
> > > Given the lack of a g3 reloc, I assumme `var` is always an address, and we're
> > > assuming it's always in the upper 48-bits? I think it'd be worth a comment as
> > > to why this is safe, or just use a g3 reloc since then it's always good per
> > > inspection.
> > >
> >
> > Upper 47 bits, yes. This is because since the 52-bit VA address space
> > overhaul, the kernel, fixmap and anything else we may want to address
> > statically here will always be in the upper 47-bit addressable part of
> > the address space. The abs_g2_s relocation sign extends that into the
> > bits above.
> >
> > I opted for as few instructions as required, as these sequences are
> > emitted into the vector table.
> >
> > > I'm a bit confused that we've put the var into the literal; I thought the idea
> > > here was that it was secret and needed to be placed in a page not mapped during
> > > userspace. Is the assumption there that it's pointless for !RELOCATABLE kernels
> > > since it can be known anyway, have I misunderstood, or something else?
> > >
> >
> > Basically, yes. !RELOCATABLE implies !RANDOMIZE_BASE, and so the
> > kernel will be running from a known address anyway. So if you are
> > using KPTI without KASLR, there is no need to use a literal load here.
>
> Fair enough; that all makes sense to me.
>
> Could we have a comment to that effect, e.g. something like:
>
> /*
> * As !RELOCATABLE implies !RANDOMIZE_BASE the address is always a
> * compile time constant (and hence not secret and not worth hiding).
> *
> * As !RELOCATABLE kernels always live in the top 47 bits of the address
> * space we can sign-extend bit 47 and avoid an instruction to load the
> * upper 16 bits (which must be 0xFFFF).
> */
>
> With something like that:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
Thanks, I'll fold that in.
_______________________________________________
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] 6+ messages in thread
end of thread, other threads:[~2022-06-22 16:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-27 10:22 [PATCH] arm64: entry: simplify trampoline data page Ard Biesheuvel
2022-06-22 14:41 ` Ard Biesheuvel
2022-06-22 15:04 ` Mark Rutland
2022-06-22 15:27 ` Ard Biesheuvel
2022-06-22 15:40 ` Mark Rutland
2022-06-22 16:07 ` Ard Biesheuvel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.