On 03/21/16 10:32, Ard Biesheuvel wrote:
> On 14 March 2016 at 13:52, Laszlo Ersek <[email protected]> wrote:
>> QEMU's ACPI table generator can only create meaningful _CRS objects --
>> apertures -- for the root buses if all of the PCI devices behind those
>> buses are actively decoding their IO and MMIO resources, at the time of
>> the firmware fetching the "etc/table-loader" fw_cfg file. This is not a
>> QEMU error; QEMU follows the definition of BARs (which are meaningless
>> when decoding is disabled).
>>
>> Currently we hook up AcpiPlatformDxe to the PCI Bus driver's
>> gEfiPciEnumerationCompleteProtocolGuid cue. Unfortunately, when the PCI
>> Bus driver installs this protocol, it's *still* not the right time for
>> fetching "etc/table-loader": although resources have been allocated and
>> BARs have been programmed with them, the PCI Bus driver has also cleared
>> IO and MMIO decoding in the command registers of the devices.
>>
>> Furthermore, we couldn't reenable IO and MMIO decoding temporarily in our
>> gEfiPciEnumerationCompleteProtocolGuid callback even if we wanted to,
>> because at that time the PCI Bus driver has not produced PciIo instances
>> yet.
>>
>> Our Platform BDSes are responsible for connecting the root bridges, hence
>> they know exactly when the PciIo instances become available -- not when
>> PCI enumeration completes (signaled by the above protocol), but when the
>> ConnectController() calls return.
>>
>> This is when our Platform BDSes should explicitly cue in AcpiPlatformDxe.
>> Then AcpiPlatformDxe can temporarily enable IO and MMIO decoding for all
>> devices, while it contacts QEMU for the ACPI payload.
>>
>> This patch introduces the non-standard protocol that we'll use for
>> unleashing AcpiPlatformDxe from or Platform BDSes.
>>
>> Cc: Ard Biesheuvel <[email protected]>
>> Cc: Gerd Hoffmann <[email protected]>
>> Cc: Jordan Justen <[email protected]>
>> Cc: Marcel Apfelbaum <[email protected]>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <[email protected]>
>> ---
>> OvmfPkg/OvmfPkg.dec | 1 +
>> OvmfPkg/Include/Protocol/RootBusesConnected.h | 33 ++++++++++++++++++++
>> 2 files changed, 34 insertions(+)
>>
>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
>> index 784542f62368..a7d887ad353c 100644
>> --- a/OvmfPkg/OvmfPkg.dec
>> +++ b/OvmfPkg/OvmfPkg.dec
>> @@ -60,14 +60,15 @@ [Guids]
>> gXenBusRootDeviceGuid = {0xa732241f, 0x383d, 0x4d9c, {0x8a,
>> 0xe1, 0x8e, 0x09, 0x83, 0x75, 0x89, 0xd7}}
>>
>> [Protocols]
>> gVirtioDeviceProtocolGuid = {0xfa920010, 0x6785, 0x4941, {0xb6,
>> 0xec, 0x49, 0x8c, 0x57, 0x9f, 0x16, 0x0a}}
>> gBlockMmioProtocolGuid = {0x6b558ce3, 0x69e5, 0x4c67, {0xa6,
>> 0x34, 0xf7, 0xfe, 0x72, 0xad, 0xbe, 0x84}}
>> gXenBusProtocolGuid = {0x3d3ca290, 0xb9a5, 0x11e3, {0xb7,
>> 0x5d, 0xb8, 0xac, 0x6f, 0x7d, 0x65, 0xe6}}
>> gXenIoProtocolGuid = {0x6efac84f, 0x0ab0, 0x4747, {0x81,
>> 0xbe, 0x85, 0x55, 0x62, 0x59, 0x04, 0x49}}
>> + gRootBusesConnectedProtocolGuid = {0x24a2d66f, 0xeedd, 0x4086, {0x90,
>> 0x42, 0xf2, 0x6e, 0x47, 0x97, 0xee, 0x69}}
>>
>> [PcdsFixedAtBuild]
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|0x0|UINT32|0
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize|0x0|UINT32|1
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|0x0|UINT32|0x15
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize|0x0|UINT32|0x16
>>
>> diff --git a/OvmfPkg/Include/Protocol/RootBusesConnected.h
>> b/OvmfPkg/Include/Protocol/RootBusesConnected.h
>> new file mode 100644
>> index 000000000000..02181cbbcbb8
>> --- /dev/null
>> +++ b/OvmfPkg/Include/Protocol/RootBusesConnected.h
>> @@ -0,0 +1,33 @@
>> +/** @file
>> + A non-standard protocol with which BDS indicates that PCI root bridges
>> have
>> + been connected, and PciIo protocol instances have become available.
>> +
>> + Note that this differs from the PCI Enumeration Complete Protocol as
>> defined
>> + in the PI 1.1 specification. That protocol is installed by the PCI bus
>> driver
>> + after enumeration and resource allocation have been completed, but before
>> + PciIo protocol instances are created.
>> +
>> + Copyright (C) 2016, Red Hat, Inc.
>> +
>> + This program and the accompanying materials are licensed and made
>> available
>> + under the terms and conditions of the BSD License which accompanies this
>> + distribution. The full text of the license may be found at
>> + http://opensource.org/licenses/bsd-license.php
>> +
>> + 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 _ROOT_BUSES_CONNECTED_H_
>> +#define _ROOT_BUSES_CONNECTED_H_
>> +
>> +#define ROOT_BUSES_CONNECTED_PROTOCOL_GUID \
>> + { 0x24a2d66f, \
>> + 0xeedd, \
>> + 0x4086, \
>> + { 0x90, 0x42, 0xf2, 0x6e, 0x47, 0x97, 0xee, 0x69 }, \
>> + }
>> +
>> +extern EFI_GUID gRootBusesConnectedProtocolGuid;
>> +
>> +#endif
>
> Recently, someone noted that declaring EFI_GUID's like this is not
> necessary in EDK2, since AutoGen.h will declare it if the guid is
> mentioned in the .inf, and the #define is never referenced either. Do
> you have any particular reason to include this .h file regardless?
>
Two reasons:
(a) stick with existing practice
(b) macros like ROOT_BUSES_CONNECTED_PROTOCOL_GUID are useful for
initializing EFI_GUID objects (including GUID objects that are
members of structures) that have static storage duration (and are
optionally constant). So the statement "the #define is never
referenced" cannot be made in general; it is up to client code.
Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
