public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 1/2] UefiPayloadPkg: Fix PciHostBridgeLib
@ 2022-03-30 18:29 Sean Rhodes
  2022-03-30 18:29 ` [PATCH 2/2] OvmfPkg: Fix PciHostBridgeLibScan Sean Rhodes
                   ` (2 more replies)
  0 siblings, 3 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,
	Sean Rhodes, Patrick Rudolph

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

* [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: [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

* 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: [edk2-devel] [PATCH 2/2] OvmfPkg: Fix PciHostBridgeLibScan
  2022-03-31  6:25   ` [edk2-devel] " Ard Biesheuvel
@ 2022-05-17 12:08     ` Sheng Lean Tan
  0 siblings, 0 replies; 6+ messages in thread
From: Sheng Lean Tan @ 2022-05-17 12:08 UTC (permalink / raw)
  To: Ard Biesheuvel, devel

[-- Attachment #1: Type: text/plain, Size: 337 bytes --]

Hi Ard,
As per discribed in the commit message, this issue is seen on actual hardware and the fix is merged:

https://github.com/tianocore/edk2/commit/bfefdc2c49ca9487b7aa0df196b2aca6c0c170a2

Since this function is the exact duplicate of the one in UefiPayloadPkg, he figured to port the fix here as well to prevent future issue.

[-- Attachment #2: Type: text/html, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-05-17 12:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-31  6:25   ` [edk2-devel] " Ard Biesheuvel
2022-05-17 12:08     ` Sheng Lean Tan
2022-03-30 18:29 ` [PATCH 1/2] UefiPayloadPkg: Fix PciHostBridgeLib Sean Rhodes
2022-03-30 23:00 ` Guo Dong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox