Re: [PATCH 2/2] ArmPkg: Add Library/MpInitLib to support EFI_MP_SERVICES_PROTOCOL

["Rebecca Cran" <rebecca@nuviainc.com>]

[-- Attachment #1: Type: text/plain, Size: 2412 bytes --]

Hi Sami,


I've been looking at this again to make sure I've addressed all the 
issues before sending out a v2 patch series. I'm stuck on one point 
though: I'm not sure how it can be split into 2-3 patches without 
breaking bisecting. The CpuDxe and MpInitLib changes introduce a 
dependency on MpInitLib, so I don't see how I can put the ArmVirtPkg 
change into a separate commit without breaking it?


-- 
Rebecca Cran


On 10/22/21 11:51 AM, Sami Mujawar wrote:
>
> Hi Rebecca,
>
> Thanks you for this patch.
>
> I think this patch could be split in 2 - 3 patches (CpuDxe & MpInitLib 
> and the ArmVirtPkg change should be a separate patch).
>
> Please find my feedback inline marked [SAMI].
>
> I think the usage of MPIDR and CpuInfo->ProcessorId needs to be 
> revisited in this patch series. I have kept my feedback on usage of 
> MPIDR and the MT bit [24] to give the context. But, it will be good to 
> only retain the affinity bits and mask all other bits of the MPIDR.
>
> Please let me know if you have any queries.
>
> Regards,
>
> Sami Mujawar
>
>
> On 18/10/2021 04:39 PM, Rebecca Cran wrote:
>> Add support for EFI_MP_SERVICES_PROTOCOL during the DXE phase under
>> AArch64.
>>
>> PSCI_CPU_ON is called to power on the core, the supplied procedure is
>> executed and PSCI_CPU_OFF is called to power off the core.
>>
>> Minimal setup is done before calling the supplied procedure: for example
>> the MMU and caches are not enabled.
>>
>> Signed-off-by: Rebecca Cran<rebecca@nuviainc.com>
>> ---
>>   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/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 +++++
>>   ArmVirtPkg/ArmVirt.dsc.inc                   |    3 +
>>   14 files changed, 3013 insertions(+)

[-- Attachment #2: Type: text/html, Size: 3494 bytes --]