KVM ARM Archive on lore.kernel.org
 help / color / Atom feed
From: Thomas Huth <thuth@redhat.com>
To: Andre Przywara <andre.przywara@arm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org
Subject: Re: [PATCH kvm-unit-tests] arm: prevent compiler from using unaligned accesses
Date: Wed, 11 Sep 2019 10:19:05 +0200
Message-ID: <c2904d78-45aa-46ef-8cfd-5e544a94e446@redhat.com> (raw)
In-Reply-To: <20190911091604.380c6df9@donnerap.cambridge.arm.com>

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

  reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-05 17:15 Andre Przywara
2019-09-06  6:30 ` Andrew Jones
2019-09-10 17:07 ` Paolo Bonzini
2019-09-10 18:15 ` Thomas Huth
2019-09-11  8:16   ` Andre Przywara
2019-09-11  8:19     ` Thomas Huth [this message]
2019-09-11  8:24 ` Thomas Huth

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c2904d78-45aa-46ef-8cfd-5e544a94e446@redhat.com \
    --to=thuth@redhat.com \
    --cc=andre.przywara@arm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

KVM ARM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvmarm/0 kvmarm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvmarm kvmarm/ https://lore.kernel.org/kvmarm \
		kvmarm@lists.cs.columbia.edu kvmarm@archiver.kernel.org
	public-inbox-index kvmarm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/edu.columbia.cs.lists.kvmarm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox