comments below
On 02/16/15 03:06, Jordan Justen wrote:
> Eventually two tables will be installed first (FACS, FACP), and the
> remaining will be installed at the ReadyToBoot event.
>
> In this patch they will still continue to all be published during the
> driver's entry point.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jordan Justen <[email protected]>
> ---
> OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c | 2 +-
> OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h | 3 +-
> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 65
> ++++++++++++++++++++++++++++++---
> 3 files changed, 62 insertions(+), 8 deletions(-)
>
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
> b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
> index 331cdc4..33cc05c 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
> @@ -255,7 +255,7 @@ AcpiPlatformEntryPoint (
> if (XenDetected ()) {
> Status = InstallXenTables (AcpiTable);
> } else {
> - Status = InstallAllQemuLinkedTables (AcpiTable);
> + Status = InstallQemuFwCfgTables (AcpiTable);
> }
>
> if (EFI_ERROR (Status)) {
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> index e757233..ae11e79 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> @@ -63,8 +63,9 @@ InstallXenTables (
>
> EFI_STATUS
> EFIAPI
> -InstallAllQemuLinkedTables (
> +InstallQemuFwCfgTables (
> IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol
> );
> +
> #endif
>
> diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> index d75394c..6fa434e 100644
> --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> @@ -414,7 +414,8 @@ Process2ndPassCmdAddPointer (
> IN CONST ORDERED_COLLECTION *Tracker,
> IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol,
> IN OUT UINTN InstalledKey[INSTALLED_TABLES_MAX],
> - IN OUT INT32 *NumInstalled
> + IN OUT INT32 *NumInstalled,
> + IN BOOLEAN EarlyTablesOnly
> )
> {
> CONST ORDERED_COLLECTION_ENTRY *TrackerEntry;
> @@ -550,10 +551,12 @@ Process2ndPassCmdAddPointer (
> AcpiProtocol->InstallAcpiTable().
>
So here is where the new info should be used ultimately, but at this
point we don't use the parameter yet. Okay.
> **/
> +STATIC
> EFI_STATUS
> EFIAPI
> -InstallAllQemuLinkedTables (
> - IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol
> +DownloadAndInstallTables (
> + IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol,
> + IN BOOLEAN EarlyTablesOnly
> )
> {
> EFI_STATUS Status;
> @@ -632,8 +635,10 @@ InstallAllQemuLinkedTables (
> Installed = 0;
> for (LoaderEntry = LoaderStart; LoaderEntry < LoaderEnd; ++LoaderEntry) {
> if (LoaderEntry->Type == QemuLoaderCmdAddPointer) {
> - Status = Process2ndPassCmdAddPointer (&LoaderEntry->Command.AddPointer,
> - Tracker, AcpiProtocol, InstalledKey, &Installed);
> + Status = Process2ndPassCmdAddPointer (
> + &LoaderEntry->Command.AddPointer, Tracker, AcpiProtocol,
> + InstalledKey, &Installed, EarlyTablesOnly
> + );
> if (EFI_ERROR (Status)) {
> break;
> }
Okay, function is renamed and it propagates the parameter.
> @@ -686,6 +691,54 @@ FreeLoader:
>
>
> /**
> + Download, process, and install ACPI table data from the QEMU loader
> + interface.
> +
> + The installation process is handled in two phase. The FACS table is
"two phase[s]"; typo
> + installed immediatedly, and the remaining tables are installed after the
> + ReadyToBoot event is signalled.
> +
> + @param[in] AcpiProtocol The ACPI table protocol used to install tables.
> +
> + @retval EFI_UNSUPPORTED Firmware configuration is unavailable, or
> QEMU
> + loader command with unsupported parameters
> + has been found.
> +
> + @retval EFI_NOT_FOUND The host doesn't export the required fw_cfg
> + files.
> +
> + @retval EFI_OUT_OF_RESOURCES Memory allocation failed, or more than
> + INSTALLED_TABLES_MAX tables found.
> +
> + @retval EFI_PROTOCOL_ERROR Found invalid fw_cfg contents.
> +
> + @return Status codes returned by
> + AcpiProtocol->InstallAcpiTable().
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +InstallQemuFwCfgTables (
> + IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol
> + )
> +{
> + EFI_STATUS Status;
> +
> + //
> + // At this early stage, we install a few tables that other code depends on.
> + //
> + // For example, the S3 code depends on the FACP table being available.
I think the comments should name the same table. As far as I understand,
the FACS is the directly required table (it carries the wake up vector
address), the FACP might "only" be relevant because it is the link
between the RSDT/XSDT and the FACS.
> + //
> + Status = DownloadAndInstallTables (AcpiProtocol, TRUE);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
Redundant, see below:
> +
> + return Status;
> +}
(But perhaps in the next patch you'll insert stuff inbetween.)
> +
> +
> +/**
> Entrypoint of QEMU fw-cfg Acpi Platform driver.
>
> @param ImageHandle
> @@ -718,6 +771,6 @@ QemuFwCfgAcpiPlatformEntryPoint (
> return EFI_ABORTED;
> }
>
> - Status = InstallAllQemuLinkedTables (AcpiTable);
> + Status = InstallQemuFwCfgTables (AcpiTable);
> return Status;
> }
>
With the comment fixes, and optionally with the condition fix (as you
see fit):
Reviewed-by: Laszlo Ersek <[email protected]>
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
