* [edk2-platform][PATCH] UefiPayloadPkg: Fix PciHostBridgeLib
@ 2022-02-07 10:01 Patrick Rudolph
2022-02-17 4:26 ` Ma, Maurice
0 siblings, 1 reply; 3+ messages in thread
From: Patrick Rudolph @ 2022-02-07 10:01 UTC (permalink / raw)
To: devel; +Cc: ray.ni, maurice.ma, benjamin.you, guo.dong
On modern platforms with TBT devices the coreboot resource allocator
opens large PCI bridge MMIO windows above 4GiB to place hotplugable
PCI BARs there as they won't fit below 4GiB. In addition modern
GPGPU devices have very big PCI bars that doesn't fit below 4GiB.
The PciHostBridgeLib made lots of assumptions about the coreboot
resource allocator that were not verified at runtime and are no
longer true.
Remove all of the 'coreboot specific' code and implement the same
logic as OvmfPkg's ScanForRootBridges.
Fixes assertion
"ASSERT [PciHostBridgeDxe] Bridge->Mem.Limit < 0x0000000100000000ULL".
Tested with coreboot as bootloader on platforms that have PCI resources
above 4GiB and on platforms that don't have resources above 4GiB.
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c | 137 +++-----------------
1 file changed, 18 insertions(+), 119 deletions(-)
diff --git a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
index bf2d10f4bf..8a890b6b53 100644
--- a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
+++ b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
@@ -18,92 +18,6 @@
#include <Library/PciLib.h>
#include "PciHostBridge.h"
-/**
- Adjust the collected PCI resource.
-
- @param[in] Io IO aperture.
-
- @param[in] Mem MMIO aperture.
-
- @param[in] MemAbove4G MMIO aperture above 4G.
-
- @param[in] PMem Prefetchable MMIO aperture.
-
- @param[in] PMemAbove4G Prefetchable MMIO aperture above 4G.
-**/
-VOID
-AdjustRootBridgeResource (
- IN PCI_ROOT_BRIDGE_APERTURE *Io,
- IN PCI_ROOT_BRIDGE_APERTURE *Mem,
- IN PCI_ROOT_BRIDGE_APERTURE *MemAbove4G,
- IN PCI_ROOT_BRIDGE_APERTURE *PMem,
- IN PCI_ROOT_BRIDGE_APERTURE *PMemAbove4G
- )
-{
- UINT64 Mask;
-
- //
- // For now try to downgrade everything into MEM32 since
- // - coreboot does not assign resource above 4GB
- // - coreboot might allocate interleaved MEM32 and PMEM32 resource
- // in some cases
- //
- if (PMem->Base < Mem->Base) {
- Mem->Base = PMem->Base;
- }
-
- if (PMem->Limit > Mem->Limit) {
- Mem->Limit = PMem->Limit;
- }
-
- PMem->Base = MAX_UINT64;
- PMem->Limit = 0;
-
- if (MemAbove4G->Base < 0x100000000ULL) {
- if (MemAbove4G->Base < Mem->Base) {
- Mem->Base = MemAbove4G->Base;
- }
-
- if (MemAbove4G->Limit > Mem->Limit) {
- Mem->Limit = MemAbove4G->Limit;
- }
-
- MemAbove4G->Base = MAX_UINT64;
- MemAbove4G->Limit = 0;
- }
-
- if (PMemAbove4G->Base < 0x100000000ULL) {
- if (PMemAbove4G->Base < Mem->Base) {
- Mem->Base = PMemAbove4G->Base;
- }
-
- if (PMemAbove4G->Limit > Mem->Limit) {
- Mem->Limit = PMemAbove4G->Limit;
- }
-
- PMemAbove4G->Base = MAX_UINT64;
- PMemAbove4G->Limit = 0;
- }
-
- //
- // Align IO resource at 4K boundary
- //
- Mask = 0xFFFULL;
- Io->Limit = ((Io->Limit + Mask) & ~Mask) - 1;
- if (Io->Base != MAX_UINT64) {
- Io->Base &= ~Mask;
- }
-
- //
- // Align MEM resource at 1MB boundary
- //
- Mask = 0xFFFFFULL;
- Mem->Limit = ((Mem->Limit + Mask) & ~Mask) - 1;
- if (Mem->Base != MAX_UINT64) {
- Mem->Base &= ~Mask;
- }
-}
-
/**
Probe a bar is existed or not.
@@ -114,28 +28,24 @@ AdjustRootBridgeResource (
STATIC
VOID
PcatPciRootBridgeBarExisted (
- IN UINT64 Address,
+ IN UINTN Address,
OUT UINT32 *OriginalValue,
OUT UINT32 *Value
)
{
- UINTN PciAddress;
-
- PciAddress = (UINTN)Address;
-
//
// Preserve the original value
//
- *OriginalValue = PciRead32 (PciAddress);
+ *OriginalValue = PciRead32 (Address);
//
// Disable timer interrupt while the BAR is probed
//
DisableInterrupts ();
- PciWrite32 (PciAddress, 0xFFFFFFFF);
- *Value = PciRead32 (PciAddress);
- PciWrite32 (PciAddress, *OriginalValue);
+ PciWrite32 (Address, 0xFFFFFFFF);
+ *Value = PciRead32 (Address);
+ PciWrite32 (Address, *OriginalValue);
//
// Enable interrupt
@@ -179,9 +89,7 @@ PcatPciRootBridgeParseBars (
IN UINTN BarOffsetEnd,
IN PCI_ROOT_BRIDGE_APERTURE *Io,
IN PCI_ROOT_BRIDGE_APERTURE *Mem,
- IN PCI_ROOT_BRIDGE_APERTURE *MemAbove4G,
- IN PCI_ROOT_BRIDGE_APERTURE *PMem,
- IN PCI_ROOT_BRIDGE_APERTURE *PMemAbove4G
+ IN PCI_ROOT_BRIDGE_APERTURE *MemAbove4G
)
{
@@ -246,11 +154,7 @@ PcatPciRootBridgeParseBars (
//
Length = ((~Length) + 1) & 0xffffffff;
- if ((Value & BIT3) == BIT3) {
- MemAperture = PMem;
- } else {
- MemAperture = Mem;
- }
+ MemAperture = Mem;
} else {
//
// 64bit
@@ -269,8 +173,8 @@ PcatPciRootBridgeParseBars (
Length = LShiftU64 (1ULL, LowBit);
}
- if ((Value & BIT3) == BIT3) {
- MemAperture = PMemAbove4G;
+ if (Base < BASE_4GB) {
+ MemAperture = Mem;
} else {
MemAperture = MemAbove4G;
}
@@ -291,6 +195,8 @@ PcatPciRootBridgeParseBars (
}
}
+STATIC PCI_ROOT_BRIDGE_APERTURE mNonExistAperture = { MAX_UINT64, 0 };
+
/**
Scan for all root bridges in platform.
@@ -317,8 +223,6 @@ ScanForRootBridges (
PCI_ROOT_BRIDGE_APERTURE Io;
PCI_ROOT_BRIDGE_APERTURE Mem;
PCI_ROOT_BRIDGE_APERTURE MemAbove4G;
- PCI_ROOT_BRIDGE_APERTURE PMem;
- PCI_ROOT_BRIDGE_APERTURE PMemAbove4G;
PCI_ROOT_BRIDGE_APERTURE *MemAperture;
PCI_ROOT_BRIDGE *RootBridges;
UINTN BarOffsetEnd;
@@ -338,9 +242,7 @@ ScanForRootBridges (
ZeroMem (&Io, sizeof (Io));
ZeroMem (&Mem, sizeof (Mem));
ZeroMem (&MemAbove4G, sizeof (MemAbove4G));
- ZeroMem (&PMem, sizeof (PMem));
- ZeroMem (&PMemAbove4G, sizeof (PMemAbove4G));
- Io.Base = Mem.Base = MemAbove4G.Base = PMem.Base = PMemAbove4G.Base = MAX_UINT64;
+ Io.Base = Mem.Base = MemAbove4G.Base = MAX_UINT64;
//
// Scan all the PCI devices on the primary bus of the PCI root bridge
//
@@ -446,16 +348,17 @@ ScanForRootBridges (
//
// Get the Prefetchable Memory range that the PPB is decoding
+ // and merge it into Memory range
//
Value = Pci.Bridge.PrefetchableMemoryBase & 0x0f;
Base = ((UINT32)Pci.Bridge.PrefetchableMemoryBase & 0xfff0) << 16;
Limit = (((UINT32)Pci.Bridge.PrefetchableMemoryLimit & 0xfff0)
<< 16) | 0xfffff;
- MemAperture = &PMem;
+ MemAperture = &Mem;
if (Value == BIT0) {
Base |= LShiftU64 (Pci.Bridge.PrefetchableBaseUpper32, 32);
Limit |= LShiftU64 (Pci.Bridge.PrefetchableLimitUpper32, 32);
- MemAperture = &PMemAbove4G;
+ MemAperture = &MemAbove4G;
}
if ((Base > 0) && (Base < Limit)) {
@@ -513,9 +416,7 @@ ScanForRootBridges (
BarOffsetEnd,
&Io,
&Mem,
- &MemAbove4G,
- &PMem,
- &PMemAbove4G
+ &MemAbove4G
);
//
@@ -593,8 +494,6 @@ ScanForRootBridges (
);
ASSERT (RootBridges != NULL);
- AdjustRootBridgeResource (&Io, &Mem, &MemAbove4G, &PMem, &PMemAbove4G);
-
InitRootBridge (
Attributes,
Attributes,
@@ -604,8 +503,8 @@ ScanForRootBridges (
&Io,
&Mem,
&MemAbove4G,
- &PMem,
- &PMemAbove4G,
+ &mNonExistAperture,
+ &mNonExistAperture,
&RootBridges[*NumberOfRootBridges]
);
RootBridges[*NumberOfRootBridges].ResourceAssigned = TRUE;
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-platform][PATCH] UefiPayloadPkg: Fix PciHostBridgeLib
2022-02-07 10:01 [edk2-platform][PATCH] UefiPayloadPkg: Fix PciHostBridgeLib Patrick Rudolph
@ 2022-02-17 4:26 ` Ma, Maurice
2022-03-21 9:35 ` [edk2-devel] " Sean Rhodes
0 siblings, 1 reply; 3+ messages in thread
From: Ma, Maurice @ 2022-02-17 4:26 UTC (permalink / raw)
To: Patrick Rudolph, devel@edk2.groups.io; +Cc: Ni, Ray, You, Benjamin, Dong, Guo
Reviewed-by: Maurice Ma <maurice.ma@intel.com>
> -----Original Message-----
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
> Sent: Monday, February 7, 2022 2:01
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Ma, Maurice <maurice.ma@intel.com>; You,
> Benjamin <benjamin.you@intel.com>; Dong, Guo <guo.dong@intel.com>
> Subject: [edk2-platform][PATCH] UefiPayloadPkg: Fix PciHostBridgeLib
>
> On modern platforms with TBT devices the coreboot resource allocator
> opens large PCI bridge MMIO windows above 4GiB to place hotplugable PCI
> BARs there as they won't fit below 4GiB. In addition modern GPGPU devices
> have very big PCI bars that doesn't fit below 4GiB.
>
> The PciHostBridgeLib made lots of assumptions about the coreboot resource
> allocator that were not verified at runtime and are no longer true.
>
> Remove all of the 'coreboot specific' code and implement the same logic as
> OvmfPkg's ScanForRootBridges.
>
> Fixes assertion
> "ASSERT [PciHostBridgeDxe] Bridge->Mem.Limit < 0x0000000100000000ULL".
>
> Tested with coreboot as bootloader on platforms that have PCI resources
> above 4GiB and on platforms that don't have resources above 4GiB.
>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
> UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c | 137 +++-
> ----------------
> 1 file changed, 18 insertions(+), 119 deletions(-)
>
> diff --git a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
> b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
> index bf2d10f4bf..8a890b6b53 100644
> --- a/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
> +++ b/UefiPayloadPkg/Library/PciHostBridgeLib/PciHostBridgeSupport.c
> @@ -18,92 +18,6 @@
> #include <Library/PciLib.h> #include "PciHostBridge.h" -/**- Adjust the
> collected PCI resource.-- @param[in] Io IO aperture.-- @param[in]
> Mem MMIO aperture.-- @param[in] MemAbove4G MMIO
> aperture above 4G.-- @param[in] PMem Prefetchable MMIO
> aperture.-- @param[in] PMemAbove4G Prefetchable MMIO aperture
> above 4G.-**/-VOID-AdjustRootBridgeResource (- IN
> PCI_ROOT_BRIDGE_APERTURE *Io,- IN PCI_ROOT_BRIDGE_APERTURE
> *Mem,- IN PCI_ROOT_BRIDGE_APERTURE *MemAbove4G,- IN
> PCI_ROOT_BRIDGE_APERTURE *PMem,- IN PCI_ROOT_BRIDGE_APERTURE
> *PMemAbove4G- )-{- UINT64 Mask;-- //- // For now try to downgrade
> everything into MEM32 since- // - coreboot does not assign resource above
> 4GB- // - coreboot might allocate interleaved MEM32 and PMEM32
> resource- // in some cases- //- if (PMem->Base < Mem->Base) {- Mem-
> >Base = PMem->Base;- }-- if (PMem->Limit > Mem->Limit) {- Mem->Limit
> = PMem->Limit;- }-- PMem->Base = MAX_UINT64;- PMem->Limit = 0;-- if
> (MemAbove4G->Base < 0x100000000ULL) {- if (MemAbove4G->Base <
> Mem->Base) {- Mem->Base = MemAbove4G->Base;- }-- if
> (MemAbove4G->Limit > Mem->Limit) {- Mem->Limit = MemAbove4G-
> >Limit;- }-- MemAbove4G->Base = MAX_UINT64;- MemAbove4G->Limit
> = 0;- }-- if (PMemAbove4G->Base < 0x100000000ULL) {- if
> (PMemAbove4G->Base < Mem->Base) {- Mem->Base = PMemAbove4G-
> >Base;- }-- if (PMemAbove4G->Limit > Mem->Limit) {- Mem->Limit =
> PMemAbove4G->Limit;- }-- PMemAbove4G->Base = MAX_UINT64;-
> PMemAbove4G->Limit = 0;- }-- //- // Align IO resource at 4K boundary- //-
> Mask = 0xFFFULL;- Io->Limit = ((Io->Limit + Mask) & ~Mask) - 1;- if (Io-
> >Base != MAX_UINT64) {- Io->Base &= ~Mask;- }-- //- // Align MEM
> resource at 1MB boundary- //- Mask = 0xFFFFFULL;- Mem->Limit =
> ((Mem->Limit + Mask) & ~Mask) - 1;- if (Mem->Base != MAX_UINT64) {-
> Mem->Base &= ~Mask;- }-}- /** Probe a bar is existed or not. @@ -114,28
> +28,24 @@ AdjustRootBridgeResource (
> STATIC VOID PcatPciRootBridgeBarExisted (- IN UINT64 Address,+ IN
> UINTN Address, OUT UINT32 *OriginalValue, OUT UINT32 *Value ) {-
> UINTN PciAddress;-- PciAddress = (UINTN)Address;- // // Preserve the
> original value //- *OriginalValue = PciRead32 (PciAddress);+ *OriginalValue
> = PciRead32 (Address); // // Disable timer interrupt while the BAR is
> probed // DisableInterrupts (); - PciWrite32 (PciAddress, 0xFFFFFFFF);-
> *Value = PciRead32 (PciAddress);- PciWrite32 (PciAddress, *OriginalValue);+
> PciWrite32 (Address, 0xFFFFFFFF);+ *Value = PciRead32 (Address);+
> PciWrite32 (Address, *OriginalValue); // // Enable interrupt@@ -179,9
> +89,7 @@ PcatPciRootBridgeParseBars (
> IN UINTN BarOffsetEnd, IN PCI_ROOT_BRIDGE_APERTURE *Io,
> IN PCI_ROOT_BRIDGE_APERTURE *Mem,- IN
> PCI_ROOT_BRIDGE_APERTURE *MemAbove4G,- IN
> PCI_ROOT_BRIDGE_APERTURE *PMem,- IN PCI_ROOT_BRIDGE_APERTURE
> *PMemAbove4G+ IN PCI_ROOT_BRIDGE_APERTURE *MemAbove4G )
> {@@ -246,11 +154,7 @@ PcatPciRootBridgeParseBars (
> // Length = ((~Length) + 1) & 0xffffffff; - if ((Value & BIT3) ==
> BIT3) {- MemAperture = PMem;- } else {- MemAperture =
> Mem;- }+ MemAperture = Mem; } else { // //
> 64bit@@ -269,8 +173,8 @@ PcatPciRootBridgeParseBars (
> Length = LShiftU64 (1ULL, LowBit); } - if ((Value & BIT3) ==
> BIT3) {- MemAperture = PMemAbove4G;+ if (Base < BASE_4GB) {+
> MemAperture = Mem; } else { MemAperture =
> MemAbove4G; }@@ -291,6 +195,8 @@ PcatPciRootBridgeParseBars (
> } } +STATIC PCI_ROOT_BRIDGE_APERTURE mNonExistAperture =
> { MAX_UINT64, 0 };+ /** Scan for all root bridges in platform. @@ -317,8
> +223,6 @@ ScanForRootBridges (
> PCI_ROOT_BRIDGE_APERTURE Io; PCI_ROOT_BRIDGE_APERTURE Mem;
> PCI_ROOT_BRIDGE_APERTURE MemAbove4G;-
> PCI_ROOT_BRIDGE_APERTURE PMem;- PCI_ROOT_BRIDGE_APERTURE
> PMemAbove4G; PCI_ROOT_BRIDGE_APERTURE *MemAperture;
> PCI_ROOT_BRIDGE *RootBridges; UINTN BarOffsetEnd;@@ -
> 338,9 +242,7 @@ ScanForRootBridges (
> ZeroMem (&Io, sizeof (Io)); ZeroMem (&Mem, sizeof (Mem));
> ZeroMem (&MemAbove4G, sizeof (MemAbove4G));- ZeroMem (&PMem,
> sizeof (PMem));- ZeroMem (&PMemAbove4G, sizeof (PMemAbove4G));-
> Io.Base = Mem.Base = MemAbove4G.Base = PMem.Base =
> PMemAbove4G.Base = MAX_UINT64;+ Io.Base = Mem.Base =
> MemAbove4G.Base = MAX_UINT64; // // Scan all the PCI devices on the
> primary bus of the PCI root bridge //@@ -446,16 +348,17 @@
> ScanForRootBridges (
> // // Get the Prefetchable Memory range that the PPB is
> decoding+ // and merge it into Memory range // Value =
> Pci.Bridge.PrefetchableMemoryBase & 0x0f; Base =
> ((UINT32)Pci.Bridge.PrefetchableMemoryBase & 0xfff0) << 16; Limit =
> (((UINT32)Pci.Bridge.PrefetchableMemoryLimit & 0xfff0) << 16) |
> 0xfffff;- MemAperture = &PMem;+ MemAperture = &Mem; if
> (Value == BIT0) { Base |= LShiftU64
> (Pci.Bridge.PrefetchableBaseUpper32, 32); Limit |= LShiftU64
> (Pci.Bridge.PrefetchableLimitUpper32, 32);- MemAperture =
> &PMemAbove4G;+ MemAperture = &MemAbove4G; } if
> ((Base > 0) && (Base < Limit)) {@@ -513,9 +416,7 @@ ScanForRootBridges (
> BarOffsetEnd, &Io, &Mem,- &MemAbove4G,-
> &PMem,- &PMemAbove4G+ &MemAbove4G ); //@@ -
> 593,8 +494,6 @@ ScanForRootBridges (
> ); ASSERT (RootBridges != NULL); -
> AdjustRootBridgeResource (&Io, &Mem, &MemAbove4G, &PMem,
> &PMemAbove4G);- InitRootBridge ( Attributes, Attributes,@@ -
> 604,8 +503,8 @@ ScanForRootBridges (
> &Io, &Mem, &MemAbove4G,- &PMem,-
> &PMemAbove4G,+ &mNonExistAperture,+ &mNonExistAperture,
> &RootBridges[*NumberOfRootBridges] );
> RootBridges[*NumberOfRootBridges].ResourceAssigned = TRUE;--
> 2.34.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [edk2-platform][PATCH] UefiPayloadPkg: Fix PciHostBridgeLib
2022-02-17 4:26 ` Ma, Maurice
@ 2022-03-21 9:35 ` Sean Rhodes
0 siblings, 0 replies; 3+ messages in thread
From: Sean Rhodes @ 2022-03-21 9:35 UTC (permalink / raw)
To: Ma, Maurice, devel
[-- Attachment #1: Type: text/plain, Size: 69 bytes --]
Hi
Can I follow up on this patch being merged?
Thanks
Sean
[-- Attachment #2: Type: text/html, Size: 109 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-03-21 9:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-07 10:01 [edk2-platform][PATCH] UefiPayloadPkg: Fix PciHostBridgeLib Patrick Rudolph
2022-02-17 4:26 ` Ma, Maurice
2022-03-21 9:35 ` [edk2-devel] " Sean Rhodes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox