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


Rebecca Cran <rebecca@...>
 

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

Join devel@edk2.groups.io to automatically receive all group messages.