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

["Ard Biesheuvel" <ardb@kernel.org>]

On Wed, 4 Jan 2023 at 16:37, Rebecca Cran <rebecca@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)

-- 
Ard.