From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gary Lin Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation) Date: Wed, 27 Apr 2016 16:26:41 +0800 Message-ID: <20160427082641.GK3109__46524.6510961608$1461745703$gmane$org@GaryWorkstation> References: <20160422144736.GD1885@perard.uk.xensource.com> <734D49CCEBEEF84792F5B80ED585239D0DA25884@SHSMSX103.ccr.corp.intel.com> <20160426073529.GC3109@GaryWorkstation> <734D49CCEBEEF84792F5B80ED585239D0DA25B98@SHSMSX103.ccr.corp.intel.com> <20160426084000.GD3109@GaryWorkstation> <734D49CCEBEEF84792F5B80ED585239D0DA25D38@SHSMSX103.ccr.corp.intel.com> <20160427042907.GH3109@GaryWorkstation> <734D49CCEBEEF84792F5B80ED585239D0DA27D1E@SHSMSX103.ccr.corp.intel.com> <20160427065428.GJ3109@GaryWorkstation> <734D49CCEBEEF84792F5B80ED585239D0DA27F59@SHSMSX103.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="xB0nW4MQa6jZONgY" Return-path: Content-Disposition: inline In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D0DA27F59@SHSMSX103.ccr.corp.intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: "Ni, Ruiyu" Cc: "edk2-devel@lists.01.org" , Xen Devel List-Id: xen-devel@lists.xenproject.org --xB0nW4MQa6jZONgY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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:glin@suse.com] > >Sent: Wednesday, April 27, 2016 2:54 PM > >To: Ni, Ruiyu > >Cc: edk2-devel@lists.01.org; Xen Devel > >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:glin@suse.com] > >> >Sent: Wednesday, April 27, 2016 12:29 PM > >> >To: Ni, Ruiyu > >> >Cc: edk2-devel@lists.01.org; Xen Devel > >> >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:glin@suse.com] > >> >> >Sent: Tuesday, April 26, 2016 4:40 PM > >> >> >To: Ni, Ruiyu > >> >> >Cc: edk2-devel@lists.01.org; Xen Devel > >> >> >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:glin@suse.com] > >> >> >> Sent: Tuesday, April 26, 2016 3:35 PM > >> >> >> To: Ni, Ruiyu > >> >> >> Cc: Anthony PERARD ; edk2-devel@lists.01.org; Xen Devel > >> >> >> 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: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 > > >> >> >> > >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 > >> >> >> edk2-devel@lists.01.org > >> >> >> https://lists.01.org/mailman/listinfo/edk2-devel > >> >> >> > >> >> _______________________________________________ > >> >> edk2-devel mailing list > >> >> edk2-devel@lists.01.org > >> >> https://lists.01.org/mailman/listinfo/edk2-devel > >> >> > >> _______________________________________________ > >> edk2-devel mailing list > >> edk2-devel@lists.01.org > >> https://lists.01.org/mailman/listinfo/edk2-devel > >> --xB0nW4MQa6jZONgY Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="ovmf-hack-2.patch" 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 #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. // --xB0nW4MQa6jZONgY Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --xB0nW4MQa6jZONgY--