Re: [Xen-devel] [edk2] OVMF broken under Xen (in PCI initialisation)

["Ni, Ruiyu" <ruiyu.ni@xxxxxxxxx>]

Copying Mike.

Regards,
Ray

>-----Original Message-----
>From: Ni, Ruiyu
>Sent: Wednesday, April 27, 2016 5:49 PM
>To: 'Gary Lin' <glin@xxxxxxxx>
>Cc: edk2-devel@xxxxxxxxxxxx; Xen Devel <xen-devel@xxxxxxxxxxxxx>; Laszlo Ersek 
><lersek@xxxxxxxxxx>
>Subject: RE: [edk2] OVMF broken under Xen (in PCI initialisation)
>
>Gary,
>Thanks for the try.
>
>I now understand why the issue happens.
>1. PciEnumeratorLight() depends on the MinBus returned from
>PciRootBridgeIo.Configuration().
>2. PciRootBridgeIo.Configuration() returns the resource descriptors
>initialized by PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources).
>3. PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources)
>is not called when PcdPciDisableBusEnumeration is TRUE
>
>I am thinking about let PciRootBridgeIo.Configuration() returns
>the correct resource consumption information even when
>PcdPciDisableBusEnumeration is TRUE.
>
>I can add a new field to PCI_ROOT_BRIDGE structure defined in
>MdeModulePkg/Include/Library/PciHostBridgeLib.h to indicate
>that the resources filled in PCI_ROOT_BRIDGE.Bus/Io/Mem/
>MemAbove4G/PMem/PMemAbove4G are already assigned
>to PCI bus. So that PciRootBridgeIo.Configuration() can return
>these value even when
>PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources)
>is not called due to PcdPciDisableBusEnumeration is TRUE.
>
>Do you know whether Xen passes the PCI device resource
>information to firmware?
>
>Copying Laszlo.
>
>Regards,
>Ray
>
>>-----Original Message-----
>>From: Gary Lin [mailto:glin@xxxxxxxx]
>>Sent: Wednesday, April 27, 2016 4:27 PM
>>To: Ni, Ruiyu <ruiyu.ni@xxxxxxxxx>
>>Cc: edk2-devel@xxxxxxxxxxxx; Xen Devel <xen-devel@xxxxxxxxxxxxx>
>>Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
>>
>>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@xxxxxxxx]
>>> >Sent: Wednesday, April 27, 2016 2:54 PM
>>> >To: Ni, Ruiyu <ruiyu.ni@xxxxxxxxx>
>>> >Cc: edk2-devel@xxxxxxxxxxxx; Xen Devel <xen-devel@xxxxxxxxxxxxx>
>>> >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@xxxxxxxx]
>>> >> >Sent: Wednesday, April 27, 2016 12:29 PM
>>> >> >To: Ni, Ruiyu <ruiyu.ni@xxxxxxxxx>
>>> >> >Cc: edk2-devel@xxxxxxxxxxxx; Xen Devel <xen-devel@xxxxxxxxxxxxx>
>>> >> >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@xxxxxxxx]
>>> >> >> >Sent: Tuesday, April 26, 2016 4:40 PM
>>> >> >> >To: Ni, Ruiyu <ruiyu.ni@xxxxxxxxx>
>>> >> >> >Cc: edk2-devel@xxxxxxxxxxxx; Xen Devel <xen-devel@xxxxxxxxxxxxx>
>>> >> >> >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@xxxxxxxx]
>>> >> >> >> Sent: Tuesday, April 26, 2016 3:35 PM
>>> >> >> >> To: Ni, Ruiyu <ruiyu.ni@xxxxxxxxx>
>>> >> >> >> Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>; 
>>> >> >> >> edk2-devel@xxxxxxxxxxxx; Xen Devel
>><xen-devel@xxxxxxxxxxxxx>
>>> >> >> >> 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@xxxxxxxxxxxx] On 
>>> >> >> >> > >Behalf Of Anthony PERARD
>>> >> >> >> > >Sent: Friday, April 22, 2016 10:48 PM
>>> >> >> >> > >To: edk2-devel@xxxxxxxxxxxx<mailto:edk2-devel@xxxxxxxxxxxx>
>>> >> >> >> > >Cc: Xen Devel 
>>> >> >> >> > ><xen-devel@xxxxxxxxxxxxx<mailto:xen-devel@xxxxxxxxxxxxx>>
>>> >> >> >> > >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@xxxxxxxxxxxx
>>> >> >> >> https://lists.01.org/mailman/listinfo/edk2-devel
>>> >> >> >>
>>> >> >> _______________________________________________
>>> >> >> edk2-devel mailing list
>>> >> >> edk2-devel@xxxxxxxxxxxx
>>> >> >> https://lists.01.org/mailman/listinfo/edk2-devel
>>> >> >>
>>> >> _______________________________________________
>>> >> edk2-devel mailing list
>>> >> edk2-devel@xxxxxxxxxxxx
>>> >> https://lists.01.org/mailman/listinfo/edk2-devel
>>> >>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel