* [PATCH 2/2] OvmfPkg: Fix PciHostBridgeLibScan
2022-03-30 18:29 [PATCH 1/2] UefiPayloadPkg: Fix PciHostBridgeLib Sean Rhodes
@ 2022-03-30 18:29 ` Sean Rhodes
2022-03-31 6:25 ` [edk2-devel] " Ard Biesheuvel
2022-03-30 18:29 ` [PATCH 1/2] UefiPayloadPkg: Fix PciHostBridgeLib Sean Rhodes
2022-03-30 23:00 ` Guo Dong
2 siblings, 1 reply; 6+ messages in thread
From: Sean Rhodes @ 2022-03-30 18:29 UTC (permalink / raw)
To: devel
Cc: Patrick Rudolph, Ard Biesheuvel, Jiewen Yao, Jordan Justen,
Gerd Hoffmann
From: Patrick Rudolph <patrick.rudolph@9elements.com>
Don't assume a 64bit register always holds an address greater than 4GB.
Check the value in the register and decide which Aperature it should be
assigned to.
The same code caused an issue on real hardware. It's unclear if this is an
issue here as well, as it's intended to run on emulated hardware only.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
.../PciHostBridgeLibScan/ScanForRootBridges.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/OvmfPkg/Library/PciHostBridgeLibScan/ScanForRootBridges.c b/OvmfPkg/Library/PciHostBridgeLibScan/ScanForRootBridges.c
index 5fb02a89b9..1ff96be57f 100644
--- a/OvmfPkg/Library/PciHostBridgeLibScan/ScanForRootBridges.c
+++ b/OvmfPkg/Library/PciHostBridgeLibScan/ScanForRootBridges.c
@@ -331,14 +331,18 @@ ScanForRootBridges (
Base = ((UINT32)Pci.Bridge.PrefetchableMemoryBase & 0xfff0) << 16;
Limit = (((UINT32)Pci.Bridge.PrefetchableMemoryLimit & 0xfff0)
<< 16) | 0xfffff;
- MemAperture = &Mem;
if (Value == BIT0) {
- Base |= LShiftU64 (Pci.Bridge.PrefetchableBaseUpper32, 32);
- Limit |= LShiftU64 (Pci.Bridge.PrefetchableLimitUpper32, 32);
- MemAperture = &MemAbove4G;
+ Base |= LShiftU64 (Pci.Bridge.PrefetchableBaseUpper32, 32);
+ Limit |= LShiftU64 (Pci.Bridge.PrefetchableLimitUpper32, 32);
}
if (Base < Limit) {
+ if (Base < BASE_4GB) {
+ MemAperture = &Mem;
+ } else {
+ MemAperture = &MemAbove4G;
+ }
+
if (MemAperture->Base > Base) {
MemAperture->Base = Base;
}
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [edk2-devel] [PATCH 2/2] OvmfPkg: Fix PciHostBridgeLibScan
2022-03-30 18:29 ` [PATCH 2/2] OvmfPkg: Fix PciHostBridgeLibScan Sean Rhodes
@ 2022-03-31 6:25 ` Ard Biesheuvel
2022-05-17 12:08 ` Sheng Lean Tan
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2022-03-31 6:25 UTC (permalink / raw)
To: edk2-devel-groups-io, sean
Cc: Patrick Rudolph, Ard Biesheuvel, Jiewen Yao, Jordan Justen,
Gerd Hoffmann
On Wed, 30 Mar 2022 at 20:29, Sean Rhodes <sean@starlabs.systems> wrote:
>
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>
> Don't assume a 64bit register always holds an address greater than 4GB.
> Check the value in the register and decide which Aperature it should be
> assigned to.
>
> The same code caused an issue on real hardware. It's unclear if this is an
> issue here as well, as it's intended to run on emulated hardware only.
>
Do you have a link to such a supported issue? Or could you elaborate?
Does it have to do with running out of 64-bit BAR space for resource
that could be located in a 32-bit region as well?
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
> .../PciHostBridgeLibScan/ScanForRootBridges.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/OvmfPkg/Library/PciHostBridgeLibScan/ScanForRootBridges.c b/OvmfPkg/Library/PciHostBridgeLibScan/ScanForRootBridges.c
> index 5fb02a89b9..1ff96be57f 100644
> --- a/OvmfPkg/Library/PciHostBridgeLibScan/ScanForRootBridges.c
> +++ b/OvmfPkg/Library/PciHostBridgeLibScan/ScanForRootBridges.c
> @@ -331,14 +331,18 @@ ScanForRootBridges (
> Base = ((UINT32)Pci.Bridge.PrefetchableMemoryBase & 0xfff0) << 16;
> Limit = (((UINT32)Pci.Bridge.PrefetchableMemoryLimit & 0xfff0)
> << 16) | 0xfffff;
> - MemAperture = &Mem;
> if (Value == BIT0) {
> - Base |= LShiftU64 (Pci.Bridge.PrefetchableBaseUpper32, 32);
> - Limit |= LShiftU64 (Pci.Bridge.PrefetchableLimitUpper32, 32);
> - MemAperture = &MemAbove4G;
> + Base |= LShiftU64 (Pci.Bridge.PrefetchableBaseUpper32, 32);
> + Limit |= LShiftU64 (Pci.Bridge.PrefetchableLimitUpper32, 32);
> }
>
> if (Base < Limit) {
> + if (Base < BASE_4GB) {
> + MemAperture = &Mem;
> + } else {
> + MemAperture = &MemAbove4G;
> + }
> +
> if (MemAperture->Base > Base) {
> MemAperture->Base = Base;
> }
> --
> 2.32.0
>
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#88266): https://edk2.groups.io/g/devel/message/88266
> Mute This Topic: https://groups.io/mt/90138165/5717338
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb+tianocore@kernel.org]
> ------------
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] UefiPayloadPkg: Fix PciHostBridgeLib
2022-03-30 18:29 [PATCH 1/2] UefiPayloadPkg: Fix PciHostBridgeLib Sean Rhodes
2022-03-30 18:29 ` [PATCH 2/2] OvmfPkg: Fix PciHostBridgeLibScan Sean Rhodes
@ 2022-03-30 18:29 ` Sean Rhodes
2022-03-30 23:00 ` Guo Dong
2 siblings, 0 replies; 6+ messages in thread
From: Sean Rhodes @ 2022-03-30 18:29 UTC (permalink / raw)
To: devel
Cc: Lean Sheng Tan, Guo Dong, Ray Ni, Maurice Ma, Benjamin You,
Patrick Rudolph
[-- Attachment #1: Type: text/plain, Size: 2334 bytes --]
Reviewed-by Sean Rhodes <sean@starlabs.systems>
On Wed, 30 Mar 2022 at 19:29, Sean Rhodes <sean@starlabs.systems> wrote:
> From: Lean Sheng Tan <sheng.tan@9elements.com>
>
> Don't assume a 64bit register always holds an address greater than 4GB.
> Check the value in the register and decide which Aperature it should be
> assigned to.
>
> Fixes assertion
> "ASSERT [PciHostBridgeDxe] Bridge->MemAbove4G.Base >=
> 0x0000000100000000ULL".
>
> Tested with coreboot as bootloader on platforms that have PCI resource
> above 4GiB and on platforms that don't have resource above 4GiB.
>
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Benjamin You <benjamin.you@intel.com>
> Cc: Sean Rhodes <sean@starlabs.systems>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
> .../Library/PciHostBridgeLib/PciHostBridgeSupport.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git
> a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
> b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
> index 8a890b6b53..e1faa24ae7 100644
> --- a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
> +++ b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
> @@ -354,14 +354,19 @@ ScanForRootBridges (
> Base = ((UINT32)Pci.Bridge.PrefetchableMemoryBase & 0xfff0) <<
> 16;
> Limit = (((UINT32)Pci.Bridge.PrefetchableMemoryLimit & 0xfff0)
> << 16) | 0xfffff;
> - MemAperture = &Mem;
> +
> if (Value == BIT0) {
> - Base |= LShiftU64 (Pci.Bridge.PrefetchableBaseUpper32,
> 32);
> - Limit |= LShiftU64 (Pci.Bridge.PrefetchableLimitUpper32,
> 32);
> - MemAperture = &MemAbove4G;
> + Base |= LShiftU64 (Pci.Bridge.PrefetchableBaseUpper32, 32);
> + Limit |= LShiftU64 (Pci.Bridge.PrefetchableLimitUpper32, 32);
> }
>
> if ((Base > 0) && (Base < Limit)) {
> + if (Base < BASE_4GB) {
> + MemAperture = &Mem;
> + } else {
> + MemAperture = &MemAbove4G;
> + }
> +
> if (MemAperture->Base > Base) {
> MemAperture->Base = Base;
> }
> --
> 2.32.0
>
>
[-- Attachment #2: Type: text/html, Size: 3432 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] UefiPayloadPkg: Fix PciHostBridgeLib
2022-03-30 18:29 [PATCH 1/2] UefiPayloadPkg: Fix PciHostBridgeLib Sean Rhodes
2022-03-30 18:29 ` [PATCH 2/2] OvmfPkg: Fix PciHostBridgeLibScan Sean Rhodes
2022-03-30 18:29 ` [PATCH 1/2] UefiPayloadPkg: Fix PciHostBridgeLib Sean Rhodes
@ 2022-03-30 23:00 ` Guo Dong
2 siblings, 0 replies; 6+ messages in thread
From: Guo Dong @ 2022-03-30 23:00 UTC (permalink / raw)
To: Rhodes, Sean, devel@edk2.groups.io
Cc: Tan, Lean Sheng, Ni, Ray, Ma, Maurice, You, Benjamin,
Rhodes, Sean, Patrick Rudolph
Reviewed-by: Guo Dong <guo.dong@intel.com>
-----Original Message-----
From: Sean Rhodes <sean@starlabs.systems>
Sent: Wednesday, March 30, 2022 11:29 AM
To: devel@edk2.groups.io
Cc: Tan, Lean Sheng <sheng.tan@9elements.com>; Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>; Rhodes, Sean <sean@starlabs.systems>; Patrick Rudolph <patrick.rudolph@9elements.com>
Subject: [PATCH 1/2] UefiPayloadPkg: Fix PciHostBridgeLib
From: Lean Sheng Tan <sheng.tan@9elements.com>
Don't assume a 64bit register always holds an address greater than 4GB.
Check the value in the register and decide which Aperature it should be assigned to.
Fixes assertion
"ASSERT [PciHostBridgeDxe] Bridge->MemAbove4G.Base >= 0x0000000100000000ULL".
Tested with coreboot as bootloader on platforms that have PCI resource above 4GiB and on platforms that don't have resource above 4GiB.
Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
.../Library/PciHostBridgeLib/PciHostBridgeSupport.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
index 8a890b6b53..e1faa24ae7 100644
--- a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
+++ b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
@@ -354,14 +354,19 @@ ScanForRootBridges (
Base = ((UINT32)Pci.Bridge.PrefetchableMemoryBase & 0xfff0) << 16; Limit = (((UINT32)Pci.Bridge.PrefetchableMemoryLimit & 0xfff0) << 16) | 0xfffff;- MemAperture = &Mem;+ if (Value == BIT0) {- Base |= LShiftU64 (Pci.Bridge.PrefetchableBaseUpper32, 32);- Limit |= LShiftU64 (Pci.Bridge.PrefetchableLimitUpper32, 32);- MemAperture = &MemAbove4G;+ Base |= LShiftU64 (Pci.Bridge.PrefetchableBaseUpper32, 32);+ Limit |= LShiftU64 (Pci.Bridge.PrefetchableLimitUpper32, 32); } if ((Base > 0) && (Base < Limit)) {+ if (Base < BASE_4GB) {+ MemAperture = &Mem;+ } else {+ MemAperture = &MemAbove4G;+ }+ if (MemAperture->Base > Base) { MemAperture->Base = Base; }--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread