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

[Laszlo Ersek <lersek@xxxxxxxxxx>]

On 04/27/16 11:50, Ni, Ruiyu wrote:
> 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.

Can you use PcdPciDisableBusEnumeration directly for this?

(If you don't like the idea, we could certainly set the new field in
PCI_ROOT_BRIDGE from the PCD too, in OvmfPkg/Library/PciHostBridgeLib.)

>> Do you know whether Xen passes the PCI device resource
>> information to firmware?

I don't think so, no.

But, given that the previous PciHostBridgeDxe driver was working on Xen,
can we perhaps emulate that behavior in
"OvmfPkg/Library/PciHostBridgeLib" somehow?

Do you think that step (2) above behaves differently between the old and
the new PCI host bridge driver? (Steps (1) and (3) should be identical,
they are initiated by the PCI Bus driver.)

Thanks
Laszlo

>>
>> 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