[PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Andre Przywara]

The ARM architecture requires all accesses to device memory to be
naturally aligned[1][2]. Normal memory does not have this strict
requirement, and in fact many systems do ignore unaligned accesses
(by the means of clearing the A bit in SCTLR and accessing normal
memory). So the default behaviour of GCC assumes that unaligned accesses
are fine, at least if happening on the stack.

Now kvm-unit-tests runs some C code with the MMU off, which degrades the
whole system memory to device memory. Now every unaligned access will
fault, regardless of the A bit.
In fact there is at least one place in lib/printf.c where GCC merges
two consecutive char* accesses into one "strh" instruction, writing to
a potentially unaligned address.
This can be reproduced by configuring kvm-unit-tests for kvmtool, but
running it on QEMU, which triggers an early printf that exercises this
particular code path.

Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
problem. Also add the respective -mno-unaligned-access flag for arm.

Thanks to Alexandru for helping debugging this.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>

[1] ARMv8 ARM DDI 0487E.a, B2.5.2
[2] ARMv7 ARM DDI 0406C.d, A3.2.1
---
 arm/Makefile.arm   | 1 +
 arm/Makefile.arm64 | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arm/Makefile.arm b/arm/Makefile.arm
index a625267..43b4be1 100644
--- a/arm/Makefile.arm
+++ b/arm/Makefile.arm
@@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
 
 CFLAGS += $(machine)
 CFLAGS += -mcpu=$(PROCESSOR)
+CFLAGS += -mno-unaligned-access
 
 arch_LDFLAGS = -Ttext=40010000
 
diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
index 02c24e8..35de5ea 100644
--- a/arm/Makefile.arm64
+++ b/arm/Makefile.arm64
@@ -7,6 +7,7 @@ bits = 64
 ldarch = elf64-littleaarch64
 
 arch_LDFLAGS = -pie -n
+CFLAGS += -mstrict-align
 
 define arch_elf_check =
 	$(if $(shell ! $(OBJDUMP) -R $(1) >&/dev/null && echo "nok"),
-- 
2.17.1

[PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Andre Przywara]

The ARM architecture requires all accesses to device memory to be
naturally aligned[1][2]. Normal memory does not have this strict
requirement, and in fact many systems do ignore unaligned accesses
(by the means of clearing the A bit in SCTLR and accessing normal
memory). So the default behaviour of GCC assumes that unaligned accesses
are fine, at least if happening on the stack.

Now kvm-unit-tests runs some C code with the MMU off, which degrades the
whole system memory to device memory. Now every unaligned access will
fault, regardless of the A bit.
In fact there is at least one place in lib/printf.c where GCC merges
two consecutive char* accesses into one "strh" instruction, writing to
a potentially unaligned address.
This can be reproduced by configuring kvm-unit-tests for kvmtool, but
running it on QEMU, which triggers an early printf that exercises this
particular code path.

Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
problem. Also add the respective -mno-unaligned-access flag for arm.

Thanks to Alexandru for helping debugging this.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>

[1] ARMv8 ARM DDI 0487E.a, B2.5.2
[2] ARMv7 ARM DDI 0406C.d, A3.2.1
---
 arm/Makefile.arm   | 1 +
 arm/Makefile.arm64 | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arm/Makefile.arm b/arm/Makefile.arm
index a625267..43b4be1 100644
--- a/arm/Makefile.arm
+++ b/arm/Makefile.arm
@@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
 
 CFLAGS += $(machine)
 CFLAGS += -mcpu=$(PROCESSOR)
+CFLAGS += -mno-unaligned-access
 
 arch_LDFLAGS = -Ttext=40010000
 
diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
index 02c24e8..35de5ea 100644
--- a/arm/Makefile.arm64
+++ b/arm/Makefile.arm64
@@ -7,6 +7,7 @@ bits = 64
 ldarch = elf64-littleaarch64
 
 arch_LDFLAGS = -pie -n
+CFLAGS += -mstrict-align
 
 define arch_elf_check =
 	$(if $(shell ! $(OBJDUMP) -R $(1) >&/dev/null && echo "nok"),
-- 
2.17.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Andrew Jones]

On Thu, Sep 05, 2019 at 06:15:02PM +0100, Andre Przywara wrote:
> The ARM architecture requires all accesses to device memory to be
> naturally aligned[1][2]. Normal memory does not have this strict
> requirement, and in fact many systems do ignore unaligned accesses
> (by the means of clearing the A bit in SCTLR and accessing normal
> memory). So the default behaviour of GCC assumes that unaligned accesses
> are fine, at least if happening on the stack.
> 
> Now kvm-unit-tests runs some C code with the MMU off, which degrades the
> whole system memory to device memory. Now every unaligned access will
> fault, regardless of the A bit.
> In fact there is at least one place in lib/printf.c where GCC merges
> two consecutive char* accesses into one "strh" instruction, writing to
> a potentially unaligned address.
> This can be reproduced by configuring kvm-unit-tests for kvmtool, but
> running it on QEMU, which triggers an early printf that exercises this
> particular code path.
> 
> Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
> problem. Also add the respective -mno-unaligned-access flag for arm.
> 
> Thanks to Alexandru for helping debugging this.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> [1] ARMv8 ARM DDI 0487E.a, B2.5.2
> [2] ARMv7 ARM DDI 0406C.d, A3.2.1
> ---
>  arm/Makefile.arm   | 1 +
>  arm/Makefile.arm64 | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index a625267..43b4be1 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
>  
>  CFLAGS += $(machine)
>  CFLAGS += -mcpu=$(PROCESSOR)
> +CFLAGS += -mno-unaligned-access
>  
>  arch_LDFLAGS = -Ttext=40010000
>  
> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> index 02c24e8..35de5ea 100644
> --- a/arm/Makefile.arm64
> +++ b/arm/Makefile.arm64
> @@ -7,6 +7,7 @@ bits = 64
>  ldarch = elf64-littleaarch64
>  
>  arch_LDFLAGS = -pie -n
> +CFLAGS += -mstrict-align
>  
>  define arch_elf_check =
>  	$(if $(shell ! $(OBJDUMP) -R $(1) >&/dev/null && echo "nok"),
> -- 
> 2.17.1
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Andrew Jones]

On Thu, Sep 05, 2019 at 06:15:02PM +0100, Andre Przywara wrote:
> The ARM architecture requires all accesses to device memory to be
> naturally aligned[1][2]. Normal memory does not have this strict
> requirement, and in fact many systems do ignore unaligned accesses
> (by the means of clearing the A bit in SCTLR and accessing normal
> memory). So the default behaviour of GCC assumes that unaligned accesses
> are fine, at least if happening on the stack.
> 
> Now kvm-unit-tests runs some C code with the MMU off, which degrades the
> whole system memory to device memory. Now every unaligned access will
> fault, regardless of the A bit.
> In fact there is at least one place in lib/printf.c where GCC merges
> two consecutive char* accesses into one "strh" instruction, writing to
> a potentially unaligned address.
> This can be reproduced by configuring kvm-unit-tests for kvmtool, but
> running it on QEMU, which triggers an early printf that exercises this
> particular code path.
> 
> Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
> problem. Also add the respective -mno-unaligned-access flag for arm.
> 
> Thanks to Alexandru for helping debugging this.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> [1] ARMv8 ARM DDI 0487E.a, B2.5.2
> [2] ARMv7 ARM DDI 0406C.d, A3.2.1
> ---
>  arm/Makefile.arm   | 1 +
>  arm/Makefile.arm64 | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index a625267..43b4be1 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
>  
>  CFLAGS += $(machine)
>  CFLAGS += -mcpu=$(PROCESSOR)
> +CFLAGS += -mno-unaligned-access
>  
>  arch_LDFLAGS = -Ttext=40010000
>  
> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> index 02c24e8..35de5ea 100644
> --- a/arm/Makefile.arm64
> +++ b/arm/Makefile.arm64
> @@ -7,6 +7,7 @@ bits = 64
>  ldarch = elf64-littleaarch64
>  
>  arch_LDFLAGS = -pie -n
> +CFLAGS += -mstrict-align
>  
>  define arch_elf_check =
>  	$(if $(shell ! $(OBJDUMP) -R $(1) >&/dev/null && echo "nok"),
> -- 
> 2.17.1
>

Reviewed-by: Andrew Jones <drjones@redhat.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Paolo Bonzini]

On 05/09/19 19:15, Andre Przywara wrote:
> The ARM architecture requires all accesses to device memory to be
> naturally aligned[1][2]. Normal memory does not have this strict
> requirement, and in fact many systems do ignore unaligned accesses
> (by the means of clearing the A bit in SCTLR and accessing normal
> memory). So the default behaviour of GCC assumes that unaligned accesses
> are fine, at least if happening on the stack.
> 
> Now kvm-unit-tests runs some C code with the MMU off, which degrades the
> whole system memory to device memory. Now every unaligned access will
> fault, regardless of the A bit.
> In fact there is at least one place in lib/printf.c where GCC merges
> two consecutive char* accesses into one "strh" instruction, writing to
> a potentially unaligned address.
> This can be reproduced by configuring kvm-unit-tests for kvmtool, but
> running it on QEMU, which triggers an early printf that exercises this
> particular code path.
> 
> Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
> problem. Also add the respective -mno-unaligned-access flag for arm.
> 
> Thanks to Alexandru for helping debugging this.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> [1] ARMv8 ARM DDI 0487E.a, B2.5.2
> [2] ARMv7 ARM DDI 0406C.d, A3.2.1
> ---
>  arm/Makefile.arm   | 1 +
>  arm/Makefile.arm64 | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index a625267..43b4be1 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
>  
>  CFLAGS += $(machine)
>  CFLAGS += -mcpu=$(PROCESSOR)
> +CFLAGS += -mno-unaligned-access
>  
>  arch_LDFLAGS = -Ttext=40010000
>  
> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> index 02c24e8..35de5ea 100644
> --- a/arm/Makefile.arm64
> +++ b/arm/Makefile.arm64
> @@ -7,6 +7,7 @@ bits = 64
>  ldarch = elf64-littleaarch64
>  
>  arch_LDFLAGS = -pie -n
> +CFLAGS += -mstrict-align
>  
>  define arch_elf_check =
>  	$(if $(shell ! $(OBJDUMP) -R $(1) >&/dev/null && echo "nok"),
> 

Queued, thanks.

Paolo

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Paolo Bonzini]

On 05/09/19 19:15, Andre Przywara wrote:
> The ARM architecture requires all accesses to device memory to be
> naturally aligned[1][2]. Normal memory does not have this strict
> requirement, and in fact many systems do ignore unaligned accesses
> (by the means of clearing the A bit in SCTLR and accessing normal
> memory). So the default behaviour of GCC assumes that unaligned accesses
> are fine, at least if happening on the stack.
> 
> Now kvm-unit-tests runs some C code with the MMU off, which degrades the
> whole system memory to device memory. Now every unaligned access will
> fault, regardless of the A bit.
> In fact there is at least one place in lib/printf.c where GCC merges
> two consecutive char* accesses into one "strh" instruction, writing to
> a potentially unaligned address.
> This can be reproduced by configuring kvm-unit-tests for kvmtool, but
> running it on QEMU, which triggers an early printf that exercises this
> particular code path.
> 
> Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
> problem. Also add the respective -mno-unaligned-access flag for arm.
> 
> Thanks to Alexandru for helping debugging this.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> [1] ARMv8 ARM DDI 0487E.a, B2.5.2
> [2] ARMv7 ARM DDI 0406C.d, A3.2.1
> ---
>  arm/Makefile.arm   | 1 +
>  arm/Makefile.arm64 | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index a625267..43b4be1 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
>  
>  CFLAGS += $(machine)
>  CFLAGS += -mcpu=$(PROCESSOR)
> +CFLAGS += -mno-unaligned-access
>  
>  arch_LDFLAGS = -Ttext=40010000
>  
> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> index 02c24e8..35de5ea 100644
> --- a/arm/Makefile.arm64
> +++ b/arm/Makefile.arm64
> @@ -7,6 +7,7 @@ bits = 64
>  ldarch = elf64-littleaarch64
>  
>  arch_LDFLAGS = -pie -n
> +CFLAGS += -mstrict-align
>  
>  define arch_elf_check =
>  	$(if $(shell ! $(OBJDUMP) -R $(1) >&/dev/null && echo "nok"),
> 

Queued, thanks.

Paolo
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Thomas Huth]

On 05/09/2019 19.15, Andre Przywara wrote:
> The ARM architecture requires all accesses to device memory to be
> naturally aligned[1][2]. Normal memory does not have this strict
> requirement, and in fact many systems do ignore unaligned accesses
> (by the means of clearing the A bit in SCTLR and accessing normal
> memory). So the default behaviour of GCC assumes that unaligned accesses
> are fine, at least if happening on the stack.
> 
> Now kvm-unit-tests runs some C code with the MMU off, which degrades the
> whole system memory to device memory. Now every unaligned access will
> fault, regardless of the A bit.
> In fact there is at least one place in lib/printf.c where GCC merges
> two consecutive char* accesses into one "strh" instruction, writing to
> a potentially unaligned address.
> This can be reproduced by configuring kvm-unit-tests for kvmtool, but
> running it on QEMU, which triggers an early printf that exercises this
> particular code path.
> 
> Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
> problem. Also add the respective -mno-unaligned-access flag for arm.
> 
> Thanks to Alexandru for helping debugging this.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> [1] ARMv8 ARM DDI 0487E.a, B2.5.2
> [2] ARMv7 ARM DDI 0406C.d, A3.2.1
> ---
>  arm/Makefile.arm   | 1 +
>  arm/Makefile.arm64 | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index a625267..43b4be1 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
>  
>  CFLAGS += $(machine)
>  CFLAGS += -mcpu=$(PROCESSOR)
> +CFLAGS += -mno-unaligned-access
>  
>  arch_LDFLAGS = -Ttext=40010000
>  
> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> index 02c24e8..35de5ea 100644
> --- a/arm/Makefile.arm64
> +++ b/arm/Makefile.arm64
> @@ -7,6 +7,7 @@ bits = 64
>  ldarch = elf64-littleaarch64
>  
>  arch_LDFLAGS = -pie -n
> +CFLAGS += -mstrict-align

Instead of adding it to both, Makefile.arm and Makefile.arm64, you could
also simply add it to Makefile.common instead.

 Thomas

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Thomas Huth]

On 05/09/2019 19.15, Andre Przywara wrote:
> The ARM architecture requires all accesses to device memory to be
> naturally aligned[1][2]. Normal memory does not have this strict
> requirement, and in fact many systems do ignore unaligned accesses
> (by the means of clearing the A bit in SCTLR and accessing normal
> memory). So the default behaviour of GCC assumes that unaligned accesses
> are fine, at least if happening on the stack.
> 
> Now kvm-unit-tests runs some C code with the MMU off, which degrades the
> whole system memory to device memory. Now every unaligned access will
> fault, regardless of the A bit.
> In fact there is at least one place in lib/printf.c where GCC merges
> two consecutive char* accesses into one "strh" instruction, writing to
> a potentially unaligned address.
> This can be reproduced by configuring kvm-unit-tests for kvmtool, but
> running it on QEMU, which triggers an early printf that exercises this
> particular code path.
> 
> Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
> problem. Also add the respective -mno-unaligned-access flag for arm.
> 
> Thanks to Alexandru for helping debugging this.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> [1] ARMv8 ARM DDI 0487E.a, B2.5.2
> [2] ARMv7 ARM DDI 0406C.d, A3.2.1
> ---
>  arm/Makefile.arm   | 1 +
>  arm/Makefile.arm64 | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index a625267..43b4be1 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
>  
>  CFLAGS += $(machine)
>  CFLAGS += -mcpu=$(PROCESSOR)
> +CFLAGS += -mno-unaligned-access
>  
>  arch_LDFLAGS = -Ttext=40010000
>  
> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> index 02c24e8..35de5ea 100644
> --- a/arm/Makefile.arm64
> +++ b/arm/Makefile.arm64
> @@ -7,6 +7,7 @@ bits = 64
>  ldarch = elf64-littleaarch64
>  
>  arch_LDFLAGS = -pie -n
> +CFLAGS += -mstrict-align

Instead of adding it to both, Makefile.arm and Makefile.arm64, you could
also simply add it to Makefile.common instead.

 Thomas
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Andre Przywara]

On Tue, 10 Sep 2019 20:15:19 +0200
Thomas Huth <thuth@redhat.com> wrote:

Hi,

> On 05/09/2019 19.15, Andre Przywara wrote:
> > The ARM architecture requires all accesses to device memory to be
> > naturally aligned[1][2]. Normal memory does not have this strict
> > requirement, and in fact many systems do ignore unaligned accesses
> > (by the means of clearing the A bit in SCTLR and accessing normal
> > memory). So the default behaviour of GCC assumes that unaligned accesses
> > are fine, at least if happening on the stack.
> > 
> > Now kvm-unit-tests runs some C code with the MMU off, which degrades the
> > whole system memory to device memory. Now every unaligned access will
> > fault, regardless of the A bit.
> > In fact there is at least one place in lib/printf.c where GCC merges
> > two consecutive char* accesses into one "strh" instruction, writing to
> > a potentially unaligned address.
> > This can be reproduced by configuring kvm-unit-tests for kvmtool, but
> > running it on QEMU, which triggers an early printf that exercises this
> > particular code path.
> > 
> > Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
> > problem. Also add the respective -mno-unaligned-access flag for arm.
> > 
> > Thanks to Alexandru for helping debugging this.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > 
> > [1] ARMv8 ARM DDI 0487E.a, B2.5.2
> > [2] ARMv7 ARM DDI 0406C.d, A3.2.1
> > ---
> >  arm/Makefile.arm   | 1 +
> >  arm/Makefile.arm64 | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> > index a625267..43b4be1 100644
> > --- a/arm/Makefile.arm
> > +++ b/arm/Makefile.arm
> > @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
> >  
> >  CFLAGS += $(machine)
> >  CFLAGS += -mcpu=$(PROCESSOR)
> > +CFLAGS += -mno-unaligned-access
> >  
> >  arch_LDFLAGS = -Ttext=40010000
> >  
> > diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> > index 02c24e8..35de5ea 100644
> > --- a/arm/Makefile.arm64
> > +++ b/arm/Makefile.arm64
> > @@ -7,6 +7,7 @@ bits = 64
> >  ldarch = elf64-littleaarch64
> >  
> >  arch_LDFLAGS = -pie -n
> > +CFLAGS += -mstrict-align  
> 
> Instead of adding it to both, Makefile.arm and Makefile.arm64, you could
> also simply add it to Makefile.common instead.

But the arguments are not the same (admittedly against intuition)?
I thought about defining arch_CFLAGS in both files, then adding that to Makefile.common, but didn't see the advantage over this straightforward approach here.

Cheers,
Andre.

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Andre Przywara]

On Tue, 10 Sep 2019 20:15:19 +0200
Thomas Huth <thuth@redhat.com> wrote:

Hi,

> On 05/09/2019 19.15, Andre Przywara wrote:
> > The ARM architecture requires all accesses to device memory to be
> > naturally aligned[1][2]. Normal memory does not have this strict
> > requirement, and in fact many systems do ignore unaligned accesses
> > (by the means of clearing the A bit in SCTLR and accessing normal
> > memory). So the default behaviour of GCC assumes that unaligned accesses
> > are fine, at least if happening on the stack.
> > 
> > Now kvm-unit-tests runs some C code with the MMU off, which degrades the
> > whole system memory to device memory. Now every unaligned access will
> > fault, regardless of the A bit.
> > In fact there is at least one place in lib/printf.c where GCC merges
> > two consecutive char* accesses into one "strh" instruction, writing to
> > a potentially unaligned address.
> > This can be reproduced by configuring kvm-unit-tests for kvmtool, but
> > running it on QEMU, which triggers an early printf that exercises this
> > particular code path.
> > 
> > Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
> > problem. Also add the respective -mno-unaligned-access flag for arm.
> > 
> > Thanks to Alexandru for helping debugging this.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > 
> > [1] ARMv8 ARM DDI 0487E.a, B2.5.2
> > [2] ARMv7 ARM DDI 0406C.d, A3.2.1
> > ---
> >  arm/Makefile.arm   | 1 +
> >  arm/Makefile.arm64 | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> > index a625267..43b4be1 100644
> > --- a/arm/Makefile.arm
> > +++ b/arm/Makefile.arm
> > @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
> >  
> >  CFLAGS += $(machine)
> >  CFLAGS += -mcpu=$(PROCESSOR)
> > +CFLAGS += -mno-unaligned-access
> >  
> >  arch_LDFLAGS = -Ttext=40010000
> >  
> > diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> > index 02c24e8..35de5ea 100644
> > --- a/arm/Makefile.arm64
> > +++ b/arm/Makefile.arm64
> > @@ -7,6 +7,7 @@ bits = 64
> >  ldarch = elf64-littleaarch64
> >  
> >  arch_LDFLAGS = -pie -n
> > +CFLAGS += -mstrict-align  
> 
> Instead of adding it to both, Makefile.arm and Makefile.arm64, you could
> also simply add it to Makefile.common instead.

But the arguments are not the same (admittedly against intuition)?
I thought about defining arch_CFLAGS in both files, then adding that to Makefile.common, but didn't see the advantage over this straightforward approach here.

Cheers,
Andre.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Thomas Huth]

On 11/09/2019 10.16, Andre Przywara wrote:
> On Tue, 10 Sep 2019 20:15:19 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
> Hi,
> 
>> On 05/09/2019 19.15, Andre Przywara wrote:
>>> The ARM architecture requires all accesses to device memory to be
>>> naturally aligned[1][2]. Normal memory does not have this strict
>>> requirement, and in fact many systems do ignore unaligned accesses
>>> (by the means of clearing the A bit in SCTLR and accessing normal
>>> memory). So the default behaviour of GCC assumes that unaligned accesses
>>> are fine, at least if happening on the stack.
>>>
>>> Now kvm-unit-tests runs some C code with the MMU off, which degrades the
>>> whole system memory to device memory. Now every unaligned access will
>>> fault, regardless of the A bit.
>>> In fact there is at least one place in lib/printf.c where GCC merges
>>> two consecutive char* accesses into one "strh" instruction, writing to
>>> a potentially unaligned address.
>>> This can be reproduced by configuring kvm-unit-tests for kvmtool, but
>>> running it on QEMU, which triggers an early printf that exercises this
>>> particular code path.
>>>
>>> Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
>>> problem. Also add the respective -mno-unaligned-access flag for arm.
>>>
>>> Thanks to Alexandru for helping debugging this.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>
>>> [1] ARMv8 ARM DDI 0487E.a, B2.5.2
>>> [2] ARMv7 ARM DDI 0406C.d, A3.2.1
>>> ---
>>>  arm/Makefile.arm   | 1 +
>>>  arm/Makefile.arm64 | 1 +
>>>  2 files changed, 2 insertions(+)
>>>
>>> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
>>> index a625267..43b4be1 100644
>>> --- a/arm/Makefile.arm
>>> +++ b/arm/Makefile.arm
>>> @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
>>>  
>>>  CFLAGS += $(machine)
>>>  CFLAGS += -mcpu=$(PROCESSOR)
>>> +CFLAGS += -mno-unaligned-access
>>>  
>>>  arch_LDFLAGS = -Ttext=40010000
>>>  
>>> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
>>> index 02c24e8..35de5ea 100644
>>> --- a/arm/Makefile.arm64
>>> +++ b/arm/Makefile.arm64
>>> @@ -7,6 +7,7 @@ bits = 64
>>>  ldarch = elf64-littleaarch64
>>>  
>>>  arch_LDFLAGS = -pie -n
>>> +CFLAGS += -mstrict-align  
>>
>> Instead of adding it to both, Makefile.arm and Makefile.arm64, you could
>> also simply add it to Makefile.common instead.
> 
> But the arguments are not the same (admittedly against intuition)?
> I thought about defining arch_CFLAGS in both files, then adding that to Makefile.common, but didn't see the advantage over this straightforward approach here.

D'oh, never mind, I didn't read the patch properly. I somehow thought
that the arguments are the same. It's quite weird that the compiler
developers chose different names here...

 Thomas

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Thomas Huth]

On 11/09/2019 10.16, Andre Przywara wrote:
> On Tue, 10 Sep 2019 20:15:19 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
> Hi,
> 
>> On 05/09/2019 19.15, Andre Przywara wrote:
>>> The ARM architecture requires all accesses to device memory to be
>>> naturally aligned[1][2]. Normal memory does not have this strict
>>> requirement, and in fact many systems do ignore unaligned accesses
>>> (by the means of clearing the A bit in SCTLR and accessing normal
>>> memory). So the default behaviour of GCC assumes that unaligned accesses
>>> are fine, at least if happening on the stack.
>>>
>>> Now kvm-unit-tests runs some C code with the MMU off, which degrades the
>>> whole system memory to device memory. Now every unaligned access will
>>> fault, regardless of the A bit.
>>> In fact there is at least one place in lib/printf.c where GCC merges
>>> two consecutive char* accesses into one "strh" instruction, writing to
>>> a potentially unaligned address.
>>> This can be reproduced by configuring kvm-unit-tests for kvmtool, but
>>> running it on QEMU, which triggers an early printf that exercises this
>>> particular code path.
>>>
>>> Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
>>> problem. Also add the respective -mno-unaligned-access flag for arm.
>>>
>>> Thanks to Alexandru for helping debugging this.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>
>>> [1] ARMv8 ARM DDI 0487E.a, B2.5.2
>>> [2] ARMv7 ARM DDI 0406C.d, A3.2.1
>>> ---
>>>  arm/Makefile.arm   | 1 +
>>>  arm/Makefile.arm64 | 1 +
>>>  2 files changed, 2 insertions(+)
>>>
>>> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
>>> index a625267..43b4be1 100644
>>> --- a/arm/Makefile.arm
>>> +++ b/arm/Makefile.arm
>>> @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
>>>  
>>>  CFLAGS += $(machine)
>>>  CFLAGS += -mcpu=$(PROCESSOR)
>>> +CFLAGS += -mno-unaligned-access
>>>  
>>>  arch_LDFLAGS = -Ttext=40010000
>>>  
>>> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
>>> index 02c24e8..35de5ea 100644
>>> --- a/arm/Makefile.arm64
>>> +++ b/arm/Makefile.arm64
>>> @@ -7,6 +7,7 @@ bits = 64
>>>  ldarch = elf64-littleaarch64
>>>  
>>>  arch_LDFLAGS = -pie -n
>>> +CFLAGS += -mstrict-align  
>>
>> Instead of adding it to both, Makefile.arm and Makefile.arm64, you could
>> also simply add it to Makefile.common instead.
> 
> But the arguments are not the same (admittedly against intuition)?
> I thought about defining arch_CFLAGS in both files, then adding that to Makefile.common, but didn't see the advantage over this straightforward approach here.

D'oh, never mind, I didn't read the patch properly. I somehow thought
that the arguments are the same. It's quite weird that the compiler
developers chose different names here...

 Thomas
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Thomas Huth]

On 05/09/2019 19.15, Andre Przywara wrote:
> The ARM architecture requires all accesses to device memory to be
> naturally aligned[1][2]. Normal memory does not have this strict
> requirement, and in fact many systems do ignore unaligned accesses
> (by the means of clearing the A bit in SCTLR and accessing normal
> memory). So the default behaviour of GCC assumes that unaligned accesses
> are fine, at least if happening on the stack.
> 
> Now kvm-unit-tests runs some C code with the MMU off, which degrades the
> whole system memory to device memory. Now every unaligned access will
> fault, regardless of the A bit.
> In fact there is at least one place in lib/printf.c where GCC merges
> two consecutive char* accesses into one "strh" instruction, writing to
> a potentially unaligned address.
> This can be reproduced by configuring kvm-unit-tests for kvmtool, but
> running it on QEMU, which triggers an early printf that exercises this
> particular code path.
> 
> Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
> problem. Also add the respective -mno-unaligned-access flag for arm.
> 
> Thanks to Alexandru for helping debugging this.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> [1] ARMv8 ARM DDI 0487E.a, B2.5.2
> [2] ARMv7 ARM DDI 0406C.d, A3.2.1
> ---
>  arm/Makefile.arm   | 1 +
>  arm/Makefile.arm64 | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index a625267..43b4be1 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
>  
>  CFLAGS += $(machine)
>  CFLAGS += -mcpu=$(PROCESSOR)
> +CFLAGS += -mno-unaligned-access
>  
>  arch_LDFLAGS = -Ttext=40010000
>  
> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> index 02c24e8..35de5ea 100644
> --- a/arm/Makefile.arm64
> +++ b/arm/Makefile.arm64
> @@ -7,6 +7,7 @@ bits = 64
>  ldarch = elf64-littleaarch64
>  
>  arch_LDFLAGS = -pie -n
> +CFLAGS += -mstrict-align
>  
>  define arch_elf_check =
>  	$(if $(shell ! $(OBJDUMP) -R $(1) >&/dev/null && echo "nok"),
> 

FWIW (after finally reading the patch properly ;-)) :
Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses

[Thomas Huth]

On 05/09/2019 19.15, Andre Przywara wrote:
> The ARM architecture requires all accesses to device memory to be
> naturally aligned[1][2]. Normal memory does not have this strict
> requirement, and in fact many systems do ignore unaligned accesses
> (by the means of clearing the A bit in SCTLR and accessing normal
> memory). So the default behaviour of GCC assumes that unaligned accesses
> are fine, at least if happening on the stack.
> 
> Now kvm-unit-tests runs some C code with the MMU off, which degrades the
> whole system memory to device memory. Now every unaligned access will
> fault, regardless of the A bit.
> In fact there is at least one place in lib/printf.c where GCC merges
> two consecutive char* accesses into one "strh" instruction, writing to
> a potentially unaligned address.
> This can be reproduced by configuring kvm-unit-tests for kvmtool, but
> running it on QEMU, which triggers an early printf that exercises this
> particular code path.
> 
> Add the -mstrict-align compiler option to the arm64 CFLAGS to fix this
> problem. Also add the respective -mno-unaligned-access flag for arm.
> 
> Thanks to Alexandru for helping debugging this.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> [1] ARMv8 ARM DDI 0487E.a, B2.5.2
> [2] ARMv7 ARM DDI 0406C.d, A3.2.1
> ---
>  arm/Makefile.arm   | 1 +
>  arm/Makefile.arm64 | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index a625267..43b4be1 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -12,6 +12,7 @@ KEEP_FRAME_POINTER := y
>  
>  CFLAGS += $(machine)
>  CFLAGS += -mcpu=$(PROCESSOR)
> +CFLAGS += -mno-unaligned-access
>  
>  arch_LDFLAGS = -Ttext=40010000
>  
> diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
> index 02c24e8..35de5ea 100644
> --- a/arm/Makefile.arm64
> +++ b/arm/Makefile.arm64
> @@ -7,6 +7,7 @@ bits = 64
>  ldarch = elf64-littleaarch64
>  
>  arch_LDFLAGS = -pie -n
> +CFLAGS += -mstrict-align
>  
>  define arch_elf_check =
>  	$(if $(shell ! $(OBJDUMP) -R $(1) >&/dev/null && echo "nok"),
> 

FWIW (after finally reading the patch properly ;-)) :
Reviewed-by: Thomas Huth <thuth@redhat.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm