public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch V3 1/2] MdePkg: Define structures for Resizable BAR Capability
@ 2021-01-04  6:59 Heng Luo
  2021-01-04  6:59 ` [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe " Heng Luo
  2021-01-04  7:52 ` [Patch V3 1/2] MdePkg: Define structures for " Ni, Ray
  0 siblings, 2 replies; 15+ messages in thread
From: Heng Luo @ 2021-01-04  6:59 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Hao A Wu

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3138

Define structures for Resizable BAR Capability in
MdePkg/Include/IndustryStandard/PciExpress21.h,
Change ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
to use new structures.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Heng Luo <heng.luo@intel.com>
---
 MdePkg/Include/IndustryStandard/PciExpress21.h    | 30 +++++++++++++++++++++++++-----
 ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c |  6 +++---
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/PciExpress21.h b/MdePkg/Include/IndustryStandard/PciExpress21.h
index 2c07cb560e..4617dc1569 100644
--- a/MdePkg/Include/IndustryStandard/PciExpress21.h
+++ b/MdePkg/Include/IndustryStandard/PciExpress21.h
@@ -1,7 +1,7 @@
 /** @file
   Support for the latest PCI standard.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
   (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -632,10 +632,30 @@ typedef struct {
 #define PCI_EXPRESS_EXTENDED_CAPABILITY_RESIZABLE_BAR_ID    0x0015
 #define PCI_EXPRESS_EXTENDED_CAPABILITY_RESIZABLE_BAR_VER1  0x1
 
+typedef union {
+  struct {
+    UINT32 Reserved:4;
+    UINT32 BarSizeCapability:28;
+  } Bits;
+  UINT32   Uint32;
+} PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CAPABILITY;
+
+
+typedef union {
+  struct {
+    UINT32 BarIndex:3;
+    UINT32 Reserved:2;
+    UINT32 ResizableBarNumber:3;
+    UINT32 BarSize:6;
+    UINT32 Reserved2:2;
+    UINT32 BarSizeCapability:16;
+  } Bits;
+  UINT32   Uint32;
+} PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CONTROL;
+
 typedef struct {
-  UINT32                                                 ResizableBarCapability;
-  UINT16                                                 ResizableBarControl;
-  UINT16                                                 Reserved;
+  PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CAPABILITY ResizableBarCapability;
+  PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CONTROL    ResizableBarControl;
 } PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY;
 
 typedef struct {
@@ -643,7 +663,7 @@ typedef struct {
   PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY  Capability[1];
 } PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR;
 
-#define GET_NUMBER_RESIZABLE_BARS(x) (((x->Capability[0].ResizableBarControl) & 0xE0) >> 5)
+#define GET_NUMBER_RESIZABLE_BARS(x) (x->Capability[0].ResizableBarControl.Bits.ResizableBarNumber)
 
 #define PCI_EXPRESS_EXTENDED_CAPABILITY_ARI_CAPABILITY_ID    0x000E
 #define PCI_EXPRESS_EXTENDED_CAPABILITY_ARI_CAPABILITY_VER1  0x1
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
index 3e138188ce..a2f04d8db5 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
@@ -1,7 +1,7 @@
 /** @file
   Main file for Pci shell Debug1 function.
 
-  Copyright (c) 2005 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2005 - 2021, Intel Corporation. All rights reserved.<BR>
   (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.<BR>
   (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -5534,8 +5534,8 @@ PrintInterpretedExtendedCompatibilityResizeableBar (
       STRING_TOKEN (STR_PCI_EXT_CAP_RESIZE_BAR),
       gShellDebug1HiiHandle,
       ItemCount+1,
-      Header->Capability[ItemCount].ResizableBarCapability,
-      Header->Capability[ItemCount].ResizableBarControl
+      Header->Capability[ItemCount].ResizableBarCapability.Uint32,
+      Header->Capability[ItemCount].ResizableBarControl.Uint32
       );
   }
 
-- 
2.24.0.windows.2


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

* [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-04  6:59 [Patch V3 1/2] MdePkg: Define structures for Resizable BAR Capability Heng Luo
@ 2021-01-04  6:59 ` Heng Luo
  2021-01-04  7:52   ` Ni, Ray
  2021-01-11 19:38   ` [edk2-devel] " Laszlo Ersek
  2021-01-04  7:52 ` [Patch V3 1/2] MdePkg: Define structures for " Ni, Ray
  1 sibling, 2 replies; 15+ messages in thread
From: Heng Luo @ 2021-01-04  6:59 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Hao A Wu

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=313

Add PcdPcieResizableBarSupport to enable/disable PCIe Resizable
BAR Capability fearture.
Program the Resizable BAR Register if the device suports PCIe Resizable
BAR Capability and PcdPcieResizableBarSupport is TRUE.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Heng Luo <heng.luo@intel.com>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h               |   4 +++-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf          |   3 ++-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c |  27 ++++++++++++++++++++++++++-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h |  12 +++++++++++-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c               | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------
 MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h               |  22 +++++++++++++++++++++-
 MdeModulePkg/MdeModulePkg.dec                         |   8 +++++++-
 7 files changed, 241 insertions(+), 20 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index d4113993c8..a619a68526 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -1,7 +1,7 @@
 /** @file
   Header files and data structures needed by PCI Bus module.
 
-Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -280,6 +280,8 @@ struct _PCI_IO_DEVICE {
   // This field is used to support this case.
   //
   UINT16                                    BridgeIoAlignment;
+  UINT32                                    ResizableBarOffset;
+  UINT32                                    ResizableBarNumber;
 };
 
 #define PCI_IO_DEVICE_FROM_PCI_IO_THIS(a) \
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index 9284998f36..e317169d9c 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -2,7 +2,7 @@
 #  The PCI bus driver will probe all PCI devices and allocate MMIO and IO space for these devices.
 #  Please use PCD feature flag PcdPciBusHotplugDeviceSupport to enable hot plug supporting.
 #
-#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -106,6 +106,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport                  ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport                ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration    ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPcieResizableBarSupport     ## CONSUMES
 
 [UserExtensions.TianoCore."ExtraFiles"]
   PciBusDxeExtra.uni
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
index 6c68a97d4e..1b64924b7b 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
@@ -1,7 +1,7 @@
 /** @file
   PCI emumeration support functions implementation for PCI Bus module.
 
-Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -2426,6 +2426,31 @@ CreatePciIoDevice (
     }
   }
 
+  PciIoDevice->ResizableBarOffset = 0;
+  if (PcdGetBool (PcdPcieResizableBarSupport)) {
+    Status = LocatePciExpressCapabilityRegBlock (
+               PciIoDevice,
+               PCI_EXPRESS_EXTENDED_CAPABILITY_RESIZABLE_BAR_ID,
+               &PciIoDevice->ResizableBarOffset,
+               NULL
+               );
+    if (!EFI_ERROR (Status)) {
+      PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CONTROL ResizableBarControl;
+      UINT32                                                  Offset;
+      Offset = PciIoDevice->ResizableBarOffset + sizeof (PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER)
+                + sizeof (PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CAPABILITY),
+      PciIo->Pci.Read (
+              PciIo,
+              EfiPciIoWidthUint8,
+              Offset,
+              sizeof (PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CONTROL),
+              &ResizableBarControl
+              );
+      PciIoDevice->ResizableBarNumber = ResizableBarControl.Bits.ResizableBarNumber;
+      PciProgramResizableBar (PciIoDevice, PciResizableBarMax);
+    }
+  }
+
   //
   // Initialize the reserved resource list
   //
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h
index d76606c7df..4581b270c9 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h
@@ -1,7 +1,7 @@
 /** @file
   PCI enumeration support functions declaration for PCI Bus module.
 
-Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -467,4 +467,14 @@ DumpPpbPaddingResource (
   IN PCI_BAR_TYPE                     ResourceType
   );
 
+/**
+  Dump the PCI BAR information.
+
+  @param PciIoDevice     PCI IO instance.
+**/
+VOID
+DumpPciBars (
+  IN PCI_IO_DEVICE                    *PciIoDevice
+  );
+
 #endif
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
index 72690ab647..6bba283671 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
@@ -1,7 +1,7 @@
 /** @file
   Internal library implementation for PCI Bus module.
 
-Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -377,6 +377,60 @@ DumpResourceMap (
   }
 }
 
+/**
+  Adjust the Devices' BAR size to minimum value if it support Resizeable BAR capability.
+
+  @param RootBridgeDev  Pointer to instance of PCI_IO_DEVICE..
+
+  @return TRUE if BAR size is adjusted.
+
+**/
+BOOLEAN
+AdjustPciDeviceBarSize (
+  IN PCI_IO_DEVICE *RootBridgeDev
+  )
+{
+  PCI_IO_DEVICE     *PciIoDevice;
+  LIST_ENTRY        *CurrentLink;
+  BOOLEAN           Adjusted;
+  UINTN             Offset;
+  UINTN             BarIndex;
+
+  Adjusted    = FALSE;
+  CurrentLink = RootBridgeDev->ChildList.ForwardLink;
+
+  while (CurrentLink != NULL && CurrentLink != &RootBridgeDev->ChildList) {
+    PciIoDevice = PCI_IO_DEVICE_FROM_LINK (CurrentLink);
+
+    if (IS_PCI_BRIDGE (&PciIoDevice->Pci)) {
+      if (AdjustPciDeviceBarSize (PciIoDevice)) {
+        Adjusted = TRUE;
+      }
+    } else {
+      if (PciIoDevice->ResizableBarOffset != 0) {
+        DEBUG ((
+          DEBUG_ERROR,
+          "PciBus: [%02x|%02x|%02x] Adjust Pci Device Bar Size\n",
+          PciIoDevice->BusNumber, PciIoDevice->DeviceNumber, PciIoDevice->FunctionNumber
+          ));
+        PciProgramResizableBar (PciIoDevice, PciResizableBarMin);
+        //
+        // Start to parse the bars
+        //
+        for (Offset = 0x10, BarIndex = 0; Offset <= 0x24 && BarIndex < PCI_MAX_BAR; BarIndex++) {
+          Offset = PciParseBar (PciIoDevice, Offset, BarIndex);
+        }
+        Adjusted = TRUE;
+        DEBUG_CODE (DumpPciBars (PciIoDevice););
+      }
+    }
+
+    CurrentLink = CurrentLink->ForwardLink;
+  }
+
+  return Adjusted;
+}
+
 /**
   Submits the I/O and memory resource requirements for the specified PCI Host Bridge.
 
@@ -422,6 +476,10 @@ PciHostBridgeResourceAllocator (
   PCI_RESOURCE_NODE                              PMem64Pool;
   EFI_DEVICE_HANDLE_EXTENDED_DATA_PAYLOAD        HandleExtendedData;
   EFI_RESOURCE_ALLOC_FAILURE_ERROR_DATA_PAYLOAD  AllocFailExtendedData;
+  BOOLEAN                                        ResizableBarNeedAdjust;
+  BOOLEAN                                        ResizableBarAdjusted;
+
+  ResizableBarNeedAdjust = PcdGetBool (PcdPcieResizableBarSupport);
 
   //
   // It may try several times if the resource allocation fails
@@ -703,19 +761,30 @@ PciHostBridgeResourceAllocator (
             sizeof (AllocFailExtendedData)
             );
 
-      Status = PciHostBridgeAdjustAllocation (
-                 &IoPool,
-                 &Mem32Pool,
-                 &PMem32Pool,
-                 &Mem64Pool,
-                 &PMem64Pool,
-                 IoResStatus,
-                 Mem32ResStatus,
-                 PMem32ResStatus,
-                 Mem64ResStatus,
-                 PMem64ResStatus
-                 );
-
+     //
+     // When resource conflict happens, adjust the BAR size first.
+     // Only when adjusting BAR size doesn't help or BAR size cannot be adjusted,
+     // reject the device who requests largest resource that causes conflict.
+     //
+      ResizableBarAdjusted = FALSE;
+      if (ResizableBarNeedAdjust) {
+        ResizableBarAdjusted = AdjustPciDeviceBarSize (RootBridgeDev);
+        ResizableBarNeedAdjust = FALSE;
+      }
+      if (!ResizableBarAdjusted) {
+        Status = PciHostBridgeAdjustAllocation (
+                  &IoPool,
+                  &Mem32Pool,
+                  &PMem32Pool,
+                  &Mem64Pool,
+                  &PMem64Pool,
+                  IoResStatus,
+                  Mem32ResStatus,
+                  PMem32ResStatus,
+                  Mem64ResStatus,
+                  PMem64ResStatus
+                  );
+      }
       //
       // Destroy all the resource tree
       //
@@ -1651,3 +1720,91 @@ PciHostBridgeEnumerator (
 
   return EFI_SUCCESS;
 }
+
+/**
+  This function is used to program the Resizable BAR Register.
+
+  @param PciIoDevice            A pointer to the PCI_IO_DEVICE.
+  @param ResizableBarOp         PciResizableBarMax: Set BAR to max size
+                                PciResizableBarMin: set BAR to min size.
+
+  @retval EFI_SUCCESS           Successfully enumerated the host bridge.
+  @retval other                 Some error occurred when enumerating the host bridge.
+
+**/
+EFI_STATUS
+PciProgramResizableBar (
+  IN PCI_IO_DEVICE                *PciIoDevice,
+  IN PCI_RESIZABLE_BAR_OPERATION  ResizableBarOp
+  )
+{
+  EFI_PCI_IO_PROTOCOL  *PciIo;
+  UINT64                Capabilities;
+  UINT32                Index;
+  UINT32                Offset;
+  INTN                  Bit;
+  UINTN                 ResizableBarNumber;
+  EFI_STATUS            Status;
+  PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY   Entries[PCI_MAX_BAR];
+
+  ASSERT (PciIoDevice->ResizableBarOffset != 0);
+
+  DEBUG ((DEBUG_INFO, "   Programs Resizable BAR register, offset: 0x%08x, number: %d\n",
+        PciIoDevice->ResizableBarOffset, PciIoDevice->ResizableBarNumber));
+
+  ResizableBarNumber = MIN (PciIoDevice->ResizableBarNumber, PCI_MAX_BAR);
+  PciIo = &PciIoDevice->PciIo;
+  Status = PciIo->Pci.Read (
+          PciIo,
+          EfiPciIoWidthUint8,
+          PciIoDevice->ResizableBarOffset + sizeof (PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER),
+          sizeof (PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY) * ResizableBarNumber,
+          (VOID *)(&Entries)
+          );
+  ASSERT_EFI_ERROR (Status);
+
+  for (Index = 0; Index < ResizableBarNumber; Index++) {
+
+    //
+    // When the bit of Capabilities Set, indicates that the Function supports
+    // operating with the BAR sized to (2^Bit) MB.
+    // Example:
+    // Bit 0 is set: supports operating with the BAR sized to 1 MB
+    // Bit 1 is set: supports operating with the BAR sized to 2 MB
+    // Bit n is set: supports operating with the BAR sized to (2^n) MB
+    //
+    Capabilities = LShiftU64(Entries[Index].ResizableBarControl.Bits.BarSizeCapability, 28)
+                  | Entries[Index].ResizableBarCapability.Bits.BarSizeCapability;
+
+    if (ResizableBarOp == PciResizableBarMax) {
+      Bit = HighBitSet64(Capabilities);
+    } else if (ResizableBarOp == PciResizableBarMin) {
+      Bit = LowBitSet64(Capabilities);
+    } else {
+      ASSERT ((ResizableBarOp == PciResizableBarMax) || (ResizableBarOp == PciResizableBarMin));
+    }
+
+    ASSERT (Bit >= 0);
+
+    Offset = PciIoDevice->ResizableBarOffset + sizeof (PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER)
+            + Index * sizeof (PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY)
+            + OFFSET_OF (PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY, ResizableBarControl);
+
+    Entries[Index].ResizableBarControl.Bits.BarSize = (UINT32) Bit;
+    DEBUG ((
+      DEBUG_INFO,
+      "   Resizable Bar: Offset = 0x%x, Bar Size Capability = 0x%016lx, New Bar Size = 0x%lx\n",
+      OFFSET_OF (PCI_TYPE00, Device.Bar[Entries[Index].ResizableBarControl.Bits.BarIndex]),
+      Capabilities, LShiftU64 (SIZE_1MB, Bit)
+      ));
+    PciIo->Pci.Write (
+            PciIo,
+            EfiPciIoWidthUint32,
+            Offset,
+            1,
+            &Entries[Index].ResizableBarControl.Uint32
+            );
+  }
+
+  return EFI_SUCCESS;
+}
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h
index 10b435d146..aeec6d6b6d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h
@@ -1,7 +1,7 @@
 /** @file
   Internal library declaration for PCI Bus module.
 
-Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -24,6 +24,10 @@ typedef struct {
   UINT8                              *AllocRes;
 } EFI_RESOURCE_ALLOC_FAILURE_ERROR_DATA_PAYLOAD;
 
+typedef enum {
+  PciResizableBarMin = 0x00,
+  PciResizableBarMax = 0xFF
+} PCI_RESIZABLE_BAR_OPERATION;
 
 /**
   Retrieve the PCI Card device BAR information via PciIo interface.
@@ -156,4 +160,20 @@ PciHostBridgeEnumerator (
   IN EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL  *PciResAlloc
   );
 
+/**
+  This function is used to program the Resizable BAR Register.
+
+  @param PciIoDevice            A pointer to the PCI_IO_DEVICE.
+  @param ResizableBarOp         PciResizableBarMax: Set BAR to max size
+                                PciResizableBarMin: set BAR to min size.
+
+  @retval EFI_SUCCESS           Successfully enumerated the host bridge.
+  @retval other                 Some error occurred when enumerating the host bridge.
+
+**/
+EFI_STATUS
+PciProgramResizableBar (
+  IN PCI_IO_DEVICE                *PciIoDevice,
+  IN PCI_RESIZABLE_BAR_OPERATION  ResizableBarOp
+  );
 #endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 9b52b34494..9173fdef83 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -4,7 +4,7 @@
 # and libraries instances, which are used for those modules.
 #
 # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
-# Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
 # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development LP<BR>
 # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
@@ -2043,6 +2043,12 @@
   # @Prompt Enable StatusCode via memory.
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE|BOOLEAN|0x00010023
 
+  ## Indicates if the PCIe Resizable BAR Capability Supported.<BR><BR>
+  #   TRUE  - PCIe Resizable BAR Capability is supported.<BR>
+  #   FALSE - PCIe Resizable BAR Capability is not supported.<BR>
+  # @Prompt Enable PCIe Resizable BAR Capability support.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPcieResizableBarSupport|TRUE|BOOLEAN|0x10000024
+
 [PcdsPatchableInModule]
   ## Specify memory size with page number for PEI code when
   #  Loading Module at Fixed Address feature is enabled.
-- 
2.24.0.windows.2


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

* Re: [Patch V3 1/2] MdePkg: Define structures for Resizable BAR Capability
  2021-01-04  6:59 [Patch V3 1/2] MdePkg: Define structures for Resizable BAR Capability Heng Luo
  2021-01-04  6:59 ` [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe " Heng Luo
@ 2021-01-04  7:52 ` Ni, Ray
  1 sibling, 0 replies; 15+ messages in thread
From: Ni, Ray @ 2021-01-04  7:52 UTC (permalink / raw)
  To: Luo, Heng, devel@edk2.groups.io; +Cc: Wu, Hao A

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Luo, Heng <heng.luo@intel.com>
> Sent: Monday, January 4, 2021 3:00 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [Patch V3 1/2] MdePkg: Define structures for Resizable BAR Capability
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3138
> 
> Define structures for Resizable BAR Capability in
> MdePkg/Include/IndustryStandard/PciExpress21.h,
> Change ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> to use new structures.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Heng Luo <heng.luo@intel.com>
> ---
>  MdePkg/Include/IndustryStandard/PciExpress21.h    | 30
> +++++++++++++++++++++++++-----
>  ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c |  6 +++---
>  2 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/MdePkg/Include/IndustryStandard/PciExpress21.h
> b/MdePkg/Include/IndustryStandard/PciExpress21.h
> index 2c07cb560e..4617dc1569 100644
> --- a/MdePkg/Include/IndustryStandard/PciExpress21.h
> +++ b/MdePkg/Include/IndustryStandard/PciExpress21.h
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    Support for the latest PCI standard.
> 
> 
> 
> -  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> 
> +  Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>    (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
> @@ -632,10 +632,30 @@ typedef struct {
>  #define PCI_EXPRESS_EXTENDED_CAPABILITY_RESIZABLE_BAR_ID    0x0015
> 
>  #define PCI_EXPRESS_EXTENDED_CAPABILITY_RESIZABLE_BAR_VER1  0x1
> 
> 
> 
> +typedef union {
> 
> +  struct {
> 
> +    UINT32 Reserved:4;
> 
> +    UINT32 BarSizeCapability:28;
> 
> +  } Bits;
> 
> +  UINT32   Uint32;
> 
> +} PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CAPABILITY;
> 
> +
> 
> +
> 
> +typedef union {
> 
> +  struct {
> 
> +    UINT32 BarIndex:3;
> 
> +    UINT32 Reserved:2;
> 
> +    UINT32 ResizableBarNumber:3;
> 
> +    UINT32 BarSize:6;
> 
> +    UINT32 Reserved2:2;
> 
> +    UINT32 BarSizeCapability:16;
> 
> +  } Bits;
> 
> +  UINT32   Uint32;
> 
> +} PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CONTROL;
> 
> +
> 
>  typedef struct {
> 
> -  UINT32                                                 ResizableBarCapability;
> 
> -  UINT16                                                 ResizableBarControl;
> 
> -  UINT16                                                 Reserved;
> 
> +  PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CAPABILITY
> ResizableBarCapability;
> 
> +  PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CONTROL
> ResizableBarControl;
> 
>  } PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY;
> 
> 
> 
>  typedef struct {
> 
> @@ -643,7 +663,7 @@ typedef struct {
>    PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY
> Capability[1];
> 
>  } PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR;
> 
> 
> 
> -#define GET_NUMBER_RESIZABLE_BARS(x) (((x-
> >Capability[0].ResizableBarControl) & 0xE0) >> 5)
> 
> +#define GET_NUMBER_RESIZABLE_BARS(x) (x-
> >Capability[0].ResizableBarControl.Bits.ResizableBarNumber)
> 
> 
> 
>  #define PCI_EXPRESS_EXTENDED_CAPABILITY_ARI_CAPABILITY_ID    0x000E
> 
>  #define PCI_EXPRESS_EXTENDED_CAPABILITY_ARI_CAPABILITY_VER1  0x1
> 
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> index 3e138188ce..a2f04d8db5 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    Main file for Pci shell Debug1 function.
> 
> 
> 
> -  Copyright (c) 2005 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +  Copyright (c) 2005 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>    (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.<BR>
> 
>    (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -5534,8 +5534,8 @@ PrintInterpretedExtendedCompatibilityResizeableBar
> (
>        STRING_TOKEN (STR_PCI_EXT_CAP_RESIZE_BAR),
> 
>        gShellDebug1HiiHandle,
> 
>        ItemCount+1,
> 
> -      Header->Capability[ItemCount].ResizableBarCapability,
> 
> -      Header->Capability[ItemCount].ResizableBarControl
> 
> +      Header->Capability[ItemCount].ResizableBarCapability.Uint32,
> 
> +      Header->Capability[ItemCount].ResizableBarControl.Uint32
> 
>        );
> 
>    }
> 
> 
> 
> --
> 2.24.0.windows.2


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

* Re: [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-04  6:59 ` [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe " Heng Luo
@ 2021-01-04  7:52   ` Ni, Ray
  2021-01-11 19:38   ` [edk2-devel] " Laszlo Ersek
  1 sibling, 0 replies; 15+ messages in thread
From: Ni, Ray @ 2021-01-04  7:52 UTC (permalink / raw)
  To: Luo, Heng, devel@edk2.groups.io; +Cc: Wu, Hao A

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Luo, Heng <heng.luo@intel.com>
> Sent: Monday, January 4, 2021 3:00 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe
> Resizable BAR Capability
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=313
> 
> Add PcdPcieResizableBarSupport to enable/disable PCIe Resizable
> BAR Capability fearture.
> Program the Resizable BAR Register if the device suports PCIe Resizable
> BAR Capability and PcdPcieResizableBarSupport is TRUE.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Heng Luo <heng.luo@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h               |   4 +++-
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf          |   3 ++-
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c |  27
> ++++++++++++++++++++++++++-
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h |  12
> +++++++++++-
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c               | 185
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++--------------
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h               |  22
> +++++++++++++++++++++-
>  MdeModulePkg/MdeModulePkg.dec                         |   8 +++++++-
>  7 files changed, 241 insertions(+), 20 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> index d4113993c8..a619a68526 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    Header files and data structures needed by PCI Bus module.
> 
> 
> 
> -Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -280,6 +280,8 @@ struct _PCI_IO_DEVICE {
>    // This field is used to support this case.
> 
>    //
> 
>    UINT16                                    BridgeIoAlignment;
> 
> +  UINT32                                    ResizableBarOffset;
> 
> +  UINT32                                    ResizableBarNumber;
> 
>  };
> 
> 
> 
>  #define PCI_IO_DEVICE_FROM_PCI_IO_THIS(a) \
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> index 9284998f36..e317169d9c 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> @@ -2,7 +2,7 @@
>  #  The PCI bus driver will probe all PCI devices and allocate MMIO and IO
> space for these devices.
> 
>  #  Please use PCD feature flag PcdPciBusHotplugDeviceSupport to enable hot
> plug supporting.
> 
>  #
> 
> -#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +#  Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>  #
> 
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #
> 
> @@ -106,6 +106,7 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport                  ## CONSUMES
> 
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport                ##
> CONSUMES
> 
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration    ##
> SOMETIMES_CONSUMES
> 
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPcieResizableBarSupport     ##
> CONSUMES
> 
> 
> 
>  [UserExtensions.TianoCore."ExtraFiles"]
> 
>    PciBusDxeExtra.uni
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> index 6c68a97d4e..1b64924b7b 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    PCI emumeration support functions implementation for PCI Bus module.
> 
> 
> 
> -Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>  (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
> @@ -2426,6 +2426,31 @@ CreatePciIoDevice (
>      }
> 
>    }
> 
> 
> 
> +  PciIoDevice->ResizableBarOffset = 0;
> 
> +  if (PcdGetBool (PcdPcieResizableBarSupport)) {
> 
> +    Status = LocatePciExpressCapabilityRegBlock (
> 
> +               PciIoDevice,
> 
> +               PCI_EXPRESS_EXTENDED_CAPABILITY_RESIZABLE_BAR_ID,
> 
> +               &PciIoDevice->ResizableBarOffset,
> 
> +               NULL
> 
> +               );
> 
> +    if (!EFI_ERROR (Status)) {
> 
> +      PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CONTROL
> ResizableBarControl;
> 
> +      UINT32                                                  Offset;
> 
> +      Offset = PciIoDevice->ResizableBarOffset + sizeof
> (PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER)
> 
> +                + sizeof
> (PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CAPABILITY),
> 
> +      PciIo->Pci.Read (
> 
> +              PciIo,
> 
> +              EfiPciIoWidthUint8,
> 
> +              Offset,
> 
> +              sizeof
> (PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_CONTROL),
> 
> +              &ResizableBarControl
> 
> +              );
> 
> +      PciIoDevice->ResizableBarNumber =
> ResizableBarControl.Bits.ResizableBarNumber;
> 
> +      PciProgramResizableBar (PciIoDevice, PciResizableBarMax);
> 
> +    }
> 
> +  }
> 
> +
> 
>    //
> 
>    // Initialize the reserved resource list
> 
>    //
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h
> index d76606c7df..4581b270c9 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    PCI enumeration support functions declaration for PCI Bus module.
> 
> 
> 
> -Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -467,4 +467,14 @@ DumpPpbPaddingResource (
>    IN PCI_BAR_TYPE                     ResourceType
> 
>    );
> 
> 
> 
> +/**
> 
> +  Dump the PCI BAR information.
> 
> +
> 
> +  @param PciIoDevice     PCI IO instance.
> 
> +**/
> 
> +VOID
> 
> +DumpPciBars (
> 
> +  IN PCI_IO_DEVICE                    *PciIoDevice
> 
> +  );
> 
> +
> 
>  #endif
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> index 72690ab647..6bba283671 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    Internal library implementation for PCI Bus module.
> 
> 
> 
> -Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>  (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
> @@ -377,6 +377,60 @@ DumpResourceMap (
>    }
> 
>  }
> 
> 
> 
> +/**
> 
> +  Adjust the Devices' BAR size to minimum value if it support Resizeable BAR
> capability.
> 
> +
> 
> +  @param RootBridgeDev  Pointer to instance of PCI_IO_DEVICE..
> 
> +
> 
> +  @return TRUE if BAR size is adjusted.
> 
> +
> 
> +**/
> 
> +BOOLEAN
> 
> +AdjustPciDeviceBarSize (
> 
> +  IN PCI_IO_DEVICE *RootBridgeDev
> 
> +  )
> 
> +{
> 
> +  PCI_IO_DEVICE     *PciIoDevice;
> 
> +  LIST_ENTRY        *CurrentLink;
> 
> +  BOOLEAN           Adjusted;
> 
> +  UINTN             Offset;
> 
> +  UINTN             BarIndex;
> 
> +
> 
> +  Adjusted    = FALSE;
> 
> +  CurrentLink = RootBridgeDev->ChildList.ForwardLink;
> 
> +
> 
> +  while (CurrentLink != NULL && CurrentLink != &RootBridgeDev->ChildList) {
> 
> +    PciIoDevice = PCI_IO_DEVICE_FROM_LINK (CurrentLink);
> 
> +
> 
> +    if (IS_PCI_BRIDGE (&PciIoDevice->Pci)) {
> 
> +      if (AdjustPciDeviceBarSize (PciIoDevice)) {
> 
> +        Adjusted = TRUE;
> 
> +      }
> 
> +    } else {
> 
> +      if (PciIoDevice->ResizableBarOffset != 0) {
> 
> +        DEBUG ((
> 
> +          DEBUG_ERROR,
> 
> +          "PciBus: [%02x|%02x|%02x] Adjust Pci Device Bar Size\n",
> 
> +          PciIoDevice->BusNumber, PciIoDevice->DeviceNumber, PciIoDevice-
> >FunctionNumber
> 
> +          ));
> 
> +        PciProgramResizableBar (PciIoDevice, PciResizableBarMin);
> 
> +        //
> 
> +        // Start to parse the bars
> 
> +        //
> 
> +        for (Offset = 0x10, BarIndex = 0; Offset <= 0x24 && BarIndex <
> PCI_MAX_BAR; BarIndex++) {
> 
> +          Offset = PciParseBar (PciIoDevice, Offset, BarIndex);
> 
> +        }
> 
> +        Adjusted = TRUE;
> 
> +        DEBUG_CODE (DumpPciBars (PciIoDevice););
> 
> +      }
> 
> +    }
> 
> +
> 
> +    CurrentLink = CurrentLink->ForwardLink;
> 
> +  }
> 
> +
> 
> +  return Adjusted;
> 
> +}
> 
> +
> 
>  /**
> 
>    Submits the I/O and memory resource requirements for the specified PCI
> Host Bridge.
> 
> 
> 
> @@ -422,6 +476,10 @@ PciHostBridgeResourceAllocator (
>    PCI_RESOURCE_NODE                              PMem64Pool;
> 
>    EFI_DEVICE_HANDLE_EXTENDED_DATA_PAYLOAD        HandleExtendedData;
> 
>    EFI_RESOURCE_ALLOC_FAILURE_ERROR_DATA_PAYLOAD
> AllocFailExtendedData;
> 
> +  BOOLEAN                                        ResizableBarNeedAdjust;
> 
> +  BOOLEAN                                        ResizableBarAdjusted;
> 
> +
> 
> +  ResizableBarNeedAdjust = PcdGetBool (PcdPcieResizableBarSupport);
> 
> 
> 
>    //
> 
>    // It may try several times if the resource allocation fails
> 
> @@ -703,19 +761,30 @@ PciHostBridgeResourceAllocator (
>              sizeof (AllocFailExtendedData)
> 
>              );
> 
> 
> 
> -      Status = PciHostBridgeAdjustAllocation (
> 
> -                 &IoPool,
> 
> -                 &Mem32Pool,
> 
> -                 &PMem32Pool,
> 
> -                 &Mem64Pool,
> 
> -                 &PMem64Pool,
> 
> -                 IoResStatus,
> 
> -                 Mem32ResStatus,
> 
> -                 PMem32ResStatus,
> 
> -                 Mem64ResStatus,
> 
> -                 PMem64ResStatus
> 
> -                 );
> 
> -
> 
> +     //
> 
> +     // When resource conflict happens, adjust the BAR size first.
> 
> +     // Only when adjusting BAR size doesn't help or BAR size cannot be
> adjusted,
> 
> +     // reject the device who requests largest resource that causes conflict.
> 
> +     //
> 
> +      ResizableBarAdjusted = FALSE;
> 
> +      if (ResizableBarNeedAdjust) {
> 
> +        ResizableBarAdjusted = AdjustPciDeviceBarSize (RootBridgeDev);
> 
> +        ResizableBarNeedAdjust = FALSE;
> 
> +      }
> 
> +      if (!ResizableBarAdjusted) {
> 
> +        Status = PciHostBridgeAdjustAllocation (
> 
> +                  &IoPool,
> 
> +                  &Mem32Pool,
> 
> +                  &PMem32Pool,
> 
> +                  &Mem64Pool,
> 
> +                  &PMem64Pool,
> 
> +                  IoResStatus,
> 
> +                  Mem32ResStatus,
> 
> +                  PMem32ResStatus,
> 
> +                  Mem64ResStatus,
> 
> +                  PMem64ResStatus
> 
> +                  );
> 
> +      }
> 
>        //
> 
>        // Destroy all the resource tree
> 
>        //
> 
> @@ -1651,3 +1720,91 @@ PciHostBridgeEnumerator (
> 
> 
>    return EFI_SUCCESS;
> 
>  }
> 
> +
> 
> +/**
> 
> +  This function is used to program the Resizable BAR Register.
> 
> +
> 
> +  @param PciIoDevice            A pointer to the PCI_IO_DEVICE.
> 
> +  @param ResizableBarOp         PciResizableBarMax: Set BAR to max size
> 
> +                                PciResizableBarMin: set BAR to min size.
> 
> +
> 
> +  @retval EFI_SUCCESS           Successfully enumerated the host bridge.
> 
> +  @retval other                 Some error occurred when enumerating the host
> bridge.
> 
> +
> 
> +**/
> 
> +EFI_STATUS
> 
> +PciProgramResizableBar (
> 
> +  IN PCI_IO_DEVICE                *PciIoDevice,
> 
> +  IN PCI_RESIZABLE_BAR_OPERATION  ResizableBarOp
> 
> +  )
> 
> +{
> 
> +  EFI_PCI_IO_PROTOCOL  *PciIo;
> 
> +  UINT64                Capabilities;
> 
> +  UINT32                Index;
> 
> +  UINT32                Offset;
> 
> +  INTN                  Bit;
> 
> +  UINTN                 ResizableBarNumber;
> 
> +  EFI_STATUS            Status;
> 
> +  PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY
> Entries[PCI_MAX_BAR];
> 
> +
> 
> +  ASSERT (PciIoDevice->ResizableBarOffset != 0);
> 
> +
> 
> +  DEBUG ((DEBUG_INFO, "   Programs Resizable BAR register, offset: 0x%08x,
> number: %d\n",
> 
> +        PciIoDevice->ResizableBarOffset, PciIoDevice->ResizableBarNumber));
> 
> +
> 
> +  ResizableBarNumber = MIN (PciIoDevice->ResizableBarNumber,
> PCI_MAX_BAR);
> 
> +  PciIo = &PciIoDevice->PciIo;
> 
> +  Status = PciIo->Pci.Read (
> 
> +          PciIo,
> 
> +          EfiPciIoWidthUint8,
> 
> +          PciIoDevice->ResizableBarOffset + sizeof
> (PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER),
> 
> +          sizeof (PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY)
> * ResizableBarNumber,
> 
> +          (VOID *)(&Entries)
> 
> +          );
> 
> +  ASSERT_EFI_ERROR (Status);
> 
> +
> 
> +  for (Index = 0; Index < ResizableBarNumber; Index++) {
> 
> +
> 
> +    //
> 
> +    // When the bit of Capabilities Set, indicates that the Function supports
> 
> +    // operating with the BAR sized to (2^Bit) MB.
> 
> +    // Example:
> 
> +    // Bit 0 is set: supports operating with the BAR sized to 1 MB
> 
> +    // Bit 1 is set: supports operating with the BAR sized to 2 MB
> 
> +    // Bit n is set: supports operating with the BAR sized to (2^n) MB
> 
> +    //
> 
> +    Capabilities =
> LShiftU64(Entries[Index].ResizableBarControl.Bits.BarSizeCapability, 28)
> 
> +                  | Entries[Index].ResizableBarCapability.Bits.BarSizeCapability;
> 
> +
> 
> +    if (ResizableBarOp == PciResizableBarMax) {
> 
> +      Bit = HighBitSet64(Capabilities);
> 
> +    } else if (ResizableBarOp == PciResizableBarMin) {
> 
> +      Bit = LowBitSet64(Capabilities);
> 
> +    } else {
> 
> +      ASSERT ((ResizableBarOp == PciResizableBarMax) || (ResizableBarOp ==
> PciResizableBarMin));
> 
> +    }
> 
> +
> 
> +    ASSERT (Bit >= 0);
> 
> +
> 
> +    Offset = PciIoDevice->ResizableBarOffset + sizeof
> (PCI_EXPRESS_EXTENDED_CAPABILITIES_HEADER)
> 
> +            + Index * sizeof
> (PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY)
> 
> +            + OFFSET_OF
> (PCI_EXPRESS_EXTENDED_CAPABILITIES_RESIZABLE_BAR_ENTRY,
> ResizableBarControl);
> 
> +
> 
> +    Entries[Index].ResizableBarControl.Bits.BarSize = (UINT32) Bit;
> 
> +    DEBUG ((
> 
> +      DEBUG_INFO,
> 
> +      "   Resizable Bar: Offset = 0x%x, Bar Size Capability = 0x%016lx, New Bar
> Size = 0x%lx\n",
> 
> +      OFFSET_OF (PCI_TYPE00,
> Device.Bar[Entries[Index].ResizableBarControl.Bits.BarIndex]),
> 
> +      Capabilities, LShiftU64 (SIZE_1MB, Bit)
> 
> +      ));
> 
> +    PciIo->Pci.Write (
> 
> +            PciIo,
> 
> +            EfiPciIoWidthUint32,
> 
> +            Offset,
> 
> +            1,
> 
> +            &Entries[Index].ResizableBarControl.Uint32
> 
> +            );
> 
> +  }
> 
> +
> 
> +  return EFI_SUCCESS;
> 
> +}
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h
> index 10b435d146..aeec6d6b6d 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    Internal library declaration for PCI Bus module.
> 
> 
> 
> -Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -24,6 +24,10 @@ typedef struct {
>    UINT8                              *AllocRes;
> 
>  } EFI_RESOURCE_ALLOC_FAILURE_ERROR_DATA_PAYLOAD;
> 
> 
> 
> +typedef enum {
> 
> +  PciResizableBarMin = 0x00,
> 
> +  PciResizableBarMax = 0xFF
> 
> +} PCI_RESIZABLE_BAR_OPERATION;
> 
> 
> 
>  /**
> 
>    Retrieve the PCI Card device BAR information via PciIo interface.
> 
> @@ -156,4 +160,20 @@ PciHostBridgeEnumerator (
>    IN EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL
> *PciResAlloc
> 
>    );
> 
> 
> 
> +/**
> 
> +  This function is used to program the Resizable BAR Register.
> 
> +
> 
> +  @param PciIoDevice            A pointer to the PCI_IO_DEVICE.
> 
> +  @param ResizableBarOp         PciResizableBarMax: Set BAR to max size
> 
> +                                PciResizableBarMin: set BAR to min size.
> 
> +
> 
> +  @retval EFI_SUCCESS           Successfully enumerated the host bridge.
> 
> +  @retval other                 Some error occurred when enumerating the host
> bridge.
> 
> +
> 
> +**/
> 
> +EFI_STATUS
> 
> +PciProgramResizableBar (
> 
> +  IN PCI_IO_DEVICE                *PciIoDevice,
> 
> +  IN PCI_RESIZABLE_BAR_OPERATION  ResizableBarOp
> 
> +  );
> 
>  #endif
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index 9b52b34494..9173fdef83 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -4,7 +4,7 @@
>  # and libraries instances, which are used for those modules.
> 
>  #
> 
>  # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> 
> -# Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.<BR>
> 
> +# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>  # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> 
>  # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development LP<BR>
> 
>  # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> 
> @@ -2043,6 +2043,12 @@
>    # @Prompt Enable StatusCode via memory.
> 
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE|BOOLE
> AN|0x00010023
> 
> 
> 
> +  ## Indicates if the PCIe Resizable BAR Capability Supported.<BR><BR>
> 
> +  #   TRUE  - PCIe Resizable BAR Capability is supported.<BR>
> 
> +  #   FALSE - PCIe Resizable BAR Capability is not supported.<BR>
> 
> +  # @Prompt Enable PCIe Resizable BAR Capability support.
> 
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdPcieResizableBarSupport|TRUE|BOOLE
> AN|0x10000024
> 
> +
> 
>  [PcdsPatchableInModule]
> 
>    ## Specify memory size with page number for PEI code when
> 
>    #  Loading Module at Fixed Address feature is enabled.
> 
> --
> 2.24.0.windows.2


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

* Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-04  6:59 ` [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe " Heng Luo
  2021-01-04  7:52   ` Ni, Ray
@ 2021-01-11 19:38   ` Laszlo Ersek
  2021-01-11 19:44     ` Laszlo Ersek
  2021-01-12  2:28     ` Ni, Ray
  1 sibling, 2 replies; 15+ messages in thread
From: Laszlo Ersek @ 2021-01-11 19:38 UTC (permalink / raw)
  To: devel, heng.luo; +Cc: Ray Ni, Hao A Wu

Hi,

On 01/04/21 07:59, Heng Luo wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=313
>
> Add PcdPcieResizableBarSupport to enable/disable PCIe Resizable
> BAR Capability fearture.
> Program the Resizable BAR Register if the device suports PCIe
> Resizable BAR Capability and PcdPcieResizableBarSupport is TRUE.
>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Heng Luo <heng.luo@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h               |   4 +++-
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf          |   3 ++-
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c |  27 ++++++++++++++++++++++++++-
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h |  12 +++++++++++-
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c               | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.h               |  22 +++++++++++++++++++++-
>  MdeModulePkg/MdeModulePkg.dec                         |   8 +++++++-
>  7 files changed, 241 insertions(+), 20 deletions(-)

I have brought this feature to the attention of our PCI experts.

First of all, let me say that the bugzilla ticket (3138), and the commit
message on this patch, are utterly useless. (To add insult to injury,
the bugzilla reference in the commit message even has a typo -- it
misses the last digit "8".)

So, given the lack of information in the bugzilla and the commit
message, I could only attempt to decipher the goal / use case myself.
This was my interpretation:

> It seems like the max BAR size is selected first, but if there's a
> "resource conflict" (running out of a particular resource type
> aperture), then the minimum BAR size is selected. I don't know what
> set of devices and/or resizable BARs this logic applies to, if there
> are multiple of them.

*IF* my interpretation of the code is correct -- which is admittedly a
"big if" --, then the logic seems misguided. This is the feedback I got
internally:

> Per the PCIe specification (revision 5.0, version 0.9) 7.8.6:
>
>   Software determines, through a proprietary mechanism, what the
>   optimal size is for the resource, and programs that size via the BAR
>   Size field of the Resizable BAR Control register.
>
> Furthermore, Table 7-114 defines the Bar Size field of the control
> register stating:
>
>   The default value of this field is equal to the default size of the
>   address space that the BAR resource is requesting via the BAR's
>   read-only bits.
>
> Therefore the maximum size is not necessarily optimal, nor should the
> minimum size be considered the default.  In fact, [we] tested various
> handoff BAR sizes for [a particular] GPU and found that Windows didn't
> like the maximum BAR size.
>
> Elsewhere in the discussion [1] the AMD author of the kernel support
> for resizeable BARs indicates that FPGA devices might implement the
> REBAR capability as part of their standard PCI wrapper ([our]
> interpretation), but the BAR usage would be determined by the actual
> bitstream written to the device, therefore there might be a full
> bitmask for the BAR sizes supported by the device.
>
> [1] https://lists.freedesktop.org/archives/dri-devel/2021-January/thread.html
>
> It would certainly make sense for the firmware to take REBAR
> capabilities into account when sizing bridge apertures, but to
> generically enable extended BAR sizes would make lots of assumptions
> about the device usage and compatibility.
>
> [...] At least for GPUs the expectation would be a default, smaller
> compatibility size expanding to some representation that allows direct
> DMA to the entire memory of the card.

So this patch should either be reverted; or minimally, the default value
of "PcdPcieResizableBarSupport" should be set to FALSE, as the policy
for BAR sizing doesn't look robust or portable.


General request for the future: if you implement some kind of policy in
core edk2, please at least *document* the policy somewhere. It's
unacceptable to have to decipher the source code for such a possibly
impactful change in the core. There is no need for a wiki page or an
RFC, but a sane bugzilla ticket and a sane commit message are required.

(The documentation of the PCD in the "MdeModulePkg.dec" file is
unsatisfactory too, and the UNI file has not been updated at all.)

Thanks
Laszlo


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

* Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-11 19:38   ` [edk2-devel] " Laszlo Ersek
@ 2021-01-11 19:44     ` Laszlo Ersek
  2021-01-12  2:28     ` Ni, Ray
  1 sibling, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2021-01-11 19:44 UTC (permalink / raw)
  To: devel, heng.luo; +Cc: Ray Ni, Hao A Wu

Hi Heng,

On 01/11/21 20:38, Laszlo Ersek wrote:

> General request for the future: if you implement some kind of policy in
> core edk2, please at least *document* the policy somewhere. It's
> unacceptable to have to decipher the source code for such a possibly
> impactful change in the core. There is no need for a wiki page or an
> RFC, but a sane bugzilla ticket and a sane commit message are required.
> 
> (The documentation of the PCD in the "MdeModulePkg.dec" file is
> unsatisfactory too, and the UNI file has not been updated at all.)

Wow, I've even found the following regression in the bug tracker:

  https://bugzilla.tianocore.org/show_bug.cgi?id=3158

Your approach in

  https://bugzilla.tianocore.org/show_bug.cgi?id=3158#c1

is entirely wrong. It is not the burden of platform owners to fix up the
regression you have introduced. It is your responsibility to *not break*
existent use cases. You can offer the feature, and platforms that want
the feature may enable it (or you can enable it by default if you can
prove or test that no platform breaks in response).

Please revert this patch immediately, or flip the PCD default to FALSE
immediately.

Laszlo


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

* Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-11 19:38   ` [edk2-devel] " Laszlo Ersek
  2021-01-11 19:44     ` Laszlo Ersek
@ 2021-01-12  2:28     ` Ni, Ray
  2021-01-12  5:25       ` Heng Luo
  2021-01-13  5:59       ` Wu, Hao A
  1 sibling, 2 replies; 15+ messages in thread
From: Ni, Ray @ 2021-01-12  2:28 UTC (permalink / raw)
  To: Laszlo Ersek, devel@edk2.groups.io, Luo, Heng
  Cc: Wu, Hao A, Kinney, Michael D

> > It seems like the max BAR size is selected first, but if there's a
> > "resource conflict" (running out of a particular resource type
> > aperture), then the minimum BAR size is selected. I don't know what
> > set of devices and/or resizable BARs this logic applies to, if there
> > are multiple of them. 

> > Per the PCIe specification (revision 5.0, version 0.9) 7.8.6:
> >
> >   Software determines, through a proprietary mechanism, what the
> >   optimal size is for the resource, and programs that size via the BAR
> >   Size field of the Resizable BAR Control register.
> >
> > Furthermore, Table 7-114 defines the Bar Size field of the control
> > register stating:
> >
> >   The default value of this field is equal to the default size of the
> >   address space that the BAR resource is requesting via the BAR's
> >   read-only bits.
> >
> > Therefore the maximum size is not necessarily optimal, nor should the
> > minimum size be considered the default.  In fact, [we] tested various
> > handoff BAR sizes for [a particular] GPU and found that Windows didn't
> > like the maximum BAR size.
> >
> > Elsewhere in the discussion [1] the AMD author of the kernel support
> > for resizeable BARs indicates that FPGA devices might implement the
> > REBAR capability as part of their standard PCI wrapper ([our]
> > interpretation), but the BAR usage would be determined by the actual
> > bitstream written to the device, therefore there might be a full
> > bitmask for the BAR sizes supported by the device.
> >
> > [1] https://lists.freedesktop.org/archives/dri-devel/2021-January/thread.html
> >
> > It would certainly make sense for the firmware to take REBAR
> > capabilities into account when sizing bridge apertures, but to
> > generically enable extended BAR sizes would make lots of assumptions
> > about the device usage and compatibility.
> >
> > [...] At least for GPUs the expectation would be a default, smaller
> > compatibility size expanding to some representation that allows direct
> > DMA to the entire memory of the card.
> 
> So this patch should either be reverted; or minimally, the default value
> of "PcdPcieResizableBarSupport" should be set to FALSE, as the policy
> for BAR sizing doesn't look robust or portable.
> 
> 
> General request for the future: if you implement some kind of policy in
> core edk2, please at least *document* the policy somewhere. It's
> unacceptable to have to decipher the source code for such a possibly
> impactful change in the core. There is no need for a wiki page or an
> RFC, but a sane bugzilla ticket and a sane commit message are required.
> 
> (The documentation of the PCD in the "MdeModulePkg.dec" file is
> unsatisfactory too, and the UNI file has not been updated at all.)
> 



Your understanding is correct. Original idea is to let platform supply the policy about
what the optimal BAR size is for each resizable BAR.
The current implementation is a try to avoid asking platform code for such policy because
we thought it's a burden for platform to supply the policy data.

I agree that we set the PCD default value as disabled and after a period of study, we will
understand whether a platform policy is really needed.

Thanks,
Ray

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

* Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-12  2:28     ` Ni, Ray
@ 2021-01-12  5:25       ` Heng Luo
  2021-01-13  5:59       ` Wu, Hao A
  1 sibling, 0 replies; 15+ messages in thread
From: Heng Luo @ 2021-01-12  5:25 UTC (permalink / raw)
  To: Ni, Ray, Laszlo Ersek, devel@edk2.groups.io; +Cc: Wu, Hao A, Kinney, Michael D

Thank Ray and Laszlo! I have sent 2 patches for this, please help to review.
https://edk2.groups.io/g/devel/message/70139
https://edk2.groups.io/g/devel/message/70140

Thanks,
Heng 

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, January 12, 2021 10:28 AM
> To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Luo, Heng
> <heng.luo@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe:
> Support PCIe Resizable BAR Capability
> 
> > > It seems like the max BAR size is selected first, but if there's a
> > > "resource conflict" (running out of a particular resource type
> > > aperture), then the minimum BAR size is selected. I don't know what
> > > set of devices and/or resizable BARs this logic applies to, if there
> > > are multiple of them.
> 
> > > Per the PCIe specification (revision 5.0, version 0.9) 7.8.6:
> > >
> > >   Software determines, through a proprietary mechanism, what the
> > >   optimal size is for the resource, and programs that size via the BAR
> > >   Size field of the Resizable BAR Control register.
> > >
> > > Furthermore, Table 7-114 defines the Bar Size field of the control
> > > register stating:
> > >
> > >   The default value of this field is equal to the default size of the
> > >   address space that the BAR resource is requesting via the BAR's
> > >   read-only bits.
> > >
> > > Therefore the maximum size is not necessarily optimal, nor should
> > > the minimum size be considered the default.  In fact, [we] tested
> > > various handoff BAR sizes for [a particular] GPU and found that
> > > Windows didn't like the maximum BAR size.
> > >
> > > Elsewhere in the discussion [1] the AMD author of the kernel support
> > > for resizeable BARs indicates that FPGA devices might implement the
> > > REBAR capability as part of their standard PCI wrapper ([our]
> > > interpretation), but the BAR usage would be determined by the actual
> > > bitstream written to the device, therefore there might be a full
> > > bitmask for the BAR sizes supported by the device.
> > >
> > > [1]
> > > https://lists.freedesktop.org/archives/dri-devel/2021-January/thread
> > > .html
> > >
> > > It would certainly make sense for the firmware to take REBAR
> > > capabilities into account when sizing bridge apertures, but to
> > > generically enable extended BAR sizes would make lots of assumptions
> > > about the device usage and compatibility.
> > >
> > > [...] At least for GPUs the expectation would be a default, smaller
> > > compatibility size expanding to some representation that allows
> > > direct DMA to the entire memory of the card.
> >
> > So this patch should either be reverted; or minimally, the default
> > value of "PcdPcieResizableBarSupport" should be set to FALSE, as the
> > policy for BAR sizing doesn't look robust or portable.
> >
> >
> > General request for the future: if you implement some kind of policy
> > in core edk2, please at least *document* the policy somewhere. It's
> > unacceptable to have to decipher the source code for such a possibly
> > impactful change in the core. There is no need for a wiki page or an
> > RFC, but a sane bugzilla ticket and a sane commit message are required.
> >
> > (The documentation of the PCD in the "MdeModulePkg.dec" file is
> > unsatisfactory too, and the UNI file has not been updated at all.)
> >
> 
> 
> 
> Your understanding is correct. Original idea is to let platform supply the
> policy about what the optimal BAR size is for each resizable BAR.
> The current implementation is a try to avoid asking platform code for such
> policy because we thought it's a burden for platform to supply the policy
> data.
> 
> I agree that we set the PCD default value as disabled and after a period of
> study, we will understand whether a platform policy is really needed.
> 
> Thanks,
> Ray

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

* Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-12  2:28     ` Ni, Ray
  2021-01-12  5:25       ` Heng Luo
@ 2021-01-13  5:59       ` Wu, Hao A
  2021-01-13  6:06         ` Ni, Ray
  1 sibling, 1 reply; 15+ messages in thread
From: Wu, Hao A @ 2021-01-13  5:59 UTC (permalink / raw)
  To: Ni, Ray, Laszlo Ersek, devel@edk2.groups.io, Luo, Heng; +Cc: Kinney, Michael D

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, January 12, 2021 10:28 AM
> To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Luo, Heng
> <heng.luo@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe:
> Support PCIe Resizable BAR Capability
> 
> > > It seems like the max BAR size is selected first, but if there's a
> > > "resource conflict" (running out of a particular resource type
> > > aperture), then the minimum BAR size is selected. I don't know what
> > > set of devices and/or resizable BARs this logic applies to, if there
> > > are multiple of them.
> 
> > > Per the PCIe specification (revision 5.0, version 0.9) 7.8.6:
> > >
> > >   Software determines, through a proprietary mechanism, what the
> > >   optimal size is for the resource, and programs that size via the BAR
> > >   Size field of the Resizable BAR Control register.
> > >
> > > Furthermore, Table 7-114 defines the Bar Size field of the control
> > > register stating:
> > >
> > >   The default value of this field is equal to the default size of the
> > >   address space that the BAR resource is requesting via the BAR's
> > >   read-only bits.
> > >
> > > Therefore the maximum size is not necessarily optimal, nor should
> > > the minimum size be considered the default.  In fact, [we] tested
> > > various handoff BAR sizes for [a particular] GPU and found that
> > > Windows didn't like the maximum BAR size.
> > >
> > > Elsewhere in the discussion [1] the AMD author of the kernel support
> > > for resizeable BARs indicates that FPGA devices might implement the
> > > REBAR capability as part of their standard PCI wrapper ([our]
> > > interpretation), but the BAR usage would be determined by the actual
> > > bitstream written to the device, therefore there might be a full
> > > bitmask for the BAR sizes supported by the device.
> > >
> > > [1]
> > > https://lists.freedesktop.org/archives/dri-devel/2021-January/thread
> > > .html
> > >
> > > It would certainly make sense for the firmware to take REBAR
> > > capabilities into account when sizing bridge apertures, but to
> > > generically enable extended BAR sizes would make lots of assumptions
> > > about the device usage and compatibility.
> > >
> > > [...] At least for GPUs the expectation would be a default, smaller
> > > compatibility size expanding to some representation that allows
> > > direct DMA to the entire memory of the card.
> >
> > So this patch should either be reverted; or minimally, the default
> > value of "PcdPcieResizableBarSupport" should be set to FALSE, as the
> > policy for BAR sizing doesn't look robust or portable.
> >
> >
> > General request for the future: if you implement some kind of policy
> > in core edk2, please at least *document* the policy somewhere. It's
> > unacceptable to have to decipher the source code for such a possibly
> > impactful change in the core. There is no need for a wiki page or an
> > RFC, but a sane bugzilla ticket and a sane commit message are required.
> >
> > (The documentation of the PCD in the "MdeModulePkg.dec" file is
> > unsatisfactory too, and the UNI file has not been updated at all.)
> >
> 
> 
> 
> Your understanding is correct. Original idea is to let platform supply the policy
> about what the optimal BAR size is for each resizable BAR.
> The current implementation is a try to avoid asking platform code for such
> policy because we thought it's a burden for platform to supply the policy data.
> 
> I agree that we set the PCD default value as disabled and after a period of
> study, we will understand whether a platform policy is really needed.


Hello Laszlo and Ray,

I saw Heng's patch series to
  1) Set the PCD default value to FALSE: https://edk2.groups.io/g/devel/message/70139
  2) Update the UNI file: https://edk2.groups.io/g/devel/message/70140
has got Reviewed-by/Acked-by tags from reviewers.

Do you have further comments for the series?
If not, I will merge this change in the next 24 hours.

Thanks in advance.

Best Regards,
Hao Wu


> 
> Thanks,
> Ray

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

* Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-13  5:59       ` Wu, Hao A
@ 2021-01-13  6:06         ` Ni, Ray
  2021-01-13  6:15           ` Heng Luo
  0 siblings, 1 reply; 15+ messages in thread
From: Ni, Ray @ 2021-01-13  6:06 UTC (permalink / raw)
  To: Wu, Hao A, Laszlo Ersek, devel@edk2.groups.io, Luo, Heng
  Cc: Kinney, Michael D

I've given R-b to the two patches. No comments from my side.

> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Wednesday, January 13, 2021 2:00 PM
> To: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> devel@edk2.groups.io; Luo, Heng <heng.luo@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe:
> Support PCIe Resizable BAR Capability
> 
> > -----Original Message-----
> > From: Ni, Ray <ray.ni@intel.com>
> > Sent: Tuesday, January 12, 2021 10:28 AM
> > To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Luo, Heng
> > <heng.luo@intel.com>
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe:
> > Support PCIe Resizable BAR Capability
> >
> > > > It seems like the max BAR size is selected first, but if there's a
> > > > "resource conflict" (running out of a particular resource type
> > > > aperture), then the minimum BAR size is selected. I don't know what
> > > > set of devices and/or resizable BARs this logic applies to, if there
> > > > are multiple of them.
> >
> > > > Per the PCIe specification (revision 5.0, version 0.9) 7.8.6:
> > > >
> > > >   Software determines, through a proprietary mechanism, what the
> > > >   optimal size is for the resource, and programs that size via the BAR
> > > >   Size field of the Resizable BAR Control register.
> > > >
> > > > Furthermore, Table 7-114 defines the Bar Size field of the control
> > > > register stating:
> > > >
> > > >   The default value of this field is equal to the default size of the
> > > >   address space that the BAR resource is requesting via the BAR's
> > > >   read-only bits.
> > > >
> > > > Therefore the maximum size is not necessarily optimal, nor should
> > > > the minimum size be considered the default.  In fact, [we] tested
> > > > various handoff BAR sizes for [a particular] GPU and found that
> > > > Windows didn't like the maximum BAR size.
> > > >
> > > > Elsewhere in the discussion [1] the AMD author of the kernel support
> > > > for resizeable BARs indicates that FPGA devices might implement the
> > > > REBAR capability as part of their standard PCI wrapper ([our]
> > > > interpretation), but the BAR usage would be determined by the actual
> > > > bitstream written to the device, therefore there might be a full
> > > > bitmask for the BAR sizes supported by the device.
> > > >
> > > > [1]
> > > > https://lists.freedesktop.org/archives/dri-devel/2021-January/thread
> > > > .html
> > > >
> > > > It would certainly make sense for the firmware to take REBAR
> > > > capabilities into account when sizing bridge apertures, but to
> > > > generically enable extended BAR sizes would make lots of assumptions
> > > > about the device usage and compatibility.
> > > >
> > > > [...] At least for GPUs the expectation would be a default, smaller
> > > > compatibility size expanding to some representation that allows
> > > > direct DMA to the entire memory of the card.
> > >
> > > So this patch should either be reverted; or minimally, the default
> > > value of "PcdPcieResizableBarSupport" should be set to FALSE, as the
> > > policy for BAR sizing doesn't look robust or portable.
> > >
> > >
> > > General request for the future: if you implement some kind of policy
> > > in core edk2, please at least *document* the policy somewhere. It's
> > > unacceptable to have to decipher the source code for such a possibly
> > > impactful change in the core. There is no need for a wiki page or an
> > > RFC, but a sane bugzilla ticket and a sane commit message are required.
> > >
> > > (The documentation of the PCD in the "MdeModulePkg.dec" file is
> > > unsatisfactory too, and the UNI file has not been updated at all.)
> > >
> >
> >
> >
> > Your understanding is correct. Original idea is to let platform supply the
> policy
> > about what the optimal BAR size is for each resizable BAR.
> > The current implementation is a try to avoid asking platform code for such
> > policy because we thought it's a burden for platform to supply the policy data.
> >
> > I agree that we set the PCD default value as disabled and after a period of
> > study, we will understand whether a platform policy is really needed.
> 
> 
> Hello Laszlo and Ray,
> 
> I saw Heng's patch series to
>   1) Set the PCD default value to FALSE:
> https://edk2.groups.io/g/devel/message/70139
>   2) Update the UNI file: https://edk2.groups.io/g/devel/message/70140
> has got Reviewed-by/Acked-by tags from reviewers.
> 
> Do you have further comments for the series?
> If not, I will merge this change in the next 24 hours.
> 
> Thanks in advance.
> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Thanks,
> > Ray

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

* Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-13  6:06         ` Ni, Ray
@ 2021-01-13  6:15           ` Heng Luo
  2021-01-13  6:16             ` Wu, Hao A
  2021-01-13  9:08             ` Laszlo Ersek
  0 siblings, 2 replies; 15+ messages in thread
From: Heng Luo @ 2021-01-13  6:15 UTC (permalink / raw)
  To: Ni, Ray, Wu, Hao A, Laszlo Ersek, devel@edk2.groups.io; +Cc: Kinney, Michael D

Hi Hao,
Please hold on merging patch now, we are still waiting for some inputs, I will let you know when we reach agreement.

Thanks,
Heng

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Wednesday, January 13, 2021 2:07 PM
> To: Wu, Hao A <hao.a.wu@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> devel@edk2.groups.io; Luo, Heng <heng.luo@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe:
> Support PCIe Resizable BAR Capability
> 
> I've given R-b to the two patches. No comments from my side.
> 
> > -----Original Message-----
> > From: Wu, Hao A <hao.a.wu@intel.com>
> > Sent: Wednesday, January 13, 2021 2:00 PM
> > To: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> > devel@edk2.groups.io; Luo, Heng <heng.luo@intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe:
> > Support PCIe Resizable BAR Capability
> >
> > > -----Original Message-----
> > > From: Ni, Ray <ray.ni@intel.com>
> > > Sent: Tuesday, January 12, 2021 10:28 AM
> > > To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Luo,
> > > Heng <heng.luo@intel.com>
> > > Cc: Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Subject: RE: [edk2-devel] [Patch V3 2/2]
> MdeModulePkg/Bus/Pci/PciBusDxe:
> > > Support PCIe Resizable BAR Capability
> > >
> > > > > It seems like the max BAR size is selected first, but if there's
> > > > > a "resource conflict" (running out of a particular resource type
> > > > > aperture), then the minimum BAR size is selected. I don't know
> > > > > what set of devices and/or resizable BARs this logic applies to,
> > > > > if there are multiple of them.
> > >
> > > > > Per the PCIe specification (revision 5.0, version 0.9) 7.8.6:
> > > > >
> > > > >   Software determines, through a proprietary mechanism, what the
> > > > >   optimal size is for the resource, and programs that size via the BAR
> > > > >   Size field of the Resizable BAR Control register.
> > > > >
> > > > > Furthermore, Table 7-114 defines the Bar Size field of the
> > > > > control register stating:
> > > > >
> > > > >   The default value of this field is equal to the default size of the
> > > > >   address space that the BAR resource is requesting via the BAR's
> > > > >   read-only bits.
> > > > >
> > > > > Therefore the maximum size is not necessarily optimal, nor
> > > > > should the minimum size be considered the default.  In fact,
> > > > > [we] tested various handoff BAR sizes for [a particular] GPU and
> > > > > found that Windows didn't like the maximum BAR size.
> > > > >
> > > > > Elsewhere in the discussion [1] the AMD author of the kernel
> > > > > support for resizeable BARs indicates that FPGA devices might
> > > > > implement the REBAR capability as part of their standard PCI
> > > > > wrapper ([our] interpretation), but the BAR usage would be
> > > > > determined by the actual bitstream written to the device,
> > > > > therefore there might be a full bitmask for the BAR sizes supported
> by the device.
> > > > >
> > > > > [1]
> > > > > https://lists.freedesktop.org/archives/dri-devel/2021-January/th
> > > > > read
> > > > > .html
> > > > >
> > > > > It would certainly make sense for the firmware to take REBAR
> > > > > capabilities into account when sizing bridge apertures, but to
> > > > > generically enable extended BAR sizes would make lots of
> > > > > assumptions about the device usage and compatibility.
> > > > >
> > > > > [...] At least for GPUs the expectation would be a default,
> > > > > smaller compatibility size expanding to some representation that
> > > > > allows direct DMA to the entire memory of the card.
> > > >
> > > > So this patch should either be reverted; or minimally, the default
> > > > value of "PcdPcieResizableBarSupport" should be set to FALSE, as
> > > > the policy for BAR sizing doesn't look robust or portable.
> > > >
> > > >
> > > > General request for the future: if you implement some kind of
> > > > policy in core edk2, please at least *document* the policy
> > > > somewhere. It's unacceptable to have to decipher the source code
> > > > for such a possibly impactful change in the core. There is no need
> > > > for a wiki page or an RFC, but a sane bugzilla ticket and a sane commit
> message are required.
> > > >
> > > > (The documentation of the PCD in the "MdeModulePkg.dec" file is
> > > > unsatisfactory too, and the UNI file has not been updated at all.)
> > > >
> > >
> > >
> > >
> > > Your understanding is correct. Original idea is to let platform
> > > supply the
> > policy
> > > about what the optimal BAR size is for each resizable BAR.
> > > The current implementation is a try to avoid asking platform code
> > > for such policy because we thought it's a burden for platform to supply
> the policy data.
> > >
> > > I agree that we set the PCD default value as disabled and after a
> > > period of study, we will understand whether a platform policy is really
> needed.
> >
> >
> > Hello Laszlo and Ray,
> >
> > I saw Heng's patch series to
> >   1) Set the PCD default value to FALSE:
> > https://edk2.groups.io/g/devel/message/70139
> >   2) Update the UNI file: https://edk2.groups.io/g/devel/message/70140
> > has got Reviewed-by/Acked-by tags from reviewers.
> >
> > Do you have further comments for the series?
> > If not, I will merge this change in the next 24 hours.
> >
> > Thanks in advance.
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > >
> > > Thanks,
> > > Ray

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

* Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-13  6:15           ` Heng Luo
@ 2021-01-13  6:16             ` Wu, Hao A
  2021-01-13  9:08             ` Laszlo Ersek
  1 sibling, 0 replies; 15+ messages in thread
From: Wu, Hao A @ 2021-01-13  6:16 UTC (permalink / raw)
  To: Luo, Heng, Ni, Ray, Laszlo Ersek, devel@edk2.groups.io; +Cc: Kinney, Michael D

> -----Original Message-----
> From: Luo, Heng <heng.luo@intel.com>
> Sent: Wednesday, January 13, 2021 2:15 PM
> To: Ni, Ray <ray.ni@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe:
> Support PCIe Resizable BAR Capability
> 
> Hi Hao,
> Please hold on merging patch now, we are still waiting for some inputs, I will
> let you know when we reach agreement.


Got it. Thanks Heng.

Best Regards,
Hao Wu


> 
> Thanks,
> Heng
> 
> > -----Original Message-----
> > From: Ni, Ray <ray.ni@intel.com>
> > Sent: Wednesday, January 13, 2021 2:07 PM
> > To: Wu, Hao A <hao.a.wu@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> > devel@edk2.groups.io; Luo, Heng <heng.luo@intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [Patch V3 2/2]
> MdeModulePkg/Bus/Pci/PciBusDxe:
> > Support PCIe Resizable BAR Capability
> >
> > I've given R-b to the two patches. No comments from my side.
> >
> > > -----Original Message-----
> > > From: Wu, Hao A <hao.a.wu@intel.com>
> > > Sent: Wednesday, January 13, 2021 2:00 PM
> > > To: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> > > devel@edk2.groups.io; Luo, Heng <heng.luo@intel.com>
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Subject: RE: [edk2-devel] [Patch V3 2/2]
> MdeModulePkg/Bus/Pci/PciBusDxe:
> > > Support PCIe Resizable BAR Capability
> > >
> > > > -----Original Message-----
> > > > From: Ni, Ray <ray.ni@intel.com>
> > > > Sent: Tuesday, January 12, 2021 10:28 AM
> > > > To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Luo,
> > > > Heng <heng.luo@intel.com>
> > > > Cc: Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
> > > > <michael.d.kinney@intel.com>
> > > > Subject: RE: [edk2-devel] [Patch V3 2/2]
> > MdeModulePkg/Bus/Pci/PciBusDxe:
> > > > Support PCIe Resizable BAR Capability
> > > >
> > > > > > It seems like the max BAR size is selected first, but if
> > > > > > there's a "resource conflict" (running out of a particular
> > > > > > resource type aperture), then the minimum BAR size is
> > > > > > selected. I don't know what set of devices and/or resizable
> > > > > > BARs this logic applies to, if there are multiple of them.
> > > >
> > > > > > Per the PCIe specification (revision 5.0, version 0.9) 7.8.6:
> > > > > >
> > > > > >   Software determines, through a proprietary mechanism, what the
> > > > > >   optimal size is for the resource, and programs that size via the BAR
> > > > > >   Size field of the Resizable BAR Control register.
> > > > > >
> > > > > > Furthermore, Table 7-114 defines the Bar Size field of the
> > > > > > control register stating:
> > > > > >
> > > > > >   The default value of this field is equal to the default size of the
> > > > > >   address space that the BAR resource is requesting via the BAR's
> > > > > >   read-only bits.
> > > > > >
> > > > > > Therefore the maximum size is not necessarily optimal, nor
> > > > > > should the minimum size be considered the default.  In fact,
> > > > > > [we] tested various handoff BAR sizes for [a particular] GPU
> > > > > > and found that Windows didn't like the maximum BAR size.
> > > > > >
> > > > > > Elsewhere in the discussion [1] the AMD author of the kernel
> > > > > > support for resizeable BARs indicates that FPGA devices might
> > > > > > implement the REBAR capability as part of their standard PCI
> > > > > > wrapper ([our] interpretation), but the BAR usage would be
> > > > > > determined by the actual bitstream written to the device,
> > > > > > therefore there might be a full bitmask for the BAR sizes
> > > > > > supported
> > by the device.
> > > > > >
> > > > > > [1]
> > > > > > https://lists.freedesktop.org/archives/dri-devel/2021-January/
> > > > > > th
> > > > > > read
> > > > > > .html
> > > > > >
> > > > > > It would certainly make sense for the firmware to take REBAR
> > > > > > capabilities into account when sizing bridge apertures, but to
> > > > > > generically enable extended BAR sizes would make lots of
> > > > > > assumptions about the device usage and compatibility.
> > > > > >
> > > > > > [...] At least for GPUs the expectation would be a default,
> > > > > > smaller compatibility size expanding to some representation
> > > > > > that allows direct DMA to the entire memory of the card.
> > > > >
> > > > > So this patch should either be reverted; or minimally, the
> > > > > default value of "PcdPcieResizableBarSupport" should be set to
> > > > > FALSE, as the policy for BAR sizing doesn't look robust or portable.
> > > > >
> > > > >
> > > > > General request for the future: if you implement some kind of
> > > > > policy in core edk2, please at least *document* the policy
> > > > > somewhere. It's unacceptable to have to decipher the source code
> > > > > for such a possibly impactful change in the core. There is no
> > > > > need for a wiki page or an RFC, but a sane bugzilla ticket and a
> > > > > sane commit
> > message are required.
> > > > >
> > > > > (The documentation of the PCD in the "MdeModulePkg.dec" file is
> > > > > unsatisfactory too, and the UNI file has not been updated at
> > > > > all.)
> > > > >
> > > >
> > > >
> > > >
> > > > Your understanding is correct. Original idea is to let platform
> > > > supply the
> > > policy
> > > > about what the optimal BAR size is for each resizable BAR.
> > > > The current implementation is a try to avoid asking platform code
> > > > for such policy because we thought it's a burden for platform to
> > > > supply
> > the policy data.
> > > >
> > > > I agree that we set the PCD default value as disabled and after a
> > > > period of study, we will understand whether a platform policy is
> > > > really
> > needed.
> > >
> > >
> > > Hello Laszlo and Ray,
> > >
> > > I saw Heng's patch series to
> > >   1) Set the PCD default value to FALSE:
> > > https://edk2.groups.io/g/devel/message/70139
> > >   2) Update the UNI file:
> > > https://edk2.groups.io/g/devel/message/70140
> > > has got Reviewed-by/Acked-by tags from reviewers.
> > >
> > > Do you have further comments for the series?
> > > If not, I will merge this change in the next 24 hours.
> > >
> > > Thanks in advance.
> > >
> > > Best Regards,
> > > Hao Wu
> > >
> > >
> > > >
> > > > Thanks,
> > > > Ray

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

* Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-13  6:15           ` Heng Luo
  2021-01-13  6:16             ` Wu, Hao A
@ 2021-01-13  9:08             ` Laszlo Ersek
  2021-01-14  0:44               ` Heng Luo
  1 sibling, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2021-01-13  9:08 UTC (permalink / raw)
  To: Luo, Heng, Ni, Ray, Wu, Hao A, devel@edk2.groups.io; +Cc: Kinney, Michael D

On 01/13/21 07:15, Luo, Heng wrote:
> Hi Hao,
> Please hold on merging patch now, we are still waiting for some inputs, I will let you know when we reach agreement.

I disagree with waiting. The original patch caused a regression. The
currently pending patch fixes the regression. Any further input
("agreement") should be processed *after* we have mitigated the regression.

The tree is currently in a wrong state. The fix has been reviewed. Hao,
please proceed with merging the fix as soon as you can.

Thanks
Laszlo

> 
> Thanks,
> Heng
> 
>> -----Original Message-----
>> From: Ni, Ray <ray.ni@intel.com>
>> Sent: Wednesday, January 13, 2021 2:07 PM
>> To: Wu, Hao A <hao.a.wu@intel.com>; Laszlo Ersek <lersek@redhat.com>;
>> devel@edk2.groups.io; Luo, Heng <heng.luo@intel.com>
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
>> Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe:
>> Support PCIe Resizable BAR Capability
>>
>> I've given R-b to the two patches. No comments from my side.
>>
>>> -----Original Message-----
>>> From: Wu, Hao A <hao.a.wu@intel.com>
>>> Sent: Wednesday, January 13, 2021 2:00 PM
>>> To: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
>>> devel@edk2.groups.io; Luo, Heng <heng.luo@intel.com>
>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
>>> Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe:
>>> Support PCIe Resizable BAR Capability
>>>
>>>> -----Original Message-----
>>>> From: Ni, Ray <ray.ni@intel.com>
>>>> Sent: Tuesday, January 12, 2021 10:28 AM
>>>> To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Luo,
>>>> Heng <heng.luo@intel.com>
>>>> Cc: Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>
>>>> Subject: RE: [edk2-devel] [Patch V3 2/2]
>> MdeModulePkg/Bus/Pci/PciBusDxe:
>>>> Support PCIe Resizable BAR Capability
>>>>
>>>>>> It seems like the max BAR size is selected first, but if there's
>>>>>> a "resource conflict" (running out of a particular resource type
>>>>>> aperture), then the minimum BAR size is selected. I don't know
>>>>>> what set of devices and/or resizable BARs this logic applies to,
>>>>>> if there are multiple of them.
>>>>
>>>>>> Per the PCIe specification (revision 5.0, version 0.9) 7.8.6:
>>>>>>
>>>>>>   Software determines, through a proprietary mechanism, what the
>>>>>>   optimal size is for the resource, and programs that size via the BAR
>>>>>>   Size field of the Resizable BAR Control register.
>>>>>>
>>>>>> Furthermore, Table 7-114 defines the Bar Size field of the
>>>>>> control register stating:
>>>>>>
>>>>>>   The default value of this field is equal to the default size of the
>>>>>>   address space that the BAR resource is requesting via the BAR's
>>>>>>   read-only bits.
>>>>>>
>>>>>> Therefore the maximum size is not necessarily optimal, nor
>>>>>> should the minimum size be considered the default.  In fact,
>>>>>> [we] tested various handoff BAR sizes for [a particular] GPU and
>>>>>> found that Windows didn't like the maximum BAR size.
>>>>>>
>>>>>> Elsewhere in the discussion [1] the AMD author of the kernel
>>>>>> support for resizeable BARs indicates that FPGA devices might
>>>>>> implement the REBAR capability as part of their standard PCI
>>>>>> wrapper ([our] interpretation), but the BAR usage would be
>>>>>> determined by the actual bitstream written to the device,
>>>>>> therefore there might be a full bitmask for the BAR sizes supported
>> by the device.
>>>>>>
>>>>>> [1]
>>>>>> https://lists.freedesktop.org/archives/dri-devel/2021-January/th
>>>>>> read
>>>>>> .html
>>>>>>
>>>>>> It would certainly make sense for the firmware to take REBAR
>>>>>> capabilities into account when sizing bridge apertures, but to
>>>>>> generically enable extended BAR sizes would make lots of
>>>>>> assumptions about the device usage and compatibility.
>>>>>>
>>>>>> [...] At least for GPUs the expectation would be a default,
>>>>>> smaller compatibility size expanding to some representation that
>>>>>> allows direct DMA to the entire memory of the card.
>>>>>
>>>>> So this patch should either be reverted; or minimally, the default
>>>>> value of "PcdPcieResizableBarSupport" should be set to FALSE, as
>>>>> the policy for BAR sizing doesn't look robust or portable.
>>>>>
>>>>>
>>>>> General request for the future: if you implement some kind of
>>>>> policy in core edk2, please at least *document* the policy
>>>>> somewhere. It's unacceptable to have to decipher the source code
>>>>> for such a possibly impactful change in the core. There is no need
>>>>> for a wiki page or an RFC, but a sane bugzilla ticket and a sane commit
>> message are required.
>>>>>
>>>>> (The documentation of the PCD in the "MdeModulePkg.dec" file is
>>>>> unsatisfactory too, and the UNI file has not been updated at all.)
>>>>>
>>>>
>>>>
>>>>
>>>> Your understanding is correct. Original idea is to let platform
>>>> supply the
>>> policy
>>>> about what the optimal BAR size is for each resizable BAR.
>>>> The current implementation is a try to avoid asking platform code
>>>> for such policy because we thought it's a burden for platform to supply
>> the policy data.
>>>>
>>>> I agree that we set the PCD default value as disabled and after a
>>>> period of study, we will understand whether a platform policy is really
>> needed.
>>>
>>>
>>> Hello Laszlo and Ray,
>>>
>>> I saw Heng's patch series to
>>>   1) Set the PCD default value to FALSE:
>>> https://edk2.groups.io/g/devel/message/70139
>>>   2) Update the UNI file: https://edk2.groups.io/g/devel/message/70140
>>> has got Reviewed-by/Acked-by tags from reviewers.
>>>
>>> Do you have further comments for the series?
>>> If not, I will merge this change in the next 24 hours.
>>>
>>> Thanks in advance.
>>>
>>> Best Regards,
>>> Hao Wu
>>>
>>>
>>>>
>>>> Thanks,
>>>> Ray


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

* Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-13  9:08             ` Laszlo Ersek
@ 2021-01-14  0:44               ` Heng Luo
  2021-01-14  1:01                 ` Wu, Hao A
  0 siblings, 1 reply; 15+ messages in thread
From: Heng Luo @ 2021-01-14  0:44 UTC (permalink / raw)
  To: devel@edk2.groups.io, lersek@redhat.com, Ni, Ray, Wu, Hao A
  Cc: Kinney, Michael D

Hi Laszlo,
Sorry for the delay. I have gotten the feedback, we can merged them now.

Hi Hao,
Could you help to merge the patches.

Thanks,
Heng

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> Ersek
> Sent: Wednesday, January 13, 2021 5:09 PM
> To: Luo, Heng <heng.luo@intel.com>; Ni, Ray <ray.ni@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe:
> Support PCIe Resizable BAR Capability
> 
> On 01/13/21 07:15, Luo, Heng wrote:
> > Hi Hao,
> > Please hold on merging patch now, we are still waiting for some inputs, I
> will let you know when we reach agreement.
> 
> I disagree with waiting. The original patch caused a regression. The currently
> pending patch fixes the regression. Any further input
> ("agreement") should be processed *after* we have mitigated the regression.
> 
> The tree is currently in a wrong state. The fix has been reviewed. Hao, please
> proceed with merging the fix as soon as you can.
> 
> Thanks
> Laszlo
> 
> >
> > Thanks,
> > Heng
> >
> >> -----Original Message-----
> >> From: Ni, Ray <ray.ni@intel.com>
> >> Sent: Wednesday, January 13, 2021 2:07 PM
> >> To: Wu, Hao A <hao.a.wu@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> >> devel@edk2.groups.io; Luo, Heng <heng.luo@intel.com>
> >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> >> Subject: RE: [edk2-devel] [Patch V3 2/2]
> MdeModulePkg/Bus/Pci/PciBusDxe:
> >> Support PCIe Resizable BAR Capability
> >>
> >> I've given R-b to the two patches. No comments from my side.
> >>
> >>> -----Original Message-----
> >>> From: Wu, Hao A <hao.a.wu@intel.com>
> >>> Sent: Wednesday, January 13, 2021 2:00 PM
> >>> To: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> >>> devel@edk2.groups.io; Luo, Heng <heng.luo@intel.com>
> >>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> >>> Subject: RE: [edk2-devel] [Patch V3 2/2]
> MdeModulePkg/Bus/Pci/PciBusDxe:
> >>> Support PCIe Resizable BAR Capability
> >>>
> >>>> -----Original Message-----
> >>>> From: Ni, Ray <ray.ni@intel.com>
> >>>> Sent: Tuesday, January 12, 2021 10:28 AM
> >>>> To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Luo,
> >>>> Heng <heng.luo@intel.com>
> >>>> Cc: Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
> >>>> <michael.d.kinney@intel.com>
> >>>> Subject: RE: [edk2-devel] [Patch V3 2/2]
> >> MdeModulePkg/Bus/Pci/PciBusDxe:
> >>>> Support PCIe Resizable BAR Capability
> >>>>
> >>>>>> It seems like the max BAR size is selected first, but if there's
> >>>>>> a "resource conflict" (running out of a particular resource type
> >>>>>> aperture), then the minimum BAR size is selected. I don't know
> >>>>>> what set of devices and/or resizable BARs this logic applies to,
> >>>>>> if there are multiple of them.
> >>>>
> >>>>>> Per the PCIe specification (revision 5.0, version 0.9) 7.8.6:
> >>>>>>
> >>>>>>   Software determines, through a proprietary mechanism, what the
> >>>>>>   optimal size is for the resource, and programs that size via the BAR
> >>>>>>   Size field of the Resizable BAR Control register.
> >>>>>>
> >>>>>> Furthermore, Table 7-114 defines the Bar Size field of the
> >>>>>> control register stating:
> >>>>>>
> >>>>>>   The default value of this field is equal to the default size of the
> >>>>>>   address space that the BAR resource is requesting via the BAR's
> >>>>>>   read-only bits.
> >>>>>>
> >>>>>> Therefore the maximum size is not necessarily optimal, nor should
> >>>>>> the minimum size be considered the default.  In fact, [we] tested
> >>>>>> various handoff BAR sizes for [a particular] GPU and found that
> >>>>>> Windows didn't like the maximum BAR size.
> >>>>>>
> >>>>>> Elsewhere in the discussion [1] the AMD author of the kernel
> >>>>>> support for resizeable BARs indicates that FPGA devices might
> >>>>>> implement the REBAR capability as part of their standard PCI
> >>>>>> wrapper ([our] interpretation), but the BAR usage would be
> >>>>>> determined by the actual bitstream written to the device,
> >>>>>> therefore there might be a full bitmask for the BAR sizes
> >>>>>> supported
> >> by the device.
> >>>>>>
> >>>>>> [1]
> >>>>>> https://lists.freedesktop.org/archives/dri-devel/2021-January/th
> >>>>>> read
> >>>>>> .html
> >>>>>>
> >>>>>> It would certainly make sense for the firmware to take REBAR
> >>>>>> capabilities into account when sizing bridge apertures, but to
> >>>>>> generically enable extended BAR sizes would make lots of
> >>>>>> assumptions about the device usage and compatibility.
> >>>>>>
> >>>>>> [...] At least for GPUs the expectation would be a default,
> >>>>>> smaller compatibility size expanding to some representation that
> >>>>>> allows direct DMA to the entire memory of the card.
> >>>>>
> >>>>> So this patch should either be reverted; or minimally, the default
> >>>>> value of "PcdPcieResizableBarSupport" should be set to FALSE, as
> >>>>> the policy for BAR sizing doesn't look robust or portable.
> >>>>>
> >>>>>
> >>>>> General request for the future: if you implement some kind of
> >>>>> policy in core edk2, please at least *document* the policy
> >>>>> somewhere. It's unacceptable to have to decipher the source code
> >>>>> for such a possibly impactful change in the core. There is no need
> >>>>> for a wiki page or an RFC, but a sane bugzilla ticket and a sane
> >>>>> commit
> >> message are required.
> >>>>>
> >>>>> (The documentation of the PCD in the "MdeModulePkg.dec" file is
> >>>>> unsatisfactory too, and the UNI file has not been updated at all.)
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>> Your understanding is correct. Original idea is to let platform
> >>>> supply the
> >>> policy
> >>>> about what the optimal BAR size is for each resizable BAR.
> >>>> The current implementation is a try to avoid asking platform code
> >>>> for such policy because we thought it's a burden for platform to
> >>>> supply
> >> the policy data.
> >>>>
> >>>> I agree that we set the PCD default value as disabled and after a
> >>>> period of study, we will understand whether a platform policy is
> >>>> really
> >> needed.
> >>>
> >>>
> >>> Hello Laszlo and Ray,
> >>>
> >>> I saw Heng's patch series to
> >>>   1) Set the PCD default value to FALSE:
> >>> https://edk2.groups.io/g/devel/message/70139
> >>>   2) Update the UNI file:
> >>> https://edk2.groups.io/g/devel/message/70140
> >>> has got Reviewed-by/Acked-by tags from reviewers.
> >>>
> >>> Do you have further comments for the series?
> >>> If not, I will merge this change in the next 24 hours.
> >>>
> >>> Thanks in advance.
> >>>
> >>> Best Regards,
> >>> Hao Wu
> >>>
> >>>
> >>>>
> >>>> Thanks,
> >>>> Ray
> 
> 
> 
> 
> 


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

* Re: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe Resizable BAR Capability
  2021-01-14  0:44               ` Heng Luo
@ 2021-01-14  1:01                 ` Wu, Hao A
  0 siblings, 0 replies; 15+ messages in thread
From: Wu, Hao A @ 2021-01-14  1:01 UTC (permalink / raw)
  To: Luo, Heng, devel@edk2.groups.io, lersek@redhat.com, Ni, Ray
  Cc: Kinney, Michael D

Sure, I will create PR to merge the series.

Best Regards,
Hao Wu

> -----Original Message-----
> From: Luo, Heng <heng.luo@intel.com>
> Sent: Thursday, January 14, 2021 8:45 AM
> To: devel@edk2.groups.io; lersek@redhat.com; Ni, Ray <ray.ni@intel.com>;
> Wu, Hao A <hao.a.wu@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe:
> Support PCIe Resizable BAR Capability
> 
> Hi Laszlo,
> Sorry for the delay. I have gotten the feedback, we can merged them now.
> 
> Hi Hao,
> Could you help to merge the patches.
> 
> Thanks,
> Heng
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> > Ersek
> > Sent: Wednesday, January 13, 2021 5:09 PM
> > To: Luo, Heng <heng.luo@intel.com>; Ni, Ray <ray.ni@intel.com>; Wu,
> > Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [Patch V3 2/2]
> MdeModulePkg/Bus/Pci/PciBusDxe:
> > Support PCIe Resizable BAR Capability
> >
> > On 01/13/21 07:15, Luo, Heng wrote:
> > > Hi Hao,
> > > Please hold on merging patch now, we are still waiting for some
> > > inputs, I
> > will let you know when we reach agreement.
> >
> > I disagree with waiting. The original patch caused a regression. The
> > currently pending patch fixes the regression. Any further input
> > ("agreement") should be processed *after* we have mitigated the
> regression.
> >
> > The tree is currently in a wrong state. The fix has been reviewed.
> > Hao, please proceed with merging the fix as soon as you can.
> >
> > Thanks
> > Laszlo
> >
> > >
> > > Thanks,
> > > Heng
> > >
> > >> -----Original Message-----
> > >> From: Ni, Ray <ray.ni@intel.com>
> > >> Sent: Wednesday, January 13, 2021 2:07 PM
> > >> To: Wu, Hao A <hao.a.wu@intel.com>; Laszlo Ersek
> > >> <lersek@redhat.com>; devel@edk2.groups.io; Luo, Heng
> > >> <heng.luo@intel.com>
> > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > >> Subject: RE: [edk2-devel] [Patch V3 2/2]
> > MdeModulePkg/Bus/Pci/PciBusDxe:
> > >> Support PCIe Resizable BAR Capability
> > >>
> > >> I've given R-b to the two patches. No comments from my side.
> > >>
> > >>> -----Original Message-----
> > >>> From: Wu, Hao A <hao.a.wu@intel.com>
> > >>> Sent: Wednesday, January 13, 2021 2:00 PM
> > >>> To: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> > >>> devel@edk2.groups.io; Luo, Heng <heng.luo@intel.com>
> > >>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > >>> Subject: RE: [edk2-devel] [Patch V3 2/2]
> > MdeModulePkg/Bus/Pci/PciBusDxe:
> > >>> Support PCIe Resizable BAR Capability
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Ni, Ray <ray.ni@intel.com>
> > >>>> Sent: Tuesday, January 12, 2021 10:28 AM
> > >>>> To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Luo,
> > >>>> Heng <heng.luo@intel.com>
> > >>>> Cc: Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
> > >>>> <michael.d.kinney@intel.com>
> > >>>> Subject: RE: [edk2-devel] [Patch V3 2/2]
> > >> MdeModulePkg/Bus/Pci/PciBusDxe:
> > >>>> Support PCIe Resizable BAR Capability
> > >>>>
> > >>>>>> It seems like the max BAR size is selected first, but if
> > >>>>>> there's a "resource conflict" (running out of a particular
> > >>>>>> resource type aperture), then the minimum BAR size is selected.
> > >>>>>> I don't know what set of devices and/or resizable BARs this
> > >>>>>> logic applies to, if there are multiple of them.
> > >>>>
> > >>>>>> Per the PCIe specification (revision 5.0, version 0.9) 7.8.6:
> > >>>>>>
> > >>>>>>   Software determines, through a proprietary mechanism, what
> the
> > >>>>>>   optimal size is for the resource, and programs that size via the
> BAR
> > >>>>>>   Size field of the Resizable BAR Control register.
> > >>>>>>
> > >>>>>> Furthermore, Table 7-114 defines the Bar Size field of the
> > >>>>>> control register stating:
> > >>>>>>
> > >>>>>>   The default value of this field is equal to the default size of the
> > >>>>>>   address space that the BAR resource is requesting via the BAR's
> > >>>>>>   read-only bits.
> > >>>>>>
> > >>>>>> Therefore the maximum size is not necessarily optimal, nor
> > >>>>>> should the minimum size be considered the default.  In fact,
> > >>>>>> [we] tested various handoff BAR sizes for [a particular] GPU
> > >>>>>> and found that Windows didn't like the maximum BAR size.
> > >>>>>>
> > >>>>>> Elsewhere in the discussion [1] the AMD author of the kernel
> > >>>>>> support for resizeable BARs indicates that FPGA devices might
> > >>>>>> implement the REBAR capability as part of their standard PCI
> > >>>>>> wrapper ([our] interpretation), but the BAR usage would be
> > >>>>>> determined by the actual bitstream written to the device,
> > >>>>>> therefore there might be a full bitmask for the BAR sizes
> > >>>>>> supported
> > >> by the device.
> > >>>>>>
> > >>>>>> [1]
> > >>>>>> https://lists.freedesktop.org/archives/dri-devel/2021-January/t
> > >>>>>> h
> > >>>>>> read
> > >>>>>> .html
> > >>>>>>
> > >>>>>> It would certainly make sense for the firmware to take REBAR
> > >>>>>> capabilities into account when sizing bridge apertures, but to
> > >>>>>> generically enable extended BAR sizes would make lots of
> > >>>>>> assumptions about the device usage and compatibility.
> > >>>>>>
> > >>>>>> [...] At least for GPUs the expectation would be a default,
> > >>>>>> smaller compatibility size expanding to some representation
> > >>>>>> that allows direct DMA to the entire memory of the card.
> > >>>>>
> > >>>>> So this patch should either be reverted; or minimally, the
> > >>>>> default value of "PcdPcieResizableBarSupport" should be set to
> > >>>>> FALSE, as the policy for BAR sizing doesn't look robust or portable.
> > >>>>>
> > >>>>>
> > >>>>> General request for the future: if you implement some kind of
> > >>>>> policy in core edk2, please at least *document* the policy
> > >>>>> somewhere. It's unacceptable to have to decipher the source code
> > >>>>> for such a possibly impactful change in the core. There is no
> > >>>>> need for a wiki page or an RFC, but a sane bugzilla ticket and a
> > >>>>> sane commit
> > >> message are required.
> > >>>>>
> > >>>>> (The documentation of the PCD in the "MdeModulePkg.dec" file is
> > >>>>> unsatisfactory too, and the UNI file has not been updated at
> > >>>>> all.)
> > >>>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>> Your understanding is correct. Original idea is to let platform
> > >>>> supply the
> > >>> policy
> > >>>> about what the optimal BAR size is for each resizable BAR.
> > >>>> The current implementation is a try to avoid asking platform code
> > >>>> for such policy because we thought it's a burden for platform to
> > >>>> supply
> > >> the policy data.
> > >>>>
> > >>>> I agree that we set the PCD default value as disabled and after a
> > >>>> period of study, we will understand whether a platform policy is
> > >>>> really
> > >> needed.
> > >>>
> > >>>
> > >>> Hello Laszlo and Ray,
> > >>>
> > >>> I saw Heng's patch series to
> > >>>   1) Set the PCD default value to FALSE:
> > >>> https://edk2.groups.io/g/devel/message/70139
> > >>>   2) Update the UNI file:
> > >>> https://edk2.groups.io/g/devel/message/70140
> > >>> has got Reviewed-by/Acked-by tags from reviewers.
> > >>>
> > >>> Do you have further comments for the series?
> > >>> If not, I will merge this change in the next 24 hours.
> > >>>
> > >>> Thanks in advance.
> > >>>
> > >>> Best Regards,
> > >>> Hao Wu
> > >>>
> > >>>
> > >>>>
> > >>>> Thanks,
> > >>>> Ray
> >
> >
> >
> > 
> >


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

end of thread, other threads:[~2021-01-14  1:02 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-04  6:59 [Patch V3 1/2] MdePkg: Define structures for Resizable BAR Capability Heng Luo
2021-01-04  6:59 ` [Patch V3 2/2] MdeModulePkg/Bus/Pci/PciBusDxe: Support PCIe " Heng Luo
2021-01-04  7:52   ` Ni, Ray
2021-01-11 19:38   ` [edk2-devel] " Laszlo Ersek
2021-01-11 19:44     ` Laszlo Ersek
2021-01-12  2:28     ` Ni, Ray
2021-01-12  5:25       ` Heng Luo
2021-01-13  5:59       ` Wu, Hao A
2021-01-13  6:06         ` Ni, Ray
2021-01-13  6:15           ` Heng Luo
2021-01-13  6:16             ` Wu, Hao A
2021-01-13  9:08             ` Laszlo Ersek
2021-01-14  0:44               ` Heng Luo
2021-01-14  1:01                 ` Wu, Hao A
2021-01-04  7:52 ` [Patch V3 1/2] MdePkg: Define structures for " Ni, Ray

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