Re: [edk2-devel] [PATCH 3/5] UefiCpuPkg/MpInitLib: Add support for multiple HOBs to SwitchApContext()

["Laszlo Ersek" <lersek@redhat.com>]

On 2/15/24 10:31, Gerd Hoffmann wrote:
> Remove the MpHandOff parameter.  This is not useful in case multiple
> HOBs are present in the system.  The function will use GetMpHandOffHob()
> to loop over all HOBs instead.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.h |  2 +-
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 35 +++++++++++++++++-----------
>  2 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 7e409cceaddf..a141a95b45ea 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -482,7 +482,7 @@ GetWakeupBuffer (
>  **/
>  VOID
>  SwitchApContext (
> -  IN MP_HAND_OFF  *MpHandOff
> +  VOID
>    );
>  
>  /**
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 8e6cf50ed171..35f47d3b1289 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1936,32 +1936,41 @@ GetBspNumber (
>    begin running the procedure called SwitchContextPerAp.
>    This procedure allows the AP to switch to another section of
>    memory and continue its loop there.
> -
> -  @param[in] MpHandOff  Pointer to MP hand-off data structure.
>  **/
>  VOID
>  SwitchApContext (
> -  IN MP_HAND_OFF  *MpHandOff
> +  VOID
>    )
>  {
> -  UINTN   Index;
> -  UINT32  BspNumber;
> +  UINTN        Index;
> +  UINT32       BspNumber;
> +  MP_HAND_OFF  *MpHandOff;
>  
>    BspNumber = GetBspNumber ();
>  
> -  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> -    if (Index != BspNumber) {
> -      *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress = (UINTN)SwitchContextPerAp;
> -      *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   = MpHandOff->StartupSignalValue;
> +  for (MpHandOff = GetMpHandOffHob (0);
> +       MpHandOff != NULL;
> +       MpHandOff = GetMpHandOffHob (MpHandOff->ProcessorIndex + MpHandOff->CpuCount))
> +  {
> +    for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> +      if (MpHandOff->ProcessorIndex + Index != BspNumber) {
> +        *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress = (UINTN)SwitchContextPerAp;
> +        *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress   = MpHandOff->StartupSignalValue;
> +      }
>      }
>    }
>  
>    //
>    // Wait all APs waken up if this is not the 1st broadcast of SIPI
>    //
> -  for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> -    if (Index != BspNumber) {
> -      WaitApWakeup ((UINT32 *)(UINTN)(MpHandOff->Info[Index].StartupSignalAddress));
> +  for (MpHandOff = GetMpHandOffHob (0);
> +       MpHandOff != NULL;
> +       MpHandOff = GetMpHandOffHob (MpHandOff->ProcessorIndex + MpHandOff->CpuCount))
> +  {
> +    for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> +      if (MpHandOff->ProcessorIndex + Index != BspNumber) {
> +        WaitApWakeup ((UINT32 *)(UINTN)(MpHandOff->Info[Index].StartupSignalAddress));
> +      }
>      }
>    }
>  }
> @@ -2226,7 +2235,7 @@ MpInitLibInitialize (
>        // enables the APs to switch to a different memory section and continue their
>        // looping process there.
>        //
> -      SwitchApContext (MpHandOff);
> +      SwitchApContext ();
>        //
>        // Wait for all APs finished initialization
>        //

Same comment as under the previous patch. We could just iterate with
MpHandOff over the GUID HOB list in the outer loops, and perform the
proper actions upon

  MpHandOff->ProcessorIndex + Index != BspNumber

in the inner loop.

It is not necessary for us to ask for the HOBs in any particular
sequence, so we shouldn't pay the O(n) lookup price for every HOB in turn.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115592): https://edk2.groups.io/g/devel/message/115592
Mute This Topic: https://groups.io/mt/104369847/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-