From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web08.15482.1607094533545997621 for ; Fri, 04 Dec 2020 07:08:53 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=bOpAHypk; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607094532; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=VZvyT+jcSnRpn9glrcDPA1JXOxzi/UN2y5t0rMTAVYk=; b=bOpAHypkB4uxkJcPMNgZslaaFgDOZkTZ4LFvngZfGL2TEwsqT9KWrH3i0PKmbLPOqda+4J 4CRZwmBJ1FvEOSeq2YdgQX496NToRcf2wFjJcYRgTOUZ5vaRflsJky1yYKlsDZPUmoDHZT QZLMzC6+J6kh1n2kmf/mz7sX7j88GD4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-184-2fTZD44oOZ-ZDw_SjHamoA-1; Fri, 04 Dec 2020 10:08:47 -0500 X-MC-Unique: 2fTZD44oOZ-ZDw_SjHamoA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4707D83DAA1; Fri, 4 Dec 2020 15:08:46 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-12.ams2.redhat.com [10.36.113.12]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5077110021AA; Fri, 4 Dec 2020 15:08:40 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 0/4] Add extra pci roots support for Arm To: Jiahui Cen , devel@edk2.groups.io Cc: jordan.l.justen@intel.com, ard.biesheuvel@arm.com, leif@nuviainc.com, xieyingtai@huawei.com, miaoyubo@huawei.com, xuxiaoyang2@huawei.com, Alex Williamson , Ray Ni References: <20201109130511.5946-1-cenjiahui@huawei.com> <5d2daf54-a57b-c4bd-a757-cf6b710cd4e5@huawei.com> From: "Laszlo Ersek" Message-ID: Date: Fri, 4 Dec 2020 16:08:39 +0100 MIME-Version: 1.0 In-Reply-To: <5d2daf54-a57b-c4bd-a757-cf6b710cd4e5@huawei.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit +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 >> . >> (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 >>> Cc: Laszlo Ersek >>> Cc: Ard Biesheuvel >>> Cc: Leif Lindholm >>> Signed-off-by: Yubo Miao >>> Signed-off-by: Jiahui Cen >>> >>> 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 >>> >> >> >> >> >> >> >> . >> >