On 03/06/16 11:54, Marcel Apfelbaum wrote: > On 03/04/2016 04:46 PM, Laszlo Ersek wrote: >> The comments in the code should speak for themselves; here we note only >> two facts: >> >> - The PCI config space writes (to the PCIEXBAR register) are performed >> using the 0xCF8 / 0xCFC IO ports, by virtue of PciLib being >> resolved to >> BasePciLibCf8. (This library resolution will permanently remain in >> place >> for the PEI phase.) >> >> - Since PCIEXBAR counts as a chipset register, it is the >> responsibility of >> the firmware to reprogram it at S3 resume. Therefore >> PciExBarInitialization() is called regardless of the boot path. >> (Marcel >> recently posted patches for SeaBIOS that implement this.) >> >> This patch suffices to enable PCIEXBAR (and the dependent ACPI table >> generation in QEMU), for the sake of "PCIeHotplug" in the Linux guest: >> >> ACPI: MCFG 0x000000007E193000 00003C >> (v01 BOCHS BXPCMCFG 00000001 BXPC 00000001) >> PCI: MMCONFIG for domain 0000 [bus 00-ff] at [mem >> 0xb0000000-0xbfffffff] >> (base 0xb0000000) >> PCI: MMCONFIG at [mem 0xb0000000-0xbfffffff] reserved in E820 >> acpi PNP0A08:00: _OSC: OS supports >> [ExtendedConfig ASPM ClockPM Segments MSI] >> acpi PNP0A08:00: _OSC: OS now controls >> [PCIeHotplug PME AER PCIeCapability] >> >> In the following patches, we'll equip the core PCI host bridge / root >> bridge driver and the rest of DXE as well to utilize ECAM on Q35. >> >> Cc: Gabriel Somlo <so...@cmu.edu> >> Cc: Jordan Justen <jordan.l.jus...@intel.com> >> Cc: Marcel Apfelbaum <mar...@redhat.com> >> Cc: Michał Zegan <webczat_...@poczta.onet.pl> >> Ref: https://github.com/tianocore/edk2/issues/32 >> Ref: http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/10548 >> Suggested-by: Marcel Apfelbaum <mar...@redhat.com> >> Reported-by: Michał Zegan <webczat_...@poczta.onet.pl> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >> --- >> OvmfPkg/OvmfPkgIa32.dsc | 8 ++ >> OvmfPkg/OvmfPkgIa32X64.dsc | 8 ++ >> OvmfPkg/OvmfPkgX64.dsc | 8 ++ >> OvmfPkg/PlatformPei/PlatformPei.inf | 3 + >> OvmfPkg/PlatformPei/Platform.c | 81 ++++++++++++++++++++ >> 5 files changed, 108 insertions(+) >> >> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >> index 2b928506e481..d1f91172044e 100644 >> --- a/OvmfPkg/OvmfPkgIa32.dsc >> +++ b/OvmfPkg/OvmfPkgIa32.dsc >> @@ -395,6 +395,14 @@ [PcdsFixedAtBuild] >> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F >> !endif >> >> + # This PCD is used to set the base address of the PCI express >> hierarchy. It >> + # is only consulted when OVMF runs on Q35. In that case it is >> programmed into >> + # the PCIEXBAR register. >> + # >> + # The value we set below matches both the QEMU default, and the >> value that >> + # SeaBIOS hard-codes. >> + gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xB0000000 >> + >> !ifdef $(SOURCE_DEBUG_ENABLE) >> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2 >> !endif >> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >> index 8d6271824f89..c9eeace4a134 100644 >> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >> @@ -400,6 +400,14 @@ [PcdsFixedAtBuild] >> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F >> !endif >> >> + # This PCD is used to set the base address of the PCI express >> hierarchy. It >> + # is only consulted when OVMF runs on Q35. In that case it is >> programmed into >> + # the PCIEXBAR register. >> + # >> + # The value we set below matches both the QEMU default, and the >> value that >> + # SeaBIOS hard-codes. >> + gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xB0000000 >> + >> !ifdef $(SOURCE_DEBUG_ENABLE) >> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2 >> !endif >> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >> index e3f97f16f28b..b6f4b252ff03 100644 >> --- a/OvmfPkg/OvmfPkgX64.dsc >> +++ b/OvmfPkg/OvmfPkgX64.dsc >> @@ -400,6 +400,14 @@ [PcdsFixedAtBuild] >> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F >> !endif >> >> + # This PCD is used to set the base address of the PCI express >> hierarchy. It >> + # is only consulted when OVMF runs on Q35. In that case it is >> programmed into >> + # the PCIEXBAR register. >> + # >> + # The value we set below matches both the QEMU default, and the >> value that >> + # SeaBIOS hard-codes. >> + gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xB0000000 >> + >> !ifdef $(SOURCE_DEBUG_ENABLE) >> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2 >> !endif >> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf >> b/OvmfPkg/PlatformPei/PlatformPei.inf >> index 8480839efc8e..6dc5ff079f53 100644 >> --- a/OvmfPkg/PlatformPei/PlatformPei.inf >> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf >> @@ -94,6 +94,9 @@ [Pcd] >> gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable >> gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress >> >> +[FixedPcd] >> + gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress >> + >> [FeaturePcd] >> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire >> >> diff --git a/OvmfPkg/PlatformPei/Platform.c >> b/OvmfPkg/PlatformPei/Platform.c >> index 7d0941209f25..f0a488bcfc2b 100644 >> --- a/OvmfPkg/PlatformPei/Platform.c >> +++ b/OvmfPkg/PlatformPei/Platform.c >> @@ -214,6 +214,7 @@ MemMapInitialization ( >> UINT32 TopOfLowRam; >> UINT32 PciBase; >> UINT32 PciSize; >> + UINT64 PciExBarBase; >> >> TopOfLowRam = GetSystemMemorySizeBelow4gb (); >> if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) { >> @@ -224,6 +225,16 @@ MemMapInitialization ( >> // >> PciBase = BASE_2GB + BASE_1GB; >> ASSERT (TopOfLowRam <= PciBase); >> + >> + // >> + // The (exclusive) end of the MMCONFIG area should be >> expressible as a >> + // UINT64. Additionally, the MMCONFIG area is expected to fall >> between >> + // the top of low RAM and the base of the 32-bit PCI host >> aperture. >> + // >> + PciExBarBase = FixedPcdGet64 (PcdPciExpressBaseAddress); >> + ASSERT (PciExBarBase <= MAX_UINT64 - SIZE_256MB); >> + ASSERT (TopOfLowRam <= PciExBarBase); >> + ASSERT (PciExBarBase + SIZE_256MB <= PciBase); >> } else { >> PciBase = (TopOfLowRam < BASE_2GB) ? BASE_2GB : TopOfLowRam; >> } >> @@ -249,6 +260,30 @@ MemMapInitialization ( >> AddIoMemoryBaseSizeHob (0xFED00000, SIZE_1KB); >> if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) { >> AddIoMemoryBaseSizeHob (ICH9_ROOT_COMPLEX_BASE, SIZE_16KB); >> + // >> + // Note: there should be an >> + // >> + // AddIoMemoryBaseSizeHob (PciExBarBase, SIZE_256MB); >> + // >> + // call below, just like the one above for RCBA. However, Linux >> insists >> + // that the MMCONFIG area be marked in the E820 or UEFI memory >> map as >> + // "reserved memory" -- Linux does not content itself with a >> simple gap >> + // in the memory map wherever the MCFG ACPI table points to. >> + // >> + // This appears to be a safety measure. The PCI Firmware >> Specification >> + // (rev 3.1) says in 4.1.2. "MCFG Table Description": "The >> resources can >> + // *optionally* be returned in [...] EFIGetMemoryMap as >> reserved memory >> + // [...]". (Emphasis added here.) >> + // >> + // Normally we add memory resource descriptor HOBs in >> + // QemuInitializeRam(), and pre-allocate from those with memory >> + // allocation HOBs in InitializeRamRegions(). However, the >> MMCONFIG area >> + // is most definitely not RAM; so, as an exception, cover it with >> + // uncacheable reserved memory right here. >> + // >> + AddReservedMemoryBaseSizeHob (PciExBarBase, SIZE_256MB, FALSE); >> + BuildMemoryAllocationHob (PciExBarBase, SIZE_256MB, >> + EfiReservedMemoryType); >> } >> AddIoMemoryBaseSizeHob (PcdGet32(PcdCpuLocalApicBaseAddress), >> SIZE_1MB); >> } >> @@ -318,6 +353,47 @@ NoexecDxeInitialization ( >> } >> >> VOID >> +PciExBarInitialization ( >> + VOID >> + ) >> +{ >> + union { >> + UINT64 Uint64; >> + UINT32 Uint32[2]; >> + } PciExBarBase; >> + >> + // >> + // We only support the 256MB size for the MMCONFIG area: >> + // 256 buses * 32 devices * 8 functions * 4096 bytes config space. >> + // >> + // The masks used below enforce the Q35 requirements that the >> MMCONFIG area >> + // be (a) correctly aligned -- here at 256 MB --, (b) located under >> 64 GB. >> + // >> + // Note that (b) also ensures that the minimum address width we have >> + // determined in AddressWidthInitialization(), i.e., 36 bits, will >> suffice >> + // for DXE's page tables to cover the MMCONFIG area. >> + // >> + PciExBarBase.Uint64 = FixedPcdGet64 (PcdPciExpressBaseAddress); >> + ASSERT ((PciExBarBase.Uint32[1] & MCH_PCIEXBAR_HIGHMASK) == 0); >> + ASSERT ((PciExBarBase.Uint32[0] & MCH_PCIEXBAR_LOWMASK) == 0); >> + >> + // >> + // Clear the PCIEXBAREN bit first, before programming the high >> register. >> + // >> + PciWrite32 (DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_LOW), 0); >> + >> + // >> + // Program the high register. Then program the low register, >> setting the >> + // MMCONFIG area size and enabling decoding at once. >> + // >> + PciWrite32 (DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_HIGH), >> PciExBarBase.Uint32[1]); >> + PciWrite32 ( >> + DRAMC_REGISTER_Q35 (MCH_PCIEXBAR_LOW), >> + PciExBarBase.Uint32[0] | MCH_PCIEXBAR_BUS_FF | MCH_PCIEXBAR_EN >> + ); >> +} >> + >> +VOID >> MiscInitialization ( >> VOID >> ) >> @@ -394,6 +470,11 @@ MiscInitialization ( >> POWER_MGMT_REGISTER_Q35 (ICH9_RCBA), >> ICH9_ROOT_COMPLEX_BASE | ICH9_RCBA_EN >> ); >> + >> + // >> + // Set PCI Express Register Range Base Address >> + // >> + PciExBarInitialization (); >> } >> } >> >> > > Hi, > > The PCI config accesses to PCIEXBAR are correct. > > Reviewed-by: Marcel Apfelbaum <mar...@redhat.com> > > Thanks, > Marcel
Thanks, Marcel! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
