| Message ID | 1384269110-23613-2-git-send-email-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 12, 2013 at 7:11 AM, Laszlo Ersek <lersek@redhat.com> wrote: > Qemu v1.7.0-rc0 features an ACPI linker/loader interface, available over > fw_cfg, written by Michael Tsirkin. > > Qemu composes all ACPI tables on the host side, according to the target > hardware configuration, and makes the tables available to any guest > firmware over fw_cfg. > > The feature moves the burden of keeping ACPI tables up-to-date from boot > firmware to qemu (which is the source of hardware configuration anyway). > > This patch adds client code for this feature. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h | 7 +- > OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c | 12 +- > OvmfPkg/AcpiPlatformDxe/Qemu.c | 204 +++++++++++++++++++++++++++++++++ > 3 files changed, 215 insertions(+), 8 deletions(-) > > diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h > index 21107cd..c643fa1 100644 > --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h > +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h > @@ -10,7 +10,7 @@ > THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > -**/ > +**/ > > #ifndef _ACPI_PLATFORM_H_INCLUDED_ > #define _ACPI_PLATFORM_H_INCLUDED_ > @@ -61,5 +61,10 @@ InstallXenTables ( > IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol > ); > > +EFI_STATUS > +EFIAPI > +InstallQemuLinkedTables ( > + IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol > + ); > #endif > > diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c > index 6e0b610..084c393 100644 > --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c > +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c > @@ -256,16 +256,14 @@ AcpiPlatformEntryPoint ( > > if (XenDetected ()) { > Status = InstallXenTables (AcpiTable); > - if (EFI_ERROR (Status)) { > - Status = FindAcpiTablesInFv (AcpiTable); > - } > } else { > + Status = InstallQemuLinkedTables (AcpiTable); > + } > + > + if (EFI_ERROR (Status)) { > Status = FindAcpiTablesInFv (AcpiTable); > } > - if (EFI_ERROR (Status)) { > - return Status; > - } > > - return EFI_SUCCESS; > + return Status; > } > > diff --git a/OvmfPkg/AcpiPlatformDxe/Qemu.c b/OvmfPkg/AcpiPlatformDxe/Qemu.c > index 06bd463..c572f8a 100644 > --- a/OvmfPkg/AcpiPlatformDxe/Qemu.c > +++ b/OvmfPkg/AcpiPlatformDxe/Qemu.c > @@ -515,3 +515,207 @@ QemuInstallAcpiTable ( > ); > } > > + > +/** > + Download the ACPI table data file from QEMU and interpret it. > + > + Starting with v1.7.0-rc0, QEMU provides the following three files for 1.7+ > + machine types: > + - etc/acpi/rsdp > + - etc/acpi/tables > + - etc/table-loader > + > + "etc/acpi/rsdp" and "etc/acpi/tables" are similar, they are only kept > + separate because they have different allocation requirements in SeaBIOS. > + > + Both of these fw_cfg files contain preformatted ACPI payload. "etc/acpi/rsdp" > + contains only the RSDP table, while "etc/acpi/tables" contains all other > + tables, concatenated. > + > + The tables in these two files have been filled in by qemu, but two kinds of > + fields are incomplete in each table: pointers to other tables, and checksums > + (which depend on the pointers). The pointers are pre-initialized with > + *relative* offsets, but their final values depend on where the actual files > + will be placed in memory by the guest. That is, the pointer fields need to be > + "relocated" (incremented) by the base addresses of where "/etc/acpi/rsdp" and > + "/etc/acpi/tables" will be placed in guest memory. > + > + This is where the third file, "/etc/table-loader" comes into the picture. It > + is a linker/loader script that has several command types: > + > + One command type instructs the guest to download the other two files. > + > + Another command type instructs the guest to increment ("absolutize") a > + pointer field (having a relative initial value) in some file, with the > + dynamic base address of the same (or another) file. > + > + The third command type instructs the guest to compute checksums over > + ranges and to store them. > + > + In edk2, EFI_ACPI_TABLE_PROTOCOL knows about table relationships -- it > + handles linkage automatically when a table is installed. The protocol takes > + care of checksumming too. RSDP is installed automatically. Hence we only need > + to care about the "etc/acpi/tables" file, determining the boundaries of each > + table and installing it. I'm wondering if some of the information in this comment might have a better home in the commit message. Will it help in maintaining the code to have it here? Or, maybe a more terse version can live here? Of course, I rarely comment my code enough, which is much worse. So, feel free to ignore this feedback. :) > + @param[in] AcpiProtocol The ACPI table protocol used to install tables. > + > + @retval EFI_UNSUPPORTED Firmware configuration is unavailable. > + > + @retval EFI_NOT_FOUND The host doesn't export the "etc/acpi/tables" > + fw_cfg file. > + > + @retval EFI_OUT_OF_RESOURCES Memory allocation failed. > + > + @return Status codes returned by > + AcpiProtocol->InstallAcpiTable(). > + > +**/ > + > +// > +// We'll be saving the keys of installed tables so that we can roll them back > +// in case of failure. 128 tables should be enough for anyone (TM). > +// > +#define INSTALLED_TABLES_MAX 128 > + > +// > +// This macro produces three arguments for DEBUG(), for the format string > +// "%-*.*a" -- left-justify, take field width, take number of characters to > +// consume, ASCII character string. The argument must be a valid pointer. > +// > +#define DBGSTR(ptr) (sizeof *(ptr)), (sizeof *(ptr)), ((CONST CHAR8 *) (ptr)) > + > +// > +// Introduce a macro for the format string as well, bracketed by embedded > +// double quotes. > +// > +#define DBGFMT "\"%-*.*a\"" I think that needing these macros is a sign that perhaps there are excessive debug messages. :) > +EFI_STATUS > +EFIAPI > +InstallQemuLinkedTables ( > + IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol > + ) > +{ > + STATIC CONST CHAR8 Func[] = "InstallQemuLinkedTables"; > + EFI_STATUS Status; > + FIRMWARE_CONFIG_ITEM TablesFile; > + UINTN TablesFileSize; > + UINT8 *Tables; > + UINTN *InstalledKey; > + UINTN Processed; > + INT32 Installed; > + > + Status = QemuFwCfgFindFile ("etc/acpi/tables", &TablesFile, &TablesFileSize); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_INFO, "%a: \"etc/acpi/tables\" interface unavailable: %r\n", > + Func, Status)); > + return Status; > + } > + > + Tables = AllocatePool (TablesFileSize); > + if (Tables == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + QemuFwCfgSelectItem (TablesFile); > + QemuFwCfgReadBytes (TablesFileSize, Tables); > + > + InstalledKey = AllocatePool (INSTALLED_TABLES_MAX * sizeof *InstalledKey); > + if (InstalledKey == NULL) { > + Status = EFI_OUT_OF_RESOURCES; > + goto FreeTables; > + } > + > + Processed = 0; > + Installed = 0; > + while (Processed < TablesFileSize) { > + UINTN Remaining; > + EFI_ACPI_DESCRIPTION_HEADER *Probe; > + > + Remaining = TablesFileSize - Processed; > + if (Remaining < sizeof *Probe) { > + DEBUG ((EFI_D_VERBOSE, "%a: truncated ACPI table header at offset " > + "0x%Lx\n", Func, (UINT64) Processed)); > + Status = EFI_PROTOCOL_ERROR; > + break; > + } > + > + Probe = (EFI_ACPI_DESCRIPTION_HEADER *) (Tables + Processed); > + DEBUG ((EFI_D_VERBOSE, "%a: offset 0x%016Lx:" > + " Signature=" DBGFMT > + " Length=0x%08x" > + " Revision=0x%02x" > + " OemId=" DBGFMT > + " OemTableId=" DBGFMT > + " OemRevision=0x%08x" > + " CreatorId=" DBGFMT > + " CreatorRevision=0x%08x\n", > + Func, (UINT64) Processed, > + DBGSTR (&Probe->Signature), > + Probe->Length, > + Probe->Revision, > + DBGSTR (&Probe->OemId), > + DBGSTR (&Probe->OemTableId), > + Probe->OemRevision, > + DBGSTR (&Probe->CreatorId), > + Probe->CreatorRevision)); Yep. :) Do we need to print all those fields? Anyway, maybe a better centralized place for this would be: MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c Also, I think we try to keep debug messages under 80 characters. > + if (Remaining < Probe->Length || Probe->Length < sizeof *Probe) { > + DEBUG ((EFI_D_VERBOSE, "%a: invalid ACPI table header at offset 0x%Lx\n", > + Func, (UINT64) Processed)); > + Status = EFI_PROTOCOL_ERROR; > + break; > + } > + > + // > + // skip automatically handled "root" tables: RSDT, XSDT > + // > + if (Probe->Signature != > + EFI_ACPI_1_0_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE && > + Probe->Signature != > + EFI_ACPI_2_0_EXTENDED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) { > + if (Installed == INSTALLED_TABLES_MAX) { > + DEBUG ((EFI_D_ERROR, "%a: can't install more than %d tables\n", Func, > + INSTALLED_TABLES_MAX)); > + Status = EFI_OUT_OF_RESOURCES; > + break; > + } > + > + Status = AcpiProtocol->InstallAcpiTable (AcpiProtocol, Probe, > + Probe->Length, &InstalledKey[Installed]); We do have a local InstallAcpiTable function. (Just remove "AcpiProtocol->") -Jordan
On 11/22/13 19:10, Jordan Justen wrote: > On Tue, Nov 12, 2013 at 7:11 AM, Laszlo Ersek <lersek@redhat.com> wrote: >> Qemu v1.7.0-rc0 features an ACPI linker/loader interface, available over >> fw_cfg, written by Michael Tsirkin. >> >> Qemu composes all ACPI tables on the host side, according to the target >> hardware configuration, and makes the tables available to any guest >> firmware over fw_cfg. >> >> The feature moves the burden of keeping ACPI tables up-to-date from boot >> firmware to qemu (which is the source of hardware configuration anyway). >> >> This patch adds client code for this feature. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h | 7 +- >> OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c | 12 +- >> OvmfPkg/AcpiPlatformDxe/Qemu.c | 204 +++++++++++++++++++++++++++++++++ >> 3 files changed, 215 insertions(+), 8 deletions(-) >> >> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h >> index 21107cd..c643fa1 100644 >> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h >> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h >> @@ -10,7 +10,7 @@ >> THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >> >> -**/ >> +**/ >> >> #ifndef _ACPI_PLATFORM_H_INCLUDED_ >> #define _ACPI_PLATFORM_H_INCLUDED_ >> @@ -61,5 +61,10 @@ InstallXenTables ( >> IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol >> ); >> >> +EFI_STATUS >> +EFIAPI >> +InstallQemuLinkedTables ( >> + IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol >> + ); >> #endif >> >> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c >> index 6e0b610..084c393 100644 >> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c >> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c >> @@ -256,16 +256,14 @@ AcpiPlatformEntryPoint ( >> >> if (XenDetected ()) { >> Status = InstallXenTables (AcpiTable); >> - if (EFI_ERROR (Status)) { >> - Status = FindAcpiTablesInFv (AcpiTable); >> - } >> } else { >> + Status = InstallQemuLinkedTables (AcpiTable); >> + } >> + >> + if (EFI_ERROR (Status)) { >> Status = FindAcpiTablesInFv (AcpiTable); >> } >> - if (EFI_ERROR (Status)) { >> - return Status; >> - } >> >> - return EFI_SUCCESS; >> + return Status; >> } >> >> diff --git a/OvmfPkg/AcpiPlatformDxe/Qemu.c b/OvmfPkg/AcpiPlatformDxe/Qemu.c >> index 06bd463..c572f8a 100644 >> --- a/OvmfPkg/AcpiPlatformDxe/Qemu.c >> +++ b/OvmfPkg/AcpiPlatformDxe/Qemu.c >> @@ -515,3 +515,207 @@ QemuInstallAcpiTable ( >> ); >> } >> >> + >> +/** >> + Download the ACPI table data file from QEMU and interpret it. >> + >> + Starting with v1.7.0-rc0, QEMU provides the following three files for 1.7+ >> + machine types: >> + - etc/acpi/rsdp >> + - etc/acpi/tables >> + - etc/table-loader >> + >> + "etc/acpi/rsdp" and "etc/acpi/tables" are similar, they are only kept >> + separate because they have different allocation requirements in SeaBIOS. >> + >> + Both of these fw_cfg files contain preformatted ACPI payload. "etc/acpi/rsdp" >> + contains only the RSDP table, while "etc/acpi/tables" contains all other >> + tables, concatenated. >> + >> + The tables in these two files have been filled in by qemu, but two kinds of >> + fields are incomplete in each table: pointers to other tables, and checksums >> + (which depend on the pointers). The pointers are pre-initialized with >> + *relative* offsets, but their final values depend on where the actual files >> + will be placed in memory by the guest. That is, the pointer fields need to be >> + "relocated" (incremented) by the base addresses of where "/etc/acpi/rsdp" and >> + "/etc/acpi/tables" will be placed in guest memory. >> + >> + This is where the third file, "/etc/table-loader" comes into the picture. It >> + is a linker/loader script that has several command types: >> + >> + One command type instructs the guest to download the other two files. >> + >> + Another command type instructs the guest to increment ("absolutize") a >> + pointer field (having a relative initial value) in some file, with the >> + dynamic base address of the same (or another) file. >> + >> + The third command type instructs the guest to compute checksums over >> + ranges and to store them. >> + >> + In edk2, EFI_ACPI_TABLE_PROTOCOL knows about table relationships -- it >> + handles linkage automatically when a table is installed. The protocol takes >> + care of checksumming too. RSDP is installed automatically. Hence we only need >> + to care about the "etc/acpi/tables" file, determining the boundaries of each >> + table and installing it. > > I'm wondering if some of the information in this comment might have a > better home in the commit message. Will it help in maintaining the > code to have it here? Or, maybe a more terse version can live here? > > Of course, I rarely comment my code enough, which is much worse. So, > feel free to ignore this feedback. :) I can move the text from "Starting with v1.7.0-rc0..." until here to the commit message. > >> + @param[in] AcpiProtocol The ACPI table protocol used to install tables. >> + >> + @retval EFI_UNSUPPORTED Firmware configuration is unavailable. >> + >> + @retval EFI_NOT_FOUND The host doesn't export the "etc/acpi/tables" >> + fw_cfg file. >> + >> + @retval EFI_OUT_OF_RESOURCES Memory allocation failed. >> + >> + @return Status codes returned by >> + AcpiProtocol->InstallAcpiTable(). >> + >> +**/ >> + >> +// >> +// We'll be saving the keys of installed tables so that we can roll them back >> +// in case of failure. 128 tables should be enough for anyone (TM). >> +// >> +#define INSTALLED_TABLES_MAX 128 >> + >> +// >> +// This macro produces three arguments for DEBUG(), for the format string >> +// "%-*.*a" -- left-justify, take field width, take number of characters to >> +// consume, ASCII character string. The argument must be a valid pointer. >> +// >> +#define DBGSTR(ptr) (sizeof *(ptr)), (sizeof *(ptr)), ((CONST CHAR8 *) (ptr)) >> + >> +// >> +// Introduce a macro for the format string as well, bracketed by embedded >> +// double quotes. >> +// >> +#define DBGFMT "\"%-*.*a\"" > > I think that needing these macros is a sign that perhaps there are > excessive debug messages. :) There's no such thing as an excessive debug message, if you can disable it :) When communication over a binary protocol goes wrong, the first thing you add is a hexdump with some human-friendly parsing. I just don't want to start from zero every time that happens. > >> +EFI_STATUS >> +EFIAPI >> +InstallQemuLinkedTables ( >> + IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol >> + ) >> +{ >> + STATIC CONST CHAR8 Func[] = "InstallQemuLinkedTables"; >> + EFI_STATUS Status; >> + FIRMWARE_CONFIG_ITEM TablesFile; >> + UINTN TablesFileSize; >> + UINT8 *Tables; >> + UINTN *InstalledKey; >> + UINTN Processed; >> + INT32 Installed; >> + >> + Status = QemuFwCfgFindFile ("etc/acpi/tables", &TablesFile, &TablesFileSize); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((EFI_D_INFO, "%a: \"etc/acpi/tables\" interface unavailable: %r\n", >> + Func, Status)); >> + return Status; >> + } >> + >> + Tables = AllocatePool (TablesFileSize); >> + if (Tables == NULL) { >> + return EFI_OUT_OF_RESOURCES; >> + } >> + >> + QemuFwCfgSelectItem (TablesFile); >> + QemuFwCfgReadBytes (TablesFileSize, Tables); >> + >> + InstalledKey = AllocatePool (INSTALLED_TABLES_MAX * sizeof *InstalledKey); >> + if (InstalledKey == NULL) { >> + Status = EFI_OUT_OF_RESOURCES; >> + goto FreeTables; >> + } >> + >> + Processed = 0; >> + Installed = 0; >> + while (Processed < TablesFileSize) { >> + UINTN Remaining; >> + EFI_ACPI_DESCRIPTION_HEADER *Probe; >> + >> + Remaining = TablesFileSize - Processed; >> + if (Remaining < sizeof *Probe) { >> + DEBUG ((EFI_D_VERBOSE, "%a: truncated ACPI table header at offset " >> + "0x%Lx\n", Func, (UINT64) Processed)); >> + Status = EFI_PROTOCOL_ERROR; >> + break; >> + } >> + >> + Probe = (EFI_ACPI_DESCRIPTION_HEADER *) (Tables + Processed); >> + DEBUG ((EFI_D_VERBOSE, "%a: offset 0x%016Lx:" >> + " Signature=" DBGFMT >> + " Length=0x%08x" >> + " Revision=0x%02x" >> + " OemId=" DBGFMT >> + " OemTableId=" DBGFMT >> + " OemRevision=0x%08x" >> + " CreatorId=" DBGFMT >> + " CreatorRevision=0x%08x\n", >> + Func, (UINT64) Processed, >> + DBGSTR (&Probe->Signature), >> + Probe->Length, >> + Probe->Revision, >> + DBGSTR (&Probe->OemId), >> + DBGSTR (&Probe->OemTableId), >> + Probe->OemRevision, >> + DBGSTR (&Probe->CreatorId), >> + Probe->CreatorRevision)); > > Yep. :) > > Do we need to print all those fields? > > Anyway, maybe a better centralized place for this would be: > MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c > > Also, I think we try to keep debug messages under 80 characters. We seem to think completely differently about debug messages :) I can cut most of the fields though, and simply keep the signature (and remove the format string macro too, because for the signature I'd only use it once). Would that work for you? I'd rather not try to patch MdeModulePkg without an actual bug to fix. > >> + if (Remaining < Probe->Length || Probe->Length < sizeof *Probe) { >> + DEBUG ((EFI_D_VERBOSE, "%a: invalid ACPI table header at offset 0x%Lx\n", >> + Func, (UINT64) Processed)); >> + Status = EFI_PROTOCOL_ERROR; >> + break; >> + } >> + >> + // >> + // skip automatically handled "root" tables: RSDT, XSDT >> + // >> + if (Probe->Signature != >> + EFI_ACPI_1_0_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE && >> + Probe->Signature != >> + EFI_ACPI_2_0_EXTENDED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) { >> + if (Installed == INSTALLED_TABLES_MAX) { >> + DEBUG ((EFI_D_ERROR, "%a: can't install more than %d tables\n", Func, >> + INSTALLED_TABLES_MAX)); >> + Status = EFI_OUT_OF_RESOURCES; >> + break; >> + } >> + >> + Status = AcpiProtocol->InstallAcpiTable (AcpiProtocol, Probe, >> + Probe->Length, &InstalledKey[Installed]); > > We do have a local InstallAcpiTable function. (Just remove "AcpiProtocol->") Yes, I saw that. But, we don't have a wrapper for the rollback on the error path (where we uninstall the tables). Since I used AcpiProtocol-> in the rollback part, I wanted to be consistent here. Of course I can add a wrapper for the uninstall function too :) Thanks Laszlo
On Fri, Nov 22, 2013 at 10:49 AM, Laszlo Ersek <lersek@redhat.com> wrote: > On 11/22/13 19:10, Jordan Justen wrote: >> Do we need to print all those fields? >> >> Anyway, maybe a better centralized place for this would be: >> MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c >> >> Also, I think we try to keep debug messages under 80 characters. > > We seem to think completely differently about debug messages :) Maybe... ? I think you can have too much debug if: * it makes it debug builds excessively slow or large * it makes it challenging to find items in the debug log * it makes the code significantly more difficult to read * it is in a code area that is just not valuable to log > I can cut most of the fields though, and simply keep the signature (and > remove the format string macro too, because for the signature I'd only > use it once). Would that work for you? Sure. >> We do have a local InstallAcpiTable function. (Just remove "AcpiProtocol->") > > Yes, I saw that. But, we don't have a wrapper for the rollback on the > error path (where we uninstall the tables). Since I used AcpiProtocol-> > in the rollback part, I wanted to be consistent here. Good point. -Jordan
diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h index 21107cd..c643fa1 100644 --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h @@ -10,7 +10,7 @@ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. -**/ +**/ #ifndef _ACPI_PLATFORM_H_INCLUDED_ #define _ACPI_PLATFORM_H_INCLUDED_ @@ -61,5 +61,10 @@ InstallXenTables ( IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol ); +EFI_STATUS +EFIAPI +InstallQemuLinkedTables ( + IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol + ); #endif diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c index 6e0b610..084c393 100644 --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c @@ -256,16 +256,14 @@ AcpiPlatformEntryPoint ( if (XenDetected ()) { Status = InstallXenTables (AcpiTable); - if (EFI_ERROR (Status)) { - Status = FindAcpiTablesInFv (AcpiTable); - } } else { + Status = InstallQemuLinkedTables (AcpiTable); + } + + if (EFI_ERROR (Status)) { Status = FindAcpiTablesInFv (AcpiTable); } - if (EFI_ERROR (Status)) { - return Status; - } - return EFI_SUCCESS; + return Status; } diff --git a/OvmfPkg/AcpiPlatformDxe/Qemu.c b/OvmfPkg/AcpiPlatformDxe/Qemu.c index 06bd463..c572f8a 100644 --- a/OvmfPkg/AcpiPlatformDxe/Qemu.c +++ b/OvmfPkg/AcpiPlatformDxe/Qemu.c @@ -515,3 +515,207 @@ QemuInstallAcpiTable ( ); } + +/** + Download the ACPI table data file from QEMU and interpret it. + + Starting with v1.7.0-rc0, QEMU provides the following three files for 1.7+ + machine types: + - etc/acpi/rsdp + - etc/acpi/tables + - etc/table-loader + + "etc/acpi/rsdp" and "etc/acpi/tables" are similar, they are only kept + separate because they have different allocation requirements in SeaBIOS. + + Both of these fw_cfg files contain preformatted ACPI payload. "etc/acpi/rsdp" + contains only the RSDP table, while "etc/acpi/tables" contains all other + tables, concatenated. + + The tables in these two files have been filled in by qemu, but two kinds of + fields are incomplete in each table: pointers to other tables, and checksums + (which depend on the pointers). The pointers are pre-initialized with + *relative* offsets, but their final values depend on where the actual files + will be placed in memory by the guest. That is, the pointer fields need to be + "relocated" (incremented) by the base addresses of where "/etc/acpi/rsdp" and + "/etc/acpi/tables" will be placed in guest memory. + + This is where the third file, "/etc/table-loader" comes into the picture. It + is a linker/loader script that has several command types: + + One command type instructs the guest to download the other two files. + + Another command type instructs the guest to increment ("absolutize") a + pointer field (having a relative initial value) in some file, with the + dynamic base address of the same (or another) file. + + The third command type instructs the guest to compute checksums over + ranges and to store them. + + In edk2, EFI_ACPI_TABLE_PROTOCOL knows about table relationships -- it + handles linkage automatically when a table is installed. The protocol takes + care of checksumming too. RSDP is installed automatically. Hence we only need + to care about the "etc/acpi/tables" file, determining the boundaries of each + table and installing it. + + @param[in] AcpiProtocol The ACPI table protocol used to install tables. + + @retval EFI_UNSUPPORTED Firmware configuration is unavailable. + + @retval EFI_NOT_FOUND The host doesn't export the "etc/acpi/tables" + fw_cfg file. + + @retval EFI_OUT_OF_RESOURCES Memory allocation failed. + + @return Status codes returned by + AcpiProtocol->InstallAcpiTable(). + +**/ + +// +// We'll be saving the keys of installed tables so that we can roll them back +// in case of failure. 128 tables should be enough for anyone (TM). +// +#define INSTALLED_TABLES_MAX 128 + +// +// This macro produces three arguments for DEBUG(), for the format string +// "%-*.*a" -- left-justify, take field width, take number of characters to +// consume, ASCII character string. The argument must be a valid pointer. +// +#define DBGSTR(ptr) (sizeof *(ptr)), (sizeof *(ptr)), ((CONST CHAR8 *) (ptr)) + +// +// Introduce a macro for the format string as well, bracketed by embedded +// double quotes. +// +#define DBGFMT "\"%-*.*a\"" + +EFI_STATUS +EFIAPI +InstallQemuLinkedTables ( + IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol + ) +{ + STATIC CONST CHAR8 Func[] = "InstallQemuLinkedTables"; + EFI_STATUS Status; + FIRMWARE_CONFIG_ITEM TablesFile; + UINTN TablesFileSize; + UINT8 *Tables; + UINTN *InstalledKey; + UINTN Processed; + INT32 Installed; + + Status = QemuFwCfgFindFile ("etc/acpi/tables", &TablesFile, &TablesFileSize); + if (EFI_ERROR (Status)) { + DEBUG ((EFI_D_INFO, "%a: \"etc/acpi/tables\" interface unavailable: %r\n", + Func, Status)); + return Status; + } + + Tables = AllocatePool (TablesFileSize); + if (Tables == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + QemuFwCfgSelectItem (TablesFile); + QemuFwCfgReadBytes (TablesFileSize, Tables); + + InstalledKey = AllocatePool (INSTALLED_TABLES_MAX * sizeof *InstalledKey); + if (InstalledKey == NULL) { + Status = EFI_OUT_OF_RESOURCES; + goto FreeTables; + } + + Processed = 0; + Installed = 0; + while (Processed < TablesFileSize) { + UINTN Remaining; + EFI_ACPI_DESCRIPTION_HEADER *Probe; + + Remaining = TablesFileSize - Processed; + if (Remaining < sizeof *Probe) { + DEBUG ((EFI_D_VERBOSE, "%a: truncated ACPI table header at offset " + "0x%Lx\n", Func, (UINT64) Processed)); + Status = EFI_PROTOCOL_ERROR; + break; + } + + Probe = (EFI_ACPI_DESCRIPTION_HEADER *) (Tables + Processed); + DEBUG ((EFI_D_VERBOSE, "%a: offset 0x%016Lx:" + " Signature=" DBGFMT + " Length=0x%08x" + " Revision=0x%02x" + " OemId=" DBGFMT + " OemTableId=" DBGFMT + " OemRevision=0x%08x" + " CreatorId=" DBGFMT + " CreatorRevision=0x%08x\n", + Func, (UINT64) Processed, + DBGSTR (&Probe->Signature), + Probe->Length, + Probe->Revision, + DBGSTR (&Probe->OemId), + DBGSTR (&Probe->OemTableId), + Probe->OemRevision, + DBGSTR (&Probe->CreatorId), + Probe->CreatorRevision)); + + if (Remaining < Probe->Length || Probe->Length < sizeof *Probe) { + DEBUG ((EFI_D_VERBOSE, "%a: invalid ACPI table header at offset 0x%Lx\n", + Func, (UINT64) Processed)); + Status = EFI_PROTOCOL_ERROR; + break; + } + + // + // skip automatically handled "root" tables: RSDT, XSDT + // + if (Probe->Signature != + EFI_ACPI_1_0_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE && + Probe->Signature != + EFI_ACPI_2_0_EXTENDED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) { + if (Installed == INSTALLED_TABLES_MAX) { + DEBUG ((EFI_D_ERROR, "%a: can't install more than %d tables\n", Func, + INSTALLED_TABLES_MAX)); + Status = EFI_OUT_OF_RESOURCES; + break; + } + + Status = AcpiProtocol->InstallAcpiTable (AcpiProtocol, Probe, + Probe->Length, &InstalledKey[Installed]); + if (EFI_ERROR (Status)) { + DEBUG ((EFI_D_ERROR, + "%a: failed to install table " DBGFMT " at offset 0x%Lx: %r\n", Func, + DBGSTR (&Probe->Signature), (UINT64) Processed, Status)); + break; + } + + ++Installed; + } + + Processed += Probe->Length; + } + + if (Processed == TablesFileSize || Status == EFI_PROTOCOL_ERROR) { + DEBUG ((EFI_D_INFO, "%a: installed %d tables\n", Func, Installed)); + Status = EFI_SUCCESS; + } else { + ASSERT (EFI_ERROR (Status)); + + // + // Roll back partial installation. + // + while (Installed > 0) { + --Installed; + AcpiProtocol->UninstallAcpiTable (AcpiProtocol, InstalledKey[Installed]); + } + } + + FreePool (InstalledKey); + +FreeTables: + FreePool (Tables); + + return Status; +}
Qemu v1.7.0-rc0 features an ACPI linker/loader interface, available over fw_cfg, written by Michael Tsirkin. Qemu composes all ACPI tables on the host side, according to the target hardware configuration, and makes the tables available to any guest firmware over fw_cfg. The feature moves the burden of keeping ACPI tables up-to-date from boot firmware to qemu (which is the source of hardware configuration anyway). This patch adds client code for this feature. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h | 7 +- OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c | 12 +- OvmfPkg/AcpiPlatformDxe/Qemu.c | 204 +++++++++++++++++++++++++++++++++ 3 files changed, 215 insertions(+), 8 deletions(-)