From: "Ard Biesheuvel" <ardb@kernel.org>
To: devel@edk2.groups.io, jonathan.cameron@huawei.com
Cc: "Gerd Hoffmann" <kraxel@redhat.com>,
"Jonathan Cameron via" <qemu-devel@nongnu.org>,
linuxarm@huawei.com,
"Richard Henderson" <richard.henderson@linaro.org>,
qemu-arm@nongnu.org, "Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Idan Horowitz" <idan.horowitz@gmail.com>
Subject: Re: [edk2-devel] [PATCH v3 5/6] target/arm: Do memory type alignment check when translation disabled
Date: Fri, 19 Apr 2024 19:38:26 +0200 [thread overview]
Message-ID: <CAMj1kXFMT=rM=2ivrcUXEtZYEqSbTG6ZokJgDcpd4ufwYm34Xw@mail.gmail.com> (raw)
In-Reply-To: <CAMj1kXE+0mUfFq_FrhZT0m_YOJkWiuPndWt3GsRn1eMyCVrmMw@mail.gmail.com>
On Fri, 19 Apr 2024 at 18:36, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 19 Apr 2024 at 18:09, Jonathan Cameron via groups.io
> <jonathan.cameron=huawei.com@groups.io> wrote:
> >
> > On Fri, 19 Apr 2024 13:52:07 +0200
> > Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > > Hi,
> > >
> > > > Gerd, any ideas? Maybe I needs something subtly different in my
> > > > edk2 build? I've not looked at this bit of the qemu infrastructure
> > > > before - is there a document on how that image is built?
> > >
> > > There is roms/Makefile for that.
> > >
> > > make -C roms help
> > > make -C roms efi
> > >
> > > So easiest would be to just update the edk2 submodule to what you
> > > need, then rebuild.
> > >
> > > The build is handled by the roms/edk2-build.py script,
> > > with the build configuration being in roms/edk2-build.config.
> > > That is usable outside the qemu source tree too, i.e. like this:
> > >
> > > python3 /path/to/qemu.git/roms/edk2-build.py \
> > > --config /path/to/qemu.git/roms/edk2-build.config \
> > > --core /path/to/edk2.git \
> > > --match armvirt \
> > > --silent --no-logs
> > >
> > > That'll try to place the images build in "../pc-bios", so maybe better
> > > work with a copy of the config file where you adjust this.
> > >
> > > HTH,
> > > Gerd
> > >
> >
> > Thanks Gerd!
> >
> > So the builds are very similar via the two method...
> > However - the QEMU build sets -D CAVIUM_ERRATUM_27456=TRUE
> >
> > And that's the difference - with that set for my other builds the alignment
> > problems go away...
> >
> > Any idea why we have that set in roms/edk2-build.config?
> > Superficially it seems rather unlikely anyone cares about thunderx1
> > (if they do we need to get them some new hardware with fresh bugs)
> > bugs now and this config file was only added last year.
> >
> >
> > However, the last comment in Ard's commit message below seems
> > highly likely to be relevant!
> >
> > Chasing through Ard's patch it has the side effect of dropping
> > an override of a requirement for strict alignment.
> > So with out the errata
> > DEFINE GCC_AARCH64_CC_XIPFLAGS = -mstrict-align -mgeneral-regs-only
> > is replaced with
> > [BuildOptions]
> > +!if $(CAVIUM_ERRATUM_27456) == TRUE^M
> > + GCC:*_*_AARCH64_PP_FLAGS = -DCAVIUM_ERRATUM_27456^M
> > +!else^M
> > GCC:*_*_AARCH64_CC_XIPFLAGS ==
> > +!endif^M
> >
> > The edk2 commit that added this was the following +CC Ard.
> >
> > Given I wasn't sure of the syntax of that file I set it
> > manually to the original value and indeed it works.
> >
> >
> > commit ec54ce1f1ab41b92782b37ae59e752fff0ef9c41
> > Author: Ard Biesheuvel <ardb@kernel.org>
> > Date: Wed Jan 4 16:51:35 2023 +0100
> >
> > ArmVirtPkg/ArmVirtQemu: Avoid early ID map on ThunderX
> >
> > The early ID map used by ArmVirtQemu uses ASID scoped non-global
> > mappings, as this allows us to switch to the permanent ID map seamlessly
> > without the need for explicit TLB maintenance.
> >
> > However, this triggers a known erratum on ThunderX, which does not
> > tolerate non-global mappings that are executable at EL1, as this appears
> > to result in I-cache corruption. (Linux disables the KPTI based Meltdown
> > mitigation on ThunderX for the same reason)
> >
> > So work around this, by detecting the CPU implementor and part number,
> > and proceeding without the early ID map if a ThunderX CPU is detected.
> >
> > Note that this requires the C code to be built with strict alignment
> > again, as we may end up executing it with the MMU and caches off.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > Acked-by: Laszlo Ersek <lersek@redhat.com>
> > Tested-by: dann frazier <dann.frazier@canonical.com>
> >
> > Test case is
> > qemu-system-aarch64 -M virt,virtualization=true, -m 4g -cpu cortex-a76 \
> > -bios QEMU_EFI.fd -d int
> >
> > Which gets alignment faults since:
> > https://lore.kernel.org/all/20240301204110.656742-6-richard.henderson@linaro.org/
> >
> > So my feeling here is EDK2 should either have yet another config for QEMU as a host
> > or should always set the alignment without needing to pick the CAVIUM 27456 errata
> > which I suspect will get dropped soonish anyway if anyone ever cleans up
> > old errata.
> >
>
> This code was never really intended for execution at EL2, but it
> happened to work, partially because TCG's lack of strict alignment
> checking when the MMU is off.
>
> Those assumptions no longer hold, so yes, let's get this fixed properly.
>
> Given VHE and nested virt (which will likely imply VHE in practice), I
> would like to extend this functionality (i.e., the use of preliminary
> page tables in NOR flash) to EL2 as well, but with VHE enabled. This
> means we can still elide TLB maintenance (and BBM checks) by using
> different ASIDs, and otherwise, fall back to entering with the MMU off
> if VHE is not available. In that case, we should enforce strict
> alignment too, so that needs to be fixed regardless.
>
> I'll try to code something up and send it round. In the mean time,
> feel free to propose a minimal patch that reinstates the strict
> alignment if you are pressed for time, and I'll merge it right away.
Actually, let's just so that first - I'll send out a patch.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118036): https://edk2.groups.io/g/devel/message/118036
Mute This Topic: https://groups.io/mt/105602816/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
prev parent reply other threads:[~2024-04-19 17:38 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240301204110.656742-1-richard.henderson@linaro.org>
[not found] ` <20240301204110.656742-6-richard.henderson@linaro.org>
[not found] ` <20240416161111.0000607c@huawei.com>
[not found] ` <0c878d25-3fbb-4f0b-bc9e-ca638f8c4f1e@linaro.org>
[not found] ` <20240418091555.00006666@Huawei.com>
2024-04-18 17:40 ` [edk2-devel] [PATCH v3 5/6] target/arm: Do memory type alignment check when translation disabled Jonathan Cameron via groups.io
2024-04-19 11:52 ` Gerd Hoffmann
2024-04-19 16:09 ` Jonathan Cameron via groups.io
2024-04-19 16:36 ` Ard Biesheuvel
2024-04-19 17:38 ` Ard Biesheuvel [this message]
Reply instructions:
You may reply publicly 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-list 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='CAMj1kXFMT=rM=2ivrcUXEtZYEqSbTG6ZokJgDcpd4ufwYm34Xw@mail.gmail.com' \
--to=devel@edk2.groups.io \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox