Re: [RFC] [PATCH 0/2] Proposal to add EFI_MP_SERVICES_PROTOCOL support for AARCH64


Ard Biesheuvel <ardb@...>
 

On Thu, 7 Oct 2021 at 13:02, Leif Lindholm <leif@nuviainc.com> wrote:

On Thu, Oct 07, 2021 at 12:03:30 +0200, Ard Biesheuvel wrote:
On Thu, 7 Oct 2021 at 11:41, Leif Lindholm <leif@nuviainc.com> wrote:

Ard/Sami - any comments?
The code changes by itself look fine to me. The only problem I see is
that we cannot run arbitrary code on other cores with the MMU off,
given that in that case,
This is a good point, and something we at the very least should point
out more explicitly - and perhaps ought to provide a helper library to
do - ...

they don't comply with the UEFI AArch64
platform requirements, most notably the one that says that unaligned
accesses must be permitted.
... but I'll note that EFI_MP_SERVICES_PROTOCOL is one of those
incorrectly named protocols defined in PI, not UEFI.

So either we severely constrain the kind of code that we permit to run
on other cores, or we enable the MMU and caches on each core as it
comes out of reset, as well as do any other CPU specific
initialization that we do for the primary core as well.
The description for StartupAllAPs() has a note:
It is the responsibility of the consumer of the
EFI_MP_SERVICES_PROTOCOL.StartupAllAPs() to make sure that the nature
of the code that is executed on the BSP and the dispatched APs is well
controlled. The MP Services Protocol does not guarantee that the
Procedure function is MP-safe. Hence, the tasks that can be run in
parallel are limited to certain independent tasks and well-controlled
exclusive code. EFI services and protocols may not be called by APs
unless otherwise specified.

So I think this is actually fine, implementation-wise. *Except* for
the SwitchBSP function (where we're currently bailing out anyway).
Ok, so that doesn't look as bad as I thought. But we'll have to be
more strict than other arches: even EFI services and protocols that
are marked as safe for execution under this MP protocol are likely to
explode if they rely on CopyMem() or SetMem() for in/outputs that are
not a multiple of 8 bytes in case the platform uses the
BaseMemoryLibOptDxe flavour of this library, since it relies heavily
on deliberately misaligned loads and stores.

Join rfc@edk2.groups.io to automatically receive all group messages.