Re: [PATCH v3 0/4] ArmPkg,ArmVirtPkg: Add support EFI_MP_SERVICES_PROTOCOL on AARCH64

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

On Thu, 16 Dec 2021 at 04:46, Rebecca Cran <rebecca@nuviainc.com> wrote:
>
> Patch 1/4 is the start of addressing the issue that the Aff0 field of
> the MPIDR is no longer guaranteed to be the core, and should be referred
> to in a more generic way: for example it could be the thread, with Aff1
> being the core and Aff2 the cluster. Clearly much more work is needed
> to fully remove that assumption.
>
> Patch 2/4 implements the EFI_MP_SERVICES_PROTOCOL for DXE in Library/MpInitLib.
> Patch 4/4 updates CpuDxe to initialize MP support: as a result, it gains
> a dependency on MpInitLib.
> Patch 3/4 updates ArmVirt.dsc.inc to add the new MpInitLib library.
>
> Cores are powered on and turned off using PSCI.
>
> Note that minimal CPU setup is done: for example the MMU and caches are
> left disabled.
>
> --------
>
> Changes from v2:
>
> o Split patches up.
> o Wrapped overly long lines.
> o Fixed code formatting issues (Uncrustify).
> o Fixed other formatting issues (ECC tool).
>
> --------
>
> Testing:
>
> o Ran CI: https://github.com/tianocore/edk2/pull/2312
> o Built packages in ArmVirtPkg at each revision for both ARM and AARCH64.
>
> Rebecca Cran (4):
>   ArmPkg: Replace CoreId and ClusterId with Mpidr in ARM_CORE_INFO
>     struct
>   ArmPkg: Add Library/MpInitLib to support EFI_MP_SERVICES_PROTOCOL
>   ArmVirtPkg: Add MpInitLib, which is dependency for CpuDxe consumers
>   ArmPkg: Update Drivers/CpuDxe to initialize MpInitLib
>

OK, this looks good to me in principe, and I am quite happy with the
implementation of the MpInitLib and the new ability to dispatch
execution to other cores.

However, looking at ArmPkg/Include/Guid/ArmMpCoreInfo.h, I was
seriously shocked at how much cruft has accumulated there:
- headers under Guid/ shouldn't carry that many vaguely related
definitions anyway
- the mailbox stuff is obsolete
- the pseudo-ACPI table (ACPI style header but published as a EFI
config table) is highly dubious as well, and should really just be
removed.

I don't want to derail this series further, so I''ll have a stab at
cleaning this up on top, unless there are any reasons for keeping this
stuff?