Laszlo, How about using a flag NoExtendedConfigSpace? Since for most cases the PCI CFG space limit is 0xFFF, so I think it's better to choose a flag whose default value meets most of the cases. For a Boolean flag inside a structure when the structure is allocated using AllocateZeroPool, the flag value is FALSE.
Regards, Ray From: Marcel Apfelbaum [mailto:[email protected]] Sent: Tuesday, March 1, 2016 1:30 AM To: Laszlo Ersek <[email protected]>; [email protected] Cc: Ni, Ruiyu <[email protected]>; Justen, Jordan L <[email protected]> Subject: Re: [PATCH v2] MdeModulePkg: PciHostBridgeDxe: don't assume extended config space On 02/29/2016 03:15 PM, Laszlo Ersek wrote: > 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 <[email protected]<mailto:[email protected]>> > Cc: Jordan Justen > <[email protected]<mailto:[email protected]>> > Cc: Marcel Apfelbaum <[email protected]<mailto:[email protected]>> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <[email protected]<mailto:[email protected]>> > --- > > Notes: > 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..f9839fed4628 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 ExtendedConfigSpace; > VOID *ConfigBuffer; > EFI_DEVICE_PATH_PROTOCOL *DevicePath; > CHAR16 *DevicePathStr; > diff --git a/MdeModulePkg/Include/Library/PciHostBridgeLib.h > b/MdeModulePkg/Include/Library/PciHostBridgeLib.h > index b1dba0f754d7..fbf3be860c13 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 ExtendedConfigSpace; ///< When TRUE, the root > bridge supports > + ///< Extended (4096-byte) > Configuration Space. > + ///< When FALSE, 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 332860eb3819..b0b1a2c18cca 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, "ExtConfSpace: %s\n", Bridge->ExtendedConfigSpace ? > 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->ExtendedConfigSpace = Bridge->ExtendedConfigSpace; > 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->ExtendedConfigSpace ? 0xFFF : 0xFF; > } > > if (Address < Base) { > With my limited OVMF knowledge it looks OK to me. Reviewed-by: Marcel Apfelbaum <[email protected]<mailto:[email protected]>> Thanks, Marcel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
