Re: [edk2-devel] [PATCH v2 0/4] Add extra pci roots support for Arm

["Laszlo Ersek" <lersek@redhat.com>]

+Alex, +Ray

comment below

On 12/04/20 07:48, Jiahui Cen wrote:
> Hi Laszlo,
> 
> During the modification of this patch series, I got a confusing problem
> that the EDK2's Alignment seems not fit with Linux on ARM.
> 
> EDK2, in CalculateResourceAperture, aligns the offset of child nodes
> in descending order and uses the *Bridge's alignment* to align the
> total length of Bridge.
> 
> However, Linux, in pbus_size_mem, would align the size of child nodes
> and use *the half of max alignment of child nodes* to align the total
> length of Bridge.
> 
> eg. A Root Bridge with Bus [d2]
>     -+-[0000:d2]---01.0-[d3]----01.0
>     where [d2:01.00] is a pcie-pci-bridge with BAR0 (mem, 64-bit, non-pref) [size=256]
>           [d3:01.00] is a PCI Device with BAR0 (mem, 64-bit, pref) [size=128K]
>                                           BAR4 (mem, 64-bit, pref) [size=64M]
> 
>     In EDK2, the Resource Map would be:
>         PciBus: Resource Map for Root Bridge PciRoot(0xD2)
>         Type =  Mem64; Base = 0x8004000000;     Length = 0x4200000;     Alignment = 0x3FFFFFF
>            Base = 0x8004000000; Length = 0x4100000;     Alignment = 0x3FFFFFF;  Owner = PPB [D2|01|00:**]; Type = PMem64
>            Base = 0x8008100000; Length = 0x100; Alignment = 0xFFF;      Owner = PPB [D2|01|00:10]
> 
>         PciBus: Resource Map for Bridge [D2|01|00]
>         Type = PMem64; Base = 0x8004000000;     Length = 0x4100000;     Alignment = 0x3FFFFFF
>            Base = 0x8004000000; Length = 0x4000000;     Alignment = 0x3FFFFFF;  Owner = PCI [D3|01|00:20]
>            Base = 0x8008000000; Length = 0x20000;       Alignment = 0x1FFFF;    Owner = PCI [D3|01|00:10]
>         Type =  Mem64; Base = 0x8008100000;     Length = 0x100; Alignment = 0xFFF
> 
>     It shows that with EDK2's alignment [d2:01.00] only requires
>     a PMem64 resource range of 0x4100000.
> 
>     However in Linux, [d2:01.00]'s BAR14 (Prefetchable Memory
>     Resource behind the Bridge) requires a PMem64 resource range
>     of 0x06000000, which comes from (0x4000000 + 0x20000) with
>     alignment=0x1FFFFFF the half of the max alignment 0x3FFFFFF.
> 
>     Therefore, the assignment for [d2:01.00]'s BAR14 will fail.
> 
> The difference could make the resource range allocated in EDK2 smaller
> than the resource range needed in Linux, which causes the assignment
> failure in Linux.
> 
> The same difference also occurs when io resource allocation.
> 
> To handle this senario, is it necessary to calculate the resource map
> in Linux way?

I don't know why this difference in BAR placement exists between edk2
and Linux, and/or perhaps even between x86_64 Linux and aarch64 Linux. I
don't really remember seeing this issue with OVMF.

It could have something to do with differences in ACPI table composition
as well.

I don't know if we've ever tested hotplug behind pxb.

> 
> Furthermore, Linux Kernel will treat pcie-root-port as a hotplugable
> bridge and try to require more resource. But EDK2 seems not support
> hotplug padding for ARM?

Try including "OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf" in
ArmVirtQemu. See commit fe4049471bdf for documentation.

QEMU's pcie-root-port device should expose the same reservation/padding
properties to the user when using the "virt" machine of
qemu-system-aarch64, I think.

Thanks
Laszlo

> 
> Thanks,
> Jiahui
> 
> On 2020/11/11 22:33, Laszlo Ersek wrote:
>> On 11/09/20 14:05, Jiahui Cen wrote:
>>> Changes with v1
>>> v1->v2:
>>> Separated into four patches.
>>> Factor the same logic parts into a new library.
>>>
>>> v1: https://edk2.groups.io/g/devel/topic/72723351#56901
>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3059
>>
>> (1) Each commit message in the series should reference this bugzilla.
>>
>> But, in itself, that's no reason for a repost; such an update can be
>> made by maintainers when they merge the series.
>>
>> (2) This is a feature addition, so it's merge material for the next
>> development cycle
>> <https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning>.
>> (Also it depends on the QEMU work being merged first.) I'm going to
>> proceed with the review now.
>>
>> Thanks,
>> Laszlo
>>
>>> QEMU: https://lore.kernel.org/qemu-devel/20201103120157.2286-1-cenjiahui@huawei.com/
>>>
>>> This patch series adds support for extra pci roots for ARM.
>>>
>>> In order to avoid duplicated codes, we introduce a new library
>>> PciHostBridgeUtilityLib which extracts common interfaces from
>>> OvmfPkg/PciHostBridgeLib. It provides conflicts informing and extra pci
>>> roots scanning. Using the utility lib, the uefi could scan for extra
>>> root buses and recognize multiple roots for ARM.
>>>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>> Cc: Leif Lindholm <leif@nuviainc.com>
>>> Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
>>> Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
>>>
>>> Yubo Miao (4):
>>>   OvmfPkg: Extract functions form PciHostBridgeLib
>>>   ArmVirtPkg: Use extracted PciHostBridgeUtilityLib
>>>   OvmfPkg: Extract functions of extra pci roots
>>>   ArmVirtPkg: Support extra pci roots
>>>
>>>  ArmVirtPkg/ArmVirt.dsc.inc                                          |   1 +
>>>  OvmfPkg/OvmfPkgIa32.dsc                                             |   1 +
>>>  OvmfPkg/OvmfPkgIa32X64.dsc                                          |   1 +
>>>  OvmfPkg/OvmfPkgX64.dsc                                              |   1 +
>>>  OvmfPkg/OvmfXen.dsc                                                 |   1 +
>>>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf      |   5 +
>>>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf               |   1 +
>>>  OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf |  51 ++++
>>>  OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h                   |  98 +++++++
>>>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c        | 221 ++++++++-------
>>>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c                 | 234 +---------------
>>>  OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c   | 283 ++++++++++++++++++++
>>>  12 files changed, 563 insertions(+), 335 deletions(-)
>>>  create mode 100644 OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>>>  create mode 100644 OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
>>>  create mode 100644 OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
>>>
>>
>>
>>
>> 
>>
>>
>> .
>>
>