diff mbox series

[v2,1/3] OvmfPkg/XenSupport: remove usage of prefetchable PCI host bridge aperture

Message ID 1554779560-26204-2-git-send-email-igor.druzhinin@citrix.com (mailing list archive)
State Superseded
Headers show
Series Xen PCI passthrough fixes | expand

Commit Message

Igor Druzhinin April 9, 2019, 3:12 a.m. UTC
This aperture doesn't exist in QEMU-XEN and hvmloader places BARs
in arbitrary order disregarding prefetchable bit. This makes
prefetchable and non-prefetchable BARs to follow each other that's
quite likely with PCI passthrough devices. In that case, the existing
code, that tries to work out aperture boundaries by reading hvmloader
BAR placement, will report a bogus prefetchable aperture which overlaps
with the regular one. It will eventually trigger an assertion in
DXE PCI initialization code.

Do the same thing as OVMF on QEMU-KVM and pass a non-existing aperture
there. It's not necessary to pass additional allocation flags as we set
ResourceAssigned flag on the root bridge which means they will be ignored.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
Changes in v2:
* remove usage of prefetchable aperture entirely
* explained rationale for the change in the description
---
 OvmfPkg/Library/PciHostBridgeLib/XenSupport.c | 34 ++++++++++-----------------
 1 file changed, 12 insertions(+), 22 deletions(-)

Comments

Anthony PERARD April 11, 2019, 1:07 p.m. UTC | #1
On Tue, Apr 09, 2019 at 04:12:38AM +0100, Igor Druzhinin wrote:
> This aperture doesn't exist in QEMU-XEN and hvmloader places BARs
> in arbitrary order disregarding prefetchable bit. This makes
> prefetchable and non-prefetchable BARs to follow each other that's
> quite likely with PCI passthrough devices. In that case, the existing
> code, that tries to work out aperture boundaries by reading hvmloader
> BAR placement, will report a bogus prefetchable aperture which overlaps
> with the regular one. It will eventually trigger an assertion in
> DXE PCI initialization code.
> 
> Do the same thing as OVMF on QEMU-KVM and pass a non-existing aperture
> there. It's not necessary to pass additional allocation flags as we set
> ResourceAssigned flag on the root bridge which means they will be ignored.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> ---
> Changes in v2:
> * remove usage of prefetchable aperture entirely
> * explained rationale for the change in the description
> ---

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,
diff mbox series

Patch

diff --git a/OvmfPkg/Library/PciHostBridgeLib/XenSupport.c b/OvmfPkg/Library/PciHostBridgeLib/XenSupport.c
index 9204179..5ea63f7 100644
--- a/OvmfPkg/Library/PciHostBridgeLib/XenSupport.c
+++ b/OvmfPkg/Library/PciHostBridgeLib/XenSupport.c
@@ -66,9 +66,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
 
 )
 {
@@ -129,11 +127,7 @@  PcatPciRootBridgeParseBars (
           //
           Length = ((~Length) + 1) & 0xffffffff;
 
-          if ((Value & BIT3) == BIT3) {
-            MemAperture = PMem;
-          } else {
-            MemAperture = Mem;
-          }
+          MemAperture = Mem;
         } else {
           //
           // 64bit
@@ -149,11 +143,7 @@  PcatPciRootBridgeParseBars (
           Length = Length | LShiftU64 ((UINT64) UpperValue, 32);
           Length = (~Length) + 1;
 
-          if ((Value & BIT3) == BIT3) {
-            MemAperture = PMemAbove4G;
-          } else {
-            MemAperture = MemAbove4G;
-          }
+          MemAperture = MemAbove4G;
         }
 
         Limit = Base + Length - 1;
@@ -170,6 +160,8 @@  PcatPciRootBridgeParseBars (
   }
 }
 
+STATIC PCI_ROOT_BRIDGE_APERTURE mNonExistAperture = { MAX_UINT64, 0 };
+
 PCI_ROOT_BRIDGE *
 ScanForRootBridges (
   UINTN      *NumberOfRootBridges
@@ -186,7 +178,7 @@  ScanForRootBridges (
   UINT64     Base;
   UINT64     Limit;
   UINT64     Value;
-  PCI_ROOT_BRIDGE_APERTURE Io, Mem, MemAbove4G, PMem, PMemAbove4G, *MemAperture;
+  PCI_ROOT_BRIDGE_APERTURE Io, Mem, MemAbove4G, *MemAperture;
   PCI_ROOT_BRIDGE *RootBridges;
   UINTN      BarOffsetEnd;
 
@@ -206,9 +198,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
     //
@@ -313,16 +303,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 < Limit) {
             if (MemAperture->Base > Base) {
@@ -373,8 +364,7 @@  ScanForRootBridges (
           OFFSET_OF (PCI_TYPE00, Device.Bar),
           BarOffsetEnd,
           &Io,
-          &Mem, &MemAbove4G,
-          &PMem, &PMemAbove4G
+          &Mem, &MemAbove4G
         );
 
         //
@@ -446,7 +436,7 @@  ScanForRootBridges (
       InitRootBridge (
         Attributes, Attributes, 0,
         (UINT8) PrimaryBus, (UINT8) SubBus,
-        &Io, &Mem, &MemAbove4G, &PMem, &PMemAbove4G,
+        &Io, &Mem, &MemAbove4G, &mNonExistAperture, &mNonExistAperture,
         &RootBridges[*NumberOfRootBridges]
       );
       RootBridges[*NumberOfRootBridges].ResourceAssigned = TRUE;