Re: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: Add MpServicesTest application to exercise MP Services

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

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

Thanks. I have a couple of comments, but will otherwise send out a v2 
patch later today.


On 11/3/21 4:54 AM, Sami Mujawar wrote:
> On 20/09/2021 04:47 PM, Rebecca Cran via groups.io wrote:
>> +
>> +  for (Index = 0; Index < NumCpus; Index++) {
>> +    Status = Mp->GetProcessorInfo (Mp, Index, &CpuInfo);
>> +    ASSERT_EFI_ERROR (Status);
>> +    if ((CpuInfo.StatusFlag & PROCESSOR_ENABLED_BIT) &&
>> +        !(CpuInfo.StatusFlag & PROCESSOR_AS_BSP_BIT)) {
>> +      if (IndexOfEnabledCpuToUse == IndexOfEnabledCpu) {
>> +        *ProcessorIndex = Index;
>> +        Status = EFI_SUCCESS;
>> +        break;
> [SAMI] Minor. I think it should be possible to return from here. In 
> that case the check below if (Index == NumCpus) is not needed and 
> EFI_NOT_FOUND can be returned at the end of the function.

That's true, but I prefer to avoid returning from within loops like this.


>> +
>> +  for (Index = 1; Index < NumCpus; Index++) {
>> +    Print (L"Switching BSP to Processor %d with EnableOldBSP FALSE...", Index);
>> +    Status = Mp->SwitchBSP (Mp, Index, FALSE);
> [SAMI] The SwitchBsp call can only be performed by the current BSP. 
> So, I am not sure if this would work in a for loop.

That makes sense. I don't think there's a good way to test this then, so 
I'll remove the SwitchBsp tests.


-- 

Rebecca Cran


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