[edk2-devel] [PATCH v1 0/1] OvmfPkg/Bhyve: build PlatformInfoHob

[edk2-devel] [PATCH v1 0/1] OvmfPkg/Bhyve: build PlatformInfoHob

[Corvin Köhne]

CI: https://github.com/tianocore/edk2/pull/4676

Corvin Köhne (1):
  OvmfPkg/Bhyve: build platform info HOB

 OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf |  1 +
 OvmfPkg/Bhyve/PlatformPei/Platform.c      | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

-- 
2.41.0



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

[edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Corvin Köhne]

Copy the function BuildPlatformInfoHob() from OvmfPkg/PlatformPei.

QemuFwCfgLib expect this HOB to be present, or fails to do anything.
InternalQemuFwCfgIsAvailable() from QemuFwCfgPeiLib module will not
check if the HOB is actually present for example and try to use a NULL
pointer.

Fixes: cda98df16228970dcf9a4ce2af5368219711b4b0 ("OvmfPkg/QemuFwCfgLib: remove mQemuFwCfgSupported + mQemuFwCfgDmaSupported")
Signed-off-by: Corvin Köhne <corvink@FreeBSD.org>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Rebecca Cran <rebecca@bsdio.com>
Cc: Peter Grehan <grehan@freebsd.org>
---
 OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf |  1 +
 OvmfPkg/Bhyve/PlatformPei/Platform.c      | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
index 739d63098b1e..07570d4e30ca 100644
--- a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
@@ -44,6 +44,7 @@ [Packages]
 
 [Guids]
   gEfiMemoryTypeInformationGuid
+  gUefiOvmfPkgPlatformInfoGuid
 
 [LibraryClasses]
   BaseLib
diff --git a/OvmfPkg/Bhyve/PlatformPei/Platform.c b/OvmfPkg/Bhyve/PlatformPei/Platform.c
index 5bfe435327c1..bdfc1614d284 100644
--- a/OvmfPkg/Bhyve/PlatformPei/Platform.c
+++ b/OvmfPkg/Bhyve/PlatformPei/Platform.c
@@ -17,6 +17,7 @@
 //
 // The Library classes this module consumes
 //
+#include <Library/BaseMemoryLib.h>
 #include <Library/BaseLib.h>
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
@@ -27,6 +28,7 @@
 #include <Library/PciLib.h>
 #include <Library/PeimEntryPoint.h>
 #include <Library/PeiServicesLib.h>
+#include <Library/PlatformInitLib.h>
 #include <Library/ResourcePublicationLib.h>
 #include <Guid/MemoryTypeInformation.h>
 #include <Ppi/MasterBootMode.h>
@@ -534,6 +536,23 @@ MaxCpuCountInitialization (
     ));
 }
 
+/**
+ * @brief Builds PlatformInfo Hob
+ */
+EFI_HOB_PLATFORM_INFO *
+BuildPlatformInfoHob (
+  VOID
+  )
+{
+  EFI_HOB_PLATFORM_INFO  PlatformInfoHob;
+  EFI_HOB_GUID_TYPE      *GuidHob;
+
+  ZeroMem (&PlatformInfoHob, sizeof PlatformInfoHob);
+  BuildGuidDataHob (&gUefiOvmfPkgPlatformInfoGuid, &PlatformInfoHob, sizeof (EFI_HOB_PLATFORM_INFO));
+  GuidHob = GetFirstGuidHob (&gUefiOvmfPkgPlatformInfoGuid);
+  return (EFI_HOB_PLATFORM_INFO *)GET_GUID_HOB_DATA (GuidHob);
+}
+
 /**
   Perform Platform PEI initialization.
 
@@ -551,6 +570,7 @@ InitializePlatform (
   )
 {
   DEBUG ((DEBUG_INFO, "Platform PEIM Loaded\n"));
+  BuildPlatformInfoHob();
 
   //
   // Initialize Local APIC Timer hardware and disable Local APIC Timer
-- 
2.41.0



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

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Rebecca Cran]

Nit: "sizeof" is normally used like a function. And this patch mixes the 
two styles.

Other than that:


Reviewed-by: Rebecca Cran <rebecca@bsdio.com>


-- 
Rebecca Cran


On 7/24/23 06:59, Corvin Köhne wrote:
> Copy the function BuildPlatformInfoHob() from OvmfPkg/PlatformPei.
>
> QemuFwCfgLib expect this HOB to be present, or fails to do anything.
> InternalQemuFwCfgIsAvailable() from QemuFwCfgPeiLib module will not
> check if the HOB is actually present for example and try to use a NULL
> pointer.
>
> Fixes: cda98df16228970dcf9a4ce2af5368219711b4b0 ("OvmfPkg/QemuFwCfgLib: remove mQemuFwCfgSupported + mQemuFwCfgDmaSupported")
> Signed-off-by: Corvin Köhne <corvink@FreeBSD.org>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Cc: Peter Grehan <grehan@freebsd.org>
> ---
>   OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf |  1 +
>   OvmfPkg/Bhyve/PlatformPei/Platform.c      | 20 ++++++++++++++++++++
>   2 files changed, 21 insertions(+)
>
> diff --git a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
> index 739d63098b1e..07570d4e30ca 100644
> --- a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
> @@ -44,6 +44,7 @@ [Packages]
>   
>   [Guids]
>     gEfiMemoryTypeInformationGuid
> +  gUefiOvmfPkgPlatformInfoGuid
>   
>   [LibraryClasses]
>     BaseLib
> diff --git a/OvmfPkg/Bhyve/PlatformPei/Platform.c b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> index 5bfe435327c1..bdfc1614d284 100644
> --- a/OvmfPkg/Bhyve/PlatformPei/Platform.c
> +++ b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> @@ -17,6 +17,7 @@
>   //
>   // The Library classes this module consumes
>   //
> +#include <Library/BaseMemoryLib.h>
>   #include <Library/BaseLib.h>
>   #include <Library/DebugLib.h>
>   #include <Library/HobLib.h>
> @@ -27,6 +28,7 @@
>   #include <Library/PciLib.h>
>   #include <Library/PeimEntryPoint.h>
>   #include <Library/PeiServicesLib.h>
> +#include <Library/PlatformInitLib.h>
>   #include <Library/ResourcePublicationLib.h>
>   #include <Guid/MemoryTypeInformation.h>
>   #include <Ppi/MasterBootMode.h>
> @@ -534,6 +536,23 @@ MaxCpuCountInitialization (
>       ));
>   }
>   
> +/**
> + * @brief Builds PlatformInfo Hob
> + */
> +EFI_HOB_PLATFORM_INFO *
> +BuildPlatformInfoHob (
> +  VOID
> +  )
> +{
> +  EFI_HOB_PLATFORM_INFO  PlatformInfoHob;
> +  EFI_HOB_GUID_TYPE      *GuidHob;
> +
> +  ZeroMem (&PlatformInfoHob, sizeof PlatformInfoHob);
> +  BuildGuidDataHob (&gUefiOvmfPkgPlatformInfoGuid, &PlatformInfoHob, sizeof (EFI_HOB_PLATFORM_INFO));
> +  GuidHob = GetFirstGuidHob (&gUefiOvmfPkgPlatformInfoGuid);
> +  return (EFI_HOB_PLATFORM_INFO *)GET_GUID_HOB_DATA (GuidHob);
> +}
> +
>   /**
>     Perform Platform PEI initialization.
>   
> @@ -551,6 +570,7 @@ InitializePlatform (
>     )
>   {
>     DEBUG ((DEBUG_INFO, "Platform PEIM Loaded\n"));
> +  BuildPlatformInfoHob();
>   
>     //
>     // Initialize Local APIC Timer hardware and disable Local APIC Timer


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

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Ard Biesheuvel]

On Tue, 8 Aug 2023 at 15:33, Rebecca Cran <rebecca@bsdio.com> wrote:
>
> Nit: "sizeof" is normally used like a function. And this patch mixes the
> two styles.
>

Actually, this is not unidiomatic for OVMF - sizeof takes either a
type name (in which case the () are required) or an expression (in
which case they are optional), and we have always permitted both
idioms for the latter case.

$ git grep -cE 'sizeof\s*[:alnum:]' OvmfPkg/
OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.c:4
OvmfPkg/EnrollDefaultKeys/AuthData.c:4
OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c:7
OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c:1
OvmfPkg/PlatformPei/MemTypeInfo.c:1
OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c:5
OvmfPkg/QemuVideoDxe/VbeShim.c:4
OvmfPkg/SmmControl2Dxe/SmiFeatures.c:4
OvmfPkg/Virtio10Dxe/Virtio10.c:1
OvmfPkg/VirtioGpuDxe/DriverBinding.c:4


> Other than that:
>
>
> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
>

Thanks


I've tried to merge this but I am getting errors and I cannot figure
out what they are:

https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=99505&view=results


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

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 2034 bytes --]

On Tue, 2023-08-08 at 18:25 +0200, Ard Biesheuvel wrote:
> On Tue, 8 Aug 2023 at 15:33, Rebecca Cran <rebecca@bsdio.com> wrote:
> > 
> > Nit: "sizeof" is normally used like a function. And this patch
> > mixes the
> > two styles.
> > 
> 
> Actually, this is not unidiomatic for OVMF - sizeof takes either a
> type name (in which case the () are required) or an expression (in
> which case they are optional), and we have always permitted both
> idioms for the latter case.
> 
> $ git grep -cE 'sizeof\s*[:alnum:]' OvmfPkg/
> OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.c:4
> OvmfPkg/EnrollDefaultKeys/AuthData.c:4
> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.
> c:7
> OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c:1
> OvmfPkg/PlatformPei/MemTypeInfo.c:1
> OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c:5
> OvmfPkg/QemuVideoDxe/VbeShim.c:4
> OvmfPkg/SmmControl2Dxe/SmiFeatures.c:4
> OvmfPkg/Virtio10Dxe/Virtio10.c:1
> OvmfPkg/VirtioGpuDxe/DriverBinding.c:4
> 
> 
> > Other than that:
> > 
> > 
> > Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
> > 
> 
> Thanks
> 
> 
> I've tried to merge this but I am getting errors and I cannot figure
> out what they are:
> 
> https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=99505&view=results
> 

Thanks. Have you tried
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting#how-to-find-uncrustify-formatting-errors-in-continuous-integration-ci

I'm stuck at step 5. I can't see something like "92.5% passed". Maybe I
don't have the permission to see it?

> 
> 
> 
> 

-- 
Kind regards,
Corvin


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



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Ard Biesheuvel]

On Thu, 10 Aug 2023 at 14:53, Corvin Köhne <corvink@freebsd.org> wrote:
>
> On Tue, 2023-08-08 at 18:25 +0200, Ard Biesheuvel wrote:
> > On Tue, 8 Aug 2023 at 15:33, Rebecca Cran <rebecca@bsdio.com> wrote:
> > >
> > > Nit: "sizeof" is normally used like a function. And this patch
> > > mixes the
> > > two styles.
> > >
> >
> > Actually, this is not unidiomatic for OVMF - sizeof takes either a
> > type name (in which case the () are required) or an expression (in
> > which case they are optional), and we have always permitted both
> > idioms for the latter case.
> >
> > $ git grep -cE 'sizeof\s*[:alnum:]' OvmfPkg/
> > OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.c:4
> > OvmfPkg/EnrollDefaultKeys/AuthData.c:4
> > OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.
> > c:7
> > OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c:1
> > OvmfPkg/PlatformPei/MemTypeInfo.c:1
> > OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c:5
> > OvmfPkg/QemuVideoDxe/VbeShim.c:4
> > OvmfPkg/SmmControl2Dxe/SmiFeatures.c:4
> > OvmfPkg/Virtio10Dxe/Virtio10.c:1
> > OvmfPkg/VirtioGpuDxe/DriverBinding.c:4
> >
> >
> > > Other than that:
> > >
> > >
> > > Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
> > >
> >
> > Thanks
> >
> >
> > I've tried to merge this but I am getting errors and I cannot figure
> > out what they are:
> >
> > https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=99505&view=results
> >
>
> Thanks. Have you tried
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting#how-to-find-uncrustify-formatting-errors-in-continuous-integration-ci
>
> I'm stuck at step 5. I can't see something like "92.5% passed". Maybe I
> don't have the permission to see it?
>

I was having the same problem. The instructions don't match the actual
situation, so it is impossible for me to figure why the CI decided to
reject the series.


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

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 3206 bytes --]

On Thu, 2023-08-10 at 14:55 +0200, Ard Biesheuvel wrote:
> On Thu, 10 Aug 2023 at 14:53, Corvin Köhne <corvink@freebsd.org>
> wrote:
> > 
> > On Tue, 2023-08-08 at 18:25 +0200, Ard Biesheuvel wrote:
> > > On Tue, 8 Aug 2023 at 15:33, Rebecca Cran <rebecca@bsdio.com>
> > > wrote:
> > > > 
> > > > Nit: "sizeof" is normally used like a function. And this patch
> > > > mixes the
> > > > two styles.
> > > > 
> > > 
> > > Actually, this is not unidiomatic for OVMF - sizeof takes either
> > > a
> > > type name (in which case the () are required) or an expression
> > > (in
> > > which case they are optional), and we have always permitted both
> > > idioms for the latter case.
> > > 
> > > $ git grep -cE 'sizeof\s*[:alnum:]' OvmfPkg/
> > > OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.c:4
> > > OvmfPkg/EnrollDefaultKeys/AuthData.c:4
> > > OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupp
> > > ort.
> > > c:7
> > > OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
> > > :1
> > > OvmfPkg/PlatformPei/MemTypeInfo.c:1
> > > OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c:5
> > > OvmfPkg/QemuVideoDxe/VbeShim.c:4
> > > OvmfPkg/SmmControl2Dxe/SmiFeatures.c:4
> > > OvmfPkg/Virtio10Dxe/Virtio10.c:1
> > > OvmfPkg/VirtioGpuDxe/DriverBinding.c:4
> > > 
> > > 
> > > > Other than that:
> > > > 
> > > > 
> > > > Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
> > > > 
> > > 
> > > Thanks
> > > 
> > > 
> > > I've tried to merge this but I am getting errors and I cannot
> > > figure
> > > out what they are:
> > > 
> > > https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=99505&view=results
> > > 
> > 
> > Thanks. Have you tried
> > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting#how-to-find-uncrustify-formatting-errors-in-continuous-integration-ci
> > 
> > I'm stuck at step 5. I can't see something like "92.5% passed".
> > Maybe I
> > don't have the permission to see it?
> > 
> 
> I was having the same problem. The instructions don't match the
> actual
> situation, so it is impossible for me to figure why the CI decided to
> reject the series.

The function is copied from OvmfPkg/PlatformPei. So, it's unlikely that
it causes issues. Nevertheless, I found a candidate:

> diff --git a/OvmfPkg/Bhyve/PlatformPei/Platform.c
b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> index 5bfe435327c1..bdfc1614d284 100644
> --- a/OvmfPkg/Bhyve/PlatformPei/Platform.c
> +++ b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> @@ -17,6 +17,7 @@
>   //
>   // The Library classes this module consumes
>   //
> +#include <Library/BaseMemoryLib.h>
>   #include <Library/BaseLib.h>

This is not in alphabetical order.

>   #include <Library/DebugLib.h>
>   #include <Library/HobLib.h>


-- 
Kind regards,
Corvin


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



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Ard Biesheuvel]

Could someone please decipher this for us please? I am sick and tired
of the CI impeding my work.


On Thu, 10 Aug 2023 at 15:33, Corvin Köhne <corvink@freebsd.org> wrote:
>
> On Thu, 2023-08-10 at 14:55 +0200, Ard Biesheuvel wrote:
> > On Thu, 10 Aug 2023 at 14:53, Corvin Köhne <corvink@freebsd.org>
> > wrote:
> > >
> > > On Tue, 2023-08-08 at 18:25 +0200, Ard Biesheuvel wrote:
> > > > On Tue, 8 Aug 2023 at 15:33, Rebecca Cran <rebecca@bsdio.com>
> > > > wrote:
> > > > >
> > > > > Nit: "sizeof" is normally used like a function. And this patch
> > > > > mixes the
> > > > > two styles.
> > > > >
> > > >
> > > > Actually, this is not unidiomatic for OVMF - sizeof takes either
> > > > a
> > > > type name (in which case the () are required) or an expression
> > > > (in
> > > > which case they are optional), and we have always permitted both
> > > > idioms for the latter case.
> > > >
> > > > $ git grep -cE 'sizeof\s*[:alnum:]' OvmfPkg/
> > > > OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.c:4
> > > > OvmfPkg/EnrollDefaultKeys/AuthData.c:4
> > > > OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupp
> > > > ort.
> > > > c:7
> > > > OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
> > > > :1
> > > > OvmfPkg/PlatformPei/MemTypeInfo.c:1
> > > > OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c:5
> > > > OvmfPkg/QemuVideoDxe/VbeShim.c:4
> > > > OvmfPkg/SmmControl2Dxe/SmiFeatures.c:4
> > > > OvmfPkg/Virtio10Dxe/Virtio10.c:1
> > > > OvmfPkg/VirtioGpuDxe/DriverBinding.c:4
> > > >
> > > >
> > > > > Other than that:
> > > > >
> > > > >
> > > > > Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
> > > > >
> > > >
> > > > Thanks
> > > >
> > > >
> > > > I've tried to merge this but I am getting errors and I cannot
> > > > figure
> > > > out what they are:
> > > >
> > > > https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=99505&view=results
> > > >
> > >
> > > Thanks. Have you tried
> > > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting#how-to-find-uncrustify-formatting-errors-in-continuous-integration-ci
> > >
> > > I'm stuck at step 5. I can't see something like "92.5% passed".
> > > Maybe I
> > > don't have the permission to see it?
> > >
> >
> > I was having the same problem. The instructions don't match the
> > actual
> > situation, so it is impossible for me to figure why the CI decided to
> > reject the series.
>
> The function is copied from OvmfPkg/PlatformPei. So, it's unlikely that
> it causes issues. Nevertheless, I found a candidate:
>
> > diff --git a/OvmfPkg/Bhyve/PlatformPei/Platform.c
> b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> > index 5bfe435327c1..bdfc1614d284 100644
> > --- a/OvmfPkg/Bhyve/PlatformPei/Platform.c
> > +++ b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> > @@ -17,6 +17,7 @@
> >   //
> >   // The Library classes this module consumes
> >   //
> > +#include <Library/BaseMemoryLib.h>
> >   #include <Library/BaseLib.h>
>
> This is not in alphabetical order.
>
> >   #include <Library/DebugLib.h>
> >   #include <Library/HobLib.h>
>
>
> --
> Kind regards,
> Corvin


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

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Michael Kubacki]

I'm not sure why test results are not being reported that may be a 
larger issue.

I pulled the branch locally, run uncrustify, and it automatically fixed 
the code to pass.

The issue is the following function call:

   BuildPlatformInfoHob();

Should be:

   BuildPlatformInfoHob ();


On 8/10/2023 11:46 AM, Ard Biesheuvel wrote:
> Could someone please decipher this for us please? I am sick and tired
> of the CI impeding my work.
> 
> 
> On Thu, 10 Aug 2023 at 15:33, Corvin Köhne <corvink@freebsd.org> wrote:
>>
>> On Thu, 2023-08-10 at 14:55 +0200, Ard Biesheuvel wrote:
>>> On Thu, 10 Aug 2023 at 14:53, Corvin Köhne <corvink@freebsd.org>
>>> wrote:
>>>>
>>>> On Tue, 2023-08-08 at 18:25 +0200, Ard Biesheuvel wrote:
>>>>> On Tue, 8 Aug 2023 at 15:33, Rebecca Cran <rebecca@bsdio.com>
>>>>> wrote:
>>>>>>
>>>>>> Nit: "sizeof" is normally used like a function. And this patch
>>>>>> mixes the
>>>>>> two styles.
>>>>>>
>>>>>
>>>>> Actually, this is not unidiomatic for OVMF - sizeof takes either
>>>>> a
>>>>> type name (in which case the () are required) or an expression
>>>>> (in
>>>>> which case they are optional), and we have always permitted both
>>>>> idioms for the latter case.
>>>>>
>>>>> $ git grep -cE 'sizeof\s*[:alnum:]' OvmfPkg/
>>>>> OvmfPkg/Bhyve/BhyveRfbDxe/VbeShim.c:4
>>>>> OvmfPkg/EnrollDefaultKeys/AuthData.c:4
>>>>> OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupp
>>>>> ort.
>>>>> c:7
>>>>> OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
>>>>> :1
>>>>> OvmfPkg/PlatformPei/MemTypeInfo.c:1
>>>>> OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c:5
>>>>> OvmfPkg/QemuVideoDxe/VbeShim.c:4
>>>>> OvmfPkg/SmmControl2Dxe/SmiFeatures.c:4
>>>>> OvmfPkg/Virtio10Dxe/Virtio10.c:1
>>>>> OvmfPkg/VirtioGpuDxe/DriverBinding.c:4
>>>>>
>>>>>
>>>>>> Other than that:
>>>>>>
>>>>>>
>>>>>> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
>>>>>>
>>>>>
>>>>> Thanks
>>>>>
>>>>>
>>>>> I've tried to merge this but I am getting errors and I cannot
>>>>> figure
>>>>> out what they are:
>>>>>
>>>>> https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=99505&view=results
>>>>>
>>>>
>>>> Thanks. Have you tried
>>>> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting#how-to-find-uncrustify-formatting-errors-in-continuous-integration-ci
>>>>
>>>> I'm stuck at step 5. I can't see something like "92.5% passed".
>>>> Maybe I
>>>> don't have the permission to see it?
>>>>
>>>
>>> I was having the same problem. The instructions don't match the
>>> actual
>>> situation, so it is impossible for me to figure why the CI decided to
>>> reject the series.
>>
>> The function is copied from OvmfPkg/PlatformPei. So, it's unlikely that
>> it causes issues. Nevertheless, I found a candidate:
>>
>>> diff --git a/OvmfPkg/Bhyve/PlatformPei/Platform.c
>> b/OvmfPkg/Bhyve/PlatformPei/Platform.c
>>> index 5bfe435327c1..bdfc1614d284 100644
>>> --- a/OvmfPkg/Bhyve/PlatformPei/Platform.c
>>> +++ b/OvmfPkg/Bhyve/PlatformPei/Platform.c
>>> @@ -17,6 +17,7 @@
>>>    //
>>>    // The Library classes this module consumes
>>>    //
>>> +#include <Library/BaseMemoryLib.h>
>>>    #include <Library/BaseLib.h>
>>
>> This is not in alphabetical order.
>>
>>>    #include <Library/DebugLib.h>
>>>    #include <Library/HobLib.h>
>>
>>
>> --
>> Kind regards,
>> Corvin
> 
> 
> 
> 


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

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

On Thu, 2023-08-10 at 15:54 -0400, Michael Kubacki wrote:
> I'm not sure why test results are not being reported that may be a 
> larger issue.
> 
> I pulled the branch locally, run uncrustify, and it automatically
> fixed 
> the code to pass.
> 
> The issue is the following function call:
> 
>    BuildPlatformInfoHob();
> 
> Should be:
> 
>    BuildPlatformInfoHob ();
> 

I already run uncrustify locally but it doesn't complain about it.
Thank you!

Should I resend an updated patch?
Can this change make it into edk-stable202308? Otherwise, fwcfg won't
be usable by bhyve in this version.


-- 
Kind regards,
Corvin


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



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Ard Biesheuvel]

On Mon, 14 Aug 2023 at 10:15, Corvin Köhne <corvink@freebsd.org> wrote:
>
> On Thu, 2023-08-10 at 15:54 -0400, Michael Kubacki wrote:
> > I'm not sure why test results are not being reported that may be a
> > larger issue.
> >
> > I pulled the branch locally, run uncrustify, and it automatically
> > fixed
> > the code to pass.
> >
> > The issue is the following function call:
> >
> >    BuildPlatformInfoHob();
> >
> > Should be:
> >
> >    BuildPlatformInfoHob ();
> >
>
> I already run uncrustify locally but it doesn't complain about it.
> Thank you!
>
> Should I resend an updated patch?
> Can this change make it into edk-stable202308? Otherwise, fwcfg won't
> be usable by bhyve in this version.
>

It is up to Liming to decide but given that

- it is low risk as it only touches bhyve platform code
- i would have merged it already if bugs in the CI wouldn't have
prevented me from doing so

I would be in favour of including this change


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

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Ard Biesheuvel]

On Tue, 15 Aug 2023 at 10:10, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 14 Aug 2023 at 10:15, Corvin Köhne <corvink@freebsd.org> wrote:
> >
> > On Thu, 2023-08-10 at 15:54 -0400, Michael Kubacki wrote:
> > > I'm not sure why test results are not being reported that may be a
> > > larger issue.
> > >
> > > I pulled the branch locally, run uncrustify, and it automatically
> > > fixed
> > > the code to pass.
> > >
> > > The issue is the following function call:
> > >
> > >    BuildPlatformInfoHob();
> > >
> > > Should be:
> > >
> > >    BuildPlatformInfoHob ();
> > >
> >
> > I already run uncrustify locally but it doesn't complain about it.
> > Thank you!
> >
> > Should I resend an updated patch?
> > Can this change make it into edk-stable202308? Otherwise, fwcfg won't
> > be usable by bhyve in this version.
> >
>
> It is up to Liming to decide but given that
>
> - it is low risk as it only touches bhyve platform code
> - i would have merged it already if bugs in the CI wouldn't have
> prevented me from doing so
>
> I would be in favour of including this change

I accidentally pushed this change to master by pushing it to a branch
that was already associated with a PR with the push label set.

I will leave it to Liming and the stewards to decide whether to leave
it or revert it.

Apologies - I wasn't expecting this to be permitted by the CI rules
during the freeze.


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

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 3585 bytes --]

On Mon, 2023-07-24 at 14:59 +0200, Corvin Köhne wrote:
> Copy the function BuildPlatformInfoHob() from OvmfPkg/PlatformPei.
> 
> QemuFwCfgLib expect this HOB to be present, or fails to do anything.
> InternalQemuFwCfgIsAvailable() from QemuFwCfgPeiLib module will not
> check if the HOB is actually present for example and try to use a
> NULL
> pointer.
> 
> Fixes: cda98df16228970dcf9a4ce2af5368219711b4b0
> ("OvmfPkg/QemuFwCfgLib: remove mQemuFwCfgSupported +
> mQemuFwCfgDmaSupported")
> Signed-off-by: Corvin Köhne <corvink@FreeBSD.org>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Cc: Peter Grehan <grehan@freebsd.org>
> ---
>  OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf |  1 +
>  OvmfPkg/Bhyve/PlatformPei/Platform.c      | 20 ++++++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
> b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
> index 739d63098b1e..07570d4e30ca 100644
> --- a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
> @@ -44,6 +44,7 @@ [Packages]
>  
>  [Guids]
>    gEfiMemoryTypeInformationGuid
> +  gUefiOvmfPkgPlatformInfoGuid
>  
>  [LibraryClasses]
>    BaseLib
> diff --git a/OvmfPkg/Bhyve/PlatformPei/Platform.c
> b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> index 5bfe435327c1..bdfc1614d284 100644
> --- a/OvmfPkg/Bhyve/PlatformPei/Platform.c
> +++ b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> @@ -17,6 +17,7 @@
>  //
>  // The Library classes this module consumes
>  //
> +#include <Library/BaseMemoryLib.h>
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/HobLib.h>
> @@ -27,6 +28,7 @@
>  #include <Library/PciLib.h>
>  #include <Library/PeimEntryPoint.h>
>  #include <Library/PeiServicesLib.h>
> +#include <Library/PlatformInitLib.h>
>  #include <Library/ResourcePublicationLib.h>
>  #include <Guid/MemoryTypeInformation.h>
>  #include <Ppi/MasterBootMode.h>
> @@ -534,6 +536,23 @@ MaxCpuCountInitialization (
>      ));
>  }
>  
> +/**
> + * @brief Builds PlatformInfo Hob
> + */
> +EFI_HOB_PLATFORM_INFO *
> +BuildPlatformInfoHob (
> +  VOID
> +  )
> +{
> +  EFI_HOB_PLATFORM_INFO  PlatformInfoHob;
> +  EFI_HOB_GUID_TYPE      *GuidHob;
> +
> +  ZeroMem (&PlatformInfoHob, sizeof PlatformInfoHob);
> +  BuildGuidDataHob (&gUefiOvmfPkgPlatformInfoGuid, &PlatformInfoHob,
> sizeof (EFI_HOB_PLATFORM_INFO));
> +  GuidHob = GetFirstGuidHob (&gUefiOvmfPkgPlatformInfoGuid);
> +  return (EFI_HOB_PLATFORM_INFO *)GET_GUID_HOB_DATA (GuidHob);
> +}
> +
>  /**
>    Perform Platform PEI initialization.
>  
> @@ -551,6 +570,7 @@ InitializePlatform (
>    )
>  {
>    DEBUG ((DEBUG_INFO, "Platform PEIM Loaded\n"));
> +  BuildPlatformInfoHob();
>  
>    //
>    // Initialize Local APIC Timer hardware and disable Local APIC
> Timer

Any comments?

CI: https://github.com/tianocore/edk2/pull/4676


-- 
Kind regards,
Corvin


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



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] OvmfPkg/Bhyve: build platform info HOB

[Ard Biesheuvel]

On Mon, 7 Aug 2023 at 13:31, Corvin Köhne <corvink@freebsd.org> wrote:
>
> On Mon, 2023-07-24 at 14:59 +0200, Corvin Köhne wrote:
> > Copy the function BuildPlatformInfoHob() from OvmfPkg/PlatformPei.
> >
> > QemuFwCfgLib expect this HOB to be present, or fails to do anything.
> > InternalQemuFwCfgIsAvailable() from QemuFwCfgPeiLib module will not
> > check if the HOB is actually present for example and try to use a
> > NULL
> > pointer.
> >
> > Fixes: cda98df16228970dcf9a4ce2af5368219711b4b0
> > ("OvmfPkg/QemuFwCfgLib: remove mQemuFwCfgSupported +
> > mQemuFwCfgDmaSupported")
> > Signed-off-by: Corvin Köhne <corvink@FreeBSD.org>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Rebecca Cran <rebecca@bsdio.com>
> > Cc: Peter Grehan <grehan@freebsd.org>
> > ---
> >  OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf |  1 +
> >  OvmfPkg/Bhyve/PlatformPei/Platform.c      | 20 ++++++++++++++++++++
> >  2 files changed, 21 insertions(+)
> >

Could someone review this please?




> > diff --git a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
> > b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
> > index 739d63098b1e..07570d4e30ca 100644
> > --- a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
> > +++ b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
> > @@ -44,6 +44,7 @@ [Packages]
> >
> >  [Guids]
> >    gEfiMemoryTypeInformationGuid
> > +  gUefiOvmfPkgPlatformInfoGuid
> >
> >  [LibraryClasses]
> >    BaseLib
> > diff --git a/OvmfPkg/Bhyve/PlatformPei/Platform.c
> > b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> > index 5bfe435327c1..bdfc1614d284 100644
> > --- a/OvmfPkg/Bhyve/PlatformPei/Platform.c
> > +++ b/OvmfPkg/Bhyve/PlatformPei/Platform.c
> > @@ -17,6 +17,7 @@
> >  //
> >  // The Library classes this module consumes
> >  //
> > +#include <Library/BaseMemoryLib.h>
> >  #include <Library/BaseLib.h>
> >  #include <Library/DebugLib.h>
> >  #include <Library/HobLib.h>
> > @@ -27,6 +28,7 @@
> >  #include <Library/PciLib.h>
> >  #include <Library/PeimEntryPoint.h>
> >  #include <Library/PeiServicesLib.h>
> > +#include <Library/PlatformInitLib.h>
> >  #include <Library/ResourcePublicationLib.h>
> >  #include <Guid/MemoryTypeInformation.h>
> >  #include <Ppi/MasterBootMode.h>
> > @@ -534,6 +536,23 @@ MaxCpuCountInitialization (
> >      ));
> >  }
> >
> > +/**
> > + * @brief Builds PlatformInfo Hob
> > + */
> > +EFI_HOB_PLATFORM_INFO *
> > +BuildPlatformInfoHob (
> > +  VOID
> > +  )
> > +{
> > +  EFI_HOB_PLATFORM_INFO  PlatformInfoHob;
> > +  EFI_HOB_GUID_TYPE      *GuidHob;
> > +
> > +  ZeroMem (&PlatformInfoHob, sizeof PlatformInfoHob);
> > +  BuildGuidDataHob (&gUefiOvmfPkgPlatformInfoGuid, &PlatformInfoHob,
> > sizeof (EFI_HOB_PLATFORM_INFO));
> > +  GuidHob = GetFirstGuidHob (&gUefiOvmfPkgPlatformInfoGuid);
> > +  return (EFI_HOB_PLATFORM_INFO *)GET_GUID_HOB_DATA (GuidHob);
> > +}
> > +
> >  /**
> >    Perform Platform PEI initialization.
> >
> > @@ -551,6 +570,7 @@ InitializePlatform (
> >    )
> >  {
> >    DEBUG ((DEBUG_INFO, "Platform PEIM Loaded\n"));
> > +  BuildPlatformInfoHob();
> >
> >    //
> >    // Initialize Local APIC Timer hardware and disable Local APIC
> > Timer
>
> Any comments?
>
> CI: https://github.com/tianocore/edk2/pull/4676
>
>
> --
> Kind regards,
> Corvin
>
> Attachments:
> signature.asc: https://edk2.groups.io/g/devel/attachment/107616/0
>
>
> 
>
>


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