Re: [PATCH v3 11/19] OvmfPkg/Microvm: PlatformPei/MemDetect tweaks

["Ard Biesheuvel" <ardb@kernel.org>]

On Thu, 9 Sept 2021 at 12:58, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > > Sure.  Suggestions?  Add a Pcd and set it in Microvm.dsc?
> > > Or is there some better way?
> >
> > It's all a question of how much we want to overengineer things :)
> >
> > I'm tempted to suggest a balanced version would be adding
> >   GCC: *_*_*_CC_FLAGS = -D PLATFORM_IS_MICROVM
> > to [BuildOptions] in the .dsc, and test for that.
>
> How about the approach below?
>

I am going to go with the original patch. All the workarounds seem
rather intrusive, and there is nothing wrong with probing the config
space and concluding that nothing is there if the response has all
bits set to 1. And to be pedantic, the reason 0xffff is an invalid
device ID is because it cannot be distinguished from a failed read. So
we are not checking whether the device exists and has an invalid
device ID, we are checking whether there's anything there to begin
with.


> take care,
>   Gerd
>
> commit 2d48e3eba022ba92eadcbad2c55e10ed281631c2
> Author: Gerd Hoffmann <kraxel@redhat.com>
> Date:   Tue Jun 1 12:38:38 2021 +0200
>
>     OvmfPkg/Microvm: PlatformPei/MemDetect tweaks
>
>     Set mHostBridgeDevId to MICROVM_PSEUDO_DEVICE_ID using a
>     compile time switch.
>
>     Skip host bridge setup on microvm.
>
>     Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3599
>     Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>     Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
>
> diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
> index 019b50de7d8f..a000c195d866 100644
> --- a/OvmfPkg/Microvm/MicrovmX64.dsc
> +++ b/OvmfPkg/Microvm/MicrovmX64.dsc
> @@ -73,6 +73,9 @@ [Defines]
>  !endif
>
>  [BuildOptions]
> +  GCC:*_*_*_CC_FLAGS                   = -DPLATFORM_IS_MICROVM
> +  INTEL:*_*_*_CC_FLAGS                 = /D PLATFORM_IS_MICROVM
> +  MSFT:*_*_*_CC_FLAGS                  = /D PLATFORM_IS_MICROVM
>    GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
>    INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
>    MSFT:RELEASE_*_*_CC_FLAGS            = /D MDEPKG_NDEBUG
> diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
> index 2c2c4641ec8a..8125644bc91a 100644
> --- a/OvmfPkg/PlatformPei/MemDetect.c
> +++ b/OvmfPkg/PlatformPei/MemDetect.c
> @@ -15,6 +15,7 @@ Module Name:
>  //
>  #include <IndustryStandard/E820.h>
>  #include <IndustryStandard/I440FxPiix4.h>
> +#include <IndustryStandard/Microvm.h>
>  #include <IndustryStandard/Q35MchIch9.h>
>  #include <PiPei.h>
>  #include <Register/Intel/SmramSaveStateMap.h>
> @@ -135,6 +136,10 @@ QemuUc32BaseInitialization (
>    UINT32 LowerMemorySize;
>    UINT32 Uc32Size;
>
> +  if (mHostBridgeDevId == MICROVM_PSEUDO_DEVICE_ID) {
> +    return;
> +  }
> +
>    if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
>      //
>      // On q35, the 32-bit area that we'll mark as UC, through variable MTRRs,
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index d3a20122a2ea..ed93d11c8ac6 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -31,6 +31,7 @@
>  #include <Library/ResourcePublicationLib.h>
>  #include <Ppi/MasterBootMode.h>
>  #include <IndustryStandard/I440FxPiix4.h>
> +#include <IndustryStandard/Microvm.h>
>  #include <IndustryStandard/Pci22.h>
>  #include <IndustryStandard/Q35MchIch9.h>
>  #include <IndustryStandard/QemuCpuHotplug.h>
> @@ -714,7 +715,11 @@ InitializePlatform (
>    //
>    // Query Host Bridge DID
>    //
> +#ifdef PLATFORM_IS_MICROVM
> +  mHostBridgeDevId = MICROVM_PSEUDO_DEVICE_ID;
> +#else
>    mHostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID);
> +#endif
>
>    MaxCpuCountInitialization ();
>
>