From patchwork Sat Feb 27 00:23:20 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laszlo Ersek X-Patchwork-Id: 63120 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp145708lbc; Fri, 26 Feb 2016 16:23:46 -0800 (PST) X-Received: by 10.66.187.36 with SMTP id fp4mr6017686pac.47.1456532626360; Fri, 26 Feb 2016 16:23:46 -0800 (PST) Return-Path: Received: from ml01.01.org (ml01.01.org. [198.145.21.10]) by mx.google.com with ESMTPS id e3si6219222pas.149.2016.02.26.16.23.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 26 Feb 2016 16:23:46 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) client-ip=198.145.21.10; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 3C4441A1E3A; Fri, 26 Feb 2016 16:23:50 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8FE951A1E34 for ; Fri, 26 Feb 2016 16:23:49 -0800 (PST) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id C2893337742; Sat, 27 Feb 2016 00:23:43 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-45.phx2.redhat.com [10.3.113.45]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1R0NdFR026697; Fri, 26 Feb 2016 19:23:42 -0500 From: Laszlo Ersek To: edk2-devel-01 Date: Sat, 27 Feb 2016 01:23:20 +0100 Message-Id: <1456532616-32475-2-git-send-email-lersek@redhat.com> In-Reply-To: <1456532616-32475-1-git-send-email-lersek@redhat.com> References: <1456532616-32475-1-git-send-email-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 Cc: Ruiyu Ni , Jordan Justen , Marcel Apfelbaum Subject: [edk2] [PATCH 01/17] MdeModulePkg: PciHostBridgeDxe: don't assume extended config space X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" 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 Cc: Jordan Justen Cc: Marcel Apfelbaum Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek --- 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 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) {