Re: [edk2-devel] [PATCH 1/1] OvmfPkg: Reserve the Ovmf work area as RT_DATA

["Min Xu" <min.m.xu@intel.com>]

On August 23, 2022 9:07 PM, Lendacky,Thomas wrote:
> On 8/23/22 03:40, Xu, Min M wrote:
> > On August 23, 2022 3:38 PM, Gerd Hoffmann wrote:
> >>    Hi,
> >>
> >>> Ah, I forget to reserve the work-area as RT_Data in below code:
> >>>        BuildMemoryAllocationHob (
> >>>          (EFI_PHYSICAL_ADDRESS)(UINTN)FixedPcdGet32
> >> (PcdOvmfWorkAreaBase),
> >>>          (UINT64)(UINTN)FixedPcdGet32 (PcdOvmfWorkAreaSize),
> >>>          PlatformInfoHob->S3Supported ? EfiACPIMemoryNVS :
> >> EfiRuntimeServicesData  <-- ACPI_NVS is not accessible either in OS-
> Runtime.
> >>>          );
> >>>
> >> https://github.com/tianocore/edk2/blob/master/OvmfPkg/Library/Platfor
> >> m
> >>> InitLib/MemDetect.c#L1022-L1026
> >>
> >> With that changed to use EfiRuntimeServicesData unconditionally the
> >> page fault is gone.
> >>
> >>> Gerd, do you think we can reserve Ovmf WorkArea as RT_Data even when
> >> S3 is supported?
> >>
> >> Hmm, not fully sure how the various memory types are handled when the
> >> VM is suspended.
> >>
> > Tom suggested that the work area should not  be kept around. In stead a
> PCD should be used.
> > https://edk2.groups.io/g/devel/message/92617
> >
> > But the dynamic PCD is not thread-safe and it cannot be accessed by APs.
> >
> > Now we have 2 solutions to fix this issue.
> > 1. Reserve the Ovmf work area as RT_DATA.
> > 2. Split the CcProbeLib into 2 instances. See
> > https://edk2.groups.io/g/devel/message/91132
> >
> > Gerd & Tom, what's your thought?
> 
> Runtime service call are restricted so that you don't have concurrent threads
> executing (see section 8.1 of the specification). Without that you would have
> problems with runtime services today.
>
One of the situation of CcProbe used is in BaseIoLib.
https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseIoLibIntrinsic/IoLibInternalTdx.c#L35-L40

BaseIoLib is a basic library and it may be called by APs. While dynamic PCD access is not allowed in APs. Of course we can cache the PCD in a variable but I think it is still not safe.

Reserving the OVMF work area as RT_DATA breaks the original intention of the design.

Then how about this solution?
https://edk2.groups.io/g/devel/message/91132
We can design 2 instances of CcProbe. One is to read the OvmfWorkArea. The other is to call Cc guest specific way to determine the type.

Thanks
Min