On Mon, Apr 25, 2016 at 01:43:01PM +0200, Laszlo Ersek wrote:
> On 04/22/16 16:47, Anthony PERARD wrote:
> > Hi,
> >
> > Following the switch to MdeModulePkg/Bus/Pci/PciHostBridgeDxe, the pci root
> > bridge does not finish to initialize and breaks under Xen.
>
> (Adding Ray Ni)
>
> > There are several issue probably due to the use of
> > PcdPciDisableBusEnumeration=TRUE.
> >
> > First one:
> > ASSERT MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c(99):
> > Bridge->Mem.Limit < 0x0000000100000000ULL
> >
> > This patch would fix the first assert:
> > diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/PlatformPei/Xen.c
> > index 7fa9019..15ec7a7 100644
> > --- a/OvmfPkg/PlatformPei/Xen.c
> > +++ b/OvmfPkg/PlatformPei/Xen.c
> > @@ -227,5 +227,11 @@ InitializeXen (
> >
> > PcdSetBool (PcdPciDisableBusEnumeration, TRUE);
> >
> > + // Values from hvmloader
> > +#define PCI_MEM_END 0xFC000000
> > +#define HVM_BELOW_4G_MMIO_START 0xF0000000
> > + PcdSet64 (PcdPciMmio32Base, HVM_BELOW_4G_MMIO_START);
> > + PcdSet64 (PcdPciMmio32Size, PCI_MEM_END - HVM_BELOW_4G_MMIO_START);
> > +
> > return EFI_SUCCESS;
> > }
>
> For this, I am to blame; sorry about the regression. Can you please
> submit the change as a standalone patch, and mention commit
> 03845e90cc432 as the one that missed it?
>
> Also, can you please add the macros to "OvmfPkg/PlatformPei/Xen.h"
> instead, near OVMF_INFO_PHYSICAL_ADDRESS?
>
> >
> >
> > But that not enough, next assert is:
> > ASSERT MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c(807): RootBridge
> > != ((void *) 0)
> >
> > I have worked around this one with the following patch. That would
> > correspond to how the former implementation in OvmfPkg was initializing the
> > struct. Maybe a better way would be to fill ResAllocated under Xen,
> > somehow. Under Xen, the ResAllocNode status is not allocated, so no
> > Descriptor are created.
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > index cda9b49..eda92b6 100644
> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > @@ -1509,15 +1509,13 @@ RootBridgeIoConfiguration (
> >
> > ResAllocNode = &RootBridge->ResAllocNode[Index];
> >
> > - if (ResAllocNode->Status != ResAllocated) {
> > - continue;
> > - }
> > -
> > Descriptor->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> > Descriptor->Len = sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - 3;
> > + if (ResAllocNode->Status != ResAllocated) {
> > Descriptor->AddrRangeMin = ResAllocNode->Base;
> > Descriptor->AddrRangeMax = ResAllocNode->Base + ResAllocNode->Length
> > - 1;
> > Descriptor->AddrLen = ResAllocNode->Length;
> > + }
> > switch (ResAllocNode->Type) {
> >
> >
> > That leads to one last assert:
> >> QemuVideo: Cirrus 5446 detected
> >> InstallProtocolInterface: 09576E91-6D3F-11D2-8E39-00A0C969723B 7B9EF598
> >> Adding Mode 0 as Cirrus Internal Mode 0: 640x480, 32-bit, 60 Hz
> >> Adding Mode 1 as Cirrus Internal Mode 1: 800x600, 32-bit, 60 Hz
> >> Adding Mode 2 as Cirrus Internal Mode 2: 1024x768, 24-bit, 60 Hz
> >> PixelBlueGreenRedReserved8BitPerColor
> >> ASSERT
> >> /local/home/sheep/work/ovmf/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c(1789):
> >> ((BOOLEAN)(0==1))
> > For this one, I've workaround by reverting patch 7b0a1ea
> > (MdeModuelPkg/PciBus: Return AddrTranslationOffset in GetBarAttributes).
> > That issue was introduce while still using the USE_OLD_PCI_HOST.
> >
> > Any idee or pointer on how to fix those issues ?
>
> I think your suggestion that PcdPciDisableBusEnumeration=TRUE causes
> these problems is likely correct. Both of the above issues seem to
> originate from PciHostBridgeDxe's assumption that resources have been
> allocated (i.e., there has been an enumeration).
>
> Please discuss these questions with Ray -- I guess PciHostBridgeDxe may
> not have received a lot of testing with
> PcdPciDisableBusEnumeration=TRUE. I can imagine that simply considering
> PcdPciDisableBusEnumeration, and acting conditionally upon its value,
> might solve these problems.
>
I made a quick fix to skip those checks when PcdPciDisableBusEnumeration
is TRUE. Plus the fix for OvmfPkg/PlatformPei/Xen.c, I can now boot to
the UEFI shell.
Ray,
Do you think this is the right way to fix it?
Thanks,
Gary Lin
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index f72598d..0fb150c 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1824,6 +1824,7 @@ PciIoGetBarAttributes (
PCI_IO_DEVICE *PciIoDevice;
EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
EFI_ACPI_END_TAG_DESCRIPTOR *End;
+ BOOLEAN EnablePciEnumeration;
PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
@@ -1918,7 +1919,8 @@ PciIoGetBarAttributes (
//
// Get the Address Translation Offset
//
- if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
+ EnablePciEnumeration = !PcdGetBool (PcdPciDisableBusEnumeration);
+ if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM && EnablePciEnumeration) {
Descriptor->AddrTranslationOffset = GetMmioAddressTranslationOffset (
PciIoDevice->PciRootBridgeIo,
Descriptor->AddrRangeMin,
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
index ab5d87e..712497d 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
@@ -53,3 +53,6 @@ [Protocols]
[Depex]
gEfiCpuIo2ProtocolGuid AND
gEfiMetronomeArchProtocolGuid
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index cda9b49..4bac355 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
+#include <Library/PcdLib.h>
#include "PciHostBridge.h"
#include "PciRootBridge.h"
#include "PciHostResource.h"
@@ -1495,6 +1496,7 @@ RootBridgeIoConfiguration (
PCI_RES_NODE *ResAllocNode;
EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
EFI_ACPI_END_TAG_DESCRIPTOR *End;
+ BOOLEAN EnablePciEnumeration;
//
// Get this instance of the Root Bridge.
@@ -1505,19 +1507,22 @@ RootBridgeIoConfiguration (
TypeMax * sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) + sizeof (EFI_ACPI_END_TAG_DESCRIPTOR)
);
Descriptor = RootBridge->ConfigBuffer;
+ EnablePciEnumeration = !PcdGetBool (PcdPciDisableBusEnumeration);
for (Index = TypeIo; Index < TypeMax; Index++) {
ResAllocNode = &RootBridge->ResAllocNode[Index];
- if (ResAllocNode->Status != ResAllocated) {
+ if (ResAllocNode->Status != ResAllocated && EnablePciEnumeration) {
continue;
}
Descriptor->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
Descriptor->Len = sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - 3;
- Descriptor->AddrRangeMin = ResAllocNode->Base;
- Descriptor->AddrRangeMax = ResAllocNode->Base + ResAllocNode->Length - 1;
- Descriptor->AddrLen = ResAllocNode->Length;
+ if (ResAllocNode->Status == ResAllocated) {
+ Descriptor->AddrRangeMin = ResAllocNode->Base;
+ Descriptor->AddrRangeMax = ResAllocNode->Base + ResAllocNode->Length - 1;
+ Descriptor->AddrLen = ResAllocNode->Length;
+ }
switch (ResAllocNode->Type) {
case TypeIo:
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
