From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f67.google.com (mail-ed1-f67.google.com [209.85.208.67]) by mx.groups.io with SMTP id smtpd.web09.9379.1576762358741459256 for ; Thu, 19 Dec 2019 05:32:39 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=N2qSFMCi; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.208.67, mailfrom: pete@akeo.ie) Received: by mail-ed1-f67.google.com with SMTP id c26so4871840eds.8 for ; Thu, 19 Dec 2019 05:32:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=LC5AD61CGE44wfCS9Kc65kYHYFB4GIru4ayaGZFWNCY=; b=N2qSFMCiezeudI+ArTWCgK6pAt8K1ZYKN0RxPBtXPPIad3GlXJjwJVmCxfLzmMZDCP e51ZDmPlk2LaPqrWz4nrc9bq2S+VXIhXv2ylCHodQbf0C/UZbsj97JkNOJu2kYUfgfRz kqM27zqGoCDjFmreJGk2A9IejTV8UnYwMU4+3JQ1scaERsovyN1RAey5BprHPdHUCpsM pBAFNFeP0xmMagZreYxqsBIyU428aKX3RbKhJMUlJrsMBVbZVSBB78b4fOEkW8Go1PIn HdngkdtiR44OVpoZtt1SwwXLHu7v5UUla/vrOexbPOLJqJFz7Re2syUKF2Vcs1ZOqFXE y/UA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=LC5AD61CGE44wfCS9Kc65kYHYFB4GIru4ayaGZFWNCY=; b=Ukl00Q7nX8DWN11g5jnhqREXJf1LCN/K6adY+Hc5h/YCfvVDr7X5JiZgQyYHHlZAyB bf06r4n5kLxcLgI4qGwuJN+jdEWMlrbGq0wbouMxR1OcpVilfZyQLb14/ilaIW7BPgnj fNv4exM7Hmxa+kiEnYat0338MLQH1wH5/+I6mQUhty13YC2os5UTUnf1dawYoA9CpFKV yg1Ry2sq6nXzN8XuoXwnwEpSCeHxiVHsV6btoClLWbOBSNL57W5ie4tUPxd5QQNtOitu 7e6E3wswshD2JqS1ze8FHFmM8nCYxy0sUad1fe1KW9N7kHLY2ZTFBD3xUGP93lGTaJIf ddHA== X-Gm-Message-State: APjAAAXcKddO/S711FA9oHFp8PqtYGTQz+v8IkDwlUwHyH1UV7xbf0GK x+VkAwYJwYNmkDL0QIfsoj/esA== X-Google-Smtp-Source: APXvYqyCT3Tj5i25Nm7+/1J1OaJAhlt8QeD1OQiEGH++c9Fq8iHEEK9yGiOoBIGva4/v3iGYNd7c8w== X-Received: by 2002:a17:906:7286:: with SMTP id b6mr9611300ejl.244.1576762357053; Thu, 19 Dec 2019 05:32:37 -0800 (PST) Return-Path: Received: from [10.0.0.122] ([84.203.77.210]) by smtp.googlemail.com with ESMTPSA id s11sm476622ejx.90.2019.12.19.05.32.35 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Dec 2019 05:32:36 -0800 (PST) Subject: Re: [edk2-platforms][PATCH v2 6/7] Platform/RPi4: Add XHCI ACPI table To: Ard Biesheuvel Cc: edk2-devel-groups-io , Leif Lindholm , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= References: <20191219121434.2856-1-pete@akeo.ie> <20191219121434.2856-7-pete@akeo.ie> From: "Pete Batard" Message-ID: Date: Thu, 19 Dec 2019 13:32:34 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit 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? Regards, /Pete > >> + 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 >>