| Message ID | 20160422144736.GD1885@perard.uk.xensource.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. Thanks! Laszlo
Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Anthony PERARD >Sent: Friday, April 22, 2016 10:48 PM >To: edk2-devel@lists.01.org >Cc: Xen Devel <xen-devel@lists.xen.org> >Subject: [edk2] OVMF broken under Xen (in PCI initialisation) > >Hi, > >Following the switch to MdeModulePkg/Bus/Pci/PciHostBridgeDxe, the pci root >bridge does not finish to initialize and breaks under Xen. > >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; > } > > >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) { > I think a more proper fix is to return EFI_UNSUPPORTED without returning any descriptors when the PcdPciDisableBusEnumeration is TRUE. Per UEFI Spec, EFI_UNSUPPORTED means "The current configuration of this PCI root bridge could not be retrieved." It's true when the PCD is true. > >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 ? When the PciRootBridgeIo.Configuration() returns EFI_UNSUPPORTED, this assertion can be fixed as well. Correct? > >Thanks, > >-- >Anthony PERARD >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org >https://lists.01.org/mailman/listinfo/edk2-devel
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; } 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) {