My recommendations would be: OvmfPkg/Include/Library/PciHostBridge.h => OvmfPkg/Include/OvmfPlatforms.h
Add DEVIDs to OvmfPkg/Include/OvmfPlatforms.h. Drop IS_Q35_HOSTBRIDGE, PCI_PM_REG. Maybe add Q35_PM_DEVICE=0x1f and PIIX4_PM_DEVICE=1. Add 2 Dynamic PCDs: * PcdPlatformHostBridgePciDevId * PcdPlatformPmDevice Set these PCDs in OvmfPkg/PlatformPei. OvmfPkg/Library/AcpiTimerLib/AcpiTimerLib.inf can be changed to read the PCD in a constructor and store the PM Device in a global variable. In OvmfPkg*.dsc: * Set LibraryClasses.common.PEIM to use MdePkg/Library/PeiPcdLib/PeiPcdLib.inf * Replace BasePcdLibNull.inf with DxePcdLib.inf as needed in DXE/UEFI LibraryClasses. (This paragraph is a bit tangential. Feel free to ignore it. :) I think this is not quite valid for pure UEFI drivers, since they should not rely on the PCD protocol. But, I think that OVMF as a platform can bend this rule. The problem is that OVMF's TimerLib would rely on the PCD protocol. We could make a separate UEFI version of the TimerLib that read the device ID and determine the PM device num rather than going through the PCD. Can you use PCI_DEVICE_ID_OFFSET rather than 2? It seems like breaking this into more than 1 patch would be appropriate. Thanks! -Jordan On 2014-09-27 15:06:45, Gabriel L. Somlo wrote: > Factor out logic to distinguish between Q35 and PIIX4 platforms > into a separate header (OvmfPkg/Include/Library/PciHostBridge.h), > then refer to these macros from all relevant source locations > throughouth OvmfPkg. > > This patch also adds a Q35-specific PIC IRQ routing initialization > function to OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Gabriel Somlo <so...@cmu.edu> > --- > > As discussed with Laszlo and Paolo in an earlier thread, this stuff should > hopefully be mostly uncontroversial and "the right thing to do" to support > Q35. Doesn't fix my weird OS X / Q35 / UHCI issues, but I guess that's a > whole different fight to be fought another day :) > > The one thing I'm a bit unsure about is PciInitializationQ35 (BdsPlatform.c); > In PciInitializationPIIX (which used to be plain PciInitialization before > this patch), there's a whole bunch of other devices (ide, video, network, > etc.) being initialized by writing to registers which should mostly be > read-only on Q35; Leaving that stuff out from PciInitializationQ35 doesn't > seem to have any effect, negative or otherwise, so I decided to stick to > LNK* routing initialization only. > > Any comments/suggestions much appreciated, as always. > > Thanks, > Gabriel > > OvmfPkg/Csm/CsmSupportLib/LegacyInterrupt.h | 4 +- > OvmfPkg/Include/Library/PciHostBridge.h | 44 ++++++++++++++++ > OvmfPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 78 > ++-------------------------- > OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 36 ++++++++++++- > OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h | 1 + > OvmfPkg/PlatformPei/Platform.c | 9 ++-- > 6 files changed, 90 insertions(+), 82 deletions(-) > create mode 100644 OvmfPkg/Include/Library/PciHostBridge.h > > diff --git a/OvmfPkg/Csm/CsmSupportLib/LegacyInterrupt.h > b/OvmfPkg/Csm/CsmSupportLib/LegacyInterrupt.h > index 193e48b..712e8d5 100644 > --- a/OvmfPkg/Csm/CsmSupportLib/LegacyInterrupt.h > +++ b/OvmfPkg/Csm/CsmSupportLib/LegacyInterrupt.h > @@ -23,9 +23,11 @@ > #include <Library/PciLib.h> > #include <Library/DebugLib.h> > #include <Library/UefiBootServicesTableLib.h> > +#include <Library/PciHostBridge.h> > + > > #define LEGACY_INT_BUS 0 > -#define LEGACY_INT_DEV 1 > +#define LEGACY_INT_DEV (IS_Q35_HOSTBRIDGE ? 0x1f : 1) > #define LEGACY_INT_FUNC 0 > > #define PIRQN 0x00 // PIRQ Null > diff --git a/OvmfPkg/Include/Library/PciHostBridge.h > b/OvmfPkg/Include/Library/PciHostBridge.h > new file mode 100644 > index 0000000..f85a413 > --- /dev/null > +++ b/OvmfPkg/Include/Library/PciHostBridge.h > @@ -0,0 +1,44 @@ > +/** @file > + PCI Host Bridge (PIIX4 vs. Q35) dependent device access > + > + Copyright (c) 2014, Gabriel L. Somlo <so...@cmu.edu> > + > + This program and the accompanying materials are licensed and made > + available under the terms and conditions of the BSD License which > + accompanies this distribution. The full text of the license may > + be found at http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > IMPLIED. > +**/ > + > +#ifndef __PCI_HOST_BRIDGE__ > +#define __PCI_HOST_BRIDGE__ > + > +#include <Library/PciLib.h> > + > +// > +// Host Bridge Device ID (DID) register values > +// > +#define PCI_DEVICE_ID_INTEL_82441 0x1237 // PIIX4 > +#define PCI_DEVICE_ID_INTEL_Q35_MCH 0x29C0 // Q35 > + > +// > +// Access Host Bridge Device ID (00:00.0, offset 0x02) > +// > +#define HOSTBRIDGE_DID PCI_LIB_ADDRESS (0, 0, 0, 0x02) > + > +// > +// Test if we're on a Q35/MCH Host Bridge (assume PIIX4 if false) > +// > +#define IS_Q35_HOSTBRIDGE (PciRead16 (HOSTBRIDGE_DID) == > PCI_DEVICE_ID_INTEL_Q35_MCH) > + > +// > +// Access ACPI Power Management register (device 00:1f.0 on Q35, 00:01.3 on > PIIX) > +// > +#define PCI_PM_REG(Register) \ > + (IS_Q35_HOSTBRIDGE ? \ > + PCI_LIB_ADDRESS (0, 0x1f, 0, Register) : PCI_LIB_ADDRESS (0, 1, 3, > Register)) > + > + > +#endif > diff --git a/OvmfPkg/Library/AcpiTimerLib/AcpiTimerLib.c > b/OvmfPkg/Library/AcpiTimerLib/AcpiTimerLib.c > index 7d324cb..52b2979 100644 > --- a/OvmfPkg/Library/AcpiTimerLib/AcpiTimerLib.c > +++ b/OvmfPkg/Library/AcpiTimerLib/AcpiTimerLib.c > @@ -19,90 +19,18 @@ > #include <Library/BaseLib.h> > #include <Library/IoLib.h> > #include <Library/PciLib.h> > +#include <Library/PciHostBridge.h> > #include <Library/DebugLib.h> > #include <Library/PcdLib.h> > #include <IndustryStandard/Pci22.h> > #include <IndustryStandard/Acpi.h> > > // > -// PCI Location of PIIX4 Power Management PCI Configuration Registers > -// > -#define PIIX4_POWER_MANAGEMENT_BUS 0x00 > -#define PIIX4_POWER_MANAGEMENT_DEVICE 0x01 > -#define PIIX4_POWER_MANAGEMENT_FUNCTION 0x03 > - > -// > -// Macro to access PIIX4 Power Management PCI Configuration Registers > -// > -#define PIIX4_PCI_POWER_MANAGEMENT_REGISTER(Register) \ > - PCI_LIB_ADDRESS ( \ > - PIIX4_POWER_MANAGEMENT_BUS, \ > - PIIX4_POWER_MANAGEMENT_DEVICE, \ > - PIIX4_POWER_MANAGEMENT_FUNCTION, \ > - Register \ > - ) > - > -// > -// PCI Location of Q35 Power Management PCI Configuration Registers > -// > -#define Q35_POWER_MANAGEMENT_BUS 0x00 > -#define Q35_POWER_MANAGEMENT_DEVICE 0x1f > -#define Q35_POWER_MANAGEMENT_FUNCTION 0x00 > - > -// > -// Macro to access Q35 Power Management PCI Configuration Registers > -// > -#define Q35_PCI_POWER_MANAGEMENT_REGISTER(Register) \ > - PCI_LIB_ADDRESS ( \ > - Q35_POWER_MANAGEMENT_BUS, \ > - Q35_POWER_MANAGEMENT_DEVICE, \ > - Q35_POWER_MANAGEMENT_FUNCTION, \ > - Register \ > - ) > - > -// > -// PCI Location of Host Bridge PCI Configuration Registers > -// > -#define HOST_BRIDGE_BUS 0x00 > -#define HOST_BRIDGE_DEVICE 0x00 > -#define HOST_BRIDGE_FUNCTION 0x00 > - > -// > -// Macro to access Host Bridge Configuration Registers > -// > -#define HOST_BRIDGE_REGISTER(Register) \ > - PCI_LIB_ADDRESS ( \ > - HOST_BRIDGE_BUS, \ > - HOST_BRIDGE_DEVICE, \ > - HOST_BRIDGE_FUNCTION, \ > - Register \ > - ) > - > -// > -// Host Bridge Device ID (DID) Register > -// > -#define HOST_BRIDGE_DID HOST_BRIDGE_REGISTER (0x02) > - > -// > -// Host Bridge DID Register values > -// > -#define PCI_DEVICE_ID_INTEL_82441 0x1237 // DID value for PIIX4 > -#define PCI_DEVICE_ID_INTEL_Q35_MCH 0x29C0 // DID value for Q35 > - > -// > -// Access Power Management PCI Config Regs based on Host Bridge type > -// > -#define PCI_POWER_MANAGEMENT_REGISTER(Register) \ > - ((PciRead16 (HOST_BRIDGE_DID) == PCI_DEVICE_ID_INTEL_Q35_MCH) ? \ > - Q35_PCI_POWER_MANAGEMENT_REGISTER (Register) : \ > - PIIX4_PCI_POWER_MANAGEMENT_REGISTER (Register)) > - > -// > // Power Management PCI Configuration Registers > // > -#define PMBA PCI_POWER_MANAGEMENT_REGISTER (0x40) > +#define PMBA PCI_PM_REG (0x40) > #define PMBA_RTE BIT0 > -#define PMREGMISC PCI_POWER_MANAGEMENT_REGISTER (0x80) > +#define PMREGMISC PCI_PM_REG (0x80) > #define PMIOSE BIT0 > > // > diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c > b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c > index 2a1ca88..75efbc2 100644 > --- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c > +++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c > @@ -716,8 +716,9 @@ Returns: > } > > > +STATIC > VOID > -PciInitialization ( > +PciInitializationPIIX ( > ) > { > // > @@ -765,6 +766,37 @@ PciInitialization ( > } > > > +STATIC > +VOID > +PciInitializationQ35 ( > + ) > +{ > + // > + // Bus 0, Device 0x1f, Function 0 - LPC Bridge: Initialize PIC IRQ routing > + // > + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x60), 0x0a); // LNKA routing > target > + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x61), 0x0a); // LNKB routing > target > + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x62), 0x0b); // LNKC routing > target > + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x63), 0x0b); // LNKD routing > target > + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x68), 0x0a); // LNKE routing > target > + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x69), 0x0a); // LNKF routing > target > + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x6a), 0x0b); // LNKG routing > target > + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x6b), 0x0b); // LNKH routing > target > +} > + > + > +VOID > +PciInitialization ( > + ) > +{ > + if (IS_Q35_HOSTBRIDGE) { > + PciInitializationQ35 (); > + } else { > + PciInitializationPIIX (); > + } > +} > + > + > VOID > AcpiInitialization ( > VOID > @@ -773,7 +805,7 @@ AcpiInitialization ( > // > // Set ACPI SCI_EN bit in PMCNTRL > // > - IoOr16 ((PciRead32 (PCI_LIB_ADDRESS (0, 1, 3, 0x40)) & ~BIT0) + 4, BIT0); > + IoOr16 ((PciRead32 (PCI_PM_REG (0x40)) & ~BIT0) + 4, BIT0); > } > > > diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h > b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h > index 72b0e14..3a3bc0c 100644 > --- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h > +++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.h > @@ -39,6 +39,7 @@ Abstract: > #include <Library/BaseLib.h> > #include <Library/PcdLib.h> > #include <Library/PciLib.h> > +#include <Library/PciHostBridge.h> > #include <Library/GenericBdsLib.h> > #include <Library/PlatformBdsLib.h> > #include <Library/HobLib.h> > diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c > index 11b4cb7..21adab8 100644 > --- a/OvmfPkg/PlatformPei/Platform.c > +++ b/OvmfPkg/PlatformPei/Platform.c > @@ -28,6 +28,7 @@ > #include <Library/MemoryAllocationLib.h> > #include <Library/PcdLib.h> > #include <Library/PciLib.h> > +#include <Library/PciHostBridge.h> > #include <Library/PeimEntryPoint.h> > #include <Library/PeiServicesLib.h> > #include <Library/QemuFwCfgLib.h> > @@ -243,13 +244,13 @@ MiscInitialization ( > // example by Xen) and skip the setup here. This matches the logic in > // AcpiTimerLibConstructor (). > // > - if ((PciRead8 (PCI_LIB_ADDRESS (0, 1, 3, 0x80)) & 0x01) == 0) { > + if ((PciRead8 (PCI_PM_REG (0x80)) & 0x01) == 0) { > // > // The PEI phase should be exited with fully accessibe PIIX4 IO space: > // 1. set PMBA > // > PciAndThenOr32 ( > - PCI_LIB_ADDRESS (0, 1, 3, 0x40), > + PCI_PM_REG (0x40), > (UINT32) ~0xFFC0, > PcdGet16 (PcdAcpiPmBaseAddress) > ); > @@ -258,14 +259,14 @@ MiscInitialization ( > // 2. set PCICMD/IOSE > // > PciOr8 ( > - PCI_LIB_ADDRESS (0, 1, 3, PCI_COMMAND_OFFSET), > + PCI_PM_REG (PCI_COMMAND_OFFSET), > EFI_PCI_COMMAND_IO_SPACE > ); > > // > // 3. set PMREGMISC/PMIOSE > // > - PciOr8 (PCI_LIB_ADDRESS (0, 1, 3, 0x80), 0x01); > + PciOr8 (PCI_PM_REG (0x80), 0x01); > } > } > > -- > 1.9.3 > > > ------------------------------------------------------------------------------ > Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer > Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports > Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper > Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer > http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel
signature.asc
Description: signature
------------------------------------------------------------------------------ Slashdot TV. Videos for Nerds. Stuff that Matters. http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
_______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
