Laszlo,
Thanks for your comments.
Yes the StatusFlag field of a given ProcessorId does change in the scenario you
mentioned. I think it's ok to call SwitchBSP() and Enable/DisableAP() after
creating the hob, since smm elects its own BSP and all CPU will enter smm after
receiving smi regardless of the StatusFlag.
So the NumberOfProcessors, the ProcessorId and Location fields remain
unchanged, the StatusFlag and NumberOfEnabledProcessors may be invalidated. I
agree that we should document specific fields of the HOB may be invalidated
between HOB production and HOB consumption. Will send V2 patch set to document
this in the HOB definition head file.
Thanks,
Dun
-----Original Message-----
From: Laszlo Ersek <ler...@redhat.com>
Sent: Monday, November 13, 2023 10:33 PM
To: devel@edk2.groups.io; Tan, Dun <dun....@intel.com>
Subject: Re: [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from
StandaloneMmPkg to UefiCpuPkg.
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 <ler...@redhat.com>
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111222): https://edk2.groups.io/g/devel/message/111222
Mute This Topic: https://groups.io/mt/102479007/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
