diff mbox

[edk2] OvmfPkg: AcpiPlatformDxe: make dependency on PCI enumeration explicit

Message ID 1416240995-17646-1-git-send-email-lersek@redhat.com
State New
Headers show

Commit Message

Laszlo Ersek Nov. 17, 2014, 4:16 p.m. UTC
The ACPI payload that OVMF downloads from QEMU via fw_cfg depends on the
PCI enumaration and resource assignment performed by
MdeModulePkg/Bus/Pci/PciBusDxe.

Namely, although the ACPI payload is pre-generated in qemu during machine
initialization, in

  main()                                            [vl.c]
    qemu_run_machine_init_done_notifiers()
      pc_guest_info_machine_done()                  [hw/i386/pc.c]
        acpi_setup()                                [hw/i386/acpi-build.c]
          acpi_build()
          acpi_add_rom_blob()
            rom_add_blob(... acpi_build_update ...) [hw/core/loader.c]
              fw_cfg_add_file_callback()            [hw/nvram/fw_cfg.c]

the ACPI data is rebuilt at the first time any of the related fw_cfg files
are read, through the acpi_build_update() fw_cfg read-callback function:

  fw_cfg_read()                                     [hw/nvram/fw_cfg.c]
    acpi_build_update()                             [hw/i386/acpi-build.c]
      acpi_build()

(See qemu commit d87072ceeccf4f84a64d4bc59124bcd64286c070 and its
containing series.)

For this reason we must not dispatch AcpiPlatformDxe before PciBusDxe
completes the enumeration.

Luckily, the PI Specification 1.3 defines
EFI_PCI_ENUMERATION_COMPLETE_GUID in Volume 5, "10.9 End of PCI
Enumeration Overview", as an indicia to inform the platform when the PCI
enumeration process has completed. PciBusDxe installs this protocol at the
end of the PciEnumerator() function.

Let's add this GUID to the Depex section of AcpiPlatformDxe, in order to
state the dependency explicitly.

On Xen, and on older QEMU where the linker/loader fw_cfg interface is
unavailable, this introduces a harmless ordering constraint -- we'll
always include PciBusDxe in OVMF, so the dependency will always be
satisfied.

I tested this change as follows:

- I dumped the ACPI tables in a Fedora 20 guest, before and after the
  change, and compared them. The only thing that actually changed was the
  FACS address. (Which I promptly tested with S3 suspend/resume.) Plus, of
  course, the FACP checksum changed, because the FACP links the FACS.

- Tested S3 in my Windows Server 2008 R2 and Windows Server 2012 R2 guests.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jordan Justen Nov. 20, 2014, 2:28 a.m. UTC | #1
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

On 2014-11-17 08:16:35, Laszlo Ersek wrote:
> The ACPI payload that OVMF downloads from QEMU via fw_cfg depends on the
> PCI enumaration and resource assignment performed by
> MdeModulePkg/Bus/Pci/PciBusDxe.
> 
> Namely, although the ACPI payload is pre-generated in qemu during machine
> initialization, in
> 
>   main()                                            [vl.c]
>     qemu_run_machine_init_done_notifiers()
>       pc_guest_info_machine_done()                  [hw/i386/pc.c]
>         acpi_setup()                                [hw/i386/acpi-build.c]
>           acpi_build()
>           acpi_add_rom_blob()
>             rom_add_blob(... acpi_build_update ...) [hw/core/loader.c]
>               fw_cfg_add_file_callback()            [hw/nvram/fw_cfg.c]
> 
> the ACPI data is rebuilt at the first time any of the related fw_cfg files
> are read, through the acpi_build_update() fw_cfg read-callback function:
> 
>   fw_cfg_read()                                     [hw/nvram/fw_cfg.c]
>     acpi_build_update()                             [hw/i386/acpi-build.c]
>       acpi_build()
> 
> (See qemu commit d87072ceeccf4f84a64d4bc59124bcd64286c070 and its
> containing series.)
> 
> For this reason we must not dispatch AcpiPlatformDxe before PciBusDxe
> completes the enumeration.
> 
> Luckily, the PI Specification 1.3 defines
> EFI_PCI_ENUMERATION_COMPLETE_GUID in Volume 5, "10.9 End of PCI
> Enumeration Overview", as an indicia to inform the platform when the PCI
> enumeration process has completed. PciBusDxe installs this protocol at the
> end of the PciEnumerator() function.
> 
> Let's add this GUID to the Depex section of AcpiPlatformDxe, in order to
> state the dependency explicitly.
> 
> On Xen, and on older QEMU where the linker/loader fw_cfg interface is
> unavailable, this introduces a harmless ordering constraint -- we'll
> always include PciBusDxe in OVMF, so the dependency will always be
> satisfied.
> 
> I tested this change as follows:
> 
> - I dumped the ACPI tables in a Fedora 20 guest, before and after the
>   change, and compared them. The only thing that actually changed was the
>   FACS address. (Which I promptly tested with S3 suspend/resume.) Plus, of
>   course, the FACP checksum changed, because the FACP links the FACS.
> 
> - Tested S3 in my Windows Server 2008 R2 and Windows Server 2012 R2 guests.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> index 3229d71..c6dced2 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> @@ -66,5 +66,5 @@
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
>  
>  [Depex]
> -  gEfiAcpiTableProtocolGuid
> +  gEfiAcpiTableProtocolGuid AND gEfiPciEnumerationCompleteProtocolGuid
>  
> -- 
> 1.8.3.1
> 
> 
> ------------------------------------------------------------------------------
> 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=157005751&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
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=157005751&iu=/4140/ostg.clktrk
diff mbox

Patch

diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
index 3229d71..c6dced2 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
@@ -66,5 +66,5 @@ 
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
 
 [Depex]
-  gEfiAcpiTableProtocolGuid
+  gEfiAcpiTableProtocolGuid AND gEfiPciEnumerationCompleteProtocolGuid