diff mbox series

[3/3] arm64: reduce trampoline data alignment

Message ID 20200319091407.51449-3-remi@remlab.net (mailing list archive)
State New, archived
Headers show
Series clean up KPTI / SDEI trampoline data alignment | expand

Commit Message

Rémi Denis-Courmont March 19, 2020, 9:14 a.m. UTC 
From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>

The trampoline data, currently consisting of two relocated pointers,
must be within a single page. However, there are no needs for it to
start a page.

This reduces the alignment to 16 bytes, which is sufficient to ensure
that the data is entirely within a single page of the fixmap.

Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
---
 arch/arm64/kernel/entry.S | 2 +-
 arch/arm64/mm/mmu.c       | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

Comments

Catalin Marinas March 21, 2020, 1:40 p.m. UTC | #1 
On Thu, Mar 19, 2020 at 11:14:07AM +0200, Rémi Denis-Courmont wrote:
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index c36733d8cd75..ecad15443655 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -858,7 +858,7 @@ SYM_CODE_END(tramp_exit_compat)
>  	.popsection				// .entry.tramp.text
>  #ifdef CONFIG_RANDOMIZE_BASE
>  	.pushsection ".rodata", "a"
> -	.align PAGE_SHIFT
> +	.align	4	// all .rodata must be in a single fixmap page
>  SYM_DATA_START(__entry_tramp_data_start)
>  	.quad	vectors
>  SYM_DATA_END(__entry_tramp_data_start)
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 9b08f7c7e6f0..6a0e75f48e7b 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -599,9 +599,8 @@ static int __init map_entry_trampoline(void)
>  	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);
> +		pa_start = __pa_symbol(__entry_tramp_data_start) & PAGE_MASK;
> +		__set_fixmap(FIX_ENTRY_TRAMP_DATA, pa_start, PAGE_KERNEL_RO);
>  	}
>  
>  	return 0;

For some reason, I haven't investigated yet, a kernel with KASAN and 64K
pages enabled does not boot (see the attached config). It seems to lock
up when starting user space. Bisected to this commit, reverting it fixes
the issue.
Mark Rutland March 23, 2020, 11:58 a.m. UTC | #2 
On Sat, Mar 21, 2020 at 01:41:01PM +0000, Catalin Marinas wrote:
> On Thu, Mar 19, 2020 at 11:14:07AM +0200, Rémi Denis-Courmont wrote:
> > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> > index c36733d8cd75..ecad15443655 100644
> > --- a/arch/arm64/kernel/entry.S
> > +++ b/arch/arm64/kernel/entry.S
> > @@ -858,7 +858,7 @@ SYM_CODE_END(tramp_exit_compat)
> >  	.popsection				// .entry.tramp.text
> >  #ifdef CONFIG_RANDOMIZE_BASE
> >  	.pushsection ".rodata", "a"
> > -	.align PAGE_SHIFT
> > +	.align	4	// all .rodata must be in a single fixmap page
> >  SYM_DATA_START(__entry_tramp_data_start)
> >  	.quad	vectors
> >  SYM_DATA_END(__entry_tramp_data_start)
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index 9b08f7c7e6f0..6a0e75f48e7b 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -599,9 +599,8 @@ static int __init map_entry_trampoline(void)
> >  	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);
> > +		pa_start = __pa_symbol(__entry_tramp_data_start) & PAGE_MASK;
> > +		__set_fixmap(FIX_ENTRY_TRAMP_DATA, pa_start, PAGE_KERNEL_RO);
> >  	}
> >  
> >  	return 0;
> 
> For some reason, I haven't investigated yet, a kernel with KASAN and 64K
> pages enabled does not boot (see the attached config). It seems to lock
> up when starting user space. Bisected to this commit, reverting it fixes
> the issue.

I think the issue might be due to ADRP + ADD :lo12: using 4K offsets,
and so patch 1 isn't quite right for !4K kernels, as we're not
accounting for 4 bits of the address when we try to generate it.

I'll check that now.

Thanks,
Mark.
diff mbox series

Patch

diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index c36733d8cd75..ecad15443655 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -858,7 +858,7 @@  SYM_CODE_END(tramp_exit_compat)
 	.popsection				// .entry.tramp.text
 #ifdef CONFIG_RANDOMIZE_BASE
 	.pushsection ".rodata", "a"
-	.align PAGE_SHIFT
+	.align	4	// all .rodata must be in a single fixmap page
 SYM_DATA_START(__entry_tramp_data_start)
 	.quad	vectors
 SYM_DATA_END(__entry_tramp_data_start)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 9b08f7c7e6f0..6a0e75f48e7b 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -599,9 +599,8 @@  static int __init map_entry_trampoline(void)
 	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);
+		pa_start = __pa_symbol(__entry_tramp_data_start) & PAGE_MASK;
+		__set_fixmap(FIX_ENTRY_TRAMP_DATA, pa_start, PAGE_KERNEL_RO);
 	}
 
 	return 0;