On 02/17/15 23:39, Jordan Justen wrote: > On 2015-02-17 12:09:54, Laszlo Ersek wrote: >> On 02/17/15 20:15, Laszlo Ersek wrote: >>> On 02/17/15 18:14, Jordan Justen wrote: >>>> In OvmfPkg/AcpiPlatformDxe, we now look at the setting of >>>> PcdPciDisableBusEnumeration. If enumeration is enabled, then we wait >>>> for a callback on the gEfiPciEnumerationCompleteProtocolGuid protocol >>>> before installing the ACPI tables. Otherwise, we install the ACPI >>>> tables immediately. >>>> >>>> https://github.com/jljusten/edk2.git acpi-pci-pcd >>>> >>>> Jordan Justen (5): >>>> OvmfPkg/AcpiPlatformDxe: FindAcpiTablesInFv => InstallOvmfFvTables >>>> OvmfPkg/AcpiPlatformDxe: InstallAllQemuLinkedTables => >>>> InstallQemuFwCfgTables >>>> ArmVirtualizationPkg: Set PcdPciDisableBusEnumeration to TRUE >>>> OvmfPkg/AcpiPlatformDxe: Conditional callback for >>>> PciEnumerationComplete >>>> OvmfPkg/AcpiPlatformDxe: Remove PciEnumerationComplete depex >>>> >>>> .../ArmVirtualizationPkg/ArmVirtualizationQemu.dsc | 4 + >>>> OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c | 95 >>>> ++++++++++++++++++---- >>>> OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h | 5 +- >>>> OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 4 +- >>>> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 82 >>>> +++++++++++++++---- >>>> .../AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf | 7 +- >>>> 6 files changed, 160 insertions(+), 37 deletions(-) >>>> >>> >>> The approach taken in this patchset is one of the first ideas I had >>> while pondering the issue over the weekend. I didn't dare suggesting it, >>> lest I be crucified for trying to repurpose PcdPciDisableBusEnumeration >>> (because it now has dual consumption). >> >> Some clarification / correction: I didn't shy away from using this PCD in: >> >> [edk2] [PATCH] OvmfPkg: AcpiPlatformDxe: PCI enumeration may be disabled >> <http://thread.gmane.org/gmane.comp.bios.tianocore.devel/12608> > > Gah. We've basically come full circle and implemented it twice. I'm > suitably ashamed. Would you like to update your patch to cover > QemuFwCfgAcpiPlatformDxe.inf as well?
Yes, I'll do that, thanks! > For what it's worth, I still want to see PciBusDxe install > gEfiPciEnumerationCompleteProtocolGuid when > PcdPciDisableBusEnumeration is TRUE. Your patch for that is still on the list pending review, so I think that should be OK. > That would take care of the issue for Xen on IA32 and X64 without this > change, but it cover the other cases when PciBusDxe is not used at > all. Exactly. > >> but that patch doesn't count as "repurposing the PCD". >> >> "Repurposing the PCD" will be when I set it to TRUE in VirtFdtDxe, on >> ARM, when the PCI host bridge will be absent, because I'll know that >> PciBusDxe won't even Start(). >> >> So, it's great that you're fine with that, thanks! > > I'm not sure I would call it repurposing. For the sake of this change, > the issue is that for QEMU, if the PCI devices may be re-assigned > resources, then we need wait to read the ACPI tables. If > PcdPciDisableBusEnumeration is TRUE, then the PCI devices won't be > reassigned resources, and therefore OvmfPkg/AcpiPlatformDxe need not > wait. > > Regarding other potential uses of PcdPciDisableBusEnumeration, I don't > know what my opinion would be on them. If it is not part of OVMF, then > it might be that I don't even notice. :) Well, there's only two uses -- the one you described (enumeration without resource allocation), and the other one you described higher up (when PciBusDxe is part of the firmware but it doesn't do anything). In the first case, keying AcpiPlatformDxe off PcdPciDisableBusEnumeration didn't seem like repurposing, because PcdPciDisableBusEnumeration was the direct cause of PciBusDxe not installing the otherwise awaited protocol. In the second case, keying AcpiPlatformDxe off the PCD seemed like repurposing, because the PCD was in practice independent of what PciBusDxe would do (which was "nothing") -- PciBusDxe would not even reach the place where it would check the PCD. Anyway I think I over-explained it like five-fold, sorry :) > > But, I would be open to discussing adding an OvmfPkg PCD item that > indicates if PCI is supported. Maybe PcdOvmfPciEnabled? This would be somewhat parallel to the new protocol (in role), but... > Would that > help? *No*. That's an emphatical no. :) I would not like to add another PCD. I very much like working off *only* PcdPciDisableBusEnumeration in AcpiPlatformDxe. > If so, maybe OvmfPkg/AcpiPlatformDxe could be updated to use > that PCD instead. (Assuming PcdBusDxe is fixed.) Indeed, that could be an option, but please let's not pursue it. I'll seek to post a new version of my patchset tomorrow, and it should include a revised "OvmfPkg: AcpiPlatformDxe: PCI enumeration may be disabled". Thanks! Laszlo ------------------------------------------------------------------------------ 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
