[edk2,01/17] MdeModulePkg: PciHostBridgeDxe: don't assume extended config space

Message ID 1456532616-32475-2-git-send-email-lersek@redhat.com
State New
Headers show

Commit Message

Laszlo Ersek Feb. 27, 2016, 12:23 a.m.
The "PcAtChipsetPkg/PciHostBridgeDxe" driver hard-codes the [0x00..0xFF]
range as valid config space offsets, in the RootBridgeIoCheckParameter()
function -- see the MAX_PCI_REG_ADDRESS macro. This driver uses IO ports
0xCF8 / 0xCFC to access PCI config space.

The (soon to be removed) "OvmfPkg/PciHostBridgeDxe" does the same.

The "ArmVirtPkg/PciHostBridgeDxe" uses PCI Express (ECAM) instead, and
hard-codes the [0x00..0xFFF] range as valid config space offsets.

The "MdeModulePkg/Bus/Pci/PciHostBridgeDxe" driver hard-codes the
[0x00..0xFFF] range as valid config space offsets, without actually
knowing if the platform uses PCI Express (ECAM) or plain PCI (0xCF8 /
0xCFC). For this generalized driver, the config access method is
ultimately decided by the platform's PciSegmentLib instance.

Quoting the "MdePkg/Include/Library/PciSegmentLib.h" library class file:

  These functions perform PCI configuration cycles using the default PCI
  configuration access method.  This may use I/O ports 0xCF8 and 0xCFC to
  perform PCI configuration accesses, or it may use MMIO registers
  relative to the PcdPciExpressBaseAddress, or it may use some alternate
  access method. [...]

Clearly the configuration access method determines the boundaries of the
config space as well, for each individual PCI function. However, the
PciSegmentLib class provides no API for PciHostBridgeDxe to query these
boundaries!

In order to remedy this, add another PCI_ROOT_BRIDGE_APERTURE field to the
root bridge structures in both PciHostBridgeDxe and the PciHostBridgeLib
class. This way platforms can control the PCI config space boundaries for
the purposes of PciHostBridgeDxe, in accordance with their config access
methods.

This bug causes the following symptoms with OVMF and QEMU:

QEMU's i440fx machine type is a PCI (0xCF8 / 0xCFC) machine. It supports
physical device assignment. When assigning a physical EVGA GTX750 GPU to
the guest, the PCI bus driver correctly determines that the GPU is a PCIe
device, and tries to locate its extended capabilities (specifically, ARI),
starting at config space offset 0x100.

When OVMF is built with "OvmfPkg/PciHostBridgeDxe", this config access is
caught and rejected by RootBridgeIoCheckParameter(), due to the limit
being 0xFF. (And the PCI bus driver gracefully recovers from this
rejection.)

However, "MdeModulePkg/Bus/Pci/PciHostBridgeDxe" lets the address through,
because its variant of the same function hard-codes 0xFFF as the config
space limit.

Then RootBridgeIoPciAccess() sends the (invalid) address down the
following chain of libraries:

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

Finally, ASSERT_INVALID_PCI_ADDRESS() in PciCf8Read32() realizes that
offset 0x100 is invalid for the 0xCF8 / 0xCFC config access method, and
the firmware halts.

Fix this by informing the generalized PciHostBridgeDxe driver about the
platform's config space boundaries, so that the driver can recognize and
reject out-of-bounds accesses in time.

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

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

-- 
1.8.3.1


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

Comments

Laszlo Ersek Feb. 27, 2016, 10:17 a.m. | #1
On 02/27/16 03:13, Ni, Ruiyu wrote:
> Basically I don't agree to add a CFG space range field.

> See embedded reply in below.

> 

> Regards,

> Ray

> 

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

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

>> Sent: Saturday, February 27, 2016 8:23 AM

>> To: edk2-devel-01 <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>

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

>>

>> The "PcAtChipsetPkg/PciHostBridgeDxe" driver hard-codes the [0x00..0xFF]

>> range as valid config space offsets, in the RootBridgeIoCheckParameter()

>> function -- see the MAX_PCI_REG_ADDRESS macro. This driver uses IO ports

>> 0xCF8 / 0xCFC to access PCI config space.

>>

>> The (soon to be removed) "OvmfPkg/PciHostBridgeDxe" does the same.

>>

>> The "ArmVirtPkg/PciHostBridgeDxe" uses PCI Express (ECAM) instead, and

>> hard-codes the [0x00..0xFFF] range as valid config space offsets.

>>

>> The "MdeModulePkg/Bus/Pci/PciHostBridgeDxe" driver hard-codes the

>> [0x00..0xFFF] range as valid config space offsets, without actually

>> knowing if the platform uses PCI Express (ECAM) or plain PCI (0xCF8 /

>> 0xCFC). For this generalized driver, the config access method is

>> ultimately decided by the platform's PciSegmentLib instance.

>>

>> Quoting the "MdePkg/Include/Library/PciSegmentLib.h" library class file:

>>

>>  These functions perform PCI configuration cycles using the default PCI

>>  configuration access method.  This may use I/O ports 0xCF8 and 0xCFC to

>>  perform PCI configuration accesses, or it may use MMIO registers

>>  relative to the PcdPciExpressBaseAddress, or it may use some alternate

>>  access method. [...]

>>

>> Clearly the configuration access method determines the boundaries of the

>> config space as well, for each individual PCI function. However, the

>> PciSegmentLib class provides no API for PciHostBridgeDxe to query these

>> boundaries!

>>

>> In order to remedy this, add another PCI_ROOT_BRIDGE_APERTURE field to the

>> root bridge structures in both PciHostBridgeDxe and the PciHostBridgeLib

>> class. This way platforms can control the PCI config space boundaries for

>> the purposes of PciHostBridgeDxe, in accordance with their config access

>> methods.

>>

>> This bug causes the following symptoms with OVMF and QEMU:

>>

>> QEMU's i440fx machine type is a PCI (0xCF8 / 0xCFC) machine. It supports

>> physical device assignment. When assigning a physical EVGA GTX750 GPU to

>> the guest, the PCI bus driver correctly determines that the GPU is a PCIe

>> device, and tries to locate its extended capabilities (specifically, ARI),

>> starting at config space offset 0x100.

>>

>> When OVMF is built with "OvmfPkg/PciHostBridgeDxe", this config access is

>> caught and rejected by RootBridgeIoCheckParameter(), due to the limit

>> being 0xFF. (And the PCI bus driver gracefully recovers from this

>> rejection.)

> 

> Does the platform expect the graceful recovery? I may treat it as a silent failure:)

> Some additional devices cannot come up due to this recovery/failure but platform

> developer without deep debugging cannot know the root cause, if ARI is disabled.

> 

>>

>> However, "MdeModulePkg/Bus/Pci/PciHostBridgeDxe" lets the address through,

>> because its variant of the same function hard-codes 0xFFF as the config

>> space limit.

>>

>> Then RootBridgeIoPciAccess() sends the (invalid) address down the

>> following chain of libraries:

>>

>>  BasePciSegmentLibPci [class: PciSegmentLib]

>>    BasePciLibCf8      [class: PciLib]

>>      BasePciCf8Lib    [class: PciCf8Lib]

>>

>> Finally, ASSERT_INVALID_PCI_ADDRESS() in PciCf8Read32() realizes that

>> offset 0x100 is invalid for the 0xCF8 / 0xCFC config access method, and

>> the firmware halts.

> 

> The silent failure is bad. So the assertion is a good to tell platform developer

> to choose the correct PciSegment library instance.


I agree that the assertion is good. BasePciCf8Lib should indeed catch
invalid addresses, and complain *loudly*.

However, my patch is not about suppressing this assertion. My patch
corrects an error in the RootBridgeIoCheckParameter() function in
"MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c".

The error in the RootBridgeIoCheckParameter() function is that it
accepts PCI config space accesses as valid up to register offset 0xFFF,
even if the platform doesn't support that. On a platform where only the
0xCF8 access method is available, accepting config space offsets up to
0xFFF is a bug. On such a platform, offsets >= 0x100 should be rejected
as invalid. That's all.

The rest of the commit message just describes how this error in
RootBridgeIoCheckParameter() leads to a failed assertion later. The
failed assertion is *justified*. It is the job of
RootBridgeIoCheckParameter() to recognize and catch the incorrect offset
*earlier*.

So, hard-coding 0xFFF in RootBridgeIoCheckParameter() is the bug. The
platform must be able to provide that value as a parameter. That's why I
listed the other host bridge drivers in the commit message, as examples:
in those drivers, the config access method and the maximum config offset
are *in sync*. In the case of this driver however, they are out of sync,
if the PciSegmentLib library can only provide 0xCF8 access.

Your suggestion that the platform developer choose the "correct"
PciSegmentLib instance is not valid -- the platform developer has zero
liberty in "choosing" the library. The platform (the hardware)
*dictates* that choice. If the platform is only capable of 0xCF8, and
incapable of PCI Express (ECAM), then 0xFFF as a config space limit is
invalid, plain and simple, and the PciSegmentLib *instance* cannot do
anything about it.

*Normally*, it would be the job of the PciSegmentLib *class* to provide
an interface where the maximum config space offset can be queried. Then
PciHostBridgeDxe could call that interface -- which would return 0xFF,
0xFFF, or whatever else matching the internals of the PciSegmentLib
*instance* -- and RootBridgeIoCheckParameter() could enforce *that* limit.

However, the PciSegmentLib *class* lacks such an API. So we need another
way to tell the limit to PciHostBridgeDxe.

Thus, this argument comprises two parts:

1. PciHostBridgeDxe needs to use a platform dependent config space
   limit: 0xFF, 0xFFF, or something else that matches the platform's
   PciSegmentLib instance. Correcting this bug is not optional, it is
   unavoidable. Hard-coding an ECAM assumption in the driver is a bug.

2. The other question is *whence exactly* this information should come.
   There are several alternatives here; let me list a few:

  * Add a new API to PciSegmentLib that returns the limit.

  * Or, add a new API to PciHostBridgeLib that returns the limit.

  * Or, add a new PCD that communicates the limit.

  * Or, in the driver, investigate PcdPciExpressBaseAddress (which is
    the central PCD for the base of the MMCONFIG / ECAM area). If the
    value is MAX_UINT64 -- which is obviously unusable as ECAM --, then
    assume a limit of 0xFF. Otherwise, assume 0xFFF.

  * Or, pass the limit as part of the structure that PciHostBridgeLib
    already produces. This patch implements this alternative.

I'm absolutely fine with any one of the options under (2). But, (1) must
be fixed in *some* way.

Thanks
Laszlo

> 

>>

>> Fix this by informing the generalized PciHostBridgeDxe driver about the

>> platform's config space boundaries, so that the driver can recognize and

>> reject out-of-bounds accesses in time.

>>

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

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

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

>> Contributed-under: TianoCore Contribution Agreement 1.0

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

>> ---

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

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

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

>> 3 files changed, 6 insertions(+), 2 deletions(-)

>>

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

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

>> index b1e83f1c9089..2dfc8963f8e9 100644

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

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

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

>>   PCI_ROOT_BRIDGE_APERTURE          PMem;

>>   PCI_ROOT_BRIDGE_APERTURE          MemAbove4G;

>>   PCI_ROOT_BRIDGE_APERTURE          PMemAbove4G;

>> +  PCI_ROOT_BRIDGE_APERTURE          Pci;

>>   BOOLEAN                           DmaAbove4G;

>>   VOID                              *ConfigBuffer;

>>   EFI_DEVICE_PATH_PROTOCOL          *DevicePath;

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

>> index b1dba0f754d7..a9e9ca308c7c 100644

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

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

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

>>   PCI_ROOT_BRIDGE_APERTURE MemAbove4G;           ///< MMIO aperture above 4GB which can be used by the

>> root bridge.

>>   PCI_ROOT_BRIDGE_APERTURE PMem;                 ///< Prefetchable MMIO aperture below 4GB which can be

>> used by the root bridge.

>>   PCI_ROOT_BRIDGE_APERTURE PMemAbove4G;          ///< Prefetchable MMIO aperture above 4GB which can be

>> used by the root bridge.

>> +  PCI_ROOT_BRIDGE_APERTURE Pci;                  ///< PCI config space range that is valid for the devices behind

>> the root bridge.

>>   EFI_DEVICE_PATH_PROTOCOL *DevicePath;          ///< Device path.

>> } PCI_ROOT_BRIDGE;

>>

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

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

>> index 332860eb3819..6b7bd74290a7 100644

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

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

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

>>   DEBUG ((EFI_D_INFO, "  MemAbove4G: %lx - %lx\n", Bridge->MemAbove4G.Base, Bridge->MemAbove4G.Limit));

>>   DEBUG ((EFI_D_INFO, "        PMem: %lx - %lx\n", Bridge->PMem.Base, Bridge->PMem.Limit));

>>   DEBUG ((EFI_D_INFO, " PMemAbove4G: %lx - %lx\n", Bridge->PMemAbove4G.Base, Bridge->PMemAbove4G.Limit));

>> +  DEBUG ((EFI_D_INFO, "         Pci: %lx - %lx\n", Bridge->Pci.Base, Bridge->Pci.Limit));

>>

>>   //

>>   // Make sure Mem and MemAbove4G apertures are valid

>> @@ -168,6 +169,7 @@ CreateRootBridge (

>>   CopyMem (&RootBridge->Io, &Bridge->Io, sizeof (PCI_ROOT_BRIDGE_APERTURE));

>>   CopyMem (&RootBridge->Mem, &Bridge->Mem, sizeof (PCI_ROOT_BRIDGE_APERTURE));

>>   CopyMem (&RootBridge->MemAbove4G, &Bridge->MemAbove4G, sizeof (PCI_ROOT_BRIDGE_APERTURE));

>> +  CopyMem (&RootBridge->Pci, &Bridge->Pci, sizeof (PCI_ROOT_BRIDGE_APERTURE));

>>

>>

>>   for (Index = TypeIo; Index < TypeMax; Index++) {

>> @@ -350,8 +352,8 @@ RootBridgeIoCheckParameter (

>>     } else {

>>       Address = PciRbAddr->Register;

>>     }

>> -    Base = 0;

>> -    Limit = 0xFFF;

>> +    Base = RootBridge->Pci.Base;

>> +    Limit = RootBridge->Pci.Limit;

>>   }

>>

>>   if (Address < Base) {

>> --

>> 1.8.3.1

>>

> 


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Feb. 29, 2016, noon | #2
On 02/28/16 08:16, Ni, Ruiyu wrote:

>>> 在 2016年2月27日,下午6:17,Laszlo Ersek <lersek@redhat.com> 写道:

>> Thus, this argument comprises two parts:
>>
>> 1. PciHostBridgeDxe needs to use a platform dependent config space
>>   limit: 0xFF, 0xFFF, or something else that matches the platform's
>>   PciSegmentLib instance. Correcting this bug is not optional, it is
>>   unavoidable. Hard-coding an ECAM assumption in the driver is a bug.

> You didn't convince me here but given the fact you told me that qemu
> is too old to support pci express way access,

Small clarification: the "motherboard (chipset) services" that QEMU provides *vary*. First, you have to pick an architecture to emulate (i386, x86_64, arm, aarch64, and so on). This is called "target". The different targets are built into different QEMU executables. For system emulation, they look like:

- qemu-system-x86_64
- qemu-system-aarch64
- etc

Once you picked the binary (i.e., you selected the target), QEMU provides several "machine types" for that target. The targets can be listed with "-machine help":

  $ qemu-system-x86_64 -machine help

  Supported machines are:
  pc                   Standard PC (i440FX + PIIX, 1996) (alias of pc-i440fx-2.6)
  pc-i440fx-2.6        Standard PC (i440FX + PIIX, 1996) (default)
  pc-i440fx-2.5        Standard PC (i440FX + PIIX, 1996)
  pc-i440fx-2.4        Standard PC (i440FX + PIIX, 1996)
  pc-i440fx-2.3        Standard PC (i440FX + PIIX, 1996)
  pc-i440fx-2.2        Standard PC (i440FX + PIIX, 1996)
  pc-i440fx-2.1        Standard PC (i440FX + PIIX, 1996)
  pc-i440fx-2.0        Standard PC (i440FX + PIIX, 1996)
  pc-i440fx-1.7        Standard PC (i440FX + PIIX, 1996)
  pc-i440fx-1.6        Standard PC (i440FX + PIIX, 1996)
  pc-i440fx-1.5        Standard PC (i440FX + PIIX, 1996)
  pc-i440fx-1.4        Standard PC (i440FX + PIIX, 1996)
  pc-1.3               Standard PC (i440FX + PIIX, 1996)
  pc-1.2               Standard PC (i440FX + PIIX, 1996)
  pc-1.1               Standard PC (i440FX + PIIX, 1996)
  pc-1.0               Standard PC (i440FX + PIIX, 1996)
  pc-0.15              Standard PC (i440FX + PIIX, 1996)
  pc-0.14              Standard PC (i440FX + PIIX, 1996)
  pc-0.13              Standard PC (i440FX + PIIX, 1996)
  pc-0.12              Standard PC (i440FX + PIIX, 1996)
  pc-0.11              Standard PC (i440FX + PIIX, 1996)
  pc-0.10              Standard PC (i440FX + PIIX, 1996)
  q35                  Standard PC (Q35 + ICH9, 2009) (alias of pc-q35-2.6)
  pc-q35-2.6           Standard PC (Q35 + ICH9, 2009)
  pc-q35-2.5           Standard PC (Q35 + ICH9, 2009)
  pc-q35-2.4           Standard PC (Q35 + ICH9, 2009)
  pc-q35-2.3           Standard PC (Q35 + ICH9, 2009)
  pc-q35-2.2           Standard PC (Q35 + ICH9, 2009)
  pc-q35-2.1           Standard PC (Q35 + ICH9, 2009)
  pc-q35-2.0           Standard PC (Q35 + ICH9, 2009)
  pc-q35-1.7           Standard PC (Q35 + ICH9, 2009)
  pc-q35-1.6           Standard PC (Q35 + ICH9, 2009)
  pc-q35-1.5           Standard PC (Q35 + ICH9, 2009)
  pc-q35-1.4           Standard PC (Q35 + ICH9, 2009)
  isapc                ISA-only PC
  none                 empty machine

You can identify two facts from this output.

First, some machine types are versioned. This is related to migration compatibility, and not very interesting here.

The other fact is that there are two machine type "families", i440fx, and q35. Those are quite different.

i440fx is the older machine type. It lacks a number of things that Q35 provides. For example:
- it lacks ECAM / MMCONFIG
- it also lacks SMM.
  (Remember <http://thread.gmane.org/gmane.comp.bios.edk2.devel/8078>?)

Q35 is the newer, more modern machine type. It does have ECAM / MMCONFIG, but Q35 is not the default machine type. i440fx is way more "mature"; it is almost universally used by users. For example, our own RHEL-7.2 release does not officially support Q35 machine types.

Hence, the clarification I'd like to make is: QEMU (the project) is definitely not too old to support ECAM. Instead, the i440fx machine type in particular, emulated by QEMU, is too old (2016 - 1996 = 20 years).

For another example, the "virt" machine type of "qemu-system-aarch64" emulates ECAM as well. (And "ArmVirtPkg/PciHostBridgeDxe" uses ECAM.)

So, it is not QEMU in general that is limited to 0xCF8 / 0xCFC; it is one specific (emulation target, machine type) pair that has this limitation.

i440fx happens to be the most widely used machine type. I'm not aware of any specific plans or timeline to switch the default to Q35.

> I agree I didn't think
> there might be a such old platform.

Yes, the virtual hardware of the i440fx machine type mostly reflects 1996, as the help output above says.

It is based on the "Intel ® 82371AB PIIX4" and the "Intel ® 440FX PCIset 82441FX (PMC)" specs. The PDFs that I have on my disk for understanding and occasionally developing i440fx are:

- document # 290549-001
- document # 290562-001
- document # 297654-006
- document # 297738-017

(They are listed in "OvmfPkg/Include/IndustryStandard/I440FxPiix4.h" as well.)

> So enhancing my driver to support
> it is acceptable.

Thank you. I hugely appreciate it.

>> 2. The other question is *whence exactly* this information should come.
>>   There are several alternatives here; let me list a few:
>>
>>  * Add a new API to PciSegmentLib that returns the limit.
> Not good because then pci lib also needs such api.
>>
>>  * Or, add a new API to PciHostBridgeLib that returns the limit.

> Intend to use this way. In a extreme case, a platform may contain two
> root bridges, one support MMIO cfg space access, the other doesn't.
> So we may need to use the way you proposed. But I think a flag is
> enough rather than a aperture range. Did you see platform that has
> range other than [0,ff] [0,fff]?

I'm not aware of anything different from 0x0-0xff and 0x0-0xfff.

Therefore a simple boolean flag (per root bridge structure) could do the job indeed.

I will update my patch and submit it separately, for your review.

>>
>>  * Or, add a new PCD that communicates the limit.
>>
>>  * Or, in the driver, investigate PcdPciExpressBaseAddress (which is
>>    the central PCD for the base of the MMCONFIG / ECAM area). If the
>>    value is MAX_UINT64 -- which is obviously unusable as ECAM --, then
>>    assume a limit of 0xFF. Otherwise, assume 0xFFF.
> A multi segment platform may also set such PCD to an invalid value because there are multiple base addresses.

Good point!

Thank you!
Laszlo

Patch

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
index b1e83f1c9089..2dfc8963f8e9 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
@@ -71,6 +71,7 @@  typedef struct {
   PCI_ROOT_BRIDGE_APERTURE          PMem;
   PCI_ROOT_BRIDGE_APERTURE          MemAbove4G;
   PCI_ROOT_BRIDGE_APERTURE          PMemAbove4G;
+  PCI_ROOT_BRIDGE_APERTURE          Pci;
   BOOLEAN                           DmaAbove4G;
   VOID                              *ConfigBuffer;
   EFI_DEVICE_PATH_PROTOCOL          *DevicePath;
diff --git a/MdeModulePkg/Include/Library/PciHostBridgeLib.h b/MdeModulePkg/Include/Library/PciHostBridgeLib.h
index b1dba0f754d7..a9e9ca308c7c 100644
--- a/MdeModulePkg/Include/Library/PciHostBridgeLib.h
+++ b/MdeModulePkg/Include/Library/PciHostBridgeLib.h
@@ -44,6 +44,7 @@  typedef struct {
   PCI_ROOT_BRIDGE_APERTURE MemAbove4G;           ///< MMIO aperture above 4GB which can be used by the root bridge.
   PCI_ROOT_BRIDGE_APERTURE PMem;                 ///< Prefetchable MMIO aperture below 4GB which can be used by the root bridge.
   PCI_ROOT_BRIDGE_APERTURE PMemAbove4G;          ///< Prefetchable MMIO aperture above 4GB which can be used by the root bridge.
+  PCI_ROOT_BRIDGE_APERTURE Pci;                  ///< PCI config space range that is valid for the devices behind the root bridge.
   EFI_DEVICE_PATH_PROTOCOL *DevicePath;          ///< Device path.
 } PCI_ROOT_BRIDGE;
 
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index 332860eb3819..6b7bd74290a7 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -90,6 +90,7 @@  CreateRootBridge (
   DEBUG ((EFI_D_INFO, "  MemAbove4G: %lx - %lx\n", Bridge->MemAbove4G.Base, Bridge->MemAbove4G.Limit));
   DEBUG ((EFI_D_INFO, "        PMem: %lx - %lx\n", Bridge->PMem.Base, Bridge->PMem.Limit));
   DEBUG ((EFI_D_INFO, " PMemAbove4G: %lx - %lx\n", Bridge->PMemAbove4G.Base, Bridge->PMemAbove4G.Limit));
+  DEBUG ((EFI_D_INFO, "         Pci: %lx - %lx\n", Bridge->Pci.Base, Bridge->Pci.Limit));
 
   //
   // Make sure Mem and MemAbove4G apertures are valid
@@ -168,6 +169,7 @@  CreateRootBridge (
   CopyMem (&RootBridge->Io, &Bridge->Io, sizeof (PCI_ROOT_BRIDGE_APERTURE));
   CopyMem (&RootBridge->Mem, &Bridge->Mem, sizeof (PCI_ROOT_BRIDGE_APERTURE));
   CopyMem (&RootBridge->MemAbove4G, &Bridge->MemAbove4G, sizeof (PCI_ROOT_BRIDGE_APERTURE));
+  CopyMem (&RootBridge->Pci, &Bridge->Pci, sizeof (PCI_ROOT_BRIDGE_APERTURE));
 
 
   for (Index = TypeIo; Index < TypeMax; Index++) {
@@ -350,8 +352,8 @@  RootBridgeIoCheckParameter (
     } else {
       Address = PciRbAddr->Register;
     }
-    Base = 0;
-    Limit = 0xFFF;
+    Base = RootBridge->Pci.Base;
+    Limit = RootBridge->Pci.Limit;
   }
 
   if (Address < Base) {