On Wed, Apr 27, 2016 at 07:18:21AM +0000, Ni, Ruiyu wrote:
> Gary,
> In PciEnumeratorLight(), please directly assign 0 to MinBus when
> Configuration() returns error, instead of calling PciGetBusRange() to
> extract MinBus from the descriptors returned from Configuration().
>
OK, I ignored PciGetBusRange() and the check of the return status of
Configuration(). Since MinBus is already initialized, I don't bother to
change the value anyway. The change is attached.
And yes, this time the shell showed.
Thanks,
Gary Lin
> Regards,
> Ray
>
> >-----Original Message-----
> >From: Gary Lin [mailto:[email protected]]
> >Sent: Wednesday, April 27, 2016 2:54 PM
> >To: Ni, Ruiyu <[email protected]>
> >Cc: [email protected]; Xen Devel <[email protected]>
> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
> >
> >On Wed, Apr 27, 2016 at 05:39:40AM +0000, Ni, Ruiyu wrote:
> >>
> >>
> >> Regards,
> >> Ray
> >>
> >> >-----Original Message-----
> >> >From: Gary Lin [mailto:[email protected]]
> >> >Sent: Wednesday, April 27, 2016 12:29 PM
> >> >To: Ni, Ruiyu <[email protected]>
> >> >Cc: [email protected]; Xen Devel <[email protected]>
> >> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
> >> >
> >> >On Tue, Apr 26, 2016 at 09:40:42AM +0000, Ni, Ruiyu wrote:
> >> >> Gary,
> >> >> I can reproduce the issue and have debugged to get the reason.
> >> >> In MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c:
> >> >> PciEnumeratorLight() calls PciRootBridgeIo->Configuration()
> >> >> while the Configuration returns EFI_UNSUPPORTED resulting
> >> >> the PciBus driver exits earlier.
> >> >> You could try to manually set MinBus to 0 and MaxBus to 0xFF.
> >> >>
> >> >Do you mean to set MinBus and MaxBus in PciGetBusRange() ?
> >> >Should I also keep the EFI_UNSUPPORTED code in Configuration() ?
> >> Keep returning EFI_UNSUPPORTED in Configuration() and set
> >> MinBus/MaxBus in PciGetBusRange().
> >>
> >> >
> >> >> The second change you need to make is in PciIo.c:
> >> >> Change GetMmioAddressTranslationOffset() to return 0 instead
> >> >> of -1 when error occurs
> >> >>
> >> >
> >> >Should I also remove "ASSERT (FALSE);" ?
> >>
> >>
> >> ASSERT() won't be hit.
> >>
> >It didn't work (see my attached patch) :(
> >BTW, if Configuration() already returns EFI_UNSUPPORTED, then there is
> >no way to invoke PciGetBusRange(). Besides, MaxBus was never used in
> >PciEnumeratorLight(), so it's meaningless to assign any value to it.
> >
> >Cheers,
> >
> >Gary Lin
> >
> >> >
> >> >Cheers,
> >> >
> >> >Gary Lin
> >> >
> >> >> With the above 2 changes, can you check whether the system can
> >> >> boot?
> >> >>
> >> >> If it can boot, then we need to think about what the proper fix is.
> >> >> For OVMF above Xen, the PCI resources are assigned by Xen other than
> >> >> PciBus driver. So PciRootBridgeIo->Configuration() doesn't know
> >> >> the resource assignment information.
> >> >> With old PciHostBridge driver, the Configuration() returns BUS resource
> >> >> descriptor with MinBus = MaxBus = 0 (default value), so that the PciBus
> >> >> driver can detect all devices in BUS 0, but I guess devices in BUS 1+
> >> >> cannot
> >> >> be detected.
> >> >>
> >> >> Does Xen pass the resource assignment information through some memory
> >> >> blob to firmware? If yes, we could think about let PciHostBridgeLib
> >> >> pass that
> >> >> information to PciHostBridge driver. If no, we also could let
> >> >> PciHostBridgeLib
> >> >> collect the information (IO/MEM/BUS resource assignment) and pass that
> >> >> to PciHostBridgeDxe driver.
> >> >>
> >> >> And we need to have a way to tell PciHostBridgeDxe the resource
> >> >> information
> >> >> passed from PciHostBridgeLib is available resource to assign, or
> >> >> already allocated.
> >> >> Maybe just depends on the PcdPciBusDisableEnumeration. still thinking.
> >> >>
> >> >>
> >> >> Regards,
> >> >> Ray
> >> >>
> >> >> >-----Original Message-----
> >> >> >From: Gary Lin [mailto:[email protected]]
> >> >> >Sent: Tuesday, April 26, 2016 4:40 PM
> >> >> >To: Ni, Ruiyu <[email protected]>
> >> >> >Cc: [email protected]; Xen Devel <[email protected]>
> >> >> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
> >> >> >
> >> >> >On Tue, Apr 26, 2016 at 08:19:49AM +0000, Ni, Ruiyu wrote:
> >> >> >> Gary,
> >> >> >> Maybe the system boots to Shell well but the video isn't initialized
> >> >> >> properly.
> >> >> >> Can you build the firmware using "-D DEBUG_ON_SERIAL_PORT" switch and
> >> >> >> give me the boot log?
> >> >> >>
> >> >> >Hi Ray,
> >> >> >
> >> >> >I already did and it's how I make the firmware spit the log :-(
> >> >> >
> >> >> >Thanks,
> >> >> >
> >> >> >Gary Lin
> >> >> >
> >> >> >> Regards,
> >> >> >> Ray
> >> >> >>
> >> >> >> From: Gary Lin [mailto:[email protected]]
> >> >> >> Sent: Tuesday, April 26, 2016 3:35 PM
> >> >> >> To: Ni, Ruiyu <[email protected]>
> >> >> >> Cc: Anthony PERARD <[email protected]>;
> >> >> >> [email protected]; Xen Devel <[email protected]>
> >> >> >> Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
> >> >> >>
> >> >> >> On Tue, Apr 26, 2016 at 06:43:56AM +0000, Ni, Ruiyu wrote:
> >> >> >> >
> >> >> >> >
> >> >> >> > Regards,
> >> >> >> > Ray
> >> >> >> >
> >> >> >> > >-----Original Message-----
> >> >> >> > >From: edk2-devel [mailto:[email protected]] On
> >> >> >> > >Behalf Of Anthony PERARD
> >> >> >> > >Sent: Friday, April 22, 2016 10:48 PM
> >> >> >> > >To: [email protected]<mailto:[email protected]>
> >> >> >> > >Cc: Xen Devel
> >> >> >> > ><[email protected]<mailto:[email protected]>>
> >> >> >> > >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?
> >> >> >>
> >> >> >> I tried to make RootBridgeIoConfiguration() return EFI_UNSUPPORTED
> >> >> >> (see the attachment), and those asserts were gone. However, the
> >> >> >> system
> >> >> >> stuck somewhere after loading shell.efi. Here are the messages from
> >> >> >> the
> >> >> >> debug log:
> >> >> >>
> >> >> >> BdsLibConnectAll
> >> >> >> Select Item: 0xE
> >> >> >> Memory Previous Current Next
> >> >> >> Type Pages Pages Pages
> >> >> >> ====== ======== ======== ========
> >> >> >> 0A 00000004 00000001 00000004
> >> >> >> 09 00000008 00000011 00000015
> >> >> >> 00 00000004 00000001 00000004
> >> >> >> 06 00000024 00000057 0000006C
> >> >> >> 05 00000030 00000063 0000007B
> >> >> >> 03 00000180 0000048D 000005B0
> >> >> >> 04 00000F00 00000B65 00000F00
> >> >> >> Booting EFI Internal Shell
> >> >> >> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B
> >> >> >> 3EA580C0
> >> >> >> Loading driver at 0x0003E03F000 EntryPoint=0x0003E03F240 Shell.efi
> >> >> >> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF
> >> >> >> 3EA58B98
> >> >> >> InstallProtocolInterface: 387477C2-69C7-11D2-8E39-00A0C969723B
> >> >> >> 3EA54320
> >> >> >> InstallProtocolInterface: 752F3136-4E16-4FDC-A22A-E5F46812F4CA
> >> >> >> 3EA53CD8
> >> >> >> InstallProtocolInterface: 6302D008-7F9B-4F30-87AC-60C9FEF5DA4E
> >> >> >> 3E16B580
> >> >> >>
> >> >> >> I couldn't find out where it stuck unless there is a way to attach
> >> >> >> gdb
> >> >> >> to Xen HVM for source level debugging :-(
> >> >> >>
> >> >> >> Thanks,
> >> >> >>
> >> >> >> Gary Lin
> >> >> >> _______________________________________________
> >> >> >> edk2-devel mailing list
> >> >> >> [email protected]
> >> >> >> https://lists.01.org/mailman/listinfo/edk2-devel
> >> >> >>
> >> >> _______________________________________________
> >> >> edk2-devel mailing list
> >> >> [email protected]
> >> >> https://lists.01.org/mailman/listinfo/edk2-devel
> >> >>
> >> _______________________________________________
> >> edk2-devel mailing list
> >> [email protected]
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> >>
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
index a6ade26..8f36e90 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
@@ -2382,21 +2382,21 @@ PciEnumeratorLight (
Status = PciRootBridgeIo->Configuration (PciRootBridgeIo, (VOID **) &Descriptors);
- if (EFI_ERROR (Status)) {
- return Status;
- }
+// if (EFI_ERROR (Status)) {
+// return Status;
+// }
- while (PciGetBusRange (&Descriptors, &MinBus, &MaxBus, NULL) == EFI_SUCCESS) {
+// while (PciGetBusRange (&Descriptors, &MinBus, &MaxBus, NULL) == EFI_SUCCESS) {
//
// Create a device node for root bridge device with a NULL host bridge controller handle
//
RootBridgeDev = CreateRootBridge (Controller);
- if (RootBridgeDev == NULL) {
- Descriptors++;
- continue;
- }
+// if (RootBridgeDev == NULL) {
+// Descriptors++;
+// continue;
+// }
//
// Record the root bridgeio protocol
@@ -2437,8 +2437,8 @@ PciEnumeratorLight (
DestroyRootBridge (RootBridgeDev);
}
- Descriptors++;
- }
+// Descriptors++;
+// }
return EFI_SUCCESS;
}
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index f72598d..39a0e4c 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1770,7 +1770,7 @@ GetMmioAddressTranslationOffset (
(VOID **) &Configuration
);
if (EFI_ERROR (Status)) {
- return (UINT64) -1;
+ return (UINT64) 0;
}
while (Configuration->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR) {
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..9ca98d4 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"
@@ -1496,6 +1497,9 @@ RootBridgeIoConfiguration (
EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
EFI_ACPI_END_TAG_DESCRIPTOR *End;
+ if (PcdGetBool (PcdPciDisableBusEnumeration))
+ return EFI_UNSUPPORTED;
+
//
// Get this instance of the Root Bridge.
//
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
