[edk2-devel] [PATCH v4 0/3] ArmPkg, MdeModulePkg: Implement EFI_MP_SERVICES_PROTOCOL for AArch64 and add an MpServicesTest application to exercise it

[Ard Biesheuvel]

On Thu, 5 Jan 2023 at 18:39, Ard Biesheuvel <ardb at kernel.org> wrote:
>
> On Wed, 4 Jan 2023 at 16:37, Rebecca Cran <rebecca at quicinc.com> wrote:
> >
> > Implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls for AArch64, and
> > add an MpServicesTest application to exercise it.
> >
> > Changes from v3:
> >
> > Removed Ard's 'Reviewed-by' line from the commits since the code has changed
> > sufficiently that it's no longer valid.
> >
> > Rebecca Cran (3):
> >   ArmPkg: Add GET_MPIDR_AFFINITY_BITS and MPIDR_MT_BIT to ArmLib.h
> >   ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls
> >   MdeModulePkg: Add new Application/MpServicesTest application
> >
> >  ArmPkg/ArmPkg.dsc                                            |    1 +
> >  MdeModulePkg/MdeModulePkg.dsc                                |    2 +
> >  ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf |   56 +
> >  MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf   |   40 +
> >  ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h     |  344 ++++
> >  ArmPkg/Include/Library/ArmLib.h                              |   16 +-
> >  MdeModulePkg/Application/MpServicesTest/Options.h            |   39 +
> >  ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c   | 1847 ++++++++++++++++++++
> >  MdeModulePkg/Application/MpServicesTest/MpServicesTest.c     |  560 ++++++
> >  MdeModulePkg/Application/MpServicesTest/Options.c            |  164 ++
> >  ArmPkg/Drivers/ArmPsciMpServicesDxe/MpFuncs.S                |   57 +
> >  11 files changed, 3119 insertions(+), 7 deletions(-)
>
> Hello Rebecca,
>
> Thanks for reposting this.
>
> Running the secondaries with MMU and caches on is a huge improvement.
> I would suggest, though, that we enable the MMU first thing out of
> reset, and do so from asm code so we don't have to reason about the
> stack (pushing something with the MMU off and popping it with the MMU
> on requires cache maintenance as well, and there is no way this can be
> done from the code itself without help from the compiler)
>
> So I propose adding the diff below - note that only the variables
> holding TCR, MAIR and TTBR0 need cache maintenance now (in addition to
> the executable image) - everything else will be accessed by the
> secondaries with the MMU enabled.
>
> https://paste.debian.net/1266242/
>
> WIth a tweak to the RPI4 platform that I sent out separately, this all
> works fine for me (both with and without the diff above applied)
>

Actually, maybe better to retain the hunk below. I saw some weird
hangs without them

diff --git a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
index ab439bffd722..eb634a25b33a 100644
--- a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
+++ b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
@@ -1345,18 +1345,6 @@ MpServicesInitialize (

   gProcessorIDs[Index] = MAX_UINT64;

-  //
-  // The global pointer variables as well as the gProcessorIDs array contents
-  // are accessed by the other cores so we must clean them to the PoC
-  //
-  WriteBackDataCacheRange (&gProcessorIDs, sizeof (UINT64 *));
-  WriteBackDataCacheRange (&gApStacksBase, sizeof (UINT64 *));
-
-  WriteBackDataCacheRange (
-    gProcessorIDs,
-    (mCpuMpData.NumberOfProcessors + 1) * sizeof (UINT64)
-    );
-
   mNonBlockingModeAllowed = TRUE;

   Status = EfiCreateEventReadyToBootEx (


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98031): https://edk2.groups.io/g/devel/message/98031
Mute This Topic: https://groups.io/mt/96076871/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-