public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif@nuviainc.com>
To: Rebecca Cran <rebecca@nuviainc.com>
Cc: devel@edk2.groups.io, Sami Mujawar <sami.mujawar@arm.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Gerd Hoffmann <kraxel@redhat.com>,
	rfc@edk2.groups.io
Subject: Re: [RFC] [PATCH 0/2] Proposal to add EFI_MP_SERVICES_PROTOCOL support for AARCH64
Date: Tue, 28 Sep 2021 12:14:35 +0100	[thread overview]
Message-ID: <20210928111435.poztq4cksagsogbw@leviathan> (raw)
In-Reply-To: <20210925021752.20678-1-rebecca@nuviainc.com>

On Fri, Sep 24, 2021 at 20:17:50 -0600, Rebecca Cran wrote:
> I'd like to propose adding EFI_MP_SERVICES_PROTOCOL support for
> AARCH64 systems. I've attached two patches to implement support for it
> in the DXE phase, based on code in EmulatorPkg and UefiCpuPkg. It's added
> under ArmPkg for now, but longer term it should probably be moved into
> UefiCpuPkg.
>
> Patch 1/2 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.

Just to add to this:
Aff0 was never defined by the architecture to be the "core", it was
just the smallest schedulable entity. The intent being that whether
you had multiple hardware threads per core or not, you could just use
the affinity to determine whether
There is also a bit in the MPIDR to indicate whether the core *had*
multiple hardware threads.

In recent processors (without any change to the architecture), ARM
thought it would be beneficial to keep software developers on their
toes by starting to use the hyperthreading layout even for processors
without hyperthreading support. I.e. Aff0 is always 0 even though MT
is 0:
https://developer.arm.com/documentation/100798/0301/Register-descriptions/AArch64-system-registers/MPIDR-EL1--Multiprocessor-Affinity-Register--EL1
The justification being that an SoC might contain both processors
with and without multiple hardware threads per core.

Anyway, the point is that from at least Cortex-A76 onwards, Aff0 no
longer maps to CoreId universally, and Aff1 no longer maps to
ClusterId, for all non-threaded implementations.
So we need to start cleaning up this use.
This will possibly break some out-of-tree platforms, but I figure
we're far enough from next stable tag for that not to matter too
much.

> Patch 2/2 implements the EFI_MP_SERVICES_PROTOCOL for DXE in Library/MpInitLib.

Worth calling out in the cover letter that this is backed by PSCI.

/
    Leif

> CpuDxe initializes the MP support, and as a result gains a dependency on
> MpInitLib. ArmVirt.dsc.inc needs updated to add the new library, as will
> all AARCH64 platforms.
> 
> I sent out a patch for MdeModulePkg last week to add a corresponding test
> application, MpServicesTest (see https://edk2.groups.io/g/devel/message/80878).
> 
> Rebecca Cran (2):
>   ArmPkg: Replace CoreId and ClusterId with Mpidr in ARM_CORE_INFO
>     struct
>   ArmPkg: Add Library/MpInitLib to support EFI_MP_SERVICES_PROTOCOL
> 
>  ArmPkg/ArmPkg.dec                                              |    4 +
>  ArmPkg/ArmPkg.dsc                                              |    4 +
>  ArmPkg/Drivers/CpuDxe/AArch64/Arch.c                           |   22 +
>  ArmPkg/Drivers/CpuDxe/Arm/Arch.c                               |   22 +
>  ArmPkg/Drivers/CpuDxe/CpuDxe.c                                 |    2 +
>  ArmPkg/Drivers/CpuDxe/CpuDxe.h                                 |   10 +
>  ArmPkg/Drivers/CpuDxe/CpuDxe.inf                               |    6 +
>  ArmPkg/Drivers/CpuDxe/CpuMpInit.c                              |  604 ++++++++
>  ArmPkg/Include/Guid/ArmMpCoreInfo.h                            |    3 +-
>  ArmPkg/Include/Library/ArmLib.h                                |    4 +
>  ArmPkg/Include/Library/MpInitLib.h                             |  366 +++++
>  ArmPkg/Library/MpInitLib/AArch64/MpFuncs.S                     |   61 +
>  ArmPkg/Library/MpInitLib/DxeMpInitLib.inf                      |   53 +
>  ArmPkg/Library/MpInitLib/DxeMpLib.c                            | 1498 ++++++++++++++++++++
>  ArmPkg/Library/MpInitLib/InternalMpInitLib.h                   |  358 +++++
>  ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.c |    8 +-
>  ArmPlatformPkg/PrePeiCore/MainMPCore.c                         |    2 +-
>  ArmPlatformPkg/PrePi/MainMPCore.c                              |    2 +-
>  ArmVirtPkg/ArmVirt.dsc.inc                                     |    3 +
>  19 files changed, 3024 insertions(+), 8 deletions(-)
>  create mode 100644 ArmPkg/Drivers/CpuDxe/AArch64/Arch.c
>  create mode 100644 ArmPkg/Drivers/CpuDxe/Arm/Arch.c
>  create mode 100644 ArmPkg/Drivers/CpuDxe/CpuMpInit.c
>  create mode 100644 ArmPkg/Include/Library/MpInitLib.h
>  create mode 100644 ArmPkg/Library/MpInitLib/AArch64/MpFuncs.S
>  create mode 100644 ArmPkg/Library/MpInitLib/DxeMpInitLib.inf
>  create mode 100644 ArmPkg/Library/MpInitLib/DxeMpLib.c
>  create mode 100644 ArmPkg/Library/MpInitLib/InternalMpInitLib.h
> 
> -- 
> 2.31.1
> 

  parent reply	other threads:[~2021-09-28 11:14 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-25  2:17 [RFC] [PATCH 0/2] Proposal to add EFI_MP_SERVICES_PROTOCOL support for AARCH64 Rebecca Cran
2021-09-25  2:17 ` [RFC] [PATCH 1/2] ArmPkg: Replace CoreId and ClusterId with Mpidr in ARM_CORE_INFO struct Rebecca Cran
2021-09-25  2:17 ` [RFC] [PATCH 2/2] ArmPkg: Add Library/MpInitLib to support EFI_MP_SERVICES_PROTOCOL Rebecca Cran
2021-09-28 11:14 ` Leif Lindholm [this message]
2021-10-07  9:41   ` [RFC] [PATCH 0/2] Proposal to add EFI_MP_SERVICES_PROTOCOL support for AARCH64 Leif Lindholm
2021-10-07 10:03     ` Ard Biesheuvel
2021-10-07 11:02       ` Leif Lindholm
2021-10-08 14:51         ` Ard Biesheuvel
2021-10-11 12:28           ` Leif Lindholm
2021-10-11 14:20             ` Samer El-Haj-Mahmoud
2021-10-11 15:01               ` [edk2-rfc] " Leif Lindholm
2021-10-11 21:52                 ` Samer El-Haj-Mahmoud
2021-10-14 13:14                   ` Leif Lindholm
2021-10-18  9:21                     ` Sami Mujawar
2021-10-18 11:54                       ` Leif Lindholm
2022-01-19 10:28                         ` [edk2-devel] " Sami Mujawar
2021-10-14 12:41   ` Rebecca Cran

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210928111435.poztq4cksagsogbw@leviathan \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox