| Message ID | 20190905171502.215183-1-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] arm: prevent compiler from using unaligned accesses | expand |
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>
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
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
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.
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
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>
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"),
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(+)