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.96680.1674800247433439596 for ; Thu, 26 Jan 2023 22:17:27 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: vivek.gautam@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 C8CF72B; Thu, 26 Jan 2023 22:18:08 -0800 (PST) Received: from [10.162.43.190] (a077843.arm.com [10.162.43.190]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 103993F5A1; Thu, 26 Jan 2023 22:17:24 -0800 (PST) Message-ID: <85bdeaf4-5509-2f1a-711f-0df402869edc@arm.com> Date: Fri, 27 Jan 2023 11:47:17 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 From: "Vivek Kumar Gautam" Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1 2/6] Platform/Sgi: add ssdt table for non-discoverable IO virtualization block To: Pierre Gondois , devel@edk2.groups.io Cc: Sami Mujawar , Ard Biesheuvel , Leif Lindholm References: <20220214121307.14608-1-vivek.gautam@arm.com> <20220214121307.14608-3-vivek.gautam@arm.com> <36c59ec4-a158-c1f1-021a-29a5d7645d0c@arm.com> In-Reply-To: <36c59ec4-a158-c1f1-021a-29a5d7645d0c@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Hi Pierre, On 12/7/22 19:04, Pierre Gondois wrote: > 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=20 > virtualization block > - [PATCH V1 3/6] Platform/Sgi: Initialize additional uart controllers > The other patches look good to me. > Thank you for your review and apologies for responding late. I was able=20 to rework the patches and get a cleaner SSDT table implementation. I=20 will post the patches soon. Please see my responses inline. > 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=20 >> multiple >> IO virtualization blocks that allow connecting PCIe root bus or non-PC= Ie >> devices to the system. For platforms that connect non-discoverable (no= n- >> PCI) devices to IO virtualization block, add a SSDT table to describe >> such devices and use PCDs for the memory region and interrupts of thes= e >> 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 >> --- >> =C2=A0 Platform/ARM/SgiPkg/SgiPlatform.dec=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 |=C2=A0 42 ++++ >> =C2=A0 Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 40 ++++ >> =C2=A0 Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 |=C2=A0 45 ++++- >> =C2=A0 Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf=C2=A0 |=C2= =A0 45 ++++- >> =C2=A0 Platform/ARM/SgiPkg/AcpiTables/SsdtNonPciIoVirtBlk.asl | 203=20 >> ++++++++++++++++++++ >> =C2=A0 5 files changed, 369 insertions(+), 6 deletions(-) >> >> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec=20 >> 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 @@ >> =C2=A0=C2=A0=C2=A0 gArmSgiTokenSpaceGuid.PcdOscLpiEnable|0|UINT32|0x00= 000025 >> =C2=A0=C2=A0=C2=A0 gArmSgiTokenSpaceGuid.PcdOscCppcEnable|0|UINT32|0x0= 0000026 >> =C2=A0 +=C2=A0 # 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). I will post the reworked patch-set addressing this. > >> + 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=20 >> 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 >> +=C2=A0 Secondary System Description Table (SSDT) for Non-PCIe IO >> +=C2=A0 Virtualization Block. >> + >> +=C2=A0 The IO virtualization block present on reference design platfo= rms >> +=C2=A0 such as RD-N2 and RD-N2-Cfg1 allows connecting PCIe and non-PC= Ie >> +=C2=A0 devices. The non-discoverable (non-PCIe) devices that are conn= ected >> +=C2=A0 to the IO virtualization block include two PL011 UART and two = PL330 >> +=C2=A0 DMA controllers. >> + >> +=C2=A0 Copyright (c) 2022, Arm Ltd. All rights reserved. >> +=C2=A0 SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> +=C2=A0 @par Specification Reference: >> +=C2=A0=C2=A0=C2=A0 - ACPI 6.4, Chapter 5, Section 5.2.11.2, Secondary= System=20 >> Description Table >> +**/ [snip] >> >> + >> +=C2=A0=C2=A0=C2=A0 // IO Virtualization Block - PL330 DMA0 >> +=C2=A0=C2=A0=C2=A0 Device (\_SB.DMA0) { >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Name (_HID, "ARMH0330") > > Is there a specification for the description of this _HID and how > it should be represented in ACPI ? Yes, this can be found in the ACPI specification here [1]. The Linux=20 kernel documentation also describes it here [2] as the primary object to=20 use in device probing. [1]=20 https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/06_Device_Configuration/Dev= ice_Configuration.html#device-identification-objects [2] https://docs.kernel.org/arm64/acpi_object_usage.html Best regards Vivek