Re: [Xen-devel] [edk2] [PATCH] OvmfPkg: AcpiPlatformDxe: PCI enumeration may be disabled
On 02/12/15 21:53, Jordan Justen wrote: I think gEfiPciEnumerationCompleteProtocolGuid should be installed by MdeModulePkg/Bus/Pci/PciBusDxe, even when PcdPciDisableBusEnumeration is set. Ray's main feedback seemed to be that we need to make sure PciBusDxe only installs the protocol once. (I'll also reply to the other related patch thread.) First, I now agree that this patch of mine should not go in, hence: Self-NAK Second, I tend to disagree that that gEfiPciEnumerationCompleteProtocolGuid should be installed even if full enumeration was eschewed. This might slightly change the de-facto meaning of the protocol (because no resource allocation took place). In general I think we should not try to touch MdeModulePkg/Bus/Pci/PciBusDxe for our need here. So let's think again what we need. We want to delay the download and installation of ACPI tables on virt platforms until PCI enumeration is complete, *except* in some cases: (1) When OVMF runs on Xen. In that case PcdPciDisableBusEnumeration is TRUE, and the PCI bus driver does not install gEfiPciEnumerationCompleteProtocolGuid (in my opinion: correctly, because no resource allocation takes place, which is the de-facto meaning of the completion protocol). This means that the depex in OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf is no longer correct: [Depex] gEfiAcpiTableProtocolGuid AND gEfiPciEnumerationCompleteProtocolGuid (2) When the platform in question lacks PCI. Right now this means ArmVirtualizationQemu. However, that is about to change (I've mostly ported PCI support to ArmVirtualizationQemu; virtio-pci, USB keyboard, std-VGA work). Importantly, presence of PCI on ARM will become a *dynamic* question, dependent on the QEMU version. Current master QEMU does not provide PCI hardware for arm/mach-virt, but Alex Graf's patches in Peter Maydell's target-arm.next branch do, and so will master soon. This means that the depex in OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf will also break: [Depex] gEfiAcpiTableProtocolGuid [Depex.IA32, Depex.X64] gEfiPciEnumerationCompleteProtocolGuid because on ARM we might or might not want to wait for enumeration completion dependent on whether the DTB ultimately describes a PCI root bridge or not. So here's what I propose. (Again, the above is *all* motivated by OvmfPkg/AcpiPlatformDxe/.) In my PCI-for-ArmVirtualizationQemu patchset, I will introduce a new protocol GUID (with NULL structure) that will simply say OvmfPkg's AcpiPlatformDxe should not wait for PCI enumeration. Then, *both* INF files under OvmfPkg/AcpiPlatformDxe shall receive the following depex: [Depex] gEfiAcpiTableProtocolGuid AND (gEfiPciEnumerationCompleteProtocolGuid OR gOvmfAcpiPlatformNoPciEnumerationProtocolGuid ) Then each particular platform driver shall unblock AcpiPlatformDxe in its own way: - OVMF on QEMU will do nothing special, it'll just go with the usual gEfiPciEnumerationCompleteProtocolGuid, installed by PciBusDxe. - OVMF on Xen will install gOvmfAcpiPlatformNoPciEnumerationProtocolGuid -- Wei, could you post a patch for this later? I think the protocol should be installed in XenBusDxeDriverBindingStart(), when it succeeds. It would be probably prudent to coordinate with Ard, wrt. Ard's series that brings Xen to ArmVirtualizationQemu. - In my PCI-for-ArmVirtualizationQemu series, I'll install gOvmfAcpiPlatformNoPciEnumerationProtocolGuid in case PCI is unavailable on the particular QEMU version. My main point is, our *real* target is OvmfPkg/AcpiPlatformDxe here, and the conditions whether to delay or not to delay its work until PCI enumeration is complete are platform dependent and *dynamic*. We should let all platform-specific drivers decide for themselves, and we should steer clear of drivers that are central to edk2, like PciBusDxe. What do you guys think? Thanks! Laszlo -Jordan On 2015-02-12 04:16:07, Laszlo Ersek wrote: SVN r16411 delayed ACPI table installation until PCI enumeration was complete, because on QEMU the ACPI-related fw_cfg files should only be downloaded after PCI enumeration. However, InitializeXen() in OvmfPkg/PlatformPei/Xen.c sets PcdPciDisableBusEnumeration to TRUE. This causes PciBusDriverBindingStart() in MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c to set gFullEnumeration to FALSE, which in turn makes PciEnumerator() in MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c branch to PciEnumeratorLight(). The installation of EFI_PCI_ENUMERATION_COMPLETE_PROTOCOL at the end of PciEnumerator() is not reached. Which means that starting with SVN r16411, AcpiPlatformDxe is never dispatched on Xen. This patch replaces the EFI_PCI_ENUMERATION_COMPLETE_PROTOCOL depex with a matching protocol registration callback for the PCI enumeration enabled (ie. QEMU) case. When PCI enumeration is disabled (ie. when running on Xen), AcpiPlatformDxe doesn't wait for EFI_PCI_ENUMERATION_COMPLETE_PROTOCOL. Contributed-under: TianoCore
Re: [Xen-devel] [edk2] [PATCH] OvmfPkg: AcpiPlatformDxe: PCI enumeration may be disabled
On Feb 14, 2015, at 1:04 PM, Laszlo Ersek ler...@redhat.com wrote: On 02/14/15 21:03, Jordan Justen wrote: On 2015-02-14 08:38:37, Laszlo Ersek wrote: On 02/12/15 21:53, Jordan Justen wrote: I think gEfiPciEnumerationCompleteProtocolGuid should be installed by MdeModulePkg/Bus/Pci/PciBusDxe, even when PcdPciDisableBusEnumeration is set. Ray's main feedback seemed to be that we need to make sure PciBusDxe only installs the protocol once. (I'll also reply to the other related patch thread.) First, I now agree that this patch of mine should not go in, hence: Self-NAK Second, I tend to disagree that that gEfiPciEnumerationCompleteProtocolGuid should be installed even if full enumeration was eschewed. This might slightly change the de-facto meaning of the protocol (because no resource allocation took place). De-facto seems the correct term here, since the PI1.2 spec says very little about the protocol. My interpretation of the protocol is that it is a signal that the EFI knows about the basic PCI topology, and has installed most PciIo instances. Maybe PcdPciDisableBusEnumeration wasn't the best name. Perhaps PcdPciBusPreEnumerated would have been better. At any rate, in the case of Xen, the hypervisor has pre-enumerated the bus. PciBusDxe uses this and 'enumerates' PCI devices by simply scanning the pre-enumerated topology. So, I still think PciBusDxe should install gEfiPciEnumerationCompleteProtocolGuid, because it still seems like it acurately reflects the phase of the boot process. Regarding the ACPI tables issue, would a callback for the ReadyToBoot event work? EFI_EVENT_GROUP_READY_TO_BOOT This event group is notified by the system when the Boot Manager is about to load and execute a boot option. (1) As far as I understand, if you boot a UEFI application, then exit it, and boot something else, then the event group will be signalled each time. We could use a static variable in AcpiPlatformDxe to avoid trying to install again and again, but it wouldn't be nice IMHO. You can always use a global. This is the secondary reason I'm not enthusiastic about EFI_EVENT_GROUP_READY_TO_BOOT. :) (2) The main reason is different: in both OVMF and ArmVirtualizationQemu we can boot a kernel directly, taken from QEMU. That happens without EFI_EVENT_GROUP_READY_TO_BOOT being signalled. However, the kernel booted thus should have both ACPI tables and configured PCI devices (if there is a PCI host). In OVMF this is ensured by: PlatformBdsPolicyBehavior() PlatformBdsConnectSequence() BdsLibConnectAll() TryRunningQemuKernel() where the BdsLibConnectAll() call listed above will cover the enumeration, and trigger the dependent ACPI table installation as well, before we go to the kernel. One idea could be to signal EFI_EVENT_GROUP_READY_TO_BOOT ourselves, That sounds like the right thing to do. before booting the kernel; however this event group seems quite tied to the Boot Manager itself (see again its definition above). The Boot Manager has a few responsibilities in EFI (and few more in PI where it is called the BDS), but in general it is a place where a lot of platform specific policy is enforced. So updating the Boot Manager do do the right thing sees like a good answer. Thanks, Andrew Fish I'll post my patch series soon; my idea that is relevant for this discussion is at its beginning (and a later patch also displays how I mean it to be used). We can discuss it there too if you like. Thanks Laszlo -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ http://goparallel.sourceforge.net/ ___ edk2-devel mailing list edk2-de...@lists.sourceforge.net mailto:edk2-de...@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel https://lists.sourceforge.net/lists/listinfo/edk2-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [edk2] [PATCH] OvmfPkg: AcpiPlatformDxe: PCI enumeration may be disabled
On 2015-02-14 08:38:37, Laszlo Ersek wrote: On 02/12/15 21:53, Jordan Justen wrote: I think gEfiPciEnumerationCompleteProtocolGuid should be installed by MdeModulePkg/Bus/Pci/PciBusDxe, even when PcdPciDisableBusEnumeration is set. Ray's main feedback seemed to be that we need to make sure PciBusDxe only installs the protocol once. (I'll also reply to the other related patch thread.) First, I now agree that this patch of mine should not go in, hence: Self-NAK Second, I tend to disagree that that gEfiPciEnumerationCompleteProtocolGuid should be installed even if full enumeration was eschewed. This might slightly change the de-facto meaning of the protocol (because no resource allocation took place). De-facto seems the correct term here, since the PI1.2 spec says very little about the protocol. My interpretation of the protocol is that it is a signal that the EFI knows about the basic PCI topology, and has installed most PciIo instances. Maybe PcdPciDisableBusEnumeration wasn't the best name. Perhaps PcdPciBusPreEnumerated would have been better. At any rate, in the case of Xen, the hypervisor has pre-enumerated the bus. PciBusDxe uses this and 'enumerates' PCI devices by simply scanning the pre-enumerated topology. So, I still think PciBusDxe should install gEfiPciEnumerationCompleteProtocolGuid, because it still seems like it acurately reflects the phase of the boot process. Regarding the ACPI tables issue, would a callback for the ReadyToBoot event work? -Jordan In general I think we should not try to touch MdeModulePkg/Bus/Pci/PciBusDxe for our need here. So let's think again what we need. We want to delay the download and installation of ACPI tables on virt platforms until PCI enumeration is complete, *except* in some cases: (1) When OVMF runs on Xen. In that case PcdPciDisableBusEnumeration is TRUE, and the PCI bus driver does not install gEfiPciEnumerationCompleteProtocolGuid (in my opinion: correctly, because no resource allocation takes place, which is the de-facto meaning of the completion protocol). This means that the depex in OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf is no longer correct: [Depex] gEfiAcpiTableProtocolGuid AND gEfiPciEnumerationCompleteProtocolGuid (2) When the platform in question lacks PCI. Right now this means ArmVirtualizationQemu. However, that is about to change (I've mostly ported PCI support to ArmVirtualizationQemu; virtio-pci, USB keyboard, std-VGA work). Importantly, presence of PCI on ARM will become a *dynamic* question, dependent on the QEMU version. Current master QEMU does not provide PCI hardware for arm/mach-virt, but Alex Graf's patches in Peter Maydell's target-arm.next branch do, and so will master soon. This means that the depex in OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf will also break: [Depex] gEfiAcpiTableProtocolGuid [Depex.IA32, Depex.X64] gEfiPciEnumerationCompleteProtocolGuid because on ARM we might or might not want to wait for enumeration completion dependent on whether the DTB ultimately describes a PCI root bridge or not. So here's what I propose. (Again, the above is *all* motivated by OvmfPkg/AcpiPlatformDxe/.) In my PCI-for-ArmVirtualizationQemu patchset, I will introduce a new protocol GUID (with NULL structure) that will simply say OvmfPkg's AcpiPlatformDxe should not wait for PCI enumeration. Then, *both* INF files under OvmfPkg/AcpiPlatformDxe shall receive the following depex: [Depex] gEfiAcpiTableProtocolGuid AND (gEfiPciEnumerationCompleteProtocolGuid OR gOvmfAcpiPlatformNoPciEnumerationProtocolGuid ) Then each particular platform driver shall unblock AcpiPlatformDxe in its own way: - OVMF on QEMU will do nothing special, it'll just go with the usual gEfiPciEnumerationCompleteProtocolGuid, installed by PciBusDxe. - OVMF on Xen will install gOvmfAcpiPlatformNoPciEnumerationProtocolGuid -- Wei, could you post a patch for this later? I think the protocol should be installed in XenBusDxeDriverBindingStart(), when it succeeds. It would be probably prudent to coordinate with Ard, wrt. Ard's series that brings Xen to ArmVirtualizationQemu. - In my PCI-for-ArmVirtualizationQemu series, I'll install gOvmfAcpiPlatformNoPciEnumerationProtocolGuid in case PCI is unavailable on the particular QEMU version. My main point is, our *real* target is OvmfPkg/AcpiPlatformDxe here, and the conditions whether to delay or not to delay its work until PCI enumeration is complete are platform dependent and *dynamic*. We should let all platform-specific drivers decide for themselves, and we should steer clear of drivers that are central to edk2, like PciBusDxe. What do you guys think? Thanks! Laszlo -Jordan On 2015-02-12 04:16:07, Laszlo Ersek wrote: SVN r16411 delayed ACPI table installation until PCI enumeration was complete, because on QEMU the
Re: [Xen-devel] [edk2] [PATCH] OvmfPkg: AcpiPlatformDxe: PCI enumeration may be disabled
On 02/14/15 21:03, Jordan Justen wrote: On 2015-02-14 08:38:37, Laszlo Ersek wrote: On 02/12/15 21:53, Jordan Justen wrote: I think gEfiPciEnumerationCompleteProtocolGuid should be installed by MdeModulePkg/Bus/Pci/PciBusDxe, even when PcdPciDisableBusEnumeration is set. Ray's main feedback seemed to be that we need to make sure PciBusDxe only installs the protocol once. (I'll also reply to the other related patch thread.) First, I now agree that this patch of mine should not go in, hence: Self-NAK Second, I tend to disagree that that gEfiPciEnumerationCompleteProtocolGuid should be installed even if full enumeration was eschewed. This might slightly change the de-facto meaning of the protocol (because no resource allocation took place). De-facto seems the correct term here, since the PI1.2 spec says very little about the protocol. My interpretation of the protocol is that it is a signal that the EFI knows about the basic PCI topology, and has installed most PciIo instances. Maybe PcdPciDisableBusEnumeration wasn't the best name. Perhaps PcdPciBusPreEnumerated would have been better. At any rate, in the case of Xen, the hypervisor has pre-enumerated the bus. PciBusDxe uses this and 'enumerates' PCI devices by simply scanning the pre-enumerated topology. So, I still think PciBusDxe should install gEfiPciEnumerationCompleteProtocolGuid, because it still seems like it acurately reflects the phase of the boot process. Regarding the ACPI tables issue, would a callback for the ReadyToBoot event work? EFI_EVENT_GROUP_READY_TO_BOOT This event group is notified by the system when the Boot Manager is about to load and execute a boot option. (1) As far as I understand, if you boot a UEFI application, then exit it, and boot something else, then the event group will be signalled each time. We could use a static variable in AcpiPlatformDxe to avoid trying to install again and again, but it wouldn't be nice IMHO. This is the secondary reason I'm not enthusiastic about EFI_EVENT_GROUP_READY_TO_BOOT. :) (2) The main reason is different: in both OVMF and ArmVirtualizationQemu we can boot a kernel directly, taken from QEMU. That happens without EFI_EVENT_GROUP_READY_TO_BOOT being signalled. However, the kernel booted thus should have both ACPI tables and configured PCI devices (if there is a PCI host). In OVMF this is ensured by: PlatformBdsPolicyBehavior() PlatformBdsConnectSequence() BdsLibConnectAll() TryRunningQemuKernel() where the BdsLibConnectAll() call listed above will cover the enumeration, and trigger the dependent ACPI table installation as well, before we go to the kernel. One idea could be to signal EFI_EVENT_GROUP_READY_TO_BOOT ourselves, before booting the kernel; however this event group seems quite tied to the Boot Manager itself (see again its definition above). I'll post my patch series soon; my idea that is relevant for this discussion is at its beginning (and a later patch also displays how I mean it to be used). We can discuss it there too if you like. Thanks Laszlo ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [edk2] [PATCH] OvmfPkg: AcpiPlatformDxe: PCI enumeration may be disabled
I think gEfiPciEnumerationCompleteProtocolGuid should be installed by
MdeModulePkg/Bus/Pci/PciBusDxe, even when PcdPciDisableBusEnumeration
is set.
Ray's main feedback seemed to be that we need to make sure PciBusDxe
only installs the protocol once. (I'll also reply to the other related
patch thread.)
-Jordan
On 2015-02-12 04:16:07, Laszlo Ersek wrote:
SVN r16411 delayed ACPI table installation until PCI enumeration was
complete, because on QEMU the ACPI-related fw_cfg files should only be
downloaded after PCI enumeration.
However, InitializeXen() in OvmfPkg/PlatformPei/Xen.c sets
PcdPciDisableBusEnumeration to TRUE. This causes
PciBusDriverBindingStart() in MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c to
set gFullEnumeration to FALSE, which in turn makes PciEnumerator() in
MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c branch to
PciEnumeratorLight(). The installation of
EFI_PCI_ENUMERATION_COMPLETE_PROTOCOL at the end of PciEnumerator() is not
reached.
Which means that starting with SVN r16411, AcpiPlatformDxe is never
dispatched on Xen.
This patch replaces the EFI_PCI_ENUMERATION_COMPLETE_PROTOCOL depex with a
matching protocol registration callback for the PCI enumeration enabled
(ie. QEMU) case. When PCI enumeration is disabled (ie. when running on
Xen), AcpiPlatformDxe doesn't wait for
EFI_PCI_ENUMERATION_COMPLETE_PROTOCOL.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek ler...@redhat.com
---
OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 4 +-
OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c | 84
+++--
2 files changed, 72 insertions(+), 16 deletions(-)
diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
index 53292bf..6b2c9d2 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
@@ -56,16 +56,18 @@
[Protocols]
gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
+ gEfiPciEnumerationCompleteProtocolGuid# PROTOCOL SOMETIMES_CONSUMED
[Guids]
gEfiXenInfoGuid
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
[Depex]
- gEfiAcpiTableProtocolGuid AND gEfiPciEnumerationCompleteProtocolGuid
+ gEfiAcpiTableProtocolGuid
diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
index 11f0ca8..9823eba 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
@@ -12,6 +12,7 @@
**/
+#include Protocol/PciEnumerationComplete.h
#include AcpiPlatform.h
EFI_STATUS
@@ -221,6 +222,47 @@ FindAcpiTablesInFv (
return EFI_SUCCESS;
}
+STATIC
+EFI_STATUS
+EFIAPI
+InstallTables (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
+
+ Status = gBS-LocateProtocol (gEfiAcpiTableProtocolGuid,
+ NULL /* Registration */, (VOID **)AcpiTable);
+ if (EFI_ERROR (Status)) {
+return EFI_ABORTED;
+ }
+
+ if (XenDetected ()) {
+Status = InstallXenTables (AcpiTable);
+ } else {
+Status = InstallAllQemuLinkedTables (AcpiTable);
+ }
+
+ if (EFI_ERROR (Status)) {
+Status = FindAcpiTablesInFv (AcpiTable);
+ }
+
+ return Status;
+}
+
+STATIC
+VOID
+EFIAPI
+OnPciEnumerated (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ InstallTables ();
+ gBS-CloseEvent (Event);
+}
+
/**
Entrypoint of Acpi Platform driver.
@@ -239,31 +281,43 @@ AcpiPlatformEntryPoint (
IN EFI_SYSTEM_TABLE *SystemTable
)
{
- EFI_STATUS Status;
- EFI_ACPI_TABLE_PROTOCOL*AcpiTable;
+ EFI_STATUS Status;
+ VOID *Interface;
+ EFI_EVENT PciEnumerated;
+ VOID *Registration;
//
- // Find the AcpiTable protocol
+ // If PCI enumeration has been disabled, or it has already completed,
install
+ // the tables at once, and let the entry point's return code reflect the
full
+ // functionality.
//
- Status = gBS-LocateProtocol (
- gEfiAcpiTableProtocolGuid,
- NULL,
- (VOID**)AcpiTable
- );
- if (EFI_ERROR (Status)) {
-return EFI_ABORTED;
+ if (PcdGetBool (PcdPciDisableBusEnumeration)) {
+return InstallTables ();
}
- if (XenDetected ()) {
-Status = InstallXenTables (AcpiTable);
- } else {
-Status = InstallAllQemuLinkedTables (AcpiTable);
+ Status = gBS-LocateProtocol (gEfiPciEnumerationCompleteProtocolGuid,
+ NULL /* Registration */, Interface);
+ if (!EFI_ERROR (Status)) {
+return InstallTables ();
}
+ ASSERT
