public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Vivek Kumar Gautam" <vivek.gautam@arm.com>
To: Pierre Gondois <pierre.gondois@arm.com>, devel@edk2.groups.io
Cc: Sami Mujawar <sami.mujawar@arm.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Leif Lindholm <leif@nuviainc.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1 2/6] Platform/Sgi: add ssdt table for non-discoverable IO virtualization block
Date: Fri, 27 Jan 2023 11:47:17 +0530	[thread overview]
Message-ID: <85bdeaf4-5509-2f1a-711f-0df402869edc@arm.com> (raw)
In-Reply-To: <36c59ec4-a158-c1f1-021a-29a5d7645d0c@arm.com>

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 
> 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 
to rework the patches and get a cleaner SSDT table implementation. I 
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 
>> 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 <vivek.gautam@arm.com>
>> ---
>>   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).

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 
>> 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
>> +**/

[snip]

>>
>> +
>> +    // 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 ?

Yes, this can be found in the ACPI specification here [1]. The Linux 
kernel documentation also describes it here [2] as the primary object to 
use in device probing.

[1] 
https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/06_Device_Configuration/Device_Configuration.html#device-identification-objects
[2] https://docs.kernel.org/arm64/acpi_object_usage.html


Best regards
Vivek


  reply	other threads:[~2023-01-27  6:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-14 12:13 [edk2-platforms][PATCH V1 0/6] Add non-discoverable IO block for Rd-N2 Vivek Kumar Gautam
2022-02-14 12:13 ` [edk2-platforms][PATCH V1 1/6] Platform/Sgi: add PCDs for SMMUv3 base address and interrupts Vivek Kumar Gautam
2022-02-14 12:13 ` [edk2-platforms][PATCH V1 2/6] Platform/Sgi: add ssdt table for non-discoverable IO virtualization block Vivek Kumar Gautam
2022-12-07 13:34   ` [edk2-devel] " PierreGondois
2023-01-27  6:17     ` Vivek Kumar Gautam [this message]
2022-02-14 12:13 ` [edk2-platforms][PATCH V1 3/6] Platform/Sgi: Initialize additional uart controllers Vivek Kumar Gautam
2022-12-07 13:34   ` [edk2-devel] " PierreGondois
2022-02-14 12:13 ` [edk2-platforms][PATCH V1 4/6] Platform/Sgi: add helper macros for ITS, SMMUv3 and DMA IORT nodes Vivek Kumar Gautam
2022-02-14 12:13 ` [edk2-platforms][PATCH V1 5/6] Platform/Sgi: add IORT table for IO virtualization block on RD-N2-Cfg1 Vivek Kumar Gautam
2022-02-14 12:13 ` [edk2-platforms][PATCH V1 6/6] Platform/Sgi: add IORT table for IO virtualization block on RD-N2 Vivek Kumar Gautam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=85bdeaf4-5509-2f1a-711f-0df402869edc@arm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox