On Thu, 7 Oct 2021 at 13:02, Leif Lindholm <l...@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 <l...@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. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#81711): https://edk2.groups.io/g/devel/message/81711 Mute This Topic: https://groups.io/mt/85854039/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
