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

["Jiahui Cen" <cenjiahui@huawei.com>]

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?

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?

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