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

Signed-off-by: Yubo Miao <miaoyubo@...>
Signed-off-by: Jiahui Cen <cenjiahui@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Leif Lindholm <leif@...>

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 +
.../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 223 +++++++-------
.../FdtPciHostBridgeLib.inf | 5 +
.../Include/Library/PciHostBridgeUtilityLib.h | 98 ++++++
.../PciHostBridgeLib/PciHostBridgeLib.c | 230 +-------------
.../PciHostBridgeLib/PciHostBridgeLib.inf | 1 +
.../PciHostBridgeUtilityLib.c | 280 ++++++++++++++++++
.../PciHostBridgeUtilityLib.inf | 51 ++++
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
12 files changed, 560 insertions(+), 333 deletions(-)
create mode 100644 OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
create mode 100644 OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
create mode 100644 OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf

--
2.19.1

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
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@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Leif Lindholm <leif@...>
Signed-off-by: Yubo Miao <miaoyubo@...>
Signed-off-by: Jiahui Cen <cenjiahui@...>

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

--
2.28.0

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@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Leif Lindholm <leif@...>
Signed-off-by: Yubo Miao <miaoyubo@...>
Signed-off-by: Jiahui Cen <cenjiahui@...>

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

Hi Laszlo,

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.

Will add it.

(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 for the material. I'll keep on developing it and the QEMU work.

Jiahui

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@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Leif Lindholm <leif@...>
Signed-off-by: Yubo Miao <miaoyubo@...>
Signed-off-by: Jiahui Cen <cenjiahui@...>

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

.

On 11/9/20 2:05 PM, 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
QEMU: https://lore.kernel.org/qemu-devel/20201103120157.2286-1-cenjiahui@huawei.com/
This patch series adds support for extra pci roots for ARM.

Why?

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@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Leif Lindholm <leif@...>
Signed-off-by: Yubo Miao <miaoyubo@...>
Signed-off-by: Jiahui Cen <cenjiahui@...>
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

On 11/12/20 09:49, Ard Biesheuvel wrote:

On 11/9/20 2:05 PM, 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
QEMU:
https://lore.kernel.org/qemu-devel/20201103120157.2286-1-cenjiahui@huawei.com/


This patch series adds support for extra pci roots for ARM.

Why?

I don't know Jiahui Cen's particular reasons; on x86, pxb-pcie is useful
for associating PCIe hierarchies with particular NUMA nodes. See
"docs/pci_expander_bridge.txt" in the QEMU tree.

Also, in Jiahui Cen's QEMU blurb linked above, the following is included:

Currently pxb-pcie is not supported by arm,
the reason for it is pxb-pcie is not described in DSDT table
and only one main host bridge is described in acpi tables,
which means it is not impossible to present different io numas
for different devices.

So I guess this work aims to extend the same PCI(e)<->NUMA association
feature, from x86 to ARM.

Thanks,
Laszlo

On 2020/11/14 3:44, Laszlo Ersek wrote:

On 11/12/20 09:49, Ard Biesheuvel wrote:

On 11/9/20 2:05 PM, 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
QEMU:
https://lore.kernel.org/qemu-devel/20201103120157.2286-1-cenjiahui@huawei.com/


This patch series adds support for extra pci roots for ARM.

Why?

I don't know Jiahui Cen's particular reasons; on x86, pxb-pcie is useful
for associating PCIe hierarchies with particular NUMA nodes. See
"docs/pci_expander_bridge.txt" in the QEMU tree.

Also, in Jiahui Cen's QEMU blurb linked above, the following is included:

Currently pxb-pcie is not supported by arm,
the reason for it is pxb-pcie is not described in DSDT table
and only one main host bridge is described in acpi tables,
which means it is not impossible to present different io numas
for different devices.

So I guess this work aims to extend the same PCI(e)<->NUMA association
feature, from x86 to ARM.

Thanks,

Laszlo


.

Right, we'd like to extend the pxb-pcie feature on ARM so that PCI(e)
can be associated with particular NUMA nodes.

Thanks,
Jiahui

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

+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@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Leif Lindholm <leif@...>
Signed-off-by: Yubo Miao <miaoyubo@...>
Signed-off-by: Jiahui Cen <cenjiahui@...>

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

.

On 2020/12/11 18:57, Ni, Ray wrote:

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jiahui Cen via groups.io
Sent: Friday, December 4, 2020 2:49 PM
To: devel@edk2.groups.io; lersek@...
Cc: Justen, Jordan L <jordan.l.justen@...>; ard.biesheuvel@...; leif@...; xieyingtai@...;
miaoyubo@...; xuxiaoyang2@...
Subject: Re: [edk2-devel] [PATCH v2 0/4] Add extra pci roots support for Arm

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.

How is 0x3FFFFFF calculated?
0x3FFFFFF = 0x8000000 / 2 - 1, and 0x8000000 is the minimum value that's
power of 2 and bigger than 0x6000000. Is my understanding correct?

The bridge has 2 resource alignments, 0x4000000 with order 26 and 0x20000
with order 20 (the minimum order is 20). Order 26 is the maximum value
whose alignment is larger than the sum of alignments with order below it.
The final alignment is 0x2000000 = 0x4000000 / 2, and the final size is
0x6000000 = ALIGN(0x4000000 + 0x20000, 0x2000000).

It seems that Kernel tries to use a suitable alignment that ensures the
final size can fit all the resource requirements under the bridge, rather
than place each resource and calculate the exact size. Therefore, the size
may be larger than needed.

I don't understand why this calculation method is chosen.
It seems a kernel bug to me.
I thought Linux doesn't handle the pci resource assignment but just use what was
assigned by firmware.

For x86, Linux does not handle the pci resource assignment. But for arm, it
would assign all the pci resources, unless we explicitly let Linux preserve
PCI resource alignment made by firmware, using "PCI Boot Configuration"
_DSM function.

What do you think of adding "PCI Boot Configuration" _DSM function into
dsdt table to make kernel use firmware's resource configuration?

Thanks,
Jiahui

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?

Hotplug padding is supported through HotPlugInit protocol in edk2.

On 12/15/20 13:52, Jiahui Cen wrote:

For x86, Linux does not handle the pci resource assignment. But for arm, it
would assign all the pci resources, unless we explicitly let Linux preserve
PCI resource alignment made by firmware, using "PCI Boot Configuration"
_DSM function.

This difference between x86 and arm seems inexplicable to me.

What do you think of adding "PCI Boot Configuration" _DSM function into
dsdt table to make kernel use firmware's resource configuration?

The relevant kernel commits seem to be:

- 18e94a338436 ("PCI: Make a shareable UUID for PCI firmware ACPI _DSM",
2015-04-08)

- a78cf9657ba5 ("PCI/ACPI: Evaluate PCI Boot Configuration _DSM",
2019-06-21)

I've also read now section "4.6.5. _DSM for Ignoring PCI Boot
Configurations" in the "PCI(TM) Firmware Specification,
Revision 3.1, December 13, 2010".

Basically if this _DSM#5 function exists, it tells the OS whether it is
required to honor the firmware-assigned resources (value 0), or if it is
free to reassign (value 1). If the function does not exist at all, then,
"the operating system may continue to use the legacy handling regarding
the boot configuration".

Without knowing more, I consider it a bug that aarch64 Linux uses a
default strategy (in the absence of the _DSM#5 function) that is the
opposite of the x86 default. But, you seem to be right that there is a
specified way to convince arm64 Linux otherwise. Can you indeed add this
_DSM#5 function to the "virt" machine's ACPI generator, returning value
0, and see if it makes a difference?

Thanks!
Laszlo

On 12/17/20 2:23 PM, Laszlo Ersek wrote:

On 12/15/20 13:52, Jiahui Cen wrote:

For x86, Linux does not handle the pci resource assignment. But for arm, it
would assign all the pci resources, unless we explicitly let Linux preserve
PCI resource alignment made by firmware, using "PCI Boot Configuration"
_DSM function.

This difference between x86 and arm seems inexplicable to me.

What do you think of adding "PCI Boot Configuration" _DSM function into
dsdt table to make kernel use firmware's resource configuration?

The relevant kernel commits seem to be:

- 18e94a338436 ("PCI: Make a shareable UUID for PCI firmware ACPI _DSM",
2015-04-08)

- a78cf9657ba5 ("PCI/ACPI: Evaluate PCI Boot Configuration _DSM",
2019-06-21)

I've also read now section "4.6.5. _DSM for Ignoring PCI Boot
Configurations" in the "PCI(TM) Firmware Specification,
Revision 3.1, December 13, 2010".

Basically if this _DSM#5 function exists, it tells the OS whether it is
required to honor the firmware-assigned resources (value 0), or if it is
free to reassign (value 1). If the function does not exist at all, then,
"the operating system may continue to use the legacy handling regarding
the boot configuration".

Without knowing more, I consider it a bug that aarch64 Linux uses a
default strategy (in the absence of the _DSM#5 function) that is the
opposite of the x86 default. But, you seem to be right that there is a
specified way to convince arm64 Linux otherwise. Can you indeed add this
_DSM#5 function to the "virt" machine's ACPI generator, returning value
0, and see if it makes a difference?

That is not going to work, unfortunately. We had a very long discussion
in the PCI SIG firmware subteam about this, and some changes were made
IIRC, but the code does not exist yet in Linux.

For historical reason, Linux/arm64 always reassigns PCI bus and memory
resources: the ARM port on which arm64 is based does not assume the
existence of any firmware, let only a PCI BIOS that carries out all
those things. When ACPI boot on Linux/arm64 came about, this nuance got
missed, and so even though the presence of rich firmware can obviously
be relied upon in this case, the PCI layer still reassigns some of the
resources in many cases. Note that Linux/x86 might do the same, but is
less likely to do so for modern systems. (My personal favorite is the
Linux/x86 quirk that bases this decision on the BIOS year from DMI)

One thing that does seem to help on AArch64 is to ensure that the bus
padding is the same for hotplug capable root ports, because this is the
most common reason for reassigning bus numbers.

HTH,
Ard.

On 2020/12/17 21:23, Laszlo Ersek wrote:

On 12/15/20 13:52, Jiahui Cen wrote:

For x86, Linux does not handle the pci resource assignment. But for arm, it
would assign all the pci resources, unless we explicitly let Linux preserve
PCI resource alignment made by firmware, using "PCI Boot Configuration"
_DSM function.

This difference between x86 and arm seems inexplicable to me.

What do you think of adding "PCI Boot Configuration" _DSM function into
dsdt table to make kernel use firmware's resource configuration?

The relevant kernel commits seem to be:

- 18e94a338436 ("PCI: Make a shareable UUID for PCI firmware ACPI _DSM",
2015-04-08)

- a78cf9657ba5 ("PCI/ACPI: Evaluate PCI Boot Configuration _DSM",
2019-06-21)

I've also read now section "4.6.5. _DSM for Ignoring PCI Boot
Configurations" in the "PCI(TM) Firmware Specification,
Revision 3.1, December 13, 2010".

Basically if this _DSM#5 function exists, it tells the OS whether it is
required to honor the firmware-assigned resources (value 0), or if it is
free to reassign (value 1). If the function does not exist at all, then,
"the operating system may continue to use the legacy handling regarding
the boot configuration".

Without knowing more, I consider it a bug that aarch64 Linux uses a
default strategy (in the absence of the _DSM#5 function) that is the
opposite of the x86 default. But, you seem to be right that there is a
specified way to convince arm64 Linux otherwise. Can you indeed add this
_DSM#5 function to the "virt" machine's ACPI generator, returning value
0, and see if it makes a difference?

OK, I've tested adding _DSM#5 function in QEMU, and it seems to work well for
ARM "virt" machine.

Thanks,
Jiahui

On Thu, 17 Dec 2020 14:37:30 +0100
Ard Biesheuvel <ard.biesheuvel@...> wrote:

On 12/17/20 2:23 PM, Laszlo Ersek wrote:

On 12/15/20 13:52, Jiahui Cen wrote:

For x86, Linux does not handle the pci resource assignment. But for arm, it
would assign all the pci resources, unless we explicitly let Linux preserve
PCI resource alignment made by firmware, using "PCI Boot Configuration"
_DSM function.

This difference between x86 and arm seems inexplicable to me.

What do you think of adding "PCI Boot Configuration" _DSM function into
dsdt table to make kernel use firmware's resource configuration?

The relevant kernel commits seem to be:

- 18e94a338436 ("PCI: Make a shareable UUID for PCI firmware ACPI _DSM",
2015-04-08)

- a78cf9657ba5 ("PCI/ACPI: Evaluate PCI Boot Configuration _DSM",
2019-06-21)

I've also read now section "4.6.5. _DSM for Ignoring PCI Boot
Configurations" in the "PCI(TM) Firmware Specification,
Revision 3.1, December 13, 2010".

Basically if this _DSM#5 function exists, it tells the OS whether it is
required to honor the firmware-assigned resources (value 0), or if it is
free to reassign (value 1). If the function does not exist at all, then,
"the operating system may continue to use the legacy handling regarding
the boot configuration".

Without knowing more, I consider it a bug that aarch64 Linux uses a
default strategy (in the absence of the _DSM#5 function) that is the
opposite of the x86 default. But, you seem to be right that there is a
specified way to convince arm64 Linux otherwise. Can you indeed add this
_DSM#5 function to the "virt" machine's ACPI generator, returning value
0, and see if it makes a difference?

That is not going to work, unfortunately. We had a very long discussion
in the PCI SIG firmware subteam about this, and some changes were made
IIRC, but the code does not exist yet in Linux.

For historical reason, Linux/arm64 always reassigns PCI bus and memory
resources: the ARM port on which arm64 is based does not assume the
existence of any firmware, let only a PCI BIOS that carries out all
those things. When ACPI boot on Linux/arm64 came about, this nuance got
missed, and so even though the presence of rich firmware can obviously
be relied upon in this case, the PCI layer still reassigns some of the
resources in many cases. Note that Linux/x86 might do the same, but is
less likely to do so for modern systems. (My personal favorite is the
Linux/x86 quirk that bases this decision on the BIOS year from DMI)

One thing that does seem to help on AArch64 is to ensure that the bus
padding is the same for hotplug capable root ports, because this is the
most common reason for reassigning bus numbers.

Hi Ard,

For an unrelated reason I've been trying to create a setup where bus numbers
get reassigned on aarch64 and been unable to do so.
I'm looking at potential issues around Generic initiators being defined via BDF
and whether we can cache the firmware configured BDF before re enumerating.

All sorts of fun happens if the initial setup is not correct (many of which
result in unusable systems) but for a 'valid' setup I have not managed to
create a case where BDFs move. I can't find a route to reassignment of
bus numbers after the _DSM is evaluated (and host->preserve_config is
first used).

https://elixir.bootlin.com/linux/latest/source/arch/arm64/kernel/pci.c#L194
The padding for bus numbers on root ports seems to be limited to the available
space. Note I'm doing messing around in qemu but have edk2 using PciHotplugInit
from OVMF to allow straight forward root port padding for hp in edk2.

Any pointers would be great!

Jonathan

HTH,
Ard.