diff mbox

[edk2,v3,2/2] MdeModulePkg: PciHostBridgeDxe: don't assume extended config space

Message ID 1456827745-17925-3-git-send-email-lersek@redhat.com
State New
Headers show

Commit Message

Laszlo Ersek March 1, 2016, 10:22 a.m. UTC 
The "pc" ("pc-i440fx-*") machine types of QEMU don't support extended
config space. Accordingly, OVMF will use the following library instances
in connection with the core PciHostBridgeDxe driver:

  BasePciSegmentLibPci [class: PciSegmentLib]
    BasePciLibCf8      [class: PciLib]
      BasePciCf8Lib    [class: PciCf8Lib]

Add a new field to the PCI_ROOT_BRIDGE structure so that
RootBridgeIoCheckParameter() can catch config space offsets above 0xFF on
such old (emulated) platforms.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>

---

Notes:
    v3:
    - invert the meaning of the field: replace ExtendedConfigSpace with
      NoExtendedConfigSpace [Ray]
    
    v2:
    - replace the PCI config space "aperture" with a dedicated BOOLEAN flag
      called ExtendedConfigSpace [Ray]
    - simplify the commit message

 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   | 1 +
 MdeModulePkg/Include/Library/PciHostBridgeLib.h         | 4 ++++
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 4 +++-
 3 files changed, 8 insertions(+), 1 deletion(-)

-- 
1.8.3.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek March 3, 2016, 9:05 a.m. UTC | #1 
On 03/03/16 06:35, Ni, Ruiyu wrote:
> 

> 

> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>


Awesome. Thank you for working with me through this!

Thanks all for the reviews; committed as b07c96658132..014b472053ae.

Cheers
Laszlo

>> -----Original Message-----

>> From: Laszlo Ersek [mailto:lersek@redhat.com]

>> Sent: Tuesday, March 1, 2016 6:22 PM

>> To: edk2-devel@ml01.01.org

>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Marcel Apfelbaum <marcel@redhat.com>;

>> Kinney, Michael D <michael.d.kinney@intel.com>

>> Subject: [PATCH v3 2/2] MdeModulePkg: PciHostBridgeDxe: don't assume extended config space

>>

>> The "pc" ("pc-i440fx-*") machine types of QEMU don't support extended

>> config space. Accordingly, OVMF will use the following library instances

>> in connection with the core PciHostBridgeDxe driver:

>>

>>  BasePciSegmentLibPci [class: PciSegmentLib]

>>    BasePciLibCf8      [class: PciLib]

>>      BasePciCf8Lib    [class: PciCf8Lib]

>>

>> Add a new field to the PCI_ROOT_BRIDGE structure so that

>> RootBridgeIoCheckParameter() can catch config space offsets above 0xFF on

>> such old (emulated) platforms.

>>

>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>

>> Cc: Jordan Justen <jordan.l.justen@intel.com>

>> Cc: Marcel Apfelbaum <marcel@redhat.com>

>> Cc: Michael Kinney <michael.d.kinney@intel.com>

>> Contributed-under: TianoCore Contribution Agreement 1.0

>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

>> ---

>>

>> Notes:

>>    v3:

>>    - invert the meaning of the field: replace ExtendedConfigSpace with

>>      NoExtendedConfigSpace [Ray]

>>

>>    v2:

>>    - replace the PCI config space "aperture" with a dedicated BOOLEAN flag

>>      called ExtendedConfigSpace [Ray]

>>    - simplify the commit message

>>

>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   | 1 +

>> MdeModulePkg/Include/Library/PciHostBridgeLib.h         | 4 ++++

>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 4 +++-

>> 3 files changed, 8 insertions(+), 1 deletion(-)

>>

>> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h

>> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h

>> index b1e83f1c9089..aa3f43a511c4 100644

>> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h

>> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h

>> @@ -72,6 +72,7 @@ typedef struct {

>>   PCI_ROOT_BRIDGE_APERTURE          MemAbove4G;

>>   PCI_ROOT_BRIDGE_APERTURE          PMemAbove4G;

>>   BOOLEAN                           DmaAbove4G;

>> +  BOOLEAN                           NoExtendedConfigSpace;

>>   VOID                              *ConfigBuffer;

>>   EFI_DEVICE_PATH_PROTOCOL          *DevicePath;

>>   CHAR16                            *DevicePathStr;

>> diff --git a/MdeModulePkg/Include/Library/PciHostBridgeLib.h b/MdeModulePkg/Include/Library/PciHostBridgeLib.h

>> index 16ad104a9368..b67ac5e17d48 100644

>> --- a/MdeModulePkg/Include/Library/PciHostBridgeLib.h

>> +++ b/MdeModulePkg/Include/Library/PciHostBridgeLib.h

>> @@ -34,6 +34,10 @@ typedef struct {

>>                                                   ///< and SetAttributes() in

>> EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.

>>   BOOLEAN                  DmaAbove4G;            ///< DMA above 4GB memory.

>>                                                   ///< Set to TRUE when root bridge supports DMA above 4GB

>> memory.

>> +  BOOLEAN                  NoExtendedConfigSpace; ///< When FALSE, the root bridge supports

>> +                                                  ///< Extended (4096-byte) Configuration Space.

>> +                                                  ///< When TRUE, the root bridge supports

>> +                                                  ///< 256-byte Configuration Space only.

>>   UINT64                   AllocationAttributes;  ///< Allocation attributes.

>>                                                   ///< Refer to

>> EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM and

>>                                                   ///< EFI_PCI_HOST_BRIDGE_MEM64_DECODE used by

>> GetAllocAttributes()

>> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c

>> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c

>> index 932aefd5d621..cda9b49b3925 100644

>> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c

>> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c

>> @@ -80,6 +80,7 @@ CreateRootBridge (

>>   DEBUG ((EFI_D_INFO, "%s\n", DevicePathStr = ConvertDevicePathToText (Bridge->DevicePath, FALSE, FALSE)));

>>   DEBUG ((EFI_D_INFO, "  Support/Attr: %lx / %lx\n", Bridge->Supports, Bridge->Attributes));

>>   DEBUG ((EFI_D_INFO, "    DmaAbove4G: %s\n", Bridge->DmaAbove4G ? L"Yes" : L"No"));

>> +  DEBUG ((EFI_D_INFO, "NoExtConfSpace: %s\n", Bridge->NoExtendedConfigSpace ? L"Yes" : L"No"));

>>   DEBUG ((EFI_D_INFO, "     AllocAttr: %lx (%s%s)\n", Bridge->AllocationAttributes,

>>           (Bridge->AllocationAttributes & EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM) != 0 ? L"CombineMemPMem

>> " : L"",

>>           (Bridge->AllocationAttributes & EFI_PCI_HOST_BRIDGE_MEM64_DECODE) != 0 ? L"Mem64Decode" : L""

>> @@ -155,6 +156,7 @@ CreateRootBridge (

>>   RootBridge->Supports = Bridge->Supports;

>>   RootBridge->Attributes = Bridge->Attributes;

>>   RootBridge->DmaAbove4G = Bridge->DmaAbove4G;

>> +  RootBridge->NoExtendedConfigSpace = Bridge->NoExtendedConfigSpace;

>>   RootBridge->AllocationAttributes = Bridge->AllocationAttributes;

>>   RootBridge->DevicePath = DuplicateDevicePath (Bridge->DevicePath);

>>   RootBridge->DevicePathStr = DevicePathStr;

>> @@ -351,7 +353,7 @@ RootBridgeIoCheckParameter (

>>       Address = PciRbAddr->Register;

>>     }

>>     Base = 0;

>> -    Limit = 0xFFF;

>> +    Limit = RootBridge->NoExtendedConfigSpace ? 0xFF : 0xFFF;

>>   }

>>

>>   if (Address < Base) {

>> --

>> 1.8.3.1

> 

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

> 


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox

Patch

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
index b1e83f1c9089..aa3f43a511c4 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
@@ -72,6 +72,7 @@  typedef struct {
   PCI_ROOT_BRIDGE_APERTURE          MemAbove4G;
   PCI_ROOT_BRIDGE_APERTURE          PMemAbove4G;
   BOOLEAN                           DmaAbove4G;
+  BOOLEAN                           NoExtendedConfigSpace;
   VOID                              *ConfigBuffer;
   EFI_DEVICE_PATH_PROTOCOL          *DevicePath;
   CHAR16                            *DevicePathStr;
diff --git a/MdeModulePkg/Include/Library/PciHostBridgeLib.h b/MdeModulePkg/Include/Library/PciHostBridgeLib.h
index 16ad104a9368..b67ac5e17d48 100644
--- a/MdeModulePkg/Include/Library/PciHostBridgeLib.h
+++ b/MdeModulePkg/Include/Library/PciHostBridgeLib.h
@@ -34,6 +34,10 @@  typedef struct {
                                                   ///< and SetAttributes() in EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.
   BOOLEAN                  DmaAbove4G;            ///< DMA above 4GB memory.
                                                   ///< Set to TRUE when root bridge supports DMA above 4GB memory.
+  BOOLEAN                  NoExtendedConfigSpace; ///< When FALSE, the root bridge supports
+                                                  ///< Extended (4096-byte) Configuration Space.
+                                                  ///< When TRUE, the root bridge supports
+                                                  ///< 256-byte Configuration Space only.
   UINT64                   AllocationAttributes;  ///< Allocation attributes.
                                                   ///< Refer to EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM and
                                                   ///< EFI_PCI_HOST_BRIDGE_MEM64_DECODE used by GetAllocAttributes()
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index 932aefd5d621..cda9b49b3925 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -80,6 +80,7 @@  CreateRootBridge (
   DEBUG ((EFI_D_INFO, "%s\n", DevicePathStr = ConvertDevicePathToText (Bridge->DevicePath, FALSE, FALSE)));
   DEBUG ((EFI_D_INFO, "  Support/Attr: %lx / %lx\n", Bridge->Supports, Bridge->Attributes));
   DEBUG ((EFI_D_INFO, "    DmaAbove4G: %s\n", Bridge->DmaAbove4G ? L"Yes" : L"No"));
+  DEBUG ((EFI_D_INFO, "NoExtConfSpace: %s\n", Bridge->NoExtendedConfigSpace ? L"Yes" : L"No"));
   DEBUG ((EFI_D_INFO, "     AllocAttr: %lx (%s%s)\n", Bridge->AllocationAttributes,
           (Bridge->AllocationAttributes & EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM) != 0 ? L"CombineMemPMem " : L"",
           (Bridge->AllocationAttributes & EFI_PCI_HOST_BRIDGE_MEM64_DECODE) != 0 ? L"Mem64Decode" : L""
@@ -155,6 +156,7 @@  CreateRootBridge (
   RootBridge->Supports = Bridge->Supports;
   RootBridge->Attributes = Bridge->Attributes;
   RootBridge->DmaAbove4G = Bridge->DmaAbove4G;
+  RootBridge->NoExtendedConfigSpace = Bridge->NoExtendedConfigSpace;
   RootBridge->AllocationAttributes = Bridge->AllocationAttributes;
   RootBridge->DevicePath = DuplicateDevicePath (Bridge->DevicePath);
   RootBridge->DevicePathStr = DevicePathStr;
@@ -351,7 +353,7 @@  RootBridgeIoCheckParameter (
       Address = PciRbAddr->Register;
     }
     Base = 0;
-    Limit = 0xFFF;
+    Limit = RootBridge->NoExtendedConfigSpace ? 0xFF : 0xFFF;
   }
 
   if (Address < Base) {