Re: [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.

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

On 11/9/23 03:51, duntan wrote:
> Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
> 
> Previously, the HOB is defined, created and consumed only in StandaloneMmPkg. The HOB contains the number
> of processors and EFI_PROCESSOR_INFORMATION structure. This is the same as the information that PiSmmCpuDxeSmm
> uses EfiMpServiceProtocolGuid to get.
> 
> The incoming plan is to create gMpInformationHobGuid for both StandaloneMm and legacy DXE_SMM in early
> phase(for example in CpuMpPei). Then PiSmmCpuDxeSmm can consume the hob, which can simplify code logic
> in PiSmmCpuDxeSmm driver.
> 
> So move this HOB definition to UefiCpuPkg in this patch series.
> 
> Dun Tan (3):
>   UefiCpuPkg: Create MpInformation.h in UefiCpuPkg
>   StandaloneMmPkg:Add UefiCpuPkg.dec in DependencyCheck
>   StandaloneMmPkg:Remove MpInformation.h
> 
>  StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf                       | 1 +
>  StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 1 +
>  StandaloneMmPkg/StandaloneMmPkg.ci.yaml                                           | 3 ++-
>  StandaloneMmPkg/StandaloneMmPkg.dec                                               | 1 -
>  {StandaloneMmPkg => UefiCpuPkg}/Include/Guid/MpInformation.h                      | 2 +-
>  UefiCpuPkg/UefiCpuPkg.dec                                                         | 3 +++
>  6 files changed, 8 insertions(+), 3 deletions(-)
>  rename {StandaloneMmPkg => UefiCpuPkg}/Include/Guid/MpInformation.h (88%)
> 

From a quick skim, the series looks OK to me, and I also agree that the
above two MP services calls (GetNumberOfProcessors, GetProcessorInfo)
seem to be the only ones in PiSmmCpuDxeSmm.

However, what about the following scenario:

- in the PI phase (or HOB producer phase), the HOB is produced

- in the DXE phase, a platform DXE driver uses EFI_MP_SERVICES_PROTOCOL
to change some aspect of the processors in the system. For example, it
calls SwitchBSP, or calls EnableDisableAP.

- Then the information in the HOB will be stale, once PiSmmCpuDxeSmm
consumes it. The EFI_PROCESSOR_INFORMATION.StatusFlag field carries
information both about the CPU in question being BSP vs. AP, and about
the CPU being enabled or disabled. So either of SwitchBSP() and
EnableDisableAP() can invalidate the StatusFlag field for a given
ProcessorId.

I don't know how StandaloneMmPkg currently deals with this scenario, and
I also don't know whether StatusFlag actually matters to PiSmmCpuSmmDxe.
Apparently, it doesn't. So technically, the replacement of the protocol
with the HOB should be fine, but for the general case, we should
document somehow that specific fields of the HOB may be invalidated
between HOB production and HOB consumption. If platform code is required
to prevent that (i.e., the platform is responsible for not calling
SwitchBSP() or EnableDisableAP()), then that requirement should also be
documented.

Acked-by: Laszlo Ersek <lersek@redhat.com>


Thanks
Laszlo



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