Re: [PATCH] OvmfPkg/BhyveBhf: install bhyve's ACPI tables
- In reply to: Corvin Köhne : "[PATCH] OvmfPkg/BhyveBhf: install bhyve's ACPI tables"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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.