1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v3 00/19] OvmfPkg: Add support for microvm machine type
  4. View patch

[edk2-devel] [PATCH v3 11/19] OvmfPkg/Microvm: PlatformPei/MemDetect tweaks

Gerd Hoffmann posted 19 patches 4 years, 5 months ago
There is a newer version of this series
[edk2-devel] [PATCH v3 11/19] OvmfPkg/Microvm: PlatformPei/MemDetect tweaks
Posted by Gerd Hoffmann 4 years, 5 months ago 
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>
---
 OvmfPkg/PlatformPei/MemDetect.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 2c2c4641ec8a..d736b85e0d90 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -135,6 +135,10 @@ QemuUc32BaseInitialization (
   UINT32 LowerMemorySize;
   UINT32 Uc32Size;
 
+  if (mHostBridgeDevId == 0xffff /* microvm */) {
+    return;
+  }
+
   if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
     //
     // On q35, the 32-bit area that we'll mark as UC, through variable MTRRs,
-- 
2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80335): https://edk2.groups.io/g/devel/message/80335
Mute This Topic: https://groups.io/mt/85454885/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 11/19] OvmfPkg/Microvm: PlatformPei/MemDetect tweaks
Posted by Leif Lindholm 4 years, 5 months ago 
On Wed, Sep 08, 2021 at 11:01:11 +0200, Gerd Hoffmann wrote:
> 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>
> ---
>  OvmfPkg/PlatformPei/MemDetect.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
> index 2c2c4641ec8a..d736b85e0d90 100644
> --- a/OvmfPkg/PlatformPei/MemDetect.c
> +++ b/OvmfPkg/PlatformPei/MemDetect.c
> @@ -135,6 +135,10 @@ QemuUc32BaseInitialization (
>    UINT32 LowerMemorySize;
>    UINT32 Uc32Size;
>  
> +  if (mHostBridgeDevId == 0xffff /* microvm */) {
> +    return;
> +  }
> +

This, and the same conditional in the subsequent patch, weirds me out
a bit. This doesn't tell us we're on microvm, it tells us the device
ID is invalid.
Since we know at compile-time that we want to skip this function,
could we achieve that some other way?

/
    Leif

>    if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
>      //
>      // On q35, the 32-bit area that we'll mark as UC, through variable MTRRs,
> -- 
> 2.31.1
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80344): https://edk2.groups.io/g/devel/message/80344
Mute This Topic: https://groups.io/mt/85454885/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 11/19] OvmfPkg/Microvm: PlatformPei/MemDetect tweaks
Posted by Gerd Hoffmann 4 years, 5 months ago 
On Wed, Sep 08, 2021 at 12:06:46PM +0100, Leif Lindholm wrote:
> On Wed, Sep 08, 2021 at 11:01:11 +0200, Gerd Hoffmann wrote:
> > 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>
> > ---
> >  OvmfPkg/PlatformPei/MemDetect.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
> > index 2c2c4641ec8a..d736b85e0d90 100644
> > --- a/OvmfPkg/PlatformPei/MemDetect.c
> > +++ b/OvmfPkg/PlatformPei/MemDetect.c
> > @@ -135,6 +135,10 @@ QemuUc32BaseInitialization (
> >    UINT32 LowerMemorySize;
> >    UINT32 Uc32Size;
> >  
> > +  if (mHostBridgeDevId == 0xffff /* microvm */) {
> > +    return;
> > +  }
> > +
> 
> This, and the same conditional in the subsequent patch, weirds me out
> a bit. This doesn't tell us we're on microvm, it tells us the device
> ID is invalid.

Well, sort of, yes.  microvm doesn't support pci config space access via
0xcf8, so any attempt to read something there returns 0xff

> Since we know at compile-time that we want to skip this function,
> could we achieve that some other way?

Sure.  Suggestions?  Add a Pcd and set it in Microvm.dsc?
Or is there some better way?

thanks,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80347): https://edk2.groups.io/g/devel/message/80347
Mute This Topic: https://groups.io/mt/85454885/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 11/19] OvmfPkg/Microvm: PlatformPei/MemDetect tweaks
Posted by Leif Lindholm 4 years, 5 months ago 
On Wed, Sep 08, 2021 at 13:33:51 +0200, Gerd Hoffmann wrote:
> On Wed, Sep 08, 2021 at 12:06:46PM +0100, Leif Lindholm wrote:
> > On Wed, Sep 08, 2021 at 11:01:11 +0200, Gerd Hoffmann wrote:
> > > 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>
> > > ---
> > >  OvmfPkg/PlatformPei/MemDetect.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
> > > index 2c2c4641ec8a..d736b85e0d90 100644
> > > --- a/OvmfPkg/PlatformPei/MemDetect.c
> > > +++ b/OvmfPkg/PlatformPei/MemDetect.c
> > > @@ -135,6 +135,10 @@ QemuUc32BaseInitialization (
> > >    UINT32 LowerMemorySize;
> > >    UINT32 Uc32Size;
> > >  
> > > +  if (mHostBridgeDevId == 0xffff /* microvm */) {
> > > +    return;
> > > +  }
> > > +
> > 
> > This, and the same conditional in the subsequent patch, weirds me out
> > a bit. This doesn't tell us we're on microvm, it tells us the device
> > ID is invalid.
> 
> Well, sort of, yes.  microvm doesn't support pci config space access via
> 0xcf8, so any attempt to read something there returns 0xff
> 
> > Since we know at compile-time that we want to skip this function,
> > could we achieve that some other way?
> 
> 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.

FixedPcds might be architecturally nicer, but should then probably be
a different one for each use-case.

/
    Leif


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80349): https://edk2.groups.io/g/devel/message/80349
Mute This Topic: https://groups.io/mt/85454885/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 11/19] OvmfPkg/Microvm: PlatformPei/MemDetect tweaks
Posted by Gerd Hoffmann 4 years, 5 months ago 
> > 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?

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 ();
 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80427): https://edk2.groups.io/g/devel/message/80427
Mute This Topic: https://groups.io/mt/85454885/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 11/19] OvmfPkg/Microvm: PlatformPei/MemDetect tweaks
Posted by Ard Biesheuvel 4 years, 4 months ago 
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 ();
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81509): https://edk2.groups.io/g/devel/message/81509
Mute This Topic: https://groups.io/mt/85454885/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.