Re: [PATCH] OvmfPkg/BhyveBhf: install bhyve's ACPI tables

From: Mario Marietto <marietto2008_at_gmail.com>
Date: Fri, 31 Mar 2023 12:59:33 UTC
Hello Corvin.

I will try your patch very soon if you  want to write carefully all the
commands that I should issue. Thanks.

On Fri, Mar 31, 2023 at 2:40 PM Corvin Köhne <corvink@freebsd.org> wrote:

> Hi,
>
> I would like to send the following patch to the EDKII project.
> Therefore, I'd like to get some feedback from the bhyve community before
> sending them to EDKII.
>
> At the moment, UEFI guests are using static ACPI tables. Modifying them
> is not easy because we need to patch them in the EDKII repo.
> Additionally, ACPI tables should be configuration dependent. If one
> assigns a TPM device to one guest, this guest requires different ACPI
> tables than other guests.
>
> Bhyve already builds it own set of ACPI tables. This patch picks them up
> an installs them in the UEFI guest. This will overcome the mentioned
> limitations.
>
> Note that this patch is required to easily implement features like
> qemu's fwcfg or a tpm device emulation.
>
> Here's the patch:
>
> It's much easier to create configuration dependend ACPI tables for bhyve
> than for OVMF. For this reason, don't use the statically created ACPI
> tables provided by OVMF. Instead prefer the dynamically created ACPI
> tables of bhyve. If bhyve provides no ACPI tables or we are unable to
> detect those, fall back to OVMF tables.
>
> Implementation is similar to OvmfPkg/XenAcpiPlatformDxe/Xen.c.
> ---
>  MdePkg/Include/Uefi/UefiBaseType.h           |   2 +
>  OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c |  15 ++
>  OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.h |   6 +
>  OvmfPkg/Bhyve/AcpiPlatformDxe/Bhyve.c        | 266 +++++++++++++++++++
>  4 files changed, 289 insertions(+)
>
> diff --git a/MdePkg/Include/Uefi/UefiBaseType.h
> b/MdePkg/Include/Uefi/UefiBaseType.h
> index 83975a08eb..b18a0760ee 100644
> --- a/MdePkg/Include/Uefi/UefiBaseType.h
> +++ b/MdePkg/Include/Uefi/UefiBaseType.h
> @@ -54,6 +54,8 @@ typedef UINT64 EFI_PHYSICAL_ADDRESS;
>  ///
>  typedef UINT64 EFI_VIRTUAL_ADDRESS;
>
> +#define NUMERIC_VALUE_AS_POINTER(Type, Value)  ((Type *) ((UINTN)(Value)))
> +
>  ///
>  /// EFI Time Abstraction:
>  ///  Year:       1900 - 9999
> diff --git a/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c
> b/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c
> index 999e9f151e..34d9fc80d0 100644
> --- a/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c
> +++ b/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.c
> @@ -243,6 +243,21 @@ InstallAcpiTables (
>  {
>    EFI_STATUS  Status;
>
> +  Status = InstallBhyveTables (AcpiTable);
> +  if (!EFI_ERROR (Status)) {
> +    return EFI_SUCCESS;
> +  }
> +
> +  if (Status != EFI_NOT_FOUND) {
> +    DEBUG ((
> +      DEBUG_INFO,
> +      "%a: unable to install bhyve's ACPI tables (%r)\n",
> +      __FUNCTION__,
> +      Status
> +      ));
> +    return Status;
> +  }
> +
>    Status = InstallOvmfFvTables (AcpiTable);
>
>    return Status;
> diff --git a/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.h
> b/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.h
> index 54d1af073e..b2724135d0 100644
> --- a/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.h
> +++ b/OvmfPkg/Bhyve/AcpiPlatformDxe/AcpiPlatform.h
> @@ -46,6 +46,12 @@ BhyveInstallAcpiTable (
>    OUT  UINTN                    *TableKey
>    );
>
> +EFI_STATUS
> +EFIAPI
> +InstallBhyveTables (
> +  IN   EFI_ACPI_TABLE_PROTOCOL  *AcpiProtocol
> +  );
> +
>  EFI_STATUS
>  EFIAPI
>  InstallXenTables (
> diff --git a/OvmfPkg/Bhyve/AcpiPlatformDxe/Bhyve.c
> b/OvmfPkg/Bhyve/AcpiPlatformDxe/Bhyve.c
> index e216a21bfa..5e1b759c01 100644
> --- a/OvmfPkg/Bhyve/AcpiPlatformDxe/Bhyve.c
> +++ b/OvmfPkg/Bhyve/AcpiPlatformDxe/Bhyve.c
> @@ -13,6 +13,18 @@
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/QemuFwCfgLib.h>             // QemuFwCfgFindFile()
>
> +#define BHYVE_ACPI_PHYSICAL_ADDRESS  ((UINTN)0x000F2400)
> +#define BHYVE_BIOS_PHYSICAL_END      ((UINTN)0x00100000)
> +
> +#pragma pack (1)
> +
> +typedef struct {
> +  EFI_ACPI_DESCRIPTION_HEADER    Header;
> +  UINT64                         Tables[0];
> +} EFI_ACPI_2_0_EXTENDED_SYSTEM_DESCRIPTION_TABLE;
> +
> +#pragma pack ()
> +
>  STATIC
>  EFI_STATUS
>  EFIAPI
> @@ -164,3 +176,257 @@ BhyveInstallAcpiTable (
>             TableKey
>             );
>  }
> +
> +/**
> +  Get the address of bhyve's ACPI Root System Description Pointer (RSDP).
> +
> +  @param  RsdpPtr             Return pointer to RSDP.
> +
> +  @return EFI_SUCCESS         Bhyve's RSDP successfully found.
> +  @return EFI_NOT_FOUND       Couldn't find bhyve's RSDP.
> +  @return EFI_UNSUPPORTED     Revision is lower than 2.
> +  @return EFI_PROTOCOL_ERROR  Invalid RSDP found.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +BhyveGetAcpiRsdp (
> +  OUT   EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  **RsdpPtr
> +  )
> +{
> +  UINTN                                         RsdpAddress;
> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
> +
> +  if (RsdpPtr == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Detect the RSDP
> +  //
> +  for (RsdpAddress = BHYVE_ACPI_PHYSICAL_ADDRESS;
> +       RsdpAddress < BHYVE_BIOS_PHYSICAL_END;
> +       RsdpAddress += 0x10)
> +  {
> +    Rsdp = NUMERIC_VALUE_AS_POINTER (
> +             EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER,
> +             RsdpAddress
> +             );
> +    if (Rsdp->Signature !=
> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_SIGNATURE) {
> +      continue;
> +    }
> +
> +    if (Rsdp->Revision < 2) {
> +      DEBUG ((DEBUG_INFO, "%a: unsupported RSDP found\n", __FUNCTION__));
> +      return EFI_UNSUPPORTED;
> +    }
> +
> +    //
> +    // For ACPI 1.0/2.0/3.0 the checksum of first 20 bytes should be 0.
> +    // For ACPI 2.0/3.0 the checksum of the entire table should be 0.
> +    //
> +    UINT8  Sum = CalculateCheckSum8 (
> +                   (CONST UINT8 *)Rsdp,
> +                   sizeof (EFI_ACPI_1_0_ROOT_SYSTEM_DESCRIPTION_POINTER)
> +                   );
> +    if (Sum != 0) {
> +      DEBUG ((
> +        DEBUG_INFO,
> +        "%a: RSDP header checksum not valid: 0x%02x\n",
> +        __FUNCTION__,
> +        Sum
> +        ));
> +      return EFI_PROTOCOL_ERROR;
> +    }
> +
> +    Sum = CalculateCheckSum8 (
> +            (CONST UINT8 *)Rsdp,
> +            sizeof (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER)
> +            );
> +    if (Sum != 0) {
> +      DEBUG ((
> +        DEBUG_INFO,
> +        "%a: RSDP table checksum not valid: 0x%02x\n",
> +        __FUNCTION__,
> +        Sum
> +        ));
> +      return EFI_PROTOCOL_ERROR;
> +    }
> +
> +    //
> +    // RSDP was found and is valid
> +    //
> +    *RsdpPtr = Rsdp;
> +
> +    return EFI_SUCCESS;
> +  }
> +
> +  DEBUG ((DEBUG_INFO, "%a: RSDP not found\n", __FUNCTION__));
> +  return EFI_NOT_FOUND;
> +}
> +
> +/**
> +  Get bhyve's ACPI tables from the RSDP. And install bhyve's ACPI tables
> +  into the RSDT/XSDT using InstallAcpiTable.
> +
> +  @param  AcpiProtocol        Protocol instance pointer.
> +
> +  @return EFI_SUCCESS         All tables were successfully inserted.
> +  @return EFI_UNSUPPORTED     Bhyve's ACPI tables doesn't include a XSDT.
> +  @return EFI_PROTOCOL_ERROR  Invalid XSDT found.
> +
> +  @return                     Error codes propagated from underlying
> functions.
> +**/
> +EFI_STATUS
> +EFIAPI
> +InstallBhyveTables (
> +  IN   EFI_ACPI_TABLE_PROTOCOL  *AcpiProtocol
> +  )
> +{
> +  EFI_STATUS                                    Status;
> +  UINTN                                         TableHandle;
> +  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
> +  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
> +  EFI_ACPI_DESCRIPTION_HEADER                   *Dsdt;
> +
> +  Rsdp = NULL;
> +  Facs = NULL;
> +  Dsdt = NULL;
> +
> +  //
> +  // Try to find bhyve ACPI tables
> +  //
> +  Status = BhyveGetAcpiRsdp (&Rsdp);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_INFO, "%a: can't get RSDP (%r)\n", __FUNCTION__,
> Status));
> +    return Status;
> +  }
> +
> +  //
> +  // Bhyve should always provide a XSDT
> +  //
> +  EFI_ACPI_2_0_EXTENDED_SYSTEM_DESCRIPTION_TABLE *CONST  Xsdt =
> +    NUMERIC_VALUE_AS_POINTER (
> +      EFI_ACPI_2_0_EXTENDED_SYSTEM_DESCRIPTION_TABLE,
> +      Rsdp->XsdtAddress
> +      );
> +
> +  if (Xsdt == NULL) {
> +    DEBUG ((DEBUG_INFO, "%a: XSDT not found\n", __FUNCTION__));
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  if (Xsdt->Header.Length < sizeof (EFI_ACPI_DESCRIPTION_HEADER)) {
> +    DEBUG ((DEBUG_INFO, "%a: invalid XSDT length\n", __FUNCTION__));
> +    return EFI_PROTOCOL_ERROR;
> +  }
> +
> +  //
> +  // Install ACPI tables
> +  //
> +  CONST UINTN  NumberOfTableEntries =
> +    (Xsdt->Header.Length - sizeof (Xsdt->Header)) / sizeof (UINT64);
> +
> +  for (UINTN Index = 0; Index < NumberOfTableEntries; Index++) {
> +    EFI_ACPI_DESCRIPTION_HEADER *CONST  CurrentTable =
> +      NUMERIC_VALUE_AS_POINTER (
> +        EFI_ACPI_DESCRIPTION_HEADER,
> +        Xsdt->Tables[Index]
> +        );
> +    Status = AcpiProtocol->InstallAcpiTable (
> +                             AcpiProtocol,
> +                             CurrentTable,
> +                             CurrentTable->Length,
> +                             &TableHandle
> +                             );
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((
> +        DEBUG_INFO,
> +        "%a: failed to install ACPI table %c%c%c%c (%r)\n",
> +        __FUNCTION__,
> +        NUMERIC_VALUE_AS_POINTER (UINT8, CurrentTable->Signature)[0],
> +        NUMERIC_VALUE_AS_POINTER (UINT8, CurrentTable->Signature)[1],
> +        NUMERIC_VALUE_AS_POINTER (UINT8, CurrentTable->Signature)[2],
> +        NUMERIC_VALUE_AS_POINTER (UINT8, CurrentTable->Signature)[3],
> +        Status
> +        ));
> +      return Status;
> +    }
> +
> +    if (CurrentTable->Signature ==
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
> +      EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *CONST  Fadt =
> +        (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *CONST)CurrentTable;
> +      if (Fadt->XFirmwareCtrl) {
> +        Facs = NUMERIC_VALUE_AS_POINTER (
> +                 EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE,
> +                 Fadt->XFirmwareCtrl
> +                 );
> +      } else {
> +        Facs = NUMERIC_VALUE_AS_POINTER (
> +                 EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE,
> +                 Fadt->FirmwareCtrl
> +                 );
> +      }
> +
> +      if (Fadt->XDsdt) {
> +        Dsdt = NUMERIC_VALUE_AS_POINTER (
> +                 EFI_ACPI_DESCRIPTION_HEADER,
> +                 Fadt->XDsdt
> +                 );
> +      } else {
> +        Dsdt = NUMERIC_VALUE_AS_POINTER (
> +                 EFI_ACPI_DESCRIPTION_HEADER,
> +                 Fadt->Dsdt
> +                 );
> +      }
> +    }
> +  }
> +
> +  //
> +  // Install FACS
> +  //
> +  if (Facs != NULL) {
> +    Status = AcpiProtocol->InstallAcpiTable (
> +                             AcpiProtocol,
> +                             Facs,
> +                             Facs->Length,
> +                             &TableHandle
> +                             );
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((
> +        DEBUG_INFO,
> +        "%a: failed to install FACS (%r)\n",
> +        __FUNCTION__,
> +        Status
> +        ));
> +      return Status;
> +    }
> +  }
> +
> +  //
> +  // Install DSDT
> +  // If it's not found, something bad happened. Don't continue execution.
> +  //
> +  if (Dsdt == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to find DSDT\n", __FUNCTION__));
> +    CpuDeadLoop ();
> +  }
> +
> +  Status = AcpiProtocol->InstallAcpiTable (
> +                           AcpiProtocol,
> +                           Dsdt,
> +                           Dsdt->Length,
> +                           &TableHandle
> +                           );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_INFO,
> +      "%a: failed to install DSDT (%r)\n",
> +      __FUNCTION__,
> +      Status
> +      ));
> +    return Status;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> --
> 2.40.0
>
>
>

-- 
Mario.