From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.15159.1670420066592054938 for ; Wed, 07 Dec 2022 05:34:27 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 9E65823A; Wed, 7 Dec 2022 05:34:32 -0800 (PST) Received: from [10.34.100.128] (pierre123.nice.arm.com [10.34.100.128]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 16D5A3F73D; Wed, 7 Dec 2022 05:34:24 -0800 (PST) Message-ID: <36c59ec4-a158-c1f1-021a-29a5d7645d0c@arm.com> Date: Wed, 7 Dec 2022 14:34:21 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 From: "PierreGondois" Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1 2/6] Platform/Sgi: add ssdt table for non-discoverable IO virtualization block To: devel@edk2.groups.io, vivek.gautam@arm.com Cc: Sami Mujawar , Ard Biesheuvel , Leif Lindholm References: <20220214121307.14608-1-vivek.gautam@arm.com> <20220214121307.14608-3-vivek.gautam@arm.com> In-Reply-To: <20220214121307.14608-3-vivek.gautam@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello Vivek, Sorry for the long wait. I think the whole patchset needs to be rebased on latest master. I just have some comments for patches: - [PATCH V1 2/6] Platform/Sgi: add ssdt table for non-discoverable IO virtualization block - [PATCH V1 3/6] Platform/Sgi: Initialize additional uart controllers The other patches look good to me. Regards, Pierre On 2/14/22 13:13, Vivek Kumar Gautam via groups.io wrote: > Arm reference design platforms such as RD-N2 and RD-N2-Cfg1 have multiple > IO virtualization blocks that allow connecting PCIe root bus or non-PCIe > devices to the system. For platforms that connect non-discoverable (non- > PCI) devices to IO virtualization block, add a SSDT table to describe > such devices and use PCDs for the memory region and interrupts of these > devices in the table entry. > There are two PL011 UART controllers and two PL330 DMA controllers > connected to the non-PCIe IO virtualization block on RD-N2 and > RD-N2-Cfg1 platforms. List them in the SSDT ACPI table. > > While we are adding SSDT table entries for RD-N2 and RD-N2-Cfg1 > remove the source file entries for incorrect SSDT and MCFG tables > for RD-N2 and RD-N2-Cfg1 platforms. > > Signed-off-by: Vivek Gautam > --- > Platform/ARM/SgiPkg/SgiPlatform.dec | 42 ++++ > Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc | 40 ++++ > Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf | 45 ++++- > Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf | 45 ++++- > Platform/ARM/SgiPkg/AcpiTables/SsdtNonPciIoVirtBlk.asl | 203 ++++++++++++++++++++ > 5 files changed, 369 insertions(+), 6 deletions(-) > > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec > index 05079743c452..6b3e28c3a08e 100644 > --- a/Platform/ARM/SgiPkg/SgiPlatform.dec > +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec > @@ -95,5 +95,47 @@ > gArmSgiTokenSpaceGuid.PcdOscLpiEnable|0|UINT32|0x00000025 > gArmSgiTokenSpaceGuid.PcdOscCppcEnable|0|UINT32|0x00000026 > > + # IO virtualization block PL011 UARTs > + gArmSgiTokenSpaceGuid.PcdIoVirtBlkUart0Base|0|UINT64|0x0000002C > + gArmSgiTokenSpaceGuid.PcdIoVirtBlkUart0End|0|UINT64|0x0000002D I think it should be possible to remove all the Pcd*End addresses and replace them with (PcdIoVirtBlkUart0Base + PcdIoVirtBlkUart0Size - 1). > + gArmSgiTokenSpaceGuid.PcdIoVirtBlkUart0Size|0|UINT64|0x0000002E > + gArmSgiTokenSpaceGuid.PcdIoVirtBlkUart0Interrupt|0|UINT32|0x0000002F > + > + gArmSgiTokenSpaceGuid.PcdIoVirtBlkUart1Base|0|UINT64|0x00000030 > + gArmSgiTokenSpaceGuid.PcdIoVirtBlkUart1End|0|UINT64|0x00000031 > + gArmSgiTokenSpaceGuid.PcdIoVirtBlkUart1Size|0|UINT64|0x00000032 > + gArmSgiTokenSpaceGuid.PcdIoVirtBlkUart1Interrupt|0|UINT32|0x00000033 > + [...] > gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress > diff --git a/Platform/ARM/SgiPkg/AcpiTables/SsdtNonPciIoVirtBlk.asl b/Platform/ARM/SgiPkg/AcpiTables/SsdtNonPciIoVirtBlk.asl > new file mode 100644 > index 000000000000..a035186b88db > --- /dev/null > +++ b/Platform/ARM/SgiPkg/AcpiTables/SsdtNonPciIoVirtBlk.asl > @@ -0,0 +1,203 @@ > +/** @file > + Secondary System Description Table (SSDT) for Non-PCIe IO > + Virtualization Block. > + > + The IO virtualization block present on reference design platforms > + such as RD-N2 and RD-N2-Cfg1 allows connecting PCIe and non-PCIe > + devices. The non-discoverable (non-PCIe) devices that are connected > + to the IO virtualization block include two PL011 UART and two PL330 > + DMA controllers. > + > + Copyright (c) 2022, Arm Ltd. All rights reserved. > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > + @par Specification Reference: > + - ACPI 6.4, Chapter 5, Section 5.2.11.2, Secondary System Description Table > +**/ > + > +#include "SgiPlatform.h" > +#include "SgiAcpiHeader.h" > + > +DefinitionBlock ("SsdtIoVirtBlk.aml", "SSDT", 2, "ARMLTD", "ARMSGI", > + EFI_ACPI_ARM_OEM_REVISION) { > + Scope (_SB) { > + > + // IO Virtualization Block - PL011 UART0 > + Device (COM4) { > + Name (_HID, "ARMH0011") > + Name (_UID, 4) > + Name (_STA, 0xF) > + > + Name (_CRS, ResourceTemplate () { > + QWordMemory ( > + ResourceProducer, > + PosDecode, > + MinFixed, > + MaxFixed, > + NonCacheable, > + ReadWrite, > + 0x0, > + FixedPcdGet64 (PcdIoVirtBlkUart0Base), > + FixedPcdGet64 (PcdIoVirtBlkUart0End), > + 0x0, > + FixedPcdGet32 (PcdIoVirtBlkUart0Size), > + , > + , > + , > + AddressRangeMemory, > + TypeStatic > + ) > + > + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { > + FixedPcdGet32 (PcdIoVirtBlkUart0Interrupt) > + } > + }) > + } > + > + // IO Virtualization Block - PL011 UART1 > + Device (COM5) { > + Name (_HID, "ARMH0011") > + Name (_UID, 5) > + Name (_STA, 0xF) > + > + Name (_CRS, ResourceTemplate () { > + QWordMemory ( > + ResourceProducer, > + PosDecode, > + MinFixed, > + MaxFixed, > + NonCacheable, > + ReadWrite, > + 0x0, > + FixedPcdGet64 (PcdIoVirtBlkUart1Base), > + FixedPcdGet64 (PcdIoVirtBlkUart1End), > + 0x0, > + FixedPcdGet32 (PcdIoVirtBlkUart1Size), > + , > + , > + , > + AddressRangeMemory, > + TypeStatic > + ) > + > + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { > + FixedPcdGet32 (PcdIoVirtBlkUart1Interrupt) > + } > + }) > + } > + > + // IO Virtualization Block - PL330 DMA0 > + Device (\_SB.DMA0) { > + Name (_HID, "ARMH0330") Is there a specification for the description of this _HID and how it should be represented in ACPI ?