From patchwork Fri Apr 8 09:45:01 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 65378 Delivered-To: patch@linaro.org Received: by 10.112.43.237 with SMTP id z13csp569389lbl; Fri, 8 Apr 2016 02:46:11 -0700 (PDT) X-Received: by 10.98.40.4 with SMTP id o4mr11583313pfo.76.1460108758091; Fri, 08 Apr 2016 02:45:58 -0700 (PDT) Return-Path: Received: from ml01.01.org (ml01.01.org. [198.145.21.10]) by mx.google.com with ESMTPS id kv12si11762pab.194.2016.04.08.02.45.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 08 Apr 2016 02:45:58 -0700 (PDT) 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; dkim=neutral (body hash did not verify) header.i=@linaro.org; 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; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id D4DDD1A1FD3; Fri, 8 Apr 2016 02:45:43 -0700 (PDT) X-Original-To: edk2-devel@lists.01.org Delivered-To: edk2-devel@lists.01.org Received: from mail-wm0-x233.google.com (mail-wm0-x233.google.com [IPv6:2a00:1450:400c:c09::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 65A171A1FBA for ; Fri, 8 Apr 2016 02:45:41 -0700 (PDT) Received: by mail-wm0-x233.google.com with SMTP id 191so12576934wmq.0 for ; Fri, 08 Apr 2016 02:45:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=LMcQthp/SfCVfJ6CFMCcZ36tVrsJOUKpluJFmoBEBuc=; b=YG5tsfqFeHAqSiU6FDcV7RKiyHwNAAlUiXhO0sQSiACspM5lu7GyfAPZVbbPkVcRYb CEkZc516DDUNgzLRSJ11m3L6ior6qT/NwTUCOEY9CPnNyID5uDShWyGG5FSombzWukGL I6vwv2xgXHa5U/0ybSNGGT8QAMVDxu1O+5s7c= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=LMcQthp/SfCVfJ6CFMCcZ36tVrsJOUKpluJFmoBEBuc=; b=l0noiWmDzm8mMFV/OWFCpMVl7VANlhQsn1bpycC17N7ja46S34MUNWe6zUURJxVTUF 0c797myrtjVTXtH16ggtDFeM6PKKbr5yiuFjd0TAAdR6cY7H/xNb+bYRbAn78EOVblGY NMOo0oq4tg9K02Kxn3aOLOMW3OwEuT/CCZOSvMrddvb65+E1bxdH+HOZxkzHV6LAIbFE mRsPFKn5Xz/E0A6jv2Rn0erJR3689n97myucIq4vd28ob300/OGg686L2wLn+bU/9Ddd H19ggI8e9JBiL7CkTZswRop2rtg8OoT90FmKQklphYp+HUE7yW/aYhSdrZDl+2tvJESP FvJQ== X-Gm-Message-State: AD7BkJIFkAhukmh+x3Ns6IprpfSHZhDXcSSazpqBDvuGmJEtkPJMItdjXG6urzlvPxnDrCzW X-Received: by 10.28.50.138 with SMTP id y132mr2817263wmy.52.1460108740074; Fri, 08 Apr 2016 02:45:40 -0700 (PDT) Received: from localhost.localdomain ([195.55.142.58]) by smtp.gmail.com with ESMTPSA id s10sm9710681wjp.3.2016.04.08.02.45.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 08 Apr 2016 02:45:39 -0700 (PDT) From: Ard Biesheuvel To: edk2-devel@lists.01.org, lersek@redhat.com Date: Fri, 8 Apr 2016 11:45:01 +0200 Message-Id: <1460108711-12122-15-git-send-email-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.5.0 In-Reply-To: <1460108711-12122-1-git-send-email-ard.biesheuvel@linaro.org> References: <1460108711-12122-1-git-send-email-ard.biesheuvel@linaro.org> Subject: [edk2] [PATCH v2 14/24] ArmVirtPkg/PciHostBridgeDxe: move to FDT client protocol X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Ard Biesheuvel MIME-Version: 1.0 Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" Instead of relying on VirtFdtDxe to populate various dynamic PCDs with information retrieved from the host provided device tree, perform the PCI ECAM related DT node parsing directly in PciHostBridgeDxe. This means some PCI related PCDs are no longer set, such as PcdPciBusMin, PcdPciBusMax, PcdPciIoBase, PcdPciIoSize, PcdPciIoTranslation, PcdPciMmio32Base and PcdPciMmio32Size. Since these PCDs are specific to ARM (and declared in ArmPlatformPkg), and not used anywhere else by the ArmVirtPkg platforms, we can simply stop populating them, and drop any references completely. As far as PcdPciExpressBaseAddress is concerned, this dynamic PCD is now set by the DXE entry point, which means any libraries this module uses can no longer depend on its value in their constructor. This has already been addressed in a preceding patch. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel --- ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c | 234 ++++++++++++++++++-- ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h | 1 + ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf | 14 +- 3 files changed, 225 insertions(+), 24 deletions(-) -- 2.5.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c index 735c7f216318..9019cd994ec5 100644 --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c @@ -79,6 +79,197 @@ PCI_HOST_BRIDGE_INSTANCE mPciHostBridgeInstanceTemplate = { // Implementation // +STATIC UINT64 mIoBase, mIoSize, mIoTranslation; +STATIC UINT64 mMmioBase, mMmioSize, mMmioTranslation; +STATIC UINT32 mBusMin, mBusMax; + +// +// We expect the "ranges" property of "pci-host-ecam-generic" to consist of +// records like this. +// +#pragma pack (1) +typedef struct { + UINT32 Type; + UINT64 ChildBase; + UINT64 CpuBase; + UINT64 Size; +} DTB_PCI_HOST_RANGE_RECORD; +#pragma pack () + +#define DTB_PCI_HOST_RANGE_RELOCATABLE BIT31 +#define DTB_PCI_HOST_RANGE_PREFETCHABLE BIT30 +#define DTB_PCI_HOST_RANGE_ALIASED BIT29 +#define DTB_PCI_HOST_RANGE_MMIO32 BIT25 +#define DTB_PCI_HOST_RANGE_MMIO64 (BIT25 | BIT24) +#define DTB_PCI_HOST_RANGE_IO BIT24 +#define DTB_PCI_HOST_RANGE_TYPEMASK (BIT31 | BIT30 | BIT29 | BIT25 | BIT24) + +/** + Process the device tree node describing the generic PCI host controller. + + param[in] FdtClient Pointer to the FDT client protocol + + param[in] Node Offset of the device tree node whose "compatible" + property is "pci-host-ecam-generic". + + @retval EFI_SUCCESS Parsing successful, properties parsed from Node + have been stored in dynamic PCDs. + + @retval EFI_PROTOCOL_ERROR Parsing failed. PCDs are left unchanged. +**/ +STATIC +EFI_STATUS +EFIAPI +ProcessPciHost ( + FDT_CLIENT_PROTOCOL *FdtClient, + IN INT32 Node + ) +{ + UINT64 ConfigBase, ConfigSize; + CONST VOID *Prop; + UINT32 Len; + UINT32 RecordIdx; + EFI_STATUS Status; + UINT32 Tmp; + + Status = FdtClient->GetNodeProperty (FdtClient, Node, "reg", &Prop, &Len); + if (EFI_ERROR (Status) || Prop == NULL || Len != 2 * sizeof(UINT64)) { + DEBUG ((EFI_D_ERROR, "%a: 'reg' property not found or invalid\n", + __FUNCTION__)); + return EFI_PROTOCOL_ERROR; + } + + // + // Fetch the ECAM window. + // + ConfigBase = SwapBytes64 (((CONST UINT64 *)Prop)[0]); + ConfigSize = SwapBytes64 (((CONST UINT64 *)Prop)[1]); + + // + // Fetch the bus range (note: inclusive). + // + Status = FdtClient->GetNodeProperty (FdtClient, Node, "bus-range", &Prop, + &Len); + if (EFI_ERROR (Status) || Prop == NULL || Len != 2 * sizeof(UINT32)) { + DEBUG ((EFI_D_ERROR, "%a: 'bus-range' not found or invalid\n", + __FUNCTION__)); + return EFI_PROTOCOL_ERROR; + } + mBusMin = SwapBytes32 (((CONST UINT32 *)Prop)[0]); + mBusMax = SwapBytes32 (((CONST UINT32 *)Prop)[1]); + + // + // Sanity check: the config space must accommodate all 4K register bytes of + // all 8 functions of all 32 devices of all buses. + // + if (mBusMax < mBusMin || mBusMax - mBusMin == MAX_UINT32 || + DivU64x32 (ConfigSize, SIZE_4KB * 8 * 32) < mBusMax - mBusMin + 1) { + DEBUG ((EFI_D_ERROR, "%a: invalid 'bus-range' and/or 'reg'\n", + __FUNCTION__)); + return EFI_PROTOCOL_ERROR; + } + + // + // Iterate over "ranges". + // + Status = FdtClient->GetNodeProperty (FdtClient, Node, "ranges", &Prop, &Len); + if (EFI_ERROR (Status) || Prop == NULL || Len == 0 || + Len % sizeof (DTB_PCI_HOST_RANGE_RECORD) != 0) { + DEBUG ((EFI_D_ERROR, "%a: 'ranges' not found or invalid\n", __FUNCTION__)); + return EFI_PROTOCOL_ERROR; + } + + // + // mIoBase, IoTranslation, mMmioBase and MmioTranslation are initialized only + // in order to suppress '-Werror=maybe-uninitialized' warnings *incorrectly* + // emitted by some gcc versions. + // + mIoBase = 0; + mIoTranslation = 0; + mMmioBase = 0; + mMmioTranslation = 0; + + // + // IoSize and MmioSize are initialized to zero because the logic below + // requires it. + // + mIoSize = 0; + mMmioSize = 0; + for (RecordIdx = 0; RecordIdx < Len / sizeof (DTB_PCI_HOST_RANGE_RECORD); + ++RecordIdx) { + CONST DTB_PCI_HOST_RANGE_RECORD *Record; + + Record = (CONST DTB_PCI_HOST_RANGE_RECORD *)Prop + RecordIdx; + switch (SwapBytes32 (Record->Type) & DTB_PCI_HOST_RANGE_TYPEMASK) { + case DTB_PCI_HOST_RANGE_IO: + mIoBase = SwapBytes64 (Record->ChildBase); + mIoSize = SwapBytes64 (Record->Size); + mIoTranslation = SwapBytes64 (Record->CpuBase) - mIoBase; + break; + + case DTB_PCI_HOST_RANGE_MMIO32: + mMmioBase = SwapBytes64 (Record->ChildBase); + mMmioSize = SwapBytes64 (Record->Size); + mMmioTranslation = SwapBytes64 (Record->CpuBase) - mMmioBase; + + if (mMmioBase > MAX_UINT32 || mMmioSize > MAX_UINT32 || + mMmioBase + mMmioSize > SIZE_4GB) { + DEBUG ((EFI_D_ERROR, "%a: MMIO32 space invalid\n", __FUNCTION__)); + return EFI_PROTOCOL_ERROR; + } + + if (mMmioTranslation != 0) { + DEBUG ((EFI_D_ERROR, "%a: unsupported nonzero MMIO32 translation " + "0x%Lx\n", __FUNCTION__, mMmioTranslation)); + return EFI_UNSUPPORTED; + } + + break; + } + } + if (mIoSize == 0 || mMmioSize == 0) { + DEBUG ((EFI_D_ERROR, "%a: %a space empty\n", __FUNCTION__, + (mIoSize == 0) ? "IO" : "MMIO32")); + return EFI_PROTOCOL_ERROR; + } + + PcdSet64 (PcdPciExpressBaseAddress, ConfigBase); + + PcdSetBool (PcdPciDisableBusEnumeration, FALSE); + + if (!FeaturePcdGet (PcdPureAcpiBoot)) { + // + // Set the /chosen/linux,pci-probe-only property to 1, so that the PCI + // setup we will perform in the firmware is honored by the Linux OS, + // rather than torn down and done from scratch. This is generally a more + // sensible approach, and aligns with what ACPI based OSes do in general. + // + // In case we are exposing an emulated VGA PCI device to the guest, which + // may subsequently get exposed via the Graphics Output protocol and + // driven as an efifb by Linux, we need this setting to prevent the + // framebuffer from becoming unresponsive. + // + Status = FdtClient->GetOrInsertChosenNode (FdtClient, &Node); + + if (Status == EFI_SUCCESS) { + Tmp = SwapBytes32 (1); + Status = FdtClient->SetNodeProperty (FdtClient, Node, + "linux,pci-probe-only", &Tmp, sizeof (Tmp)); + } + if (EFI_ERROR (Status)) { + DEBUG ((EFI_D_WARN, + "Failed to set /chosen/linux,pci-probe-only property\n")); + } + } + + DEBUG ((EFI_D_INFO, "%a: Config[0x%Lx+0x%Lx) Bus[0x%x..0x%x] " + "Io[0x%Lx+0x%Lx)@0x%Lx Mem[0x%Lx+0x%Lx)@0x%Lx\n", __FUNCTION__, ConfigBase, + ConfigSize, mBusMin, mBusMax, mIoBase, mIoSize, mIoTranslation, mMmioBase, + mMmioSize, mMmioTranslation)); + return EFI_SUCCESS; +} + + /** Entry point of this driver @@ -97,6 +288,8 @@ InitializePciHostBridge ( IN EFI_SYSTEM_TABLE *SystemTable ) { + FDT_CLIENT_PROTOCOL *FdtClient; + INT32 PciDtNode; UINT64 MmioAttributes; EFI_STATUS Status; UINTN Loop1; @@ -104,24 +297,33 @@ InitializePciHostBridge ( PCI_HOST_BRIDGE_INSTANCE *HostBridge; PCI_ROOT_BRIDGE_INSTANCE *PrivateData; - if (PcdGet64 (PcdPciExpressBaseAddress) == 0) { + Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL, + (VOID **)&FdtClient); + ASSERT_EFI_ERROR (Status); + + Status = FdtClient->FindCompatibleNode (FdtClient, "pci-host-ecam-generic", + &PciDtNode); + if (EFI_ERROR (Status)) { DEBUG ((EFI_D_INFO, "%a: PCI host bridge not present\n", __FUNCTION__)); return EFI_ABORTED; } + Status = ProcessPciHost (FdtClient, PciDtNode); + if (EFI_ERROR (Status)) { + return EFI_ABORTED; + } + mDriverImageHandle = ImageHandle; - mResAperture[0][0].BusBase = PcdGet32 (PcdPciBusMin); - mResAperture[0][0].BusLimit = PcdGet32 (PcdPciBusMax); + mResAperture[0][0].BusBase = mBusMin; + mResAperture[0][0].BusLimit = mBusMax; - mResAperture[0][0].MemBase = PcdGet32 (PcdPciMmio32Base); - mResAperture[0][0].MemLimit = (UINT64)PcdGet32 (PcdPciMmio32Base) + - PcdGet32 (PcdPciMmio32Size) - 1; + mResAperture[0][0].MemBase = mMmioBase; + mResAperture[0][0].MemLimit = (UINT64)mMmioBase + mMmioSize - 1; - mResAperture[0][0].IoBase = PcdGet64 (PcdPciIoBase); - mResAperture[0][0].IoLimit = PcdGet64 (PcdPciIoBase) + - PcdGet64 (PcdPciIoSize) - 1; - mResAperture[0][0].IoTranslation = PcdGet64 (PcdPciIoTranslation); + mResAperture[0][0].IoBase = mIoBase; + mResAperture[0][0].IoLimit = mIoBase + mIoSize - 1; + mResAperture[0][0].IoTranslation = mIoTranslation; // // Add IO and MMIO memory space, so that resources can be allocated in the @@ -129,8 +331,8 @@ InitializePciHostBridge ( // Status = gDS->AddIoSpace ( EfiGcdIoTypeIo, - PcdGet64 (PcdPciIoBase), - PcdGet64 (PcdPciIoSize) + mIoBase, + mIoSize ); ASSERT_EFI_ERROR (Status); @@ -139,8 +341,8 @@ InitializePciHostBridge ( Status = gDS->AddMemorySpace ( EfiGcdMemoryTypeMemoryMappedIo, - PcdGet32 (PcdPciMmio32Base), - PcdGet32 (PcdPciMmio32Size), + mMmioBase, + mMmioSize, MmioAttributes ); if (EFI_ERROR (Status)) { @@ -149,8 +351,8 @@ InitializePciHostBridge ( } Status = gDS->SetMemorySpaceAttributes ( - PcdGet32 (PcdPciMmio32Base), - PcdGet32 (PcdPciMmio32Size), + mMmioBase, + mMmioSize, MmioAttributes ); if (EFI_ERROR (Status)) { diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h index 8161b546ff87..647fe1a52a7d 100644 --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h @@ -24,6 +24,7 @@ #include #include #include +#include #include diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf index 6d782582e02d..032cbeda44bc 100644 --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf @@ -22,6 +22,7 @@ [Defines] ENTRY_POINT = InitializePciHostBridge [Packages] + MdeModulePkg/MdeModulePkg.dec MdePkg/MdePkg.dec ArmPlatformPkg/ArmPlatformPkg.dec ArmVirtPkg/ArmVirtPkg.dec @@ -50,20 +51,17 @@ [Protocols] gEfiPciRootBridgeIoProtocolGuid ## PRODUCES gEfiMetronomeArchProtocolGuid ## CONSUMES gEfiDevicePathProtocolGuid ## PRODUCES + gFdtClientProtocolGuid ## CONSUMES [Pcd] - gArmPlatformTokenSpaceGuid.PcdPciBusMin - gArmPlatformTokenSpaceGuid.PcdPciBusMax - gArmPlatformTokenSpaceGuid.PcdPciIoBase - gArmPlatformTokenSpaceGuid.PcdPciIoSize - gArmPlatformTokenSpaceGuid.PcdPciIoTranslation - gArmPlatformTokenSpaceGuid.PcdPciMmio32Base - gArmPlatformTokenSpaceGuid.PcdPciMmio32Size + gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress [FeaturePcd] gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached + gArmVirtTokenSpaceGuid.PcdPureAcpiBoot [depex] gEfiMetronomeArchProtocolGuid AND - gEfiCpuArchProtocolGuid + gEfiCpuArchProtocolGuid AND + gFdtClientProtocolGuid