2021-09-29 19:44:00

by Nick Desaulniers

[permalink] [raw]
Subject: [PATCH] ARM: mm: proc-macros: ensure *_tlb_fns are 4B aligned

QEMU 6.1.0 is more correct about trapping on misaligned accesses. A
kernel built with CONFIG_THUMB2_KERNEL=y and using clang as the
assembler could generate non-naturally-aligned v7wbi_tlb_fns which
results in a boot failure. The original commit adding the macro missed
the .align directive on this data.

Fixes: 66a625a88174 ("ARM: mm: proc-macros: Add generic proc/cache/tlb struct definition macros")
Link: https://github.com/ClangBuiltLinux/linux/issues/1447
Link: https://lore.kernel.org/all/[email protected]/
Debugged-by: Ard Biesheuvel <[email protected]>
Debugged-by: Nathan Chancellor <[email protected]>
Debugged-by: Richard Henderson <[email protected]>
Suggested-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/mm/proc-macros.S | 1 +
2 files changed, 2 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index fc196421b2ce..b760dd45b734 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -92,6 +92,7 @@ config ARM
select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL && !CC_IS_CLANG
select HAVE_FUNCTION_TRACER if !XIP_KERNEL
+ select HAVE_FUTEX_CMPXCHG if FUTEX
select HAVE_GCC_PLUGINS
select HAVE_HW_BREAKPOINT if PERF_EVENTS && (CPU_V6 || CPU_V6K || CPU_V7)
select HAVE_IRQ_TIME_ACCOUNTING
diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
index e2c743aa2eb2..d9f7dfe2a7ed 100644
--- a/arch/arm/mm/proc-macros.S
+++ b/arch/arm/mm/proc-macros.S
@@ -340,6 +340,7 @@ ENTRY(\name\()_cache_fns)

.macro define_tlb_functions name:req, flags_up:req, flags_smp
.type \name\()_tlb_fns, #object
+ .align 2
ENTRY(\name\()_tlb_fns)
.long \name\()_flush_user_tlb_range
.long \name\()_flush_kern_tlb_range
--
2.33.0.685.g46640cef36-goog


2021-09-29 19:48:05

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] ARM: mm: proc-macros: ensure *_tlb_fns are 4B aligned

On Wed, Sep 29, 2021 at 12:10 PM Nick Desaulniers
<[email protected]> wrote:
>
> + Dave (Dave, I got a bounceback on your linaro.org email address.
> Consider sending a patch updating the .mailmap)
>
> On Wed, Sep 29, 2021 at 12:08 PM Nick Desaulniers
> <[email protected]> wrote:
> >
> > QEMU 6.1.0 is more correct about trapping on misaligned accesses. A
> > kernel built with CONFIG_THUMB2_KERNEL=y and using clang as the
> > assembler could generate non-naturally-aligned v7wbi_tlb_fns which
> > results in a boot failure. The original commit adding the macro missed
> > the .align directive on this data.
> >
> > Fixes: 66a625a88174 ("ARM: mm: proc-macros: Add generic proc/cache/tlb struct definition macros")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1447
> > Link: https://lore.kernel.org/all/[email protected]/
> > Debugged-by: Ard Biesheuvel <[email protected]>
> > Debugged-by: Nathan Chancellor <[email protected]>
> > Debugged-by: Richard Henderson <[email protected]>
> > Suggested-by: Ard Biesheuvel <[email protected]>
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > ---
> > arch/arm/Kconfig | 1 +

d'oh, had an unintentional `-a` when I did `git commit`. Will resend a v2...

> > arch/arm/mm/proc-macros.S | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index fc196421b2ce..b760dd45b734 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -92,6 +92,7 @@ config ARM
> > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL && !CC_IS_CLANG
> > select HAVE_FUNCTION_TRACER if !XIP_KERNEL
> > + select HAVE_FUTEX_CMPXCHG if FUTEX
> > select HAVE_GCC_PLUGINS
> > select HAVE_HW_BREAKPOINT if PERF_EVENTS && (CPU_V6 || CPU_V6K || CPU_V7)
> > select HAVE_IRQ_TIME_ACCOUNTING
> > diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> > index e2c743aa2eb2..d9f7dfe2a7ed 100644
> > --- a/arch/arm/mm/proc-macros.S
> > +++ b/arch/arm/mm/proc-macros.S
> > @@ -340,6 +340,7 @@ ENTRY(\name\()_cache_fns)
> >
> > .macro define_tlb_functions name:req, flags_up:req, flags_smp
> > .type \name\()_tlb_fns, #object
> > + .align 2
> > ENTRY(\name\()_tlb_fns)
> > .long \name\()_flush_user_tlb_range
> > .long \name\()_flush_kern_tlb_range
> > --
> > 2.33.0.685.g46640cef36-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



--
Thanks,
~Nick Desaulniers

2021-09-29 19:48:06

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH] ARM: mm: proc-macros: ensure *_tlb_fns are 4B aligned

On Wed, 29 Sept 2021 at 21:08, Nick Desaulniers <[email protected]> wrote:
>
> QEMU 6.1.0 is more correct about trapping on misaligned accesses. A
> kernel built with CONFIG_THUMB2_KERNEL=y and using clang as the
> assembler could generate non-naturally-aligned v7wbi_tlb_fns which
> results in a boot failure. The original commit adding the macro missed
> the .align directive on this data.
>
> Fixes: 66a625a88174 ("ARM: mm: proc-macros: Add generic proc/cache/tlb struct definition macros")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1447
> Link: https://lore.kernel.org/all/[email protected]/
> Debugged-by: Ard Biesheuvel <[email protected]>
> Debugged-by: Nathan Chancellor <[email protected]>
> Debugged-by: Richard Henderson <[email protected]>
> Suggested-by: Ard Biesheuvel <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/mm/proc-macros.S | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index fc196421b2ce..b760dd45b734 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -92,6 +92,7 @@ config ARM
> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL && !CC_IS_CLANG
> select HAVE_FUNCTION_TRACER if !XIP_KERNEL
> + select HAVE_FUTEX_CMPXCHG if FUTEX
> select HAVE_GCC_PLUGINS
> select HAVE_HW_BREAKPOINT if PERF_EVENTS && (CPU_V6 || CPU_V6K || CPU_V7)
> select HAVE_IRQ_TIME_ACCOUNTING

I take it this hunk got included by accident?

> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index e2c743aa2eb2..d9f7dfe2a7ed 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -340,6 +340,7 @@ ENTRY(\name\()_cache_fns)
>
> .macro define_tlb_functions name:req, flags_up:req, flags_smp
> .type \name\()_tlb_fns, #object
> + .align 2
> ENTRY(\name\()_tlb_fns)
> .long \name\()_flush_user_tlb_range
> .long \name\()_flush_kern_tlb_range
> --
> 2.33.0.685.g46640cef36-goog
>

For this part,

Acked-by: Ard Biesheuvel <[email protected]>

2021-09-29 19:49:08

by Nick Desaulniers

[permalink] [raw]
Subject: [PATCH v2] ARM: mm: proc-macros: ensure *_tlb_fns are 4B aligned

QEMU 6.1.0 is more correct about trapping on misaligned accesses. A
kernel built with CONFIG_THUMB2_KERNEL=y and using clang as the
assembler could generate non-naturally-aligned v7wbi_tlb_fns which
results in a boot failure. The original commit adding the macro missed
the .align directive on this data.

Fixes: 66a625a88174 ("ARM: mm: proc-macros: Add generic proc/cache/tlb struct definition macros")
Link: https://github.com/ClangBuiltLinux/linux/issues/1447
Link: https://lore.kernel.org/all/[email protected]/
Debugged-by: Ard Biesheuvel <[email protected]>
Debugged-by: Nathan Chancellor <[email protected]>
Debugged-by: Richard Henderson <[email protected]>
Suggested-by: Ard Biesheuvel <[email protected]>
Acked-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
Changes V1 -> V2:
* Drop accidentally committed Kconfig change.
* Pick up Ard's AB tag.

arch/arm/mm/proc-macros.S | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
index e2c743aa2eb2..d9f7dfe2a7ed 100644
--- a/arch/arm/mm/proc-macros.S
+++ b/arch/arm/mm/proc-macros.S
@@ -340,6 +340,7 @@ ENTRY(\name\()_cache_fns)

.macro define_tlb_functions name:req, flags_up:req, flags_smp
.type \name\()_tlb_fns, #object
+ .align 2
ENTRY(\name\()_tlb_fns)
.long \name\()_flush_user_tlb_range
.long \name\()_flush_kern_tlb_range

base-commit: 02d5e016800d082058b3d3b7c3ede136cdc6ddcb
prerequisite-patch-id: 3edbe4a8485c7a75a61dbbe299e8ce1985d87ee0
--
2.33.0.685.g46640cef36-goog

2021-09-29 20:41:55

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: mm: proc-macros: ensure *_tlb_fns are 4B aligned

On Wed, Sep 29, 2021 at 12:20:24PM -0700, Nick Desaulniers wrote:
> QEMU 6.1.0 is more correct about trapping on misaligned accesses. A
> kernel built with CONFIG_THUMB2_KERNEL=y and using clang as the
> assembler could generate non-naturally-aligned v7wbi_tlb_fns which
> results in a boot failure. The original commit adding the macro missed
> the .align directive on this data.
>
> Fixes: 66a625a88174 ("ARM: mm: proc-macros: Add generic proc/cache/tlb struct definition macros")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1447
> Link: https://lore.kernel.org/all/[email protected]/
> Debugged-by: Ard Biesheuvel <[email protected]>
> Debugged-by: Nathan Chancellor <[email protected]>
> Debugged-by: Richard Henderson <[email protected]>
> Suggested-by: Ard Biesheuvel <[email protected]>
> Acked-by: Ard Biesheuvel <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>

Tested-by: Nathan Chancellor <[email protected]>

> ---
> Changes V1 -> V2:
> * Drop accidentally committed Kconfig change.
> * Pick up Ard's AB tag.
>
> arch/arm/mm/proc-macros.S | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index e2c743aa2eb2..d9f7dfe2a7ed 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -340,6 +340,7 @@ ENTRY(\name\()_cache_fns)
>
> .macro define_tlb_functions name:req, flags_up:req, flags_smp
> .type \name\()_tlb_fns, #object
> + .align 2
> ENTRY(\name\()_tlb_fns)
> .long \name\()_flush_user_tlb_range
> .long \name\()_flush_kern_tlb_range
>
> base-commit: 02d5e016800d082058b3d3b7c3ede136cdc6ddcb
> prerequisite-patch-id: 3edbe4a8485c7a75a61dbbe299e8ce1985d87ee0
> --
> 2.33.0.685.g46640cef36-goog
>
>

2021-09-29 21:21:03

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] ARM: mm: proc-macros: ensure *_tlb_fns are 4B aligned

+ Dave (Dave, I got a bounceback on your linaro.org email address.
Consider sending a patch updating the .mailmap)

On Wed, Sep 29, 2021 at 12:08 PM Nick Desaulniers
<[email protected]> wrote:
>
> QEMU 6.1.0 is more correct about trapping on misaligned accesses. A
> kernel built with CONFIG_THUMB2_KERNEL=y and using clang as the
> assembler could generate non-naturally-aligned v7wbi_tlb_fns which
> results in a boot failure. The original commit adding the macro missed
> the .align directive on this data.
>
> Fixes: 66a625a88174 ("ARM: mm: proc-macros: Add generic proc/cache/tlb struct definition macros")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1447
> Link: https://lore.kernel.org/all/[email protected]/
> Debugged-by: Ard Biesheuvel <[email protected]>
> Debugged-by: Nathan Chancellor <[email protected]>
> Debugged-by: Richard Henderson <[email protected]>
> Suggested-by: Ard Biesheuvel <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/mm/proc-macros.S | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index fc196421b2ce..b760dd45b734 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -92,6 +92,7 @@ config ARM
> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL && !CC_IS_CLANG
> select HAVE_FUNCTION_TRACER if !XIP_KERNEL
> + select HAVE_FUTEX_CMPXCHG if FUTEX
> select HAVE_GCC_PLUGINS
> select HAVE_HW_BREAKPOINT if PERF_EVENTS && (CPU_V6 || CPU_V6K || CPU_V7)
> select HAVE_IRQ_TIME_ACCOUNTING
> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index e2c743aa2eb2..d9f7dfe2a7ed 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -340,6 +340,7 @@ ENTRY(\name\()_cache_fns)
>
> .macro define_tlb_functions name:req, flags_up:req, flags_smp
> .type \name\()_tlb_fns, #object
> + .align 2
> ENTRY(\name\()_tlb_fns)
> .long \name\()_flush_user_tlb_range
> .long \name\()_flush_kern_tlb_range
> --
> 2.33.0.685.g46640cef36-goog
>


--
Thanks,
~Nick Desaulniers

2021-09-29 21:34:47

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: mm: proc-macros: ensure *_tlb_fns are 4B aligned

On Wed, 29 Sept 2021 at 21:20, Nick Desaulniers <[email protected]> wrote:
>
> QEMU 6.1.0 is more correct about trapping on misaligned accesses.

Btw, this is not entirely relevant. QEMU now behaves like every single
hardware implementation does, and reports an alignment fault when
using a load-multiple instruction on an address that is not 32-bit
aligned, as the architecture requires.


> A
> kernel built with CONFIG_THUMB2_KERNEL=y and using clang as the
> assembler could generate non-naturally-aligned v7wbi_tlb_fns which
> results in a boot failure. The original commit adding the macro missed
> the .align directive on this data.
>
> Fixes: 66a625a88174 ("ARM: mm: proc-macros: Add generic proc/cache/tlb struct definition macros")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1447
> Link: https://lore.kernel.org/all/[email protected]/
> Debugged-by: Ard Biesheuvel <[email protected]>
> Debugged-by: Nathan Chancellor <[email protected]>
> Debugged-by: Richard Henderson <[email protected]>
> Suggested-by: Ard Biesheuvel <[email protected]>
> Acked-by: Ard Biesheuvel <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> Changes V1 -> V2:
> * Drop accidentally committed Kconfig change.
> * Pick up Ard's AB tag.
>
> arch/arm/mm/proc-macros.S | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> index e2c743aa2eb2..d9f7dfe2a7ed 100644
> --- a/arch/arm/mm/proc-macros.S
> +++ b/arch/arm/mm/proc-macros.S
> @@ -340,6 +340,7 @@ ENTRY(\name\()_cache_fns)
>
> .macro define_tlb_functions name:req, flags_up:req, flags_smp
> .type \name\()_tlb_fns, #object
> + .align 2
> ENTRY(\name\()_tlb_fns)
> .long \name\()_flush_user_tlb_range
> .long \name\()_flush_kern_tlb_range
>
> base-commit: 02d5e016800d082058b3d3b7c3ede136cdc6ddcb
> prerequisite-patch-id: 3edbe4a8485c7a75a61dbbe299e8ce1985d87ee0
> --
> 2.33.0.685.g46640cef36-goog
>

2021-09-29 21:34:52

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: mm: proc-macros: ensure *_tlb_fns are 4B aligned

On Wed, Sep 29, 2021 at 1:57 PM Ard Biesheuvel <[email protected]> wrote:
>
> On Wed, 29 Sept 2021 at 21:20, Nick Desaulniers <[email protected]> wrote:
> >
> > QEMU 6.1.0 is more correct about trapping on misaligned accesses.
>
> Btw, this is not entirely relevant. QEMU now behaves like every single
> hardware implementation does, and reports an alignment fault when
> using a load-multiple instruction on an address that is not 32-bit
> aligned, as the architecture requires.

Sure, I can drop that line from the commit message when submitting to
RMK's queue.

>
>
> > A
> > kernel built with CONFIG_THUMB2_KERNEL=y and using clang as the
> > assembler could generate non-naturally-aligned v7wbi_tlb_fns which
> > results in a boot failure. The original commit adding the macro missed
> > the .align directive on this data.
> >
> > Fixes: 66a625a88174 ("ARM: mm: proc-macros: Add generic proc/cache/tlb struct definition macros")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1447
> > Link: https://lore.kernel.org/all/[email protected]/
> > Debugged-by: Ard Biesheuvel <[email protected]>
> > Debugged-by: Nathan Chancellor <[email protected]>
> > Debugged-by: Richard Henderson <[email protected]>
> > Suggested-by: Ard Biesheuvel <[email protected]>
> > Acked-by: Ard Biesheuvel <[email protected]>
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > ---
> > Changes V1 -> V2:
> > * Drop accidentally committed Kconfig change.
> > * Pick up Ard's AB tag.
> >
> > arch/arm/mm/proc-macros.S | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/mm/proc-macros.S b/arch/arm/mm/proc-macros.S
> > index e2c743aa2eb2..d9f7dfe2a7ed 100644
> > --- a/arch/arm/mm/proc-macros.S
> > +++ b/arch/arm/mm/proc-macros.S
> > @@ -340,6 +340,7 @@ ENTRY(\name\()_cache_fns)
> >
> > .macro define_tlb_functions name:req, flags_up:req, flags_smp
> > .type \name\()_tlb_fns, #object
> > + .align 2
> > ENTRY(\name\()_tlb_fns)
> > .long \name\()_flush_user_tlb_range
> > .long \name\()_flush_kern_tlb_range
> >
> > base-commit: 02d5e016800d082058b3d3b7c3ede136cdc6ddcb
> > prerequisite-patch-id: 3edbe4a8485c7a75a61dbbe299e8ce1985d87ee0
> > --
> > 2.33.0.685.g46640cef36-goog
> >



--
Thanks,
~Nick Desaulniers