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.web11.14394.1675439767888576392 for ; Fri, 03 Feb 2023 07:56:08 -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 B87D1C14; Fri, 3 Feb 2023 07:56:49 -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 8FF5F3F71E; Fri, 3 Feb 2023 07:56:06 -0800 (PST) Message-ID: <9f5b591a-e6b6-2bb2-b5bf-6173b13e4fff@arm.com> Date: Fri, 3 Feb 2023 16:56:05 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [edk2-platforms][PATCH V2 3/5] Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block To: Vivek Gautam , devel@edk2.groups.io Cc: ardb+tianocore@kernel.org, leif@nuviainc.com, Sami.Mujawar@arm.com References: <20230127092338.72056-1-vivek.gautam@arm.com> <20230127092338.72056-4-vivek.gautam@arm.com> From: "PierreGondois" In-Reply-To: <20230127092338.72056-4-vivek.gautam@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello Vivek, On 1/27/23 10:23, Vivek Gautam wrote: > Arm reference design platforms have multiple IO virtualization blocks > that allow connecting PCIe root bus or non-PCIe SoC peripherals to the > system. Each of these IO virtualization blocks consists of an instance > of SMMUv3, a GIC-ITS and a NCI (network chip interconnect) to support > traffic flow and address mapping, as required. > > The SoC expansion blocks that connect to the IO virtualization block > include devices such as UARTs, DMAs and few additional memory nodes. For > platforms having SoC expansion block connected to the IO virtualization > block add a SSDT table to describe devices included in the SoC expansion > block. Preprocessor macros are also added in this change to allow > scalability for platforms that implement multiple instances of these SoC > expansion blocks. > > Signed-off-by: Vivek Gautam > --- > Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc | 5 + > Platform/ARM/SgiPkg/Include/IoVirtSoCExp.h | 189 ++++++++++++++++++++ > Platform/ARM/SgiPkg/AcpiTables/SsdtIoVirtSocExp.asl | 96 ++++++++++ > Platform/ARM/SgiPkg/SgiPlatform.dec | 5 + > 4 files changed, 295 insertions(+) > > diff --git a/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc b/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc > index 12dcd82eb132..14734fb65828 100644 > --- a/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc > +++ b/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc > @@ -72,3 +72,8 @@ > gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress|0x0C1D0000 > gArmSgiTokenSpaceGuid.PcdGpioController0Size|0x00010000 > gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt|392 > + > + # IO virtualization block > + gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base|0x1080000000 > + gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset|0x10000000 > + gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkResourceSize|0x10000 > diff --git a/Platform/ARM/SgiPkg/Include/IoVirtSoCExp.h b/Platform/ARM/SgiPkg/Include/IoVirtSoCExp.h > new file mode 100644 > index 000000000000..8e73b8989b16 > --- /dev/null > +++ b/Platform/ARM/SgiPkg/Include/IoVirtSoCExp.h > @@ -0,0 +1,189 @@ > +/** @file > +* > +* Copyright (c) 2023, Arm Limited. All rights reserved. > +* > +* SPDX-License-Identifier: BSD-2-Clause-Patent > +* > +**/ > + > +#include "SgiPlatform.h" > + > +#define IO_VIRT_BLK_BASE FixedPcdGet64 (PcdIoVirtSocExpBlk0Base) > +#define RESOURCE_SIZE FixedPcdGet32 (PcdIoVirtSocExpBlkResourceSize) > + > +/** Macros to calculate base addresses of UART and DMA devices within IO > + virtualization SoC expansion block address space. > + > + @param [in] n Index of UART or DMA device within SoC expansion block. > + Should be either 0 or 1. > + > + The base address offsets of UART and DMA devices within a SoC expansion block > + are shown below. The UARTs are at offset (2 * index + 0x1000_0000), while the I think it's (2 * index * offset) (the offset is missing). > + DMAs are at offsets ((2 * index + 1) + 0x1000_0000). > + +----------------------------------------------+ > + | Port # | Peripheral | Base address offset | > + |--------|---------------|---------------------| > + | x4_0 | PL011_UART0 | 0x0000_0000 | > + |--------|---------------|---------------------| > + | x4_1 | PL011_DMA0_NS | 0x1000_0000 | > + |--------|---------------|---------------------| > + | x8 | PL011_UART1 | 0x2000_0000 | > + |--------|---------------|---------------------| > + | x16 | PL011_DMA1_NS | 0x3000_0000 | > + +----------------------------------------------+ > +**/ > +#define UART_START(n) IO_VIRT_BLK_BASE + \ > + (2 * n * FixedPcdGet32 (PcdIoVirtSocExpBlkPeriOffset)) > +#define DMA_START(n) IO_VIRT_BLK_BASE + \ > + (((2 * n) + 1) * FixedPcdGet32 (PcdIoVirtSocExpBlkPeriOffset)) The values of: - PcdIoVirtSocExpBlk0Base - PcdIoVirtSocExpBlkPeriOffset - PcdIoVirtSocExpBlkResourceSize are the same for all Rdn2[|Cfg1|Cfg2] platforms and the documentation above is referring to hard-coded values (e.g. 0x1000_0000), so would it be worth defining them as local macros only ? > + > +// Interrupt numbers of PL330 DMA-0 and DMA-1 devices in the SoC expansion > +// connected to the IO Virtualization block. Each DMA PL330 controller uses > +// eight data channel interrupts and one instruction channel interrupt to > +// notify aborts. > +#define RD_IOVIRT_SOC_EXP_DMA0_INTERRUPTS_INIT \ > + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { \ > + 493, 494, 495, 496, 497, 498, 499, 500, 501 \ > + } > +#define RD_IOVIRT_SOC_EXP_DMA1_INTERRUPTS_INIT \ > + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { \ > + 503, 504, 505, 506, 507, 508, 509, 510, 511 \ > + } > + > +#define RD_IOVIRT_SOC_EXP_DMA2_INTERRUPTS_INIT \ > + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { \ > + 973, 974, 975, 976, 977, 978, 979, 980, 981 \ > + } > + > +#define RD_IOVIRT_SOC_EXP_DMA3_INTERRUPTS_INIT \ > + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { \ > + 983, 984, 985, 986, 987, 988, 989, 990, 991 \ > + } > + > +#define RD_IOVIRT_SOC_EXP_DMA4_INTERRUPTS_INIT \ > + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { \ > + 4557, 4558, 4559, 4560, 4561, 4562, 4563, 4564, 4565 \ > + } > + > +#define RD_IOVIRT_SOC_EXP_DMA5_INTERRUPTS_INIT \ > + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { \ > + 4567, 4568, 4569, 4570, 4571, 4572, 4573, 4574, 4575 \ > + } > + > +#define RD_IOVIRT_SOC_EXP_DMA6_INTERRUPTS_INIT \ > + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { \ > + 5037, 5038, 5039, 5040, 5041, 5042, 5043, 5044, 5045 \ > + } > + > +#define RD_IOVIRT_SOC_EXP_DMA7_INTERRUPTS_INIT \ > + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { \ > + 5047, 5048, 5049, 5050, 5051, 5052, 5053, 5054, 5055 \ > + } > + > +/** Macro for PL011 UART controller node instantiation in SSDT table. > + > + See section 5.2.11.2 of ACPI specification v6.4 for the definition of SSDT > + table. Use of this macro is restricted to ASL file and not to TDL file. Out of cur > + > + @param [in] ComIdx Index of Com device to be initializaed; > + to be passed as 2-digit index, such as 01 to > + support multichip platforms as well. > + @param [in] ChipIdx Index of chip to which this DMA device belongs > + @param [in] StartOff Starting offset of this device within IO > + virtualization block memory map > + @param [in] IrqNum Interrupt ID used for the device > +**/ > +#define RD_IOVIRT_SOC_EXP_COM_INIT(ComIdx, ChipIdx, StartOff, IrqNum) \ > + Device (COM ##ComIdx) { \ > + Name (_HID, "ARMH0011") \ > + Name (_UID, ComIdx) \ > + Name (_STA, 0xF) \ > + \ > + Method (_CRS, 0, Serialized) { \ > + Name (RBUF, ResourceTemplate () { \ > + QWordMemory ( \ > + ResourceProducer, \ > + PosDecode, \ > + MinFixed, \ > + MaxFixed, \ > + NonCacheable, \ > + ReadWrite, \ > + 0x0, \ > + 0, \ > + 1, \ > + 0x0, \ > + 2, \ > + , \ > + , \ > + MMI1, \ > + AddressRangeMemory, \ > + TypeStatic \ > + ) \ > + \ > + Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { \ > + IrqNum \ > + } \ > + }) /* end Name(RBUF) */ \ > + /* Work around ASL's inability to add in a resource definition */ \ > + CreateQwordField (RBUF, MMI1._MIN, MIN1) \ > + CreateQwordField (RBUF, MMI1._MAX, MAX1) \ > + CreateQwordField (RBUF, MMI1._LEN, LEN1) \ > + Add (SGI_REMOTE_CHIP_MEM_OFFSET(ChipIdx), StartOff, MIN1) \ > + Add (MIN1, RESOURCE_SIZE - 1, MAX1) \ > + Add (RESOURCE_SIZE, 0, LEN1) \ This seems like a complicated way to do additions. I guess the asl compiler doesn't allow doing this. The DynamicTablesPkg could allow generating such resources. Is there a reason not to use it ? > + \ > + Return (RBUF) \ > + } /* end Method(_CRS) */ \ > + } > + > +/** Macro for PL330 DMA controller node instantiation in SSDT table. > + > + See section 5.2.11.2 of ACPI specification v6.4 for the definition of SSDT > + table. Use of this macro is restricted to ASL file and not to TDL file. > + > + @param [in] DmaIdx Index of DMA device to be initializaed > + @param [in] ChipIdx Index of chip to which this DMA device belongs > + @param [in] StartOff Starting offset of this device within IO > + virtualization block memory map > +**/ > +#define RD_IOVIRT_SOC_EXP_DMA_INIT(DmaIdx, ChipIdx, StartOff) \ > + Device (\_SB.DMA ##DmaIdx) { \ > + Name (_HID, "ARMH0330") \ > + Name (_UID, DmaIdx) \ > + Name (_CCA, 1) \ > + Name (_STA, 0xF) \ > + \ > + Method (_CRS, 0, Serialized) { \ > + Name (RBUF, ResourceTemplate () { \ > + QWordMemory ( \ > + ResourceProducer, \ > + PosDecode, \ > + MinFixed, \ > + MaxFixed, \ > + NonCacheable, \ > + ReadWrite, \ > + 0x0, \ > + 0, \ > + 1, \ > + 0x0, \ > + 2, \ > + , \ > + , \ > + MMI2, \ > + AddressRangeMemory, \ > + TypeStatic \ > + ) \ > + \ > + RD_IOVIRT_SOC_EXP_DMA ##DmaIdx## _INTERRUPTS_INIT \ > + }) /* end Name(RBUF) */ \ > + /* Work around ASL's inability to add in a resource definition */ \ > + CreateQwordField (RBUF, MMI2._MIN, MIN2) \ > + CreateQwordField (RBUF, MMI2._MAX, MAX2) \ > + CreateQwordField (RBUF, MMI2._LEN, LEN2) \ > + Add (SGI_REMOTE_CHIP_MEM_OFFSET(ChipIdx), StartOff, MIN2) \ > + Add (MIN2, RESOURCE_SIZE - 1, MAX2) \ > + Add (RESOURCE_SIZE, 0, LEN2) \ > + \ > + Return (RBUF) \ > + } /* end Method(_CRS) */ \ > + } > diff --git a/Platform/ARM/SgiPkg/AcpiTables/SsdtIoVirtSocExp.asl b/Platform/ARM/SgiPkg/AcpiTables/SsdtIoVirtSocExp.asl > new file mode 100644 > index 000000000000..47cd3cb017a2 > --- /dev/null > +++ b/Platform/ARM/SgiPkg/AcpiTables/SsdtIoVirtSocExp.asl > @@ -0,0 +1,96 @@ > +/** @file > + Secondary System Description Table (SSDT) for IO Virtualization SoC Expansion > + > + The IO virtualization blocks on Arm Reference Design (RD) platforms allow > + connecting PCIe root bus as well as other non-PCIe SoC peripherals. Each of > + these IO virtualization blocks consists of an instance of SMMUv3, a GIC-ITS > + and a NCI (network chip interconnect) to support traffic flow and address > + mapping, as required. The PCIe root bus or the SoC peripherals connect to the > + IO virtualization block over ports namely x4_0, x4_1, x8 and x16. > + > + Some of the RD platforms utilize one or more IO virtualization blocks to > + connect non-PCIe devices mapped in the SoC expansion address space. One > + such instance of SoC expansion block consists of a set of non-PCIe devices > + that includes two PL011 UART controllers, two PL330 DMA controllers and > + few additional memory nodes. The devices in this SoC expansion block are > + placed at fixed offsets from a base address in the SoC expansion address > + space and the read/write accesses to these devices are routed by the IO > + virtualization block. > + > + The table below lists the address offset, address space size and interrupts > + used for the devices present in each instance of this SoC expansion block > + that is connected to the IO Virtualization block. > + +-------------------------------------------------------------------------------+ > + | Port | Peripheral | Memory map | Size | Interrupt | > + | # | |-------------------------------------| | ID | > + | | | Start Addr Offset | End Addr Offset | | | > + +-------------------------------------------------------------------------------+ > + | x4_0 | PL011_UART0 | 0x0000_0000 | 0x0000_FFFF | 64KB | 492 | > + |-------------------------------------------------------------------------------| > + | x4_1 | PL011_DMA0_NS | 0x1000_0000 | 0x1000_FFFF | 64KB | 493-501 | > + |-------------------------------------------------------------------------------| > + | x8 | PL011_UART1 | 0x2000_0000 | 0x2000_FFFF | 64KB | 502 | > + |-------------------------------------------------------------------------------| > + | x16 | PL011_DMA1_NS | 0x3000_0000 | 0x3000_FFFF | 64KB | 503-511 | > + +-------------------------------------------------------------------------------+ > + > + This SSDT ACPI table lists the SoC expansion block devices connected via the > + IO Virtualization block on RD-N2 platform variants and mapped to SoC expansion > + address at an offset of 0x10_8000_0000 from each chip's base address. > + > + Copyright (c) 2023, Arm Limited. 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 "IoVirtSoCExp.h" > +#include "SgiAcpiHeader.h" > + > +DefinitionBlock ("SsdtIoVirtSocExp.aml", "SSDT", 2, "ARMLTD", "ARMSGI", > + EFI_ACPI_ARM_OEM_REVISION) { > + Scope (_SB) > + { > + > + // IO Virtualization SoC Expansion - PL011 UART > + if (LEqual (FixedPcdGet32 (PcdIoVirtSocExpBlkUartEnable), 1)) { > + RD_IOVIRT_SOC_EXP_COM_INIT(2, 0, UART_START(0), 492) > + RD_IOVIRT_SOC_EXP_COM_INIT(3, 0, UART_START(1), 502) > + > + if (LGreater (FixedPcdGet32 (PcdChipCount), 1)) { > + RD_IOVIRT_SOC_EXP_COM_INIT(4, 1, UART_START(0), 972) > + RD_IOVIRT_SOC_EXP_COM_INIT(5, 1, UART_START(1), 982) > + } > + > + if (LGreater (FixedPcdGet32 (PcdChipCount), 2)) { > + RD_IOVIRT_SOC_EXP_COM_INIT(6, 2, UART_START(0), 4556) > + RD_IOVIRT_SOC_EXP_COM_INIT(7, 2, UART_START(1), 4566) > + } > + > + if (LGreater (FixedPcdGet32 (PcdChipCount), 3)) { > + RD_IOVIRT_SOC_EXP_COM_INIT(8, 3, UART_START(0), 5036) > + RD_IOVIRT_SOC_EXP_COM_INIT(9, 3, UART_START(1), 5046) > + } > + } > + > + // IO Virtualization SoC Expansion - PL330 DMA > + RD_IOVIRT_SOC_EXP_DMA_INIT(0, 0, DMA_START(0)) > + RD_IOVIRT_SOC_EXP_DMA_INIT(1, 0, DMA_START(1)) > + > + if (LGreater (FixedPcdGet32 (PcdChipCount), 1)) { > + RD_IOVIRT_SOC_EXP_DMA_INIT(2, 1, DMA_START(0)) > + RD_IOVIRT_SOC_EXP_DMA_INIT(3, 1, DMA_START(1)) > + } > + > + if (LGreater (FixedPcdGet32 (PcdChipCount), 2)) { > + RD_IOVIRT_SOC_EXP_DMA_INIT(4, 2, DMA_START(0)) > + RD_IOVIRT_SOC_EXP_DMA_INIT(5, 2, DMA_START(1)) > + } > + > + if (LGreater (FixedPcdGet32 (PcdChipCount), 3)) { > + RD_IOVIRT_SOC_EXP_DMA_INIT(6, 3, DMA_START(0)) > + RD_IOVIRT_SOC_EXP_DMA_INIT(7, 3, DMA_START(1)) > + } Is it possible to decide to include the definitions at build time with: #if (FixedPcdGet32 (PcdChipCount) > 3) ? Same comment for other 'if (LGreater (...' > + } // Scope(_SB) > +} > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec > index e878af24d56b..407f03c1c3e8 100644 > --- a/Platform/ARM/SgiPkg/SgiPlatform.dec > +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec > @@ -98,5 +98,10 @@ > # Address bus width > gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip|0x0|UINT64|0x00000027 > > + # IO virtualization block > + gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base|0|UINT64|0x0000002B > + gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset|0|UINT32|0x0000002C > + gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkResourceSize|0|UINT32|0x0000002D > + > [Ppis] > gNtFwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }