From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web12.4839.1628257334684140710 for ; Fri, 06 Aug 2021 06:42:14 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=VZDgfwqw; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id 25F7B6113C for ; Fri, 6 Aug 2021 13:42:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1628257334; bh=RhelpXSRZR0qrFUrCy0zpxWzfEFIHdxE0BpdyZJjHVE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=VZDgfwqw8FUG9C943fH7ZzHjXQ+tU5MkLa7TlmBuqBWkHpifEAJJ0TKjEeve95pWo 6ADIyGtSLiKsgN28v+avWucDkJ8irJw5NPB4oCYDDeghaLCAIawe/D1hPynDuSEDUa KFM5d+h2jS2l3tNOY9toCF5pekupPHvw5kUGqvTYU7/vgxExUyUVLua9W6q4x5raT7 cjh121xo91OSOQkENWVZRbFRLHA8RoN2yp4VqejYwOnuZqyf8qRJ9QYRybPDkip1NW ITpOPDxZ7Dw6vozSxewrSEZzZmkl6UlfwzEYKhk2aJ5n+FnJlWumLaoaC8IHO/AWk9 cyne/m03mwEwA== Received: by mail-ot1-f41.google.com with SMTP id 61-20020a9d0d430000b02903eabfc221a9so8974577oti.0 for ; Fri, 06 Aug 2021 06:42:14 -0700 (PDT) X-Gm-Message-State: AOAM532YnwIl1G5GHn6JgiI2f0+IXc75Y/Kq/OYpwiIaHnGsB0Jj/sGr VguxHuYuLA282wz/7hASYbgxR2FzUXN1ht9JCl4= X-Google-Smtp-Source: ABdhPJwkX7X3TnrcxIhoXZm+tGodhkNQ7TMRP0sGNFEfHuAQ/o+mCXNIs3yqjQSeQyuoBkNKe5wTAcUWc18w4d8OONE= X-Received: by 2002:a05:6830:34a6:: with SMTP id c38mr7392273otu.108.1628257333408; Fri, 06 Aug 2021 06:42:13 -0700 (PDT) MIME-Version: 1.0 References: <20210805163551.488035-1-jeremy.linton@arm.com> <20210805163551.488035-4-jeremy.linton@arm.com> In-Reply-To: <20210805163551.488035-4-jeremy.linton@arm.com> From: "Ard Biesheuvel" Date: Fri, 6 Aug 2021 15:42:02 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 3/5] Platform/RaspberryPi: Add PCIe SSDT To: Jeremy Linton Cc: edk2-devel-groups-io , Peter Batard , Ard Biesheuvel , Andrei Warkentin , Sunny Wang , Samer El-Haj-Mahmoud Content-Type: text/plain; charset="UTF-8" On Thu, 5 Aug 2021 at 18:36, Jeremy Linton wrote: > > Since we plan on toggling between XHCI and PCI the PCI > root needs to be in its own SSDT. This is all thats needed > of UEFI. The SMC conduit is provided directly to the running > OS. When the OS detects this PCIe port, on a machine without > a MADT it attempts to connect to the SMC conduit. The RPi > definition doesn't have any power mgmt, and only provides > a description of the root port. > > Signed-off-by: Jeremy Linton > --- > Platform/RaspberryPi/AcpiTables/AcpiTables.inf | 3 + > Platform/RaspberryPi/AcpiTables/Pci.asl | 237 +++++++++++++++++++++ > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 6 + > 3 files changed, 246 insertions(+) > create mode 100644 Platform/RaspberryPi/AcpiTables/Pci.asl > > diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf > index f3e8d950c1..da2a6db85f 100644 > --- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf > +++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf > @@ -39,6 +39,7 @@ > Pptt.aslc > SsdtThermal.asl > Xhci.asl > + Pci.asl > > [Packages] > ArmPkg/ArmPkg.dec > @@ -59,6 +60,8 @@ > gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase > gArmTokenSpaceGuid.PcdGicDistributorBase > gBcm27xxTokenSpaceGuid.PcdBcm27xxPciCpuMmioAdr > + gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioAdr > + gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioLen > gBcm27xxTokenSpaceGuid.PcdBcm27xxPciRegBase > gBcm27xxTokenSpaceGuid.PcdBcmGenetRegistersAddress > gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress > diff --git a/Platform/RaspberryPi/AcpiTables/Pci.asl b/Platform/RaspberryPi/AcpiTables/Pci.asl > new file mode 100644 > index 0000000000..34474f13ef > --- /dev/null > +++ b/Platform/RaspberryPi/AcpiTables/Pci.asl > @@ -0,0 +1,237 @@ > +/** @file > + * > + * Copyright (c) 2019 Linaro, Limited. All rights reserved. > + * Copyright (c) 2021 Arm > + * > + * SPDX-License-Identifier: BSD-2-Clause-Patent > + * > + **/ > + > +#include > + > +#include "AcpiTables.h" > + > +/* > + * The following can be used to remove parenthesis from > + * defined macros that the compiler complains about. > + */ > +#define ISOLATE_ARGS(...) __VA_ARGS__ > +#define REMOVE_PARENTHESES(x) ISOLATE_ARGS x > + > +#define SANITIZED_PCIE_CPU_MMIO_WINDOW REMOVE_PARENTHESES(PCIE_CPU_MMIO_WINDOW) > +#define SANITIZED_PCIE_MMIO_LEN REMOVE_PARENTHESES(PCIE_BRIDGE_MMIO_LEN) > +#define SANITIZED_PCIE_PCI_MMIO_BEGIN REMOVE_PARENTHESES(PCIE_TOP_OF_MEM_WIN) > + > +/* > + * According to UEFI boot log for the VLI device on Pi 4. > + */ > +#define RT_REG_LENGTH 0x1000 > + > +// copy paste job from juno > +#define LNK_DEVICE(Unique_Id, Link_Name, irq) \ > + Device(Link_Name) { \ > + Name(_HID, EISAID("PNP0C0F")) \ > + Name(_UID, Unique_Id) \ > + Name(_PRS, ResourceTemplate() { \ > + Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { irq } \ > + }) \ > + Method (_CRS, 0) { Return (_PRS) } \ > + Method (_SRS, 1) { } \ > + Method (_DIS) { } \ > + } > + > +#define PRT_ENTRY(Address, Pin, Link) \ > + Package (4) { \ > + Address, /* uses the same format as _ADR */ \ > + Pin, /* The PCI pin number of the device (0-INTA, 1-INTB, 2-INTC, 3-INTD). */ \ > + Link, /* Interrupt allocated via Link device. */ \ > + Zero /* global system interrupt number (no used) */ \ > + } > +#define ROOT_PRT_ENTRY(Pin, Link) PRT_ENTRY(0x0000FFFF, Pin, Link) > + This can be done in a much simpler way - SynQuacer uses this, for instance Name (_PRT, Package () { Package () { 0xFFFF, 0, Zero, 222 }, // INTA Package () { 0xFFFF, 1, Zero, 222 }, // INTB Package () { 0xFFFF, 2, Zero, 222 }, // INTC Package () { 0xFFFF, 3, Zero, 222 }, // INTD }) > +DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN", "RPI4PCIE", 2) > +{ > + Scope (\_SB_) > + { > + > + Device (SCB0) { > + Name (_HID, "ACPI0004") > + Name (_UID, 0x0) Even if this file and the xhci one should never be exposed to the OS at the same time, can we please use unique UIDs? > + Name (_CCA, 0x0) > + > + Method (_CRS, 0, Serialized) { > + // Container devices with _DMA must have _CRS, > + // meaning SCB0 to provide all resources that > + // PCI0 consumes (except interrupts). > + Name (RBUF, ResourceTemplate () { > + QWordMemory (ResourceProducer, > + , > + MinFixed, > + MaxFixed, > + NonCacheable, > + ReadWrite, > + 0x0, > + SANITIZED_PCIE_CPU_MMIO_WINDOW, // MIN > + SANITIZED_PCIE_CPU_MMIO_WINDOW, // MAX > + 0x0, > + 0x1, // LEN > + , > + , > + MMIO > + ) > + }) > + CreateQwordField (RBUF, MMIO._MAX, MMBE) > + CreateQwordField (RBUF, MMIO._LEN, MMLE) > + Add (MMBE, RT_REG_LENGTH - 1, MMBE) > + Add (MMLE, RT_REG_LENGTH - 1, MMLE) > + Return (RBUF) > + } > + > + Name (_DMA, ResourceTemplate() { > + // PCIe can only DMA to first 3GB with early SOC's > + // But we keep the restriction on the later ones > + // To avoid DMA translation problems. > + QWordMemory (ResourceProducer, > + , > + MinFixed, > + MaxFixed, > + NonCacheable, > + ReadWrite, > + 0x0, > + 0x0, // MIN > + 0xbfffffff, // MAX > + 0x0, // TRA > + 0xc0000000, // LEN > + , > + , > + ) > + }) > + > + // > + // PCI Root Complex > + // > + LNK_DEVICE(1, LNKA, 175) > + LNK_DEVICE(2, LNKB, 176) > + LNK_DEVICE(3, LNKC, 177) > + LNK_DEVICE(4, LNKD, 178) > + > + Device(PCI0) > + { > + Name(_HID, EISAID("PNP0A08")) // PCI Express Root Bridge > + Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge > + Name(_SEG, Zero) // PCI Segment Group number > + Name(_BBN, Zero) // PCI Base Bus Number > + Name(_CCA, 0) // Mark the PCI noncoherent > + > + // Root Complex 0 > + Device (RP0) { > + Name(_ADR, 0xF0000000) // Dev 0, Func 0 > + } > + Can we just drop this? > + Name (_DMA, ResourceTemplate() { > + QWordMemory (ResourceConsumer, > + , > + MinFixed, > + MaxFixed, > + NonCacheable, > + ReadWrite, > + 0x0, > + 0x0, // MIN > + 0xbfffffff, // MAX > + 0x0, // TRA > + 0xc0000000, // LEN > + , > + , > + ) > + }) > + Do we need this method on the host bridge device as well as on the container? > + // PCI Routing Table > + Name(_PRT, Package() { > + ROOT_PRT_ENTRY(0, LNKA), // INTA > + ROOT_PRT_ENTRY(1, LNKB), // INTB > + ROOT_PRT_ENTRY(2, LNKC), // INTC > + ROOT_PRT_ENTRY(3, LNKD), // INTD > + }) > + // Root complex resources > + Method (_CRS, 0, Serialized) { > + Name (RBUF, ResourceTemplate () { > + WordBusNumber ( // Bus numbers assigned to this root > + ResourceProducer, > + MinFixed, MaxFixed, PosDecode, > + 0, // AddressGranularity > + 0, // AddressMinimum - Minimum Bus Number > + 255, // AddressMaximum - Maximum Bus Number > + 0, // AddressTranslation - Set to 0 > + 256 // RangeLength - Number of Busses > + ) > + > + QWordMemory ( // 32-bit BAR Windows in 64-bit addr > + ResourceProducer, PosDecode, > + MinFixed, MaxFixed, > + NonCacheable, ReadWrite, //cacheable? is that right? > + 0x00000000, // Granularity > + 0, // SANITIZED_PCIE_PCI_MMIO_BEGIN > + 1, // SANITIZED_PCIE_MMIO_LEN + SANITIZED_PCIE_PCI_MMIO_BEGIN > + SANITIZED_PCIE_CPU_MMIO_WINDOW, // SANITIZED_PCIE_PCI_MMIO_BEGIN - SANITIZED_PCIE_CPU_MMIO_WINDOW > + 2 // SANITIZED_PCIE_MMIO_LEN + 1 > + ,,,MMI1,,TypeTranslation > + ) > + }) // end Name(RBUF) > + > + // Work around ASL's inability to add in a resource definition > + // or for that matter compute the min,max,len properly > + CreateQwordField (RBUF, MMI1._MIN, MMIB) > + CreateQwordField (RBUF, MMI1._MAX, MMIE) > + CreateQwordField (RBUF, MMI1._TRA, MMIT) > + CreateQwordField (RBUF, MMI1._LEN, MMIL) > + Add (MMIB, SANITIZED_PCIE_PCI_MMIO_BEGIN, MMIB) > + Add (SANITIZED_PCIE_MMIO_LEN, SANITIZED_PCIE_PCI_MMIO_BEGIN, MMIE) > + Subtract (MMIT, SANITIZED_PCIE_PCI_MMIO_BEGIN, MMIT) > + Add (SANITIZED_PCIE_MMIO_LEN, 1 , MMIL) > + > + Return (RBUF) > + } // end Method(_CRS) > + // > + // OS Control Handoff > + // > + Name(SUPP, Zero) // PCI _OSC Support Field value > + Name(CTRL, Zero) // PCI _OSC Control Field value > + > + // See [1] 6.2.10, [2] 4.5 > + Method(_OSC,4) { > + // Note, This code is very similar to the code in the PCIe firmware > + // specification which can be used as a reference > + // Check for proper UUID > + If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) { > + // Create DWord-adressable fields from the Capabilities Buffer > + CreateDWordField(Arg3,0,CDW1) > + CreateDWordField(Arg3,4,CDW2) > + CreateDWordField(Arg3,8,CDW3) > + // Save Capabilities DWord2 & 3 > + Store(CDW2,SUPP) > + Store(CDW3,CTRL) > + // Mask out Native HotPlug > + And(CTRL,0x1E,CTRL) > + // Always allow native PME, AER (no dependencies) > + // Never allow SHPC (no SHPC controller in this system) > + And(CTRL,0x1D,CTRL) > + > + If(LNotEqual(Arg1,One)) { // Unknown revision > + Or(CDW1,0x08,CDW1) > + } > + > + If(LNotEqual(CDW3,CTRL)) { // Capabilities bits were masked > + Or(CDW1,0x10,CDW1) > + } > + // Update DWORD3 in the buffer > + Store(CTRL,CDW3) > + Return(Arg3) > + } Else { > + Or(CDW1,4,CDW1) // Unrecognized UUID > + Return(Arg3) > + } > + } // End _OSC > + } // PCI0 > + } //end SCB0 > + } //end scope sb > +} //end definition block > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > index 7c5786303d..4c40820858 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > @@ -821,6 +821,12 @@ STATIC CONST NAMESPACE_TABLES SdtTables[] = { > PcdToken(PcdXhciPci), > NULL > }, > + { > + SIGNATURE_64 ('R', 'P', 'I', '4', 'P', 'C', 'I', 'E'), > + PcdToken(PcdXhciPci), > + 0, > + NULL > + }, > #endif > { // DSDT > SIGNATURE_64 ('R', 'P', 'I', 0, 0, 0, 0, 0), > -- > 2.13.7 >