From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by mx.groups.io with SMTP id smtpd.web10.9678.1576762422750885152 for ; Thu, 19 Dec 2019 05:33:43 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=yGO4EQ3p; spf=pass (domain: linaro.org, ip: 209.85.221.65, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f65.google.com with SMTP id d16so5971377wre.10 for ; Thu, 19 Dec 2019 05:33:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=nFyGedIgMvx1ZKCkOyRZJ7wOIAZMJWVLKanSjLntm4Y=; b=yGO4EQ3p81nXRRAMyEgKJSQGegPO4x9J1zz9lCEzyCT6mIBTBKlF0GPGyVTCCMFV/Q tV5Kb/opiRuWFvGHes6M2+wDOcpg3tX0LPejHbG5fuGJKR8ALXIQbcveTxEN3UC08n/S 9f+f8auIudRXWKRVaK+vLfvn2+eecZhyPgGBJ+XZ5AwoyoRTr7iEheKY9JzuQA0cpxps l5/MBAmAHOGp+ENImOLF6cwEM/N24H6NVf7fXRV90qynmarfduH1pjqVM88TGSkNhXSv EBbhRzLzdDp1cE/jA4TomHxLoObVJ/8S/W/c3BuYPZJrKnBeCHP/6bpeyDfvDFR58gCT Adhw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=nFyGedIgMvx1ZKCkOyRZJ7wOIAZMJWVLKanSjLntm4Y=; b=BXLm+zl9Z4Zobogk69ChbMJBKq7DqnN8HgJkihc7hRnnmXzXnK9NiW1ddpiAUrRcrS FS9IARAU2MSoEWHjHx00x1PSwI25spCIOCISx9hU0RUvON+2OMiI7RV4zxUhYr6jt/40 IgaSWSzXoBUHo8WcNoPaJ2dR3dd3o0foAC4KoqJyUJtDulF2EPKqKc/5jefLjnxjy5Ey dqAiNpOs3p0M8KVpoezz0bEz85ActBDfs9fF09ROHz8JqY7Unlm5JwPsBJiN8KA2zzkm JIhhSgTho3KFS5J8iM1Lej/lkcUDjxXiyAFhX1nm7oq73Nu8/FJS5bXG8bHNQqpCtuKE LDfA== X-Gm-Message-State: APjAAAUMLnVbvbmNV5YjspbDNTgelcwGFlIMQ/6ZOJ2GsChcODWmGewO yqDh9PgCjWt8HrV4cEZWphSibr3ottMvMfbUYWDMfg== X-Google-Smtp-Source: APXvYqy/9ocZjJfQNFl843Iw35cyPEqRN7yN44+5Er6eAyZn7TVROyI1a8KAGxmeNBcbnq13yaWFd4bC7ZngJpsC4AI= X-Received: by 2002:a5d:6652:: with SMTP id f18mr9843696wrw.246.1576762421240; Thu, 19 Dec 2019 05:33:41 -0800 (PST) MIME-Version: 1.0 References: <20191219121434.2856-1-pete@akeo.ie> <20191219121434.2856-7-pete@akeo.ie> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 19 Dec 2019 13:33:35 +0000 Message-ID: Subject: Re: [edk2-platforms][PATCH v2 6/7] Platform/RPi4: Add XHCI ACPI table To: Pete Batard Cc: edk2-devel-groups-io , Leif Lindholm , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= Content-Type: text/plain; charset="UTF-8" On Thu, 19 Dec 2019 at 15:32, Pete Batard wrote: > > Hi Ard, > > On 2019.12.19 13:12, Ard Biesheuvel wrote: > > Hi Pete, > > > > On Thu, 19 Dec 2019 at 14:14, Pete Batard wrote: > >> > >> From: Andrei Warkentin > >> > >> Since the RPi4 PCIe host bridge is not ECAM compliant, we can > >> not expose it as a host bridge to the OS via ACPI. However, > >> given the hardwired nature of this platform, we can expose the > >> xHCI controller that is guaranteed to live at the base of the > >> MMIO32 BAR window as a platform device directly. > >> > >> It should be noted that the xHCI table is not finalized at this > >> stage, as Windows xHCI support is still a major question mark. > >> > >> Signed-off-by: Pete Batard > >> --- > >> Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf | 3 + > >> Platform/RaspberryPi/RPi4/AcpiTables/Dsdt.asl | 1 + > >> Platform/RaspberryPi/RPi4/AcpiTables/Xhci.asl | 137 ++++++++++++++++++++ > >> 3 files changed, 141 insertions(+) > >> > > ... > >> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Xhci.asl b/Platform/RaspberryPi/RPi4/AcpiTables/Xhci.asl > >> new file mode 100644 > >> index 000000000000..e1fd501ab895 > >> --- /dev/null > >> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Xhci.asl > >> @@ -0,0 +1,137 @@ > >> +/** @file > >> + * > >> + * Copyright (c) 2019 Linaro, Limited. All rights reserved. > >> + * Copyright (c) 2019 Andrei Warkentin > >> + * > >> + * SPDX-License-Identifier: BSD-2-Clause-Patent > >> + * > >> + **/ > >> + > >> +#include > >> + > >> +/* > >> + * 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_REG_BASE REMOVE_PARENTHESES(PCIE_REG_BASE) > >> + > >> +/* > >> + * According to UEFI boot log for the VLI device on Pi 4. > >> + */ > >> +#define XHCI_REG_LENGTH 0x1000 > >> + > >> +Device (SCB0) { > >> + Name (_HID, "ACPI0004") > >> + Name (_UID, 0x0) > >> + Name (_CCA, 0x0) > >> + > >> + Method (_CRS, 0, Serialized) { // _CRS: Current Resource Settings > >> + /* > >> + * Container devices with _DMA must have _CRS, meaning SCB0 > >> + * to provide all resources that XHC0 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, XHCI_REG_LENGTH - 1, MMBE) > >> + Add (MMLE, XHCI_REG_LENGTH - 1, MMLE) > >> + Return (RBUF) > >> + } > >> + > >> + Name (_DMA, ResourceTemplate() { > >> + /* > >> + * XHC0 is limited to DMA to first 3GB. Note this > >> + * only applies to PCIe, not GENET or other devices > >> + * next to the A72. > >> + */ > >> + QWordMemory (ResourceConsumer, > >> + , > >> + MinFixed, > >> + MaxFixed, > >> + NonCacheable, > >> + ReadWrite, > >> + 0x0, > >> + 0x0, // MIN > >> + 0xbfffffff, // MAX > >> + 0x0, // TRA > >> + 0xc0000000, // LEN > >> + , > >> + , > >> + ) > >> + }) > >> + > >> + Device (XHC0) > >> + { > >> + Name (_HID, "11063483") // _HID: Hardware ID > > > > I failed to spot this detail before. Even if MS appears to do so, I > > don't think it is OK to string random digits together to create > > hardware identifiers (and yes, I am aware it's the vendor and device > > IDs concatenated) > > > > What's wrong with using the below as the _HID? > > Well, considering that we haven't managed to get Windows booting with > xHCI yet anyway, and that this doesn't impact Linux boot (I've just > validated this, to be safe), I see nothing wrong with using "PNP0D10" > for now and then figure out later if there's a need to use something else. > > If there are other requests leading to a v3, I'll apply that change. But > if not, do you (or the person who's going to do it) mind changing the > string during integration? > No, that's fine, I'll fix it up when applying. > > > >> + Name (_CID, "PNP0D10") // _CID: Hardware ID > >> + Name (_UID, 0x0) // _UID: Unique ID > >> + Name (_CCA, 0x0) // _CCA: Cache Coherency Attribute > >> + > >> + Method (_CRS, 0, Serialized) { // _CRS: Current Resource Settings > >> + Name (RBUF, ResourceTemplate () { > >> + QWordMemory (ResourceConsumer, > >> + , > >> + MinFixed, > >> + MaxFixed, > >> + NonCacheable, > >> + ReadWrite, > >> + 0x0, > >> + SANITIZED_PCIE_CPU_MMIO_WINDOW, // MIN > >> + SANITIZED_PCIE_CPU_MMIO_WINDOW, // MAX > >> + 0x0, > >> + 0x1, // LEN > >> + , > >> + , > >> + MMIO > >> + ) > >> + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, ) { > >> + 175 > >> + } > >> + }) > >> + CreateQwordField (RBUF, MMIO._MAX, MMBE) > >> + CreateQwordField (RBUF, MMIO._LEN, MMLE) > >> + Add (MMBE, XHCI_REG_LENGTH - 1, MMBE) > >> + Add (MMLE, XHCI_REG_LENGTH - 1, MMLE) > >> + Return (RBUF) > >> + } > >> + > >> + Method (_INI, 0, Serialized) { > >> + OperationRegion (PCFG, SystemMemory, SANITIZED_PCIE_REG_BASE + PCIE_EXT_CFG_DATA, 0x1000) > >> + Field (PCFG, AnyAcc, NoLock, Preserve) { > >> + Offset (0), > >> + VNID, 16, // Vendor ID > >> + DVID, 16, // Device ID > >> + CMND, 16, // Command register > >> + STAT, 16, // Status register > >> + } > >> + > >> + // Set command register to: > >> + // 1) decode MMIO (set bit 1) > >> + // 2) enable DMA (set bit 2) > >> + // 3) enable interrupts (clear bit 10) > >> + Debug = "xHCI enable" > >> + Store (0x6, CMND) > >> + } > >> + } > >> +} > >> -- > >> 2.21.0.windows.1 > >> >