Re: [edk2-devel] [PATCH v4 3/3] MdeModulePkg: Add new Application/MpServicesTest application

["Rebecca Cran" <quic_rcran@quicinc.com>]

Thanks, I hadn't seen that. Unless people want the additional control 
this application provides via the command-line options, I can drop this 
patch.

-- 
Rebecca Cran

On 1/8/23 18:32, Ni, Ray wrote:
> Rebecca,
> Have you reviewed UefiCpuPkg\Test\UnitTest\EfiMpServicesPpiProtocol? It's based on UnitTestFrameworkPkg for unit testing on MP PPI and MP protocol.
> Do you think if the MpServicesTest app and the EfiMpServicesPpiProtocol have some overlapped functionalities?
> 
> 
> 
> Thanks,
> Ray
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Rebecca Cran
>> Sent: Sunday, January 8, 2023 12:56 PM
>> To: Kun Qin <kuqin12@gmail.com>; devel@edk2.groups.io; Sami Mujawar <sami.mujawar@arm.com>; Ard Biesheuvel
>> <ardb+tianocore@kernel.org>; Leif Lindholm <quic_llindhol@quicinc.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao,
>> Liming <gaoliming@byosoft.com.cn>; Tiger Liu <TigerLiu@zhaoxin.com>
>> Subject: Re: [edk2-devel] [PATCH v4 3/3] MdeModulePkg: Add new Application/MpServicesTest application
>>
>> Thanks! I've fixed all the issues you noted and will send out a v5 patch
>> in the next couple of days.
>>
>> --
>> Rebecca Cran
>>
>> On 1/6/23 15:33, Kun Qin wrote:
>>> Hi Rebecca,
>>>
>>> Thanks for sharing this patch. I found a few minor issues when running
>>> this test app. Please see comments with [KQ] below.
>>>
>>> Regards,
>>> Kun
>>>
>>> On 1/4/2023 7:37 AM, Rebecca Cran wrote:
>>>> The MpServicesTest application exercises the EFI_MP_SERVICES_PROTOCOL.
>>>>
>>>> usage:
>>>>     MpServicesTest -A [-O]
>>>>     MpServicesTest -T <Timeout>
>>>>     MpServicesTest -S <Processor #>
>>>>     MpServicesTest -P
>>>>     MpServicesTest -U
>>>>     MpServicesTest -W
>>>>     MpServicesTest -E <Processor #>
>>>>     MpServicesTest -D <Processor #>
>>>>     MpServicesTest -h
>>>>
>>>> Parameter:
>>>>     -A:  Run all APs.
>>>>     -O:  Run APs sequentially (use with -A).
>>>>     -T:  Specify timeout in milliseconds. Default is to wait forever.
>>>>     -S:  Specify the single AP to run.
>>>>     -P:  Print processor information.
>>>>     -U:  Set the specified AP to the Unhealthy status (use with -E/-D).
>>>>     -W:  Run WhoAmI and print index of BSP.
>>>>     -E:  Enable the specified AP.
>>>>     -D:  Disable the specified AP.
>>>>     -h:  Print this help page.
>>>>
>>>> Signed-off-by: Rebecca Cran <rebecca@quicinc.com>
>>>> ---
>>>>    MdeModulePkg/MdeModulePkg.dsc                              |   2 +
>>>>    MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf |  40 ++
>>>>    MdeModulePkg/Application/MpServicesTest/Options.h          |  39 ++
>>>>    MdeModulePkg/Application/MpServicesTest/MpServicesTest.c   | 560
>>>> ++++++++++++++++++++
>>>>    MdeModulePkg/Application/MpServicesTest/Options.c          | 164 ++++++
>>>>    5 files changed, 805 insertions(+)
>>>>
>>>> diff --git a/MdeModulePkg/MdeModulePkg.dsc
>>>> b/MdeModulePkg/MdeModulePkg.dsc
>>>> index 659482ab737f..6992b3ae8db6 100644
>>>> --- a/MdeModulePkg/MdeModulePkg.dsc
>>>> +++ b/MdeModulePkg/MdeModulePkg.dsc
>>>> @@ -166,6 +166,7 @@ [LibraryClasses.common.UEFI_APPLICATION]
>>>>
>>>> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>>>>      DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
>>>>      FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
>>>> +  ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
>>>>    [LibraryClasses.common.MM_STANDALONE]
>>>>      HobLib|MdeModulePkg/Library/BaseHobLibNull/BaseHobLibNull.inf
>>>> @@ -445,6 +446,7 @@ [Components]
>>>>
>>>> MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
>>>>    [Components.IA32, Components.X64, Components.AARCH64]
>>>> +  MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf
>>>>      MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
>>>>      MdeModulePkg/Universal/EbcDxe/EbcDebugger.inf
>>>>      MdeModulePkg/Universal/EbcDxe/EbcDebuggerConfig.inf
>>>> diff --git
>>>> a/MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf
>>>> b/MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf
>>>> new file mode 100644
>>>> index 000000000000..07ee4afec845
>>>> --- /dev/null
>>>> +++ b/MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf
>>>> @@ -0,0 +1,40 @@
>>>> +## @file
>>>> +#  UEFI Application to exercise EFI_MP_SERVICES_PROTOCOL.
>>>> +#
>>>> +#  Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights
>>>> reserved.<BR>
>>>> +#
>>>> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>>>> +#
>>>> +##
>>>> +
>>>> +[Defines]
>>>> +  INF_VERSION                    = 1.29
>>>> +  BASE_NAME                      = MpServicesTest
>>>> +  FILE_GUID                      = 43e9defa-7209-4b0d-b136-cc4ca02cb469
>>>> +  MODULE_TYPE                    = UEFI_APPLICATION
>>>> +  VERSION_STRING                 = 0.1
>>>> +  ENTRY_POINT                    = UefiMain
>>>> +
>>>> +#
>>>> +# The following information is for reference only and not required by
>>>> the build tools.
>>>> +#
>>>> +#  VALID_ARCHITECTURES           = IA32 X64 AARCH64
>>>> +#
>>>> +
>>>> +[Sources]
>>>> +  MpServicesTest.c
>>>> +  Options.c
>>>> +  Options.h
>>>> +
>>>> +[Packages]
>>>> +  MdePkg/MdePkg.dec
>>>> +
>>>> +[LibraryClasses]
>>>> +  BaseLib
>>>> +  ShellLib
>>>> +  UefiApplicationEntryPoint
>>>> +  UefiLib
>>>> +
>>>> +[Protocols]
>>>> +  gEfiMpServiceProtocolGuid    ## CONSUMES
>>>> +
>>>> diff --git a/MdeModulePkg/Application/MpServicesTest/Options.h
>>>> b/MdeModulePkg/Application/MpServicesTest/Options.h
>>>> new file mode 100644
>>>> index 000000000000..cb28230ab095
>>>> --- /dev/null
>>>> +++ b/MdeModulePkg/Application/MpServicesTest/Options.h
>>>> @@ -0,0 +1,39 @@
>>>> +/** @file
>>>> +  Options handling code.
>>>> +
>>>> +  Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights
>>>> reserved.<BR>
>>>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>>>> +**/
>>>> +
>>>> +#ifndef MPSERVICESTEST_OPTIONS_H_
>>>> +#define MPSERVICESTEST_OPTIONS_H_
>>>> +
>>>> +#define INFINITE_TIMEOUT  0
>>>> +
>>>> +typedef struct {
>>>> +  UINTN      Timeout;
>>>> +  UINTN      ProcessorIndex;
>>>> +  BOOLEAN    RunAllAPs;
>>>> +  BOOLEAN    RunSingleAP;
>>>> +  BOOLEAN    DisableProcessor;
>>>> +  BOOLEAN    EnableProcessor;
>>>> +  BOOLEAN    SetProcessorHealthy;
>>>> +  BOOLEAN    SetProcessorUnhealthy;
>>>> +  BOOLEAN    PrintProcessorInformation;
>>>> +  BOOLEAN    PrintBspProcessorIndex;
>>>> +  BOOLEAN    RunAPsSequentially;
>>>> +} MP_SERVICES_TEST_OPTIONS;
>>>> +
>>>> +/**
>>>> +  Parses any arguments provided on the command line.
>>>> +
>>>> +  @param Options  The arguments structure.
>>>> +
>>>> +  @return EFI_SUCCESS on success, or an error code.
>>>> +**/
>>>> +EFI_STATUS
>>>> +ParseArguments (
>>>> +  MP_SERVICES_TEST_OPTIONS  *Options
>>>> +  );
>>>> +
>>>> +#endif /* MPSERVICESTEST_OPTIONS_H_ */
>>>> diff --git a/MdeModulePkg/Application/MpServicesTest/MpServicesTest.c
>>>> b/MdeModulePkg/Application/MpServicesTest/MpServicesTest.c
>>>> new file mode 100644
>>>> index 000000000000..3f3d9752d500
>>>> --- /dev/null
>>>> +++ b/MdeModulePkg/Application/MpServicesTest/MpServicesTest.c
>>>> @@ -0,0 +1,560 @@
>>>> +/** @file
>>>> +  UEFI Application to exercise EFI_MP_SERVICES_PROTOCOL.
>>>> +
>>>> +  Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights
>>>> reserved.<BR>
>>>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>>>> +**/
>>>> +
>>>> +#include <Uefi.h>
>>>> +#include <Library/BaseMemoryLib.h>
>>>> +#include <Library/DebugLib.h>
>>>> +#include <Library/MemoryAllocationLib.h>
>>>> +#include <Library/PrintLib.h>
>>>> +#include <Library/UefiBootServicesTableLib.h>
>>>> +#include <Library/UefiLib.h>
>>>> +#include <Pi/PiMultiPhase.h>
>>>> +#include <Protocol/MpService.h>
>>>> +
>>>> +#include "Options.h"
>>>> +
>>>> +#define APFUNC_BUFFER_LEN  256
>>>> +
>>>> +typedef struct {
>>>> +  EFI_MP_SERVICES_PROTOCOL    *Mp;
>>>> +  CHAR16                      **Buffer;
>>>> +} APFUNC_ARG;
>>>> +
>>>> +/** The procedure to run with the MP Services interface.
>>>> +
>>>> +  @param Arg The procedure argument.
>>>> +
>>>> +**/
>>>> +STATIC
>>>> +VOID
>>>> +EFIAPI
>>>> +ApFunction (
>>>> +  IN OUT VOID  *Arg
>>>> +  )
>>>> +{
>>>> +  APFUNC_ARG  *Param;
>>>> +  UINTN       ProcessorId;
>>>> +
>>>> +  if (Arg != NULL) {
>>>> +    Param = Arg;
>>>> +
>>>> +    Param->Mp->WhoAmI (Param->Mp, &ProcessorId);
>>>> +    UnicodeSPrint (Param->Buffer[ProcessorId], APFUNC_BUFFER_LEN,
>>>> L"Hello from CPU %ld\n", ProcessorId);
>>>> +  }
>>>> +}
>>>> +
>>>> +/**
>>>> +  Fetches the number of processors and which processor is the BSP.
>>>> +
>>>> +  @param Mp  MP Services Protocol.
>>>> +  @param NumProcessors Number of processors.
>>>> +  @param BspIndex      The index of the BSP.
>>>> +**/
>>>> +STATIC
>>>> +EFI_STATUS
>>>> +GetProcessorInformation (
>>>> +  IN  EFI_MP_SERVICES_PROTOCOL  *Mp,
>>>> +  OUT UINTN                     *NumProcessors,
>>>> +  OUT UINTN                     *BspIndex
>>>> +  )
>>>> +{
>>>> +  EFI_STATUS  Status;
>>>> +  UINTN       NumEnabledProcessors;
>>>> +
>>>> +  Status = Mp->GetNumberOfProcessors (Mp, NumProcessors,
>>>> &NumEnabledProcessors);
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    return Status;
>>>> +  }
>>>> +
>>>> +  Status = Mp->WhoAmI (Mp, BspIndex);
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    return Status;
>>>> +  }
>>>> +
>>>> +  return EFI_SUCCESS;
>>>> +}
>>>> +
>>>> +/** Displays information returned from MP Services Protocol.
>>>> +
>>>> +  @param Mp       The MP Services Protocol
>>>> +  @param BspIndex On return, contains the index of the BSP.
>>>> +
>>>> +  @return The number of CPUs in the system.
>>>> +
>>>> +**/
>>>> +STATIC
>>>> +UINTN
>>>> +PrintProcessorInformation (
>>>> +  IN   EFI_MP_SERVICES_PROTOCOL  *Mp,
>>>> +  OUT  UINTN                     *BspIndex
>>>> +  )
>>>> +{
>>>> +  EFI_STATUS                 Status;
>>>> +  EFI_PROCESSOR_INFORMATION  CpuInfo;
>>>> +  UINTN                      Index;
>>>> +  UINTN                      NumCpu;
>>>> +  UINTN                      NumEnabledCpu;
>>> [KQ] The NumCpu and NumEnabledCpu probably should be initialized to 0s?
>>> Otherwise if the
>>> GetNumberOfProcessors function somehow fails, the rest of the call will
>>> essentially be no-op,
>>> instead of running into undefined number of CPUs.
>>>> +
>>>> +  Status = Mp->GetNumberOfProcessors (Mp, &NumCpu, &NumEnabledCpu);
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    Print (L"GetNumberOfProcessors failed: %r\n", Status);
>>>> +  } else {
>>>> +    Print (L"Number of CPUs: %ld, Enabled: %d\n", NumCpu,
>>>> NumEnabledCpu);
>>>> +  }
>>>> +
>>>> +  for (Index = 0; Index < NumCpu; Index++) {
>>>> +    Status = Mp->GetProcessorInfo (Mp, CPU_V2_EXTENDED_TOPOLOGY |
>>>> Index, &CpuInfo);
>>>> +    if (EFI_ERROR (Status)) {
>>>> +      Print (L"GetProcessorInfo for Processor %d failed: %r\n",
>>>> Index, Status);
>>>> +    } else {
>>>> +      Print (
>>>> +        L"Processor %d:\n"
>>>> +        L"\tID: %016lx\n"
>>>> +        L"\tStatus: %s | ",
>>>> +        Index,
>>>> +        CpuInfo.ProcessorId,
>>>> +        (CpuInfo.StatusFlag & PROCESSOR_AS_BSP_BIT) ? L"BSP" : L"AP"
>>>> +        );
>>>> +
>>>> +      if ((CpuInfo.StatusFlag & PROCESSOR_AS_BSP_BIT) && (BspIndex !=
>>>> NULL)) {
>>>> +        *BspIndex = Index;
>>>> +      }
>>>> +
>>>> +      Print (L"%s | ", (CpuInfo.StatusFlag & PROCESSOR_ENABLED_BIT) ?
>>>> L"Enabled" : L"Disabled");
>>>> +      Print (L"%s\n", (CpuInfo.StatusFlag &
>>>> PROCESSOR_HEALTH_STATUS_BIT) ? L"Healthy" : L"Faulted");
>>>> +
>>>> +      Print (
>>>> +        L"\tLocation: Package %d, Core %d, Thread %d\n"
>>>> +        L"\tExtended Information: Package %d, Module %d, Tile %d, Die
>>>> %d, Core %d, Thread %d\n\n",
>>>> +        CpuInfo.Location.Package,
>>>> +        CpuInfo.Location.Core,
>>>> +        CpuInfo.Location.Thread,
>>>> +        CpuInfo.ExtendedInformation.Location2.Package,
>>>> +        CpuInfo.ExtendedInformation.Location2.Module,
>>>> +        CpuInfo.ExtendedInformation.Location2.Tile,
>>>> +        CpuInfo.ExtendedInformation.Location2.Die,
>>>> +        CpuInfo.ExtendedInformation.Location2.Core,
>>>> +        CpuInfo.ExtendedInformation.Location2.Thread
>>>> +        );
>>>> +    }
>>>> +  }
>>>> +
>>>> +  return NumCpu;
>>>> +}
>>>> +
>>>> +/** Allocates memory in ApArg for the single AP specified.
>>>> +
>>>> +  @param ApArg          Pointer to the AP argument structure.
>>>> +  @param Mp             The MP Services Protocol.
>>>> +  @param ProcessorIndex The index of the AP.
>>>> +
>>>> +  @retval EFI_SUCCESS          Memory was successfully allocated.
>>>> +  @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
>>>> +
>>>> +**/
>>>> +STATIC
>>>> +EFI_STATUS
>>>> +AllocateApFuncBufferSingleAP (
>>>> +  IN APFUNC_ARG                *ApArg,
>>>> +  IN EFI_MP_SERVICES_PROTOCOL  *Mp,
>>>> +  IN UINTN                     ProcessorIndex
>>>> +  )
>>>> +{
>>>> +  ApArg->Mp = Mp;
>>>> +
>>>> +  ApArg->Buffer = AllocateZeroPool ((ProcessorIndex + 1) * sizeof
>>>> (VOID *));
>>>> +  if (ApArg->Buffer == NULL) {
>>>> +    Print (L"Failed to allocate buffer for AP buffer\n");
>>>> +    return EFI_OUT_OF_RESOURCES;
>>>> +  }
>>>> +
>>>> +  ApArg->Buffer[ProcessorIndex] = AllocateZeroPool (APFUNC_BUFFER_LEN);
>>>> +  if (ApArg->Buffer[ProcessorIndex] == NULL) {
>>>> +    Print (L"Failed to allocate buffer for AP buffer\n");
>>>> +    FreePool (ApArg->Buffer);
>>>> +    return EFI_OUT_OF_RESOURCES;
>>>> +  }
>>>> +
>>>> +  return EFI_SUCCESS;
>>>> +}
>>>> +
>>>> +/** Allocates memory in ApArg for all APs.
>>>> +
>>>> +  @param ApArg   Pointer to the AP argument structure.
>>>> +  @param Mp      The MP Services Protocol.
>>>> +  @param NumCpus The number of CPUs.
>>>> +
>>>> +  @retval EFI_SUCCESS          Memory was successfully allocated.
>>>> +  @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
>>>> +
>>>> +**/
>>>> +STATIC
>>>> +EFI_STATUS
>>>> +AllocateApFuncBufferAllAPs (
>>>> +  IN APFUNC_ARG                *ApArg,
>>>> +  IN EFI_MP_SERVICES_PROTOCOL  *Mp,
>>>> +  IN UINTN                     NumCpus
>>>> +  )
>>>> +{
>>>> +  UINT32  Index;
>>> [KQ] The Index of UINT32 compared to NumCpus of UINTN could make some
>>> compilers unhappy.
>>>> +
>>>> +  ApArg->Mp = Mp;
>>>> +
>>>> +  ApArg->Buffer = AllocateZeroPool (NumCpus * sizeof (VOID *));
>>>> +  if (ApArg->Buffer == NULL) {
>>>> +    Print (L"Failed to allocate buffer for AP message\n");
>>>> +    return EFI_OUT_OF_RESOURCES;
>>>> +  }
>>>> +
>>>> +  for (Index = 0; Index < NumCpus; Index++) {
>>>> +    ApArg->Buffer[Index] = AllocateZeroPool (APFUNC_BUFFER_LEN);
>>>> +    if (ApArg->Buffer[Index] == NULL) {
>>>> +      Print (L"Failed to allocate buffer for AP message\n");
>>>> +      for (--Index; Index >= 0; Index++) {
>>> [KQ] This Index increment could cause the loop not ending as expected.
>>>> +        FreePool (ApArg->Buffer[Index]);
>>>> +      }
>>>> +
>>>> +      FreePool (ApArg->Buffer);
>>>> +      return EFI_OUT_OF_RESOURCES;
>>>> +    }
>>>> +  }
>>>> +
>>>> +  return EFI_SUCCESS;
>>>> +}
>>>> +
>>>> +/** Frees memory in ApArg for all APs.
>>>> +
>>>> +  @param ApArg   Pointer to the AP argument structure.
>>>> +  @param NumCpus The number of CPUs.
>>>> +
>>>> +**/
>>>> +STATIC
>>>> +VOID
>>>> +FreeApFuncBuffer (
>>>> +  APFUNC_ARG  *ApArg,
>>>> +  UINTN       NumCpus
>>>> +  )
>>>> +{
>>>> +  UINTN  Index;
>>>> +
>>>> +  for (Index = 0; Index < NumCpus; Index++) {
>>>> +    if (ApArg->Buffer[Index] != NULL) {
>>>> +      FreePool (ApArg->Buffer[Index]);
>>>> +    }
>>>> +  }
>>>> +
>>>> +  FreePool (ApArg->Buffer);
>>>> +}
>>>> +
>>>> +/** Runs a specified AP.
>>>> +
>>>> +  @param Mp             The MP Services Protocol.
>>>> +  @param ProcessorIndex The processor index.
>>>> +  @param Timeout        Timeout in milliseconds.
>>>> +
>>>> +  @return EFI_SUCCESS on success, or an error code.
>>>> +
>>>> +**/
>>>> +STATIC
>>>> +EFI_STATUS
>>>> +StartupThisAP (
>>>> +  IN EFI_MP_SERVICES_PROTOCOL  *Mp,
>>>> +  IN UINTN                     ProcessorIndex,
>>>> +  IN UINTN                     Timeout
>>>> +  )
>>>> +{
>>>> +  EFI_STATUS  Status;
>>>> +  APFUNC_ARG  ApArg;
>>>> +
>>>> +  Status = AllocateApFuncBufferSingleAP (&ApArg, Mp, ProcessorIndex);
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    return Status;
>>>> +  }
>>>> +
>>>> +  Status = AllocateApFuncBufferSingleAP (&ApArg, Mp, ProcessorIndex);
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    return Status;
>>>> +  }
>>>> +
>>> [KQ] I guess the above double calls are not intended?
>>>> +  Print (
>>>> +    L"StartupThisAP on Processor %d with %d%s timeout...",
>>>> +    ProcessorIndex,
>>>> +    Timeout,
>>>> +    (Timeout == INFINITE_TIMEOUT) ? L" (infinite)" : L"ms"
>>>> +    );
>>>> +  Status = Mp->StartupThisAP (
>>>> +                 Mp,
>>>> +                 ApFunction,
>>>> +                 ProcessorIndex,
>>>> +                 NULL,
>>>> +                 Timeout * 1000,
>>>> +                 &ApArg,
>>>> +                 NULL
>>>> +                 );
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    Print (L"failed: %r\n", Status);
>>>> +    return Status;
>>>> +  } else {
>>>> +    Print (L"done.\n");
>>>> +    Print (ApArg.Buffer[ProcessorIndex]);
>>>> +  }
>>>> +
>>>> +  FreeApFuncBuffer (&ApArg, ProcessorIndex + 1);
>>>> +
>>>> +  return EFI_SUCCESS;
>>>> +}
>>>> +
>>>> +/** Runs all APs.
>>>> +
>>>> +  @param Mp                 The MP Services Protocol.
>>>> +  @param NumCpus            The number of CPUs in the system.
>>>> +  @param Timeout            Timeout in milliseconds.
>>>> +  @param RunAPsSequentially Run APs sequentially (FALSE: run
>>>> simultaneously)
>>>> +
>>>> +  @return EFI_SUCCESS on success, or an error code.
>>>> +
>>>> +**/
>>>> +STATIC
>>>> +EFI_STATUS
>>>> +StartupAllAPs (
>>>> +  IN EFI_MP_SERVICES_PROTOCOL  *Mp,
>>>> +  IN UINTN                     NumCpus,
>>>> +  IN UINTN                     Timeout,
>>>> +  IN BOOLEAN                   RunAPsSequentially
>>>> +  )
>>>> +{
>>>> +  EFI_STATUS  Status;
>>>> +  UINTN       Index;
>>>> +  APFUNC_ARG  ApArg;
>>>> +
>>>> +  Status = AllocateApFuncBufferAllAPs (&ApArg, Mp, NumCpus);
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    return Status;
>>>> +  }
>>>> +
>>>> +  Print (
>>>> +    L"Running with SingleThread %s, %u%s timeout...",
>>>> +    (RunAPsSequentially) ? L"TRUE" : L"FALSE",
>>>> +    Timeout,
>>>> +    (Timeout == INFINITE_TIMEOUT) ? L" (infinite)" : L"ms"
>>>> +    );
>>>> +
>>>> +  Status = Mp->StartupAllAPs (
>>>> +                 Mp,
>>>> +                 ApFunction,
>>>> +                 RunAPsSequentially,
>>>> +                 NULL,
>>>> +                 Timeout * 1000,
>>>> +                 &ApArg,
>>>> +                 NULL
>>>> +                 );
>>>> +
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    Print (L"failed: %r\n", Status);
>>>> +
>>>> +    return Status;
>>>> +  } else {
>>>> +    Print (L"done.\n");
>>>> +
>>>> +    for (Index = 0; Index < NumCpus; Index++) {
>>>> +      Print (ApArg.Buffer[Index]);
>>>> +    }
>>>> +  }
>>>> +
>>>> +  FreeApFuncBuffer (&ApArg, NumCpus);
>>>> +  return EFI_SUCCESS;
>>>> +}
>>>> +
>>>> +/**
>>>> +  Enables the specified AP.
>>>> +
>>>> +  @param Mp               The MP Services Protocol.
>>>> +  @param ProcessorIndex   The processor to enable.
>>>> +  @param ProcessorHealthy The health status of the processor.
>>>> +
>>>> +  @return EFI_SUCCESS on success, or an error code.
>>>> +**/
>>>> +STATIC
>>>> +EFI_STATUS
>>>> +EnableAP (
>>>> +  IN EFI_MP_SERVICES_PROTOCOL  *Mp,
>>>> +  UINTN                        ProcessorIndex,
>>>> +  BOOLEAN                      ProcessorHealthy
>>> [KQ] These parameters should have the "IN" attributes?
>>>> +  )
>>>> +{
>>>> +  EFI_STATUS  Status;
>>>> +  UINT32      HealthFlag;
>>>> +
>>>> +  if (ProcessorHealthy) {
>>>> +    Print (L"Enabling Processor %d with HealthFlag healthy...",
>>>> ProcessorIndex);
>>>> +    HealthFlag = PROCESSOR_HEALTH_STATUS_BIT;
>>>> +  } else {
>>>> +    Print (L"Enabling Processor %d with HealthFlag faulted...",
>>>> ProcessorIndex);
>>>> +    HealthFlag = 0;
>>>> +  }
>>>> +
>>>> +  Status = Mp->EnableDisableAP (Mp, ProcessorIndex, TRUE, &HealthFlag);
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    Print (L"failed: %r\n", Status);
>>>> +    return Status;
>>>> +  } else {
>>>> +    Print (L"done.\n");
>>>> +  }
>>>> +
>>>> +  return Status;
>>>> +}
>>>> +
>>>> +/**
>>>> +  Disables the specified AP.
>>>> +
>>>> +  @param Mp               The MP Services Protocol.
>>>> +  @param ProcessorIndex   The processor to disable.
>>>> +  @param ProcessorHealthy The health status of the processor.
>>>> +
>>>> +  @return EFI_SUCCESS on success, or an error code.
>>>> +**/
>>>> +STATIC
>>>> +EFI_STATUS
>>>> +DisableAP (
>>>> +  IN EFI_MP_SERVICES_PROTOCOL  *Mp,
>>>> +  UINTN                        ProcessorIndex,
>>>> +  BOOLEAN                      ProcessorHealthy
>>> [KQ] These parameters should have the "IN" attributes?
>>>> +  )
>>>> +{
>>>> +  EFI_STATUS  Status;
>>>> +  UINT32      HealthFlag;
>>>> +
>>>> +  if (ProcessorHealthy) {
>>>> +    Print (L"Disabling Processor %d with HealthFlag healthy...",
>>>> ProcessorIndex);
>>>> +    HealthFlag = PROCESSOR_HEALTH_STATUS_BIT;
>>>> +  } else {
>>>> +    Print (L"Disabling Processor %d with HealthFlag faulted...",
>>>> ProcessorIndex);
>>>> +    HealthFlag = 0;
>>>> +  }
>>>> +
>>>> +  Status = Mp->EnableDisableAP (Mp, ProcessorIndex, FALSE, &HealthFlag);
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    Print (L"failed: %r\n", Status);
>>>> +    return Status;
>>>> +  } else {
>>>> +    Print (L"done.\n");
>>>> +  }
>>>> +
>>>> +  return Status;
>>>> +}
>>>> +
>>>> +/**
>>>> +  The user Entry Point for Application. The user code starts with
>>>> this function
>>>> +  as the real entry point for the application.
>>>> +
>>>> +  @param[in] ImageHandle    The firmware allocated handle for the EFI
>>>> image.
>>>> +  @param[in] SystemTable    A pointer to the EFI System Table.
>>>> +
>>>> +  @retval EFI_SUCCESS       The entry point is executed successfully.
>>>> +  @retval other             Some error occurs when executing this
>>>> entry point.
>>>> +
>>>> +**/
>>>> +EFI_STATUS
>>>> +EFIAPI
>>>> +UefiMain (
>>>> +  IN EFI_HANDLE        ImageHandle,
>>>> +  IN EFI_SYSTEM_TABLE  *SystemTable
>>>> +  )
>>>> +{
>>>> +  EFI_STATUS                Status;
>>>> +  EFI_MP_SERVICES_PROTOCOL  *Mp;
>>>> +  UINTN                     BspIndex;
>>>> +  UINTN                     CpuIndex;
>>>> +  UINTN                     NumCpus;
>>>> +  BOOLEAN                   ProcessorHealthy;
>>>> +  MP_SERVICES_TEST_OPTIONS  Options;
>>>> +
>>>> +  BspIndex = 0;
>>>> +
>>>> +  Status = gBS->LocateProtocol (
>>>> +                  &gEfiMpServiceProtocolGuid,
>>>> +                  NULL,
>>>> +                  (VOID **)&Mp
>>>> +                  );
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    Print (L"Failed to locate EFI_MP_SERVICES_PROTOCOL (%r). Not
>>>> installed on platform?\n", Status);
>>>> +    return EFI_NOT_FOUND;
>>>> +  }
>>>> +
>>>> +  Status = ParseArguments (&Options);
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    return EFI_INVALID_PARAMETER;
>>>> +  }
>>>> +
>>>> +  Status = GetProcessorInformation (Mp, &NumCpus, &BspIndex);
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    Print (L"Error: Failed to fetch processor information.\n");
>>>> +    return Status;
>>>> +  }
>>>> +
>>>> +  if (Options.PrintBspProcessorIndex) {
>>>> +    Status = Mp->WhoAmI (Mp, &CpuIndex);
>>>> +    if (EFI_ERROR (Status)) {
>>>> +      Print (L"WhoAmI failed: %r\n", Status);
>>>> +      return Status;
>>>> +    } else {
>>>> +      Print (L"BSP: %016lx\n", CpuIndex);
>>>> +    }
>>>> +  }
>>>> +
>>>> +  if (Options.PrintProcessorInformation) {
>>>> +    NumCpus = PrintProcessorInformation (Mp, &BspIndex);
>>>> +    if (NumCpus < 2) {
>>>> +      Print (L"Error: Uniprocessor system found.\n");
>>>> +      return EFI_INVALID_PARAMETER;
>>>> +    }
>>>> +  }
>>>> +
>>>> +  if (Options.RunSingleAP) {
>>>> +    Status = StartupThisAP (
>>>> +               Mp,
>>>> +               Options.ProcessorIndex,
>>>> +               Options.Timeout
>>>> +               );
>>>> +    if (EFI_ERROR (Status)) {
>>>> +      return Status;
>>>> +    }
>>>> +  }
>>>> +
>>>> +  if (Options.RunAllAPs) {
>>>> +    Status = StartupAllAPs (Mp, NumCpus, Options.Timeout,
>>>> Options.RunAPsSequentially);
>>>> +    if (EFI_ERROR (Status)) {
>>>> +      return Status;
>>>> +    }
>>>> +  }
>>>> +
>>>> +  if (Options.EnableProcessor) {
>>>> +    ProcessorHealthy = TRUE;
>>>> +    if (Options.SetProcessorUnhealthy) {
>>>> +      ProcessorHealthy = FALSE;
>>>> +    }
>>>> +
>>>> +    Status = EnableAP (Mp, Options.ProcessorIndex, ProcessorHealthy);
>>>> +    if (EFI_ERROR (Status)) {
>>>> +      return Status;
>>>> +    }
>>>> +  }
>>>> +
>>>> +  if (Options.DisableProcessor) {
>>>> +    ProcessorHealthy = TRUE;
>>>> +    if (Options.SetProcessorUnhealthy) {
>>>> +      ProcessorHealthy = FALSE;
>>>> +    }
>>>> +
>>>> +    Status = DisableAP (Mp, Options.ProcessorIndex, ProcessorHealthy);
>>>> +    if (EFI_ERROR (Status)) {
>>>> +      return Status;
>>>> +    }
>>>> +  }
>>>> +
>>>> +  return EFI_SUCCESS;
>>>> +}
>>>> diff --git a/MdeModulePkg/Application/MpServicesTest/Options.c
>>>> b/MdeModulePkg/Application/MpServicesTest/Options.c
>>>> new file mode 100644
>>>> index 000000000000..e820c061e1ec
>>>> --- /dev/null
>>>> +++ b/MdeModulePkg/Application/MpServicesTest/Options.c
>>>> @@ -0,0 +1,164 @@
>>>> +/** @file
>>>> +  Options handling code.
>>>> +
>>>> +  Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights
>>>> reserved.<BR>
>>>> +  Copyright (c) 2010-2019  Finnbarr P. Murphy.   All rights
>>>> reserved.<BR>
>>>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>>>> +**/
>>>> +
>>>> +#include <Uefi.h>
>>>> +#include <Library/BaseMemoryLib.h>
>>>> +#include <Protocol/ShellParameters.h>
>>>> +#include <Library/BaseLib.h>
>>>> +#include <Library/UefiLib.h>
>>>> +#include <Library/UefiBootServicesTableLib.h>
>>>> +
>>>> +#include "Options.h"
>>>> +
>>>> +STATIC UINTN   Argc;
>>>> +STATIC CHAR16  **Argv;
>>>> +
>>>> +/**
>>>> +
>>>> +  This function provides argc and argv.
>>>> +
>>>> +  @return Status
>>>> +**/
>>>> +EFI_STATUS
>>>> +GetArg (
>>>> +  VOID
>>>> +  )
>>>> +{
>>>> +  EFI_STATUS                     Status;
>>>> +  EFI_SHELL_PARAMETERS_PROTOCOL  *ShellParameters;
>>>> +
>>>> +  Status = gBS->HandleProtocol (
>>>> +                  gImageHandle,
>>>> +                  &gEfiShellParametersProtocolGuid,
>>>> +                  (VOID **)&ShellParameters
>>>> +                  );
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    return Status;
>>>> +  }
>>>> +
>>>> +  Argc = ShellParameters->Argc;
>>>> +  Argv = ShellParameters->Argv;
>>>> +  return EFI_SUCCESS;
>>>> +}
>>>> +
>>>> +/**
>>>> +  Print app usage.
>>>> +**/
>>>> +STATIC
>>>> +VOID
>>>> +PrintUsage (
>>>> +  VOID
>>>> +  )
>>>> +{
>>>> +  Print (L"MpServicesTest:  usage\n");
>>>> +  Print (L"  MpServicesTest -A [-O]\n");
>>>> +  Print (L"  MpServicesTest -T <Timeout>\n");
>>>> +  Print (L"  MpServicesTest -S <Processor #>\n");
>>>> +  Print (L"  MpServicesTest -P\n");
>>>> +  Print (L"  MpServicesTest -U\n");
>>>> +  Print (L"  MpServicesTest -W\n");
>>>> +  Print (L"  MpServicesTest -E <Processor #>\n");
>>>> +  Print (L"  MpServicesTest -D <Processor #>\n");
>>>> +  Print (L"  MpServicesTest -h\n");
>>>> +  Print (L"Parameter:\n");
>>>> +  Print (L"  -A:  Run all APs.\n");
>>>> +  Print (L"  -O:  Run APs sequentially (use with -A).\n");
>>>> +  Print (L"  -T:  Specify timeout in milliseconds. Default is to wait
>>>> forever.\n");
>>>> +  Print (L"  -S:  Specify the single AP to run.\n");
>>>> +  Print (L"  -P:  Print processor information.\n");
>>>> +  Print (L"  -U:  Set the specified AP to the Unhealthy status (use
>>>> with -E/-D).\n");
>>>> +  Print (L"  -W:  Run WhoAmI and print index of BSP.\n");
>>>> +  Print (L"  -E:  Enable the specified AP.\n");
>>>> +  Print (L"  -D:  Disable the specified AP.\n");
>>>> +  Print (L"  -h:  Print this help page.\n");
>>>> +}
>>>> +
>>>> +/**
>>>> +  Parses any arguments provided on the command line.
>>>> +
>>>> +  @param Options  The arguments structure.
>>>> +
>>>> +  @return EFI_SUCCESS on success, or an error code.
>>>> +**/
>>>> +EFI_STATUS
>>>> +ParseArguments (
>>>> +  MP_SERVICES_TEST_OPTIONS  *Options
>>>> +  )
>>>> +{
>>>> +  EFI_STATUS    Status;
>>>> +  UINT32        ArgIndex;
>>> [KQ] Similar to the other comment, ArgIndex is of UINT32 is compared to
>>> Argc of UINTN could make some compilers unhappy.
>>>> +  CONST CHAR16  *Argument;
>>>> +  BOOLEAN       NeedsValue;
>>>> +  UINTN         *Value;
>>>> +
>>>> +  Status = GetArg ();
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    Print (L"Please use the UEFI Shell to run this application!\n",
>>>> Status);
>>>> +    return Status;
>>>> +  }
>>>> +
>>>> +  if (Argc == 1) {
>>>> +    PrintUsage ();
>>>> +  }
>>>> +
>>>> +  ZeroMem (Options, sizeof (MP_SERVICES_TEST_OPTIONS));
>>>> +
>>>> +  for (ArgIndex = 1; ArgIndex < Argc; ArgIndex++) {
>>>> +    Argument   = Argv[ArgIndex];
>>>> +    NeedsValue = FALSE;
>>>> +
>>>> +    if (StrCmp (Argument, L"-A") == 0) {
>>>> +      Options->RunAllAPs = TRUE;
>>>> +    } else if (StrCmp (Argument, L"-O") == 0) {
>>>> +      Options->RunAPsSequentially = TRUE;
>>>> +    } else if (StrCmp (Argument, L"-T") == 0) {
>>>> +      NeedsValue = TRUE;
>>>> +      Value      = &Options->Timeout;
>>>> +    } else if (StrCmp (Argument, L"-S") == 0) {
>>>> +      Options->RunSingleAP = TRUE;
>>>> +      NeedsValue           = TRUE;
>>>> +      Value                = &Options->ProcessorIndex;
>>>> +    } else if (StrCmp (Argument, L"-P") == 0) {
>>>> +      Options->PrintProcessorInformation = TRUE;
>>>> +    } else if (StrCmp (Argument, L"-U") == 0) {
>>>> +      Options->SetProcessorUnhealthy = TRUE;
>>>> +    } else if (StrCmp (Argument, L"-W") == 0) {
>>>> +      Options->PrintBspProcessorIndex = TRUE;
>>>> +    } else if (StrCmp (Argument, L"-E") == 0) {
>>>> +      Options->EnableProcessor = TRUE;
>>>> +      NeedsValue               = TRUE;
>>>> +      Value                    = &Options->ProcessorIndex;
>>>> +    } else if (StrCmp (Argument, L"-D") == 0) {
>>>> +      Options->DisableProcessor = TRUE;
>>>> +      NeedsValue                = TRUE;
>>>> +      Value                     = &Options->ProcessorIndex;
>>>> +    } else {
>>>> +      PrintUsage ();
>>>> +      ZeroMem (Options, sizeof (MP_SERVICES_TEST_OPTIONS));
>>>> +      return EFI_SUCCESS;
>>>> +    }
>>>> +
>>>> +    if (NeedsValue) {
>>>> +      if ((ArgIndex + 1) < Argc) {
>>>> +        Status = StrDecimalToUintnS (Argv[ArgIndex + 1], NULL, Value);
>>>> +        if (EFI_ERROR (Status)) {
>>>> +          Print (L"Error: option value must be a positive integer.\n");
>>>> +          PrintUsage ();
>>>> +          return EFI_INVALID_PARAMETER;
>>>> +        }
>>>> +
>>>> +        ArgIndex++;
>>>> +      } else {
>>>> +        PrintUsage ();
>>>> +        return EFI_INVALID_PARAMETER;
>>>> +      }
>>>> +    }
>>>> +  }
>>>> +
>>>> +  return EFI_SUCCESS;
>>>> +}
>>
>>
>> 
>>
>