Re: [edk2-devel] [PATCH v6] OvmfPkg/Bhyve: add support for QemuFwCfg

["Rebecca Cran" <quic_rcran@quicinc.com>]

The Standard_Error_Output.log file isn't even really useful (it should 
show a diff), but there's something wrong with the coding style in Bhyve.c:

Files with formatting errors:
  Formatting errors in Bhyve/AcpiPlatformDxe/Bhyve.c


-- 
Rebecca Cran


On 4/29/22 07:55, Ard Biesheuvel wrote:
> (+ sean, mike)
>
> I tried to merge this but it is rejected by CI, and I got lost in the
> maze of CI status and report pages. Could someone please figure out
> what's wrong?
>
> https://github.com/tianocore/edk2/pull/2841
>
> On Wed, 27 Apr 2022 at 12:07, Corvin Köhne <c.koehne@beckhoff.com> wrote:
>> QemuFwCfg is much more powerful than BhyveFwCtl. Sadly, BhyveFwCtl
>> decided to use the same IO ports as QemuFwCfg. It's not possible to use
>> both interfaces simultaneously. So, prefer QemuFwCfg over BhyveFwCtl.
>>
>> Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
>> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
>> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>> Acked-by: Peter Grehan <grehan@freebsd.org>
>> Acked-by: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: EDKII devel group <devel@edk2.groups.io>
>> Cc: FreeBSD Virtualization <freebsd-virtualization@freebsd.org>
>> ---
>>   OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatformDxe.inf |  1 +
>>   OvmfPkg/Bhyve/AcpiPlatformDxe/Bhyve.c             | 41 ++++++++++++++++++++---
>>   OvmfPkg/Bhyve/BhyveX64.dsc                        |  4 +--
>>   3 files changed, 40 insertions(+), 6 deletions(-)
>>
>> diff --git a/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> index 595fd055f9..94c65f32dc 100644
>> --- a/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> +++ b/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> @@ -43,6 +43,7 @@
>>     MemoryAllocationLib
>>     OrderedCollectionLib
>>     PcdLib
>> +  QemuFwCfgLib
>>     UefiBootServicesTableLib
>>     UefiDriverEntryPoint
>>     UefiLib
>> diff --git a/OvmfPkg/Bhyve/AcpiPlatformDxe/Bhyve.c b/OvmfPkg/Bhyve/AcpiPlatformDxe/Bhyve.c
>> index 8e80aa33e1..e216a21bfa 100644
>> --- a/OvmfPkg/Bhyve/AcpiPlatformDxe/Bhyve.c
>> +++ b/OvmfPkg/Bhyve/AcpiPlatformDxe/Bhyve.c
>> @@ -11,6 +11,41 @@
>>   #include <Library/BaseMemoryLib.h>
>>   #include <Library/BhyveFwCtlLib.h>
>>   #include <Library/MemoryAllocationLib.h>
>> +#include <Library/QemuFwCfgLib.h>             // QemuFwCfgFindFile()
>> +
>> +STATIC
>> +EFI_STATUS
>> +EFIAPI
>> +BhyveGetCpuCount (
>> +  OUT UINT32  *CpuCount
>> +  )
>> +{
>> +  FIRMWARE_CONFIG_ITEM  Item;
>> +  UINTN                 Size;
>> +
>> +  if (QemuFwCfgIsAvailable ()) {
>> +    if (EFI_ERROR (QemuFwCfgFindFile ("opt/bhyve/hw.ncpu", &Item, &Size))) {
>> +      return EFI_NOT_FOUND;
>> +    } else if (Size != sizeof (*CpuCount)) {
>> +      return EFI_BAD_BUFFER_SIZE;
>> +    }
>> +
>> +    QemuFwCfgSelectItem (Item);
>> +    QemuFwCfgReadBytes (Size, CpuCount);
>> +
>> +    return EFI_SUCCESS;
>> +  }
>> +
>> +  //
>> +  // QemuFwCfg not available, try BhyveFwCtl.
>> +  //
>> +  Size = sizeof (*CpuCount);
>> +  if (BhyveFwCtlGet ("hw.ncpu", CpuCount, &Size) == RETURN_SUCCESS) {
>> +    return EFI_SUCCESS;
>> +  }
>> +
>> +  return EFI_UNSUPPORTED;
>> +}
>>
>>   STATIC
>>   EFI_STATUS
>> @@ -23,7 +58,6 @@ BhyveInstallAcpiMadtTable (
>>     )
>>   {
>>     UINT32                                               CpuCount;
>> -  UINTN                                                cSize;
>>     UINTN                                                NewBufferSize;
>>     EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER  *Madt;
>>     EFI_ACPI_1_0_PROCESSOR_LOCAL_APIC_STRUCTURE          *LocalApic;
>> @@ -36,9 +70,8 @@ BhyveInstallAcpiMadtTable (
>>     ASSERT (AcpiTableBufferSize >= sizeof (EFI_ACPI_DESCRIPTION_HEADER));
>>
>>     // Query the host for the number of vCPUs
>> -  CpuCount = 0;
>> -  cSize    = sizeof (CpuCount);
>> -  if (BhyveFwCtlGet ("hw.ncpu", &CpuCount, &cSize) == RETURN_SUCCESS) {
>> +  Status = BhyveGetCpuCount (&CpuCount);
>> +  if (!EFI_ERROR (Status)) {
>>       DEBUG ((DEBUG_INFO, "Retrieved CpuCount %d\n", CpuCount));
>>       ASSERT (CpuCount >= 1);
>>     } else {
>> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
>> index a8fa4d38ab..002cef32a3 100644
>> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
>> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
>> @@ -164,8 +164,7 @@
>>     SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
>>     UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
>>     SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
>> -  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf
>> -  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf
>> +  QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
>>     BhyveFwCtlLib|OvmfPkg/Library/BhyveFwCtlLib/BhyveFwCtlLib.inf
>>     VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
>>     MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
>> @@ -358,6 +357,7 @@
>>   !endif
>>     PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>>     MpInitLib|UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.inf
>> +  QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
>>
>>   [LibraryClasses.common.UEFI_APPLICATION]
>>     PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>> --
>> 2.11.0
>>
>> Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
>> Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
>>
>>
>>
>
> 
>
>