public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 for RD-N2 variants
@ 2023-01-27  9:23 Vivek Kumar Gautam
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 1/5] Platform/Sgi: Add SSDT table for Virtio-P9 Vivek Kumar Gautam
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Vivek Kumar Gautam @ 2023-01-27  9:23 UTC (permalink / raw)
  To: devel; +Cc: ardb+tianocore, leif, Sami.Mujawar, Pierre.Gondois, Vivek.Gautam

Arm reference design Fixed Virtual Platforms (FVPs) such as the RD-N2
platform variants 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 Arm SMMUv3, a
GIC-ITS and a NCI (network chip interconnect).

SoC expansion blocks connect to the IO virtualization blocks via x4, x8
or x16 ports exposed by the system. A SoC expansion block implementation
includes 2 UARTs, 2 DMA devices and 2 Memory nodes.

In addition, Arm reference design platforms support Virtio-P9 device as
part of the Rest of System (RoS). The Virtio-P9 device implements a
subset of the Plan 9 file protocol over a virtio transport that enables
accessing a shared directory on the host's filesystem from a running
FVP platform.

This patch series adds SSDT tables for various RD-N2 platforms such as
RD-N2, RD-N2-Cfg1, and RD-N2-Cfg2 to describe the SoC expansion block
devices - UARTs, and DMAs and the Virtio-P9 devices present on the
platforms. The patches also add support for platform DXE driver to
initialize the UARTs that are present in SoC expansion blocks. By
default these UARTs are kept disabled and can be enabled with a Pcd -
PcdIoVirtSocExpBlkUartEnable.

This patch series is now a combination of two patch series [1] and [2]
that added Virtio-P9 support and SoC expansion block (non-discoverable)
IO block for RD-N2:
[edk2-platforms][PATCH V1 0/2] Enable Virtio-P9 on RD-N2 platforms
[edk2-platforms][PATCH V1 0/6] Add non-discoverable IO block for Rd-N2

[1] https://edk2.groups.io/g/devel/message/94936
[2] https://edk2.groups.io/g/devel/message/86646

Changes since v1:
 - Minor update to Virtio-P9 SSDT table:
   - Name of the DefinitionBlock() is set to SsdtRosVirtioP9.aml rather
     than SsdtRosVirtioP9Table.aml
 - Updates to SoC expansion block:
   - Removed IORT table for SoC expansion block and kept only the SSDT
     table for devices.
   - SSDT table now uses arithmetic operations to calculate the start
     and end addresses of the devices in QWordMemory() blocks.
   - The number of PCDs for UARTs and DMAs are now reduced as the
     addresses are now calculated within the SSDT table based on the
     SoC expansion block base address and device offsets.
   - Defined macros for Interrupt() block for various DMA nodes.
   - Removed the first patch of the series that added PCDs for SMMU:
     [PATCH V1 1/6] Platform/Sgi: add PCDs for SMMUv3 base address and interrupts
   - Added support for SoC expansion block on RD-N2-Cfg2 platform as
     well.

Shriram K (1):
  Platform/Sgi: Initialize additional UART controllers

Vivek Gautam (4):
  Platform/Sgi: Add SSDT table for Virtio-P9
  Platform/Sgi: Enable virtio-p9 device on RD-N2 platform variants
  Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block
  Platform/Sgi: Enable SoC expansion block for RD-N2 variants

 Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc                |  12 +-
 Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf        |  15 +-
 Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf    |  15 +-
 Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf    |  11 +-
 Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf  |  10 +-
 Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf  |   7 +-
 Platform/ARM/SgiPkg/Include/IoVirtSoCExp.h               | 189 ++++++++++++++++++++
 Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c    |  64 ++++++-
 Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c |  43 ++++-
 Platform/ARM/SgiPkg/AcpiTables/SsdtIoVirtSocExp.asl      |  96 ++++++++++
 Platform/ARM/SgiPkg/AcpiTables/SsdtRosVirtioP9.asl       |  42 +++++
 Platform/ARM/SgiPkg/SgiPlatform.dec                      |  13 +-
 12 files changed, 503 insertions(+), 14 deletions(-)
 create mode 100644 Platform/ARM/SgiPkg/Include/IoVirtSoCExp.h
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/SsdtIoVirtSocExp.asl
 create mode 100644 Platform/ARM/SgiPkg/AcpiTables/SsdtRosVirtioP9.asl

-- 
2.25.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [edk2-platforms][PATCH V2 1/5] Platform/Sgi: Add SSDT table for Virtio-P9
  2023-01-27  9:23 [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 for RD-N2 variants Vivek Kumar Gautam
@ 2023-01-27  9:23 ` Vivek Kumar Gautam
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 2/5] Platform/Sgi: Enable virtio-p9 device on RD-N2 platform variants Vivek Kumar Gautam
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Vivek Kumar Gautam @ 2023-01-27  9:23 UTC (permalink / raw)
  To: devel; +Cc: ardb+tianocore, leif, Sami.Mujawar, Pierre.Gondois, Vivek.Gautam

Some of the Arm reference design FVP platforms support the Virtio-p9
device as part of the RoS subsystem. Add an entry for this device in
the SSDT acpi table.

The device entry is listed in a new SSDT file as only some of the
reference design FVP platforms support it and so this file is included
in the build for only the platforms that support Virtio-P9 device.

Signed-off-by: Vivek Gautam <vivek.gautam@arm.com>
---
 Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc          |  7 +++-
 Platform/ARM/SgiPkg/AcpiTables/SsdtRosVirtioP9.asl | 42 ++++++++++++++++++++
 Platform/ARM/SgiPkg/SgiPlatform.dec                |  7 +++-
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc b/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc
index 78ee48e354a8..12dcd82eb132 100644
--- a/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2020 - 2022, Arm Limited. All rights reserved.
+#  Copyright (c) 2020 - 2023, Arm Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -38,6 +38,11 @@
   gArmSgiTokenSpaceGuid.PcdVirtioNetBaseAddress|0x0C150000
   gArmSgiTokenSpaceGuid.PcdVirtioNetInterrupt|460
 
+  # Virtio P9
+  gArmSgiTokenSpaceGuid.PcdVirtioP9BaseAddress|0x0C190000
+  gArmSgiTokenSpaceGuid.PcdVirtioP9Size|0x10000
+  gArmSgiTokenSpaceGuid.PcdVirtioP9Interrupt|459
+
   # PCIe
   gArmTokenSpaceGuid.PcdPciMmio32Base|0x60000000
   gArmTokenSpaceGuid.PcdPciMmio32Size|0x10000000
diff --git a/Platform/ARM/SgiPkg/AcpiTables/SsdtRosVirtioP9.asl b/Platform/ARM/SgiPkg/AcpiTables/SsdtRosVirtioP9.asl
new file mode 100644
index 000000000000..a1aab5e094b3
--- /dev/null
+++ b/Platform/ARM/SgiPkg/AcpiTables/SsdtRosVirtioP9.asl
@@ -0,0 +1,42 @@
+/** @file
+* Secondary System Description Table Fields (SSDT) for Virtio-P9 device.
+*
+* Some of the Arm Reference Design FVP platforms support the Virtio-P9 device
+* as part of the RoS subsystem. The Virtio-P9 device implements a subset of the
+* Plan 9 file protocol over a virtio transport. It enables accessing a shared
+* directory on the host's filesystem from a running FVP platform.
+* This file describes the SSDT entry for this Virtio-P9 device
+*
+* Copyright (c) 2023, 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 "SgiAcpiHeader.h"
+#include "SgiPlatform.h"
+
+DefinitionBlock ("SsdtRosVirtioP9.aml", "SSDT", 2, "ARMLTD", "ARMSGI",
+                 EFI_ACPI_ARM_OEM_REVISION) {
+  Scope (_SB) {
+    // VIRTIO P9 device
+    Device (VP90) {
+      Name (_HID, "LNRO0005")
+      Name (_UID, 2)
+      Name (_CCA, 1)    // mark the device coherent
+
+      Name (_CRS, ResourceTemplate() {
+        Memory32Fixed (
+          ReadWrite,
+          FixedPcdGet32 (PcdVirtioP9BaseAddress),
+          FixedPcdGet32 (PcdVirtioP9Size)
+        )
+        Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
+          FixedPcdGet32 (PcdVirtioP9Interrupt)
+        }
+      })
+    }
+  } // Scope(_SB)
+}
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
index b9be5c9060b6..e878af24d56b 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dec
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2018 - 2022, Arm Limited. All rights reserved.
+#  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -46,6 +46,11 @@
   gArmSgiTokenSpaceGuid.PcdVirtioNetSize|0x00000000|UINT32|0x00000008
   gArmSgiTokenSpaceGuid.PcdVirtioNetInterrupt|0x00000000|UINT32|0x00000009
 
+  # Virtio P9
+  gArmSgiTokenSpaceGuid.PcdVirtioP9BaseAddress|0x00000000|UINT32|0x00000028
+  gArmSgiTokenSpaceGuid.PcdVirtioP9Size|0x00000000|UINT32|0x00000029
+  gArmSgiTokenSpaceGuid.PcdVirtioP9Interrupt|0x00000000|UINT32|0x0000002A
+
   # Chip count on the platform
   gArmSgiTokenSpaceGuid.PcdChipCount|1|UINT32|0x0000000B
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-platforms][PATCH V2 2/5] Platform/Sgi: Enable virtio-p9 device on RD-N2 platform variants
  2023-01-27  9:23 [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 for RD-N2 variants Vivek Kumar Gautam
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 1/5] Platform/Sgi: Add SSDT table for Virtio-P9 Vivek Kumar Gautam
@ 2023-01-27  9:23 ` Vivek Kumar Gautam
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 3/5] Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block Vivek Kumar Gautam
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Vivek Kumar Gautam @ 2023-01-27  9:23 UTC (permalink / raw)
  To: devel; +Cc: ardb+tianocore, leif, Sami.Mujawar, Pierre.Gondois, Vivek.Gautam

Enable the virtio-p9 device that is present as part of the RoS
peripherals on RD-N2 platform variants. This will allow filesystem
sharing between the Host PC and target platform.

Signed-off-by: Vivek Gautam <vivek.gautam@arm.com>
---
 Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf     | 8 ++++++--
 Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf | 8 ++++++--
 Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf | 6 +++++-
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
index 66d5422df36b..833f87c3e4be 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
@@ -1,7 +1,7 @@
 ## @file
 #  ACPI table data and ASL sources required to boot the platform.
 #
-#  Copyright (c) 2020-2021, Arm Ltd. All rights reserved.
+#  Copyright (c) 2020 - 2023, Arm Ltd. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -25,8 +25,9 @@
   RdN2/Pptt.aslc
   Spcr.aslc
   Ssdt.asl
-  SsdtRos.asl
   SsdtEvents.asl
+  SsdtRos.asl
+  SsdtRosVirtioP9.asl
 
 [Packages]
   ArmPkg/ArmPkg.dec
@@ -70,6 +71,9 @@
   gArmSgiTokenSpaceGuid.PcdVirtioNetBaseAddress
   gArmSgiTokenSpaceGuid.PcdVirtioNetSize
   gArmSgiTokenSpaceGuid.PcdVirtioNetInterrupt
+  gArmSgiTokenSpaceGuid.PcdVirtioP9BaseAddress
+  gArmSgiTokenSpaceGuid.PcdVirtioP9Size
+  gArmSgiTokenSpaceGuid.PcdVirtioP9Interrupt
   gArmSgiTokenSpaceGuid.PcdWdogWS0Gsiv
   gArmSgiTokenSpaceGuid.PcdWdogWS1Gsiv
 
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
index 742734ab7348..e3e4e55bc410 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
@@ -1,7 +1,7 @@
 ## @file
 #  ACPI table data and ASL sources required to boot the platform.
 #
-#  Copyright (c) 2021, Arm Ltd. All rights reserved.
+#  Copyright (c) 2021 - 2023, Arm Ltd. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -25,8 +25,9 @@
   RdN2Cfg1/Pptt.aslc
   Spcr.aslc
   Ssdt.asl
-  SsdtRos.asl
   SsdtEvents.asl
+  SsdtRos.asl
+  SsdtRosVirtioP9.asl
 
 [Packages]
   ArmPkg/ArmPkg.dec
@@ -71,6 +72,9 @@
   gArmSgiTokenSpaceGuid.PcdVirtioNetBaseAddress
   gArmSgiTokenSpaceGuid.PcdVirtioNetSize
   gArmSgiTokenSpaceGuid.PcdVirtioNetInterrupt
+  gArmSgiTokenSpaceGuid.PcdVirtioP9BaseAddress
+  gArmSgiTokenSpaceGuid.PcdVirtioP9Size
+  gArmSgiTokenSpaceGuid.PcdVirtioP9Interrupt
   gArmSgiTokenSpaceGuid.PcdWdogWS0Gsiv
   gArmSgiTokenSpaceGuid.PcdWdogWS1Gsiv
 
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf
index 2354f2dc65eb..6ce78582da35 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf
@@ -1,7 +1,7 @@
 ## @file
 #  ACPI table data and ASL sources required to boot the platform.
 #
-#  Copyright (c) 2022, Arm Limited. All rights reserved.
+#  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -24,6 +24,7 @@
   RdN2Cfg2/Srat.aslc
   Spcr.aslc
   SsdtRos.asl
+  SsdtRosVirtioP9.asl
 
 [Packages]
   ArmPkg/ArmPkg.dec
@@ -65,6 +66,9 @@
   gArmSgiTokenSpaceGuid.PcdVirtioNetBaseAddress
   gArmSgiTokenSpaceGuid.PcdVirtioNetSize
   gArmSgiTokenSpaceGuid.PcdVirtioNetInterrupt
+  gArmSgiTokenSpaceGuid.PcdVirtioP9BaseAddress
+  gArmSgiTokenSpaceGuid.PcdVirtioP9Size
+  gArmSgiTokenSpaceGuid.PcdVirtioP9Interrupt
   gArmSgiTokenSpaceGuid.PcdWdogWS0Gsiv
   gArmSgiTokenSpaceGuid.PcdWdogWS1Gsiv
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-platforms][PATCH V2 3/5] Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block
  2023-01-27  9:23 [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 for RD-N2 variants Vivek Kumar Gautam
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 1/5] Platform/Sgi: Add SSDT table for Virtio-P9 Vivek Kumar Gautam
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 2/5] Platform/Sgi: Enable virtio-p9 device on RD-N2 platform variants Vivek Kumar Gautam
@ 2023-01-27  9:23 ` Vivek Kumar Gautam
  2023-02-03 15:56   ` PierreGondois
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 4/5] Platform/Sgi: Initialize additional UART controllers Vivek Kumar Gautam
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Vivek Kumar Gautam @ 2023-01-27  9:23 UTC (permalink / raw)
  To: devel; +Cc: ardb+tianocore, leif, Sami.Mujawar, Pierre.Gondois, Vivek.Gautam

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 <vivek.gautam@arm.com>
---
 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
+  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))
+
+// 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.
+
+  @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)                                             \
+                                                                               \
+      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))
+    }
+  } // 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 } }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-platforms][PATCH V2 4/5] Platform/Sgi: Initialize additional UART controllers
  2023-01-27  9:23 [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 for RD-N2 variants Vivek Kumar Gautam
                   ` (2 preceding siblings ...)
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 3/5] Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block Vivek Kumar Gautam
@ 2023-01-27  9:23 ` Vivek Kumar Gautam
  2023-02-03 15:55   ` PierreGondois
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 5/5] Platform/Sgi: Enable SoC expansion block for RD-N2 variants Vivek Kumar Gautam
  2023-02-03 15:58 ` [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 " PierreGondois
  5 siblings, 1 reply; 14+ messages in thread
From: Vivek Kumar Gautam @ 2023-01-27  9:23 UTC (permalink / raw)
  To: devel; +Cc: ardb+tianocore, leif, Sami.Mujawar, Pierre.Gondois, Vivek.Gautam

From: Shriram K <shriram.k@arm.com>

The IO virtualization block on reference design platforms allow
connecting SoC expansion devices such as PL011 UART. On platforms
that support this, initialize the UART controller connected to the
IO virtualization block.

Signed-off-by: Shriram K <shriram.k@arm.com>
Signed-off-by: Vivek Gautam <vivek.gautam@arm.com>
---
 Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf  | 10 ++-
 Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf  |  7 ++-
 Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c    | 64 +++++++++++++++++++-
 Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c | 43 ++++++++++++-
 Platform/ARM/SgiPkg/SgiPlatform.dec                      |  1 +
 5 files changed, 118 insertions(+), 7 deletions(-)

diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
index 9d89314a594e..42feadaf5f6f 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2018, ARM Limited. All rights reserved.
+#  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -17,6 +17,7 @@
   VirtioDevices.c
 
 [Packages]
+  ArmPlatformPkg/ArmPlatformPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
@@ -37,10 +38,17 @@
   gArmSgiTokenSpaceGuid.PcdVirtioNetSupported
 
 [FixedPcd]
+  gArmSgiTokenSpaceGuid.PcdChipCount
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable
+  gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
   gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
   gArmSgiTokenSpaceGuid.PcdVirtioBlkSize
   gArmSgiTokenSpaceGuid.PcdVirtioNetBaseAddress
   gArmSgiTokenSpaceGuid.PcdVirtioNetSize
 
+  gArmPlatformTokenSpaceGuid.PcdSerialDbgUartClkInHz
+
 [Depex]
   TRUE
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
index 1ca7679b4191..4459b20ecb06 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2018 - 2022, Arm Limited. All rights reserved.
+#  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -41,10 +41,13 @@
   gArmPlatformTokenSpaceGuid.PcdCoreCount
   gArmPlatformTokenSpaceGuid.PcdArmHdLcdBase
 
-  gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
   gArmSgiTokenSpaceGuid.PcdDramBlock2Base
   gArmSgiTokenSpaceGuid.PcdDramBlock2Size
   gArmSgiTokenSpaceGuid.PcdGicSize
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable
+  gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
 
   gArmTokenSpaceGuid.PcdSystemMemoryBase
   gArmTokenSpaceGuid.PcdSystemMemorySize
diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
index 2f72e7152ff3..b3a998bc1585 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
@@ -1,6 +1,6 @@
 /** @file
 *
-*  Copyright (c) 2018, ARM Limited. All rights reserved.
+*  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
@@ -9,6 +9,9 @@
 #include <Library/AcpiLib.h>
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
+#include <Library/PL011UartLib.h>
+
+#include <IoVirtSoCExp.h>
 #include <SgiPlatform.h>
 
 VOID
@@ -16,6 +19,64 @@ InitVirtioDevices (
   VOID
   );
 
+/**
+  Initialize UART controllers connected to IO Virtualization block.
+
+  Use PL011UartLib Library to initialize UART controllers that are present in
+  the SoC expansion block. This SoC expansion block is connected to the IO
+  virtualization block on Arm infrastructure reference design (RD) platforms.
+
+  @retval  None
+**/
+STATIC
+VOID
+InitIoVirtSocExpBlkUartControllers (VOID)
+{
+  EFI_STATUS                 Status;
+  EFI_PARITY_TYPE            Parity;
+  EFI_STOP_BITS_TYPE         StopBits;
+  UINT64                     BaudRate;
+  UINT32                     ReceiveFifoDepth;
+  UINT8                      DataBits;
+  UINT8                      UartIdx;
+  UINT32                     ChipIdx;
+  UINT64                     UartAddr;
+
+  if (FixedPcdGet32 (PcdIoVirtSocExpBlkUartEnable) == 0)
+    return;
+
+  ReceiveFifoDepth = 0;
+  Parity = 1;
+  DataBits = 8;
+  StopBits = 1;
+  BaudRate = 115200;
+
+  for (ChipIdx = 0; ChipIdx < FixedPcdGet32 (PcdChipCount); ChipIdx++) {
+    for (UartIdx = 0; UartIdx < 2; UartIdx++) {
+      UartAddr = SGI_REMOTE_CHIP_MEM_OFFSET(ChipIdx) + UART_START(UartIdx);
+
+      Status = PL011UartInitializePort (
+                 (UINTN)UartAddr,
+                 FixedPcdGet32 (PcdSerialDbgUartClkInHz),
+                 &BaudRate,
+                 &ReceiveFifoDepth,
+                 &Parity,
+                 &DataBits,
+                 &StopBits
+                 );
+
+      if (EFI_ERROR (Status)) {
+        DEBUG ((
+          DEBUG_ERROR,
+          "Failed to init PL011_UART%u on IO Virt Block port, status: %r\n",
+          UartIdx,
+          Status
+          ));
+      }
+    }
+  }
+}
+
 EFI_STATUS
 EFIAPI
 ArmSgiPkgEntryPoint (
@@ -32,6 +93,7 @@ ArmSgiPkgEntryPoint (
   }
 
   InitVirtioDevices ();
+  InitIoVirtSocExpBlkUartControllers ();
 
   return Status;
 }
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
index 8139b75d8ee4..08aa9bf64940 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
@@ -1,6 +1,6 @@
 /** @file
 *
-*  Copyright (c) 2018-2020, ARM Limited. All rights reserved.
+*  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
@@ -13,11 +13,23 @@
 #include <Library/IoLib.h>
 #include <Library/MemoryAllocationLib.h>
 
+#include <IoVirtSoCExp.h>
 #include <SgiPlatform.h>
 
 // Total number of descriptors, including the final "end-of-table" descriptor.
-#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS                 \
-          (14 + (FixedPcdGet32 (PcdChipCount) * 2))
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS                                     \
+          ((14 + (FixedPcdGet32 (PcdChipCount) * 2)) +                         \
+           (FixedPcdGet32 (PcdIoVirtSocExpBlkUartEnable) *                     \
+            FixedPcdGet32 (PcdChipCount) * 2))
+
+// Memory Map descriptor for IO Virtualization SoC Expansion Block UART
+#define IO_VIRT_SOC_EXP_BLK_UART_MMAP(UartIdx, ChipIdx)                        \
+  VirtualMemoryTable[++Index].PhysicalBase =                                   \
+    SGI_REMOTE_CHIP_MEM_OFFSET(ChipIdx) + UART_START(UartIdx);                 \
+  VirtualMemoryTable[Index].VirtualBase    =                                   \
+    SGI_REMOTE_CHIP_MEM_OFFSET(ChipIdx) + UART_START(UartIdx);                 \
+  VirtualMemoryTable[Index].Length         = SIZE_64KB;                        \
+  VirtualMemoryTable[Index].Attributes     = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 
 /**
   Returns the Virtual Memory Map of the platform.
@@ -171,6 +183,31 @@ ArmPlatformGetVirtualMemoryMap (
   VirtualMemoryTable[Index].Length          = SIZE_64KB;
   VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 
+#if (FixedPcdGet32 (PcdIoVirtSocExpBlkUartEnable) == 1)
+  // Chip-0 IO Virtualization SoC Expansion Block - UART0
+  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 0)
+  // Chip-0 IO Virtualization SoC Expansion Block - UART0
+  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 0)
+#if (FixedPcdGet32 (PcdChipCount) > 1)
+  // Chip-1 IO Virtualization SoC Expansion Block - UART0
+  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 1)
+  // Chip-1 IO Virtualization SoC Expansion Block - UART1
+  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 1)
+#if (FixedPcdGet32 (PcdChipCount) > 2)
+  // Chip-2 IO Virtualization SoC Expansion Block - UART0
+  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 2)
+  // Chip-2 IO Virtualization SoC Expansion Block - UART1
+  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 2)
+#if (FixedPcdGet32 (PcdChipCount) > 3)
+  // Chip-3 IO Virtualization SoC Expansion Block - UART0
+  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 3)
+  // Chip-3 IO Virtualization SoC Expansion Block - UART1
+  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 3)
+#endif
+#endif
+#endif
+#endif
+
   // DDR - (2GB - 16MB)
   VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 (PcdSystemMemoryBase);
   VirtualMemoryTable[Index].VirtualBase     = PcdGet64 (PcdSystemMemoryBase);
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
index 407f03c1c3e8..43d350ec48bb 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dec
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
@@ -102,6 +102,7 @@
   gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base|0|UINT64|0x0000002B
   gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset|0|UINT32|0x0000002C
   gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkResourceSize|0|UINT32|0x0000002D
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable|0|UINT32|0x0000002E
 
 [Ppis]
   gNtFwConfigDtInfoPpiGuid     = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [edk2-platforms][PATCH V2 5/5] Platform/Sgi: Enable SoC expansion block for RD-N2 variants
  2023-01-27  9:23 [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 for RD-N2 variants Vivek Kumar Gautam
                   ` (3 preceding siblings ...)
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 4/5] Platform/Sgi: Initialize additional UART controllers Vivek Kumar Gautam
@ 2023-01-27  9:23 ` Vivek Kumar Gautam
  2023-02-03 15:58 ` [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 " PierreGondois
  5 siblings, 0 replies; 14+ messages in thread
From: Vivek Kumar Gautam @ 2023-01-27  9:23 UTC (permalink / raw)
  To: devel; +Cc: ardb+tianocore, leif, Sami.Mujawar, Pierre.Gondois, Vivek.Gautam

For all the RD-N2 platform variants, include the SSDT ACPI table that
describes the devices present in SoC expansion block that is connected
to the IO virtualization block.

Signed-off-by: Vivek Gautam <vivek.gautam@arm.com>
---
 Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf     | 7 +++++++
 Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf | 7 +++++++
 Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf | 5 +++++
 3 files changed, 19 insertions(+)

diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
index 833f87c3e4be..a2de6f754b34 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf
@@ -26,6 +26,7 @@
   Spcr.aslc
   Ssdt.asl
   SsdtEvents.asl
+  SsdtIoVirtSocExp.asl
   SsdtRos.asl
   SsdtRosVirtioP9.asl
 
@@ -55,11 +56,17 @@
   gArmTokenSpaceGuid.PcdPciBusMin
   gArmTokenSpaceGuid.PcdPciBusMax
 
+  gArmSgiTokenSpaceGuid.PcdChipCount
   gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress
   gArmSgiTokenSpaceGuid.PcdGpioController0Size
   gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
   gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
   gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkResourceSize
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable
+  gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
   gArmSgiTokenSpaceGuid.PcdOscLpiEnable
   gArmSgiTokenSpaceGuid.PcdOscCppcEnable
   gArmSgiTokenSpaceGuid.PcdSp804DualTimerBaseAddress
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
index e3e4e55bc410..f72cb612161b 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf
@@ -26,6 +26,7 @@
   Spcr.aslc
   Ssdt.asl
   SsdtEvents.asl
+  SsdtIoVirtSocExp.asl
   SsdtRos.asl
   SsdtRosVirtioP9.asl
 
@@ -55,11 +56,17 @@
   gArmTokenSpaceGuid.PcdPciBusMin
   gArmTokenSpaceGuid.PcdPciBusMax
 
+  gArmSgiTokenSpaceGuid.PcdChipCount
   gArmSgiTokenSpaceGuid.PcdGpioController0BaseAddress
   gArmSgiTokenSpaceGuid.PcdGpioController0Size
   gArmSgiTokenSpaceGuid.PcdGpioController0Interrupt
   gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
   gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkResourceSize
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable
+  gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
   gArmSgiTokenSpaceGuid.PcdOscLpiEnable
   gArmSgiTokenSpaceGuid.PcdOscCppcEnable
   gArmSgiTokenSpaceGuid.PcdSmmuBase
diff --git a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf
index 6ce78582da35..db1fd283c5a4 100644
--- a/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf
+++ b/Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf
@@ -23,6 +23,7 @@
   RdN2Cfg2/Pptt.aslc
   RdN2Cfg2/Srat.aslc
   Spcr.aslc
+  SsdtIoVirtSocExp.asl
   SsdtRos.asl
   SsdtRosVirtioP9.asl
 
@@ -57,6 +58,10 @@
   gArmSgiTokenSpaceGuid.PcdDramBlock2Size
   gArmSgiTokenSpaceGuid.PcdGtFrame0Gsiv
   gArmSgiTokenSpaceGuid.PcdGtFrame1Gsiv
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkResourceSize
+  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable
   gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
   gArmSgiTokenSpaceGuid.PcdOscLpiEnable
   gArmSgiTokenSpaceGuid.PcdOscCppcEnable
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [edk2-platforms][PATCH V2 4/5] Platform/Sgi: Initialize additional UART controllers
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 4/5] Platform/Sgi: Initialize additional UART controllers Vivek Kumar Gautam
@ 2023-02-03 15:55   ` PierreGondois
  2023-02-07  6:10     ` [edk2-devel] " Vivek Kumar Gautam
  0 siblings, 1 reply; 14+ messages in thread
From: PierreGondois @ 2023-02-03 15:55 UTC (permalink / raw)
  To: Vivek Gautam, devel; +Cc: ardb+tianocore, leif, Sami.Mujawar

Hello Vivek,

On 1/27/23 10:23, Vivek Gautam wrote:
> From: Shriram K <shriram.k@arm.com>
> 
> The IO virtualization block on reference design platforms allow
> connecting SoC expansion devices such as PL011 UART. On platforms
> that support this, initialize the UART controller connected to the
> IO virtualization block.
> 
> Signed-off-by: Shriram K <shriram.k@arm.com>
> Signed-off-by: Vivek Gautam <vivek.gautam@arm.com>
> ---
>   Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf  | 10 ++-
>   Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf  |  7 ++-
>   Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c    | 64 +++++++++++++++++++-
>   Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c | 43 ++++++++++++-
>   Platform/ARM/SgiPkg/SgiPlatform.dec                      |  1 +
>   5 files changed, 118 insertions(+), 7 deletions(-)
> 
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> index 9d89314a594e..42feadaf5f6f 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -1,5 +1,5 @@
>   #
> -#  Copyright (c) 2018, ARM Limited. All rights reserved.
> +#  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
>   #
>   #  SPDX-License-Identifier: BSD-2-Clause-Patent
>   #
> @@ -17,6 +17,7 @@
>     VirtioDevices.c
>   
>   [Packages]
> +  ArmPlatformPkg/ArmPlatformPkg.dec
>     EmbeddedPkg/EmbeddedPkg.dec
>     MdePkg/MdePkg.dec
>     OvmfPkg/OvmfPkg.dec
> @@ -37,10 +38,17 @@
>     gArmSgiTokenSpaceGuid.PcdVirtioNetSupported
>   
>   [FixedPcd]
> +  gArmSgiTokenSpaceGuid.PcdChipCount
> +  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base
> +  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset
> +  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable
> +  gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
>     gArmSgiTokenSpaceGuid.PcdVirtioBlkBaseAddress
>     gArmSgiTokenSpaceGuid.PcdVirtioBlkSize
>     gArmSgiTokenSpaceGuid.PcdVirtioNetBaseAddress
>     gArmSgiTokenSpaceGuid.PcdVirtioNetSize
>   
> +  gArmPlatformTokenSpaceGuid.PcdSerialDbgUartClkInHz
> +
>   [Depex]
>     TRUE
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> index 1ca7679b4191..4459b20ecb06 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> @@ -1,5 +1,5 @@
>   #
> -#  Copyright (c) 2018 - 2022, Arm Limited. All rights reserved.
> +#  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
>   #
>   #  SPDX-License-Identifier: BSD-2-Clause-Patent
>   #
> @@ -41,10 +41,13 @@
>     gArmPlatformTokenSpaceGuid.PcdCoreCount
>     gArmPlatformTokenSpaceGuid.PcdArmHdLcdBase
>   
> -  gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
>     gArmSgiTokenSpaceGuid.PcdDramBlock2Base
>     gArmSgiTokenSpaceGuid.PcdDramBlock2Size
>     gArmSgiTokenSpaceGuid.PcdGicSize
> +  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base
> +  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset
> +  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable
> +  gArmSgiTokenSpaceGuid.PcdMaxAddressBitsPerChip
>   
>     gArmTokenSpaceGuid.PcdSystemMemoryBase
>     gArmTokenSpaceGuid.PcdSystemMemorySize
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> index 2f72e7152ff3..b3a998bc1585 100644
> --- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c
> @@ -1,6 +1,6 @@
>   /** @file
>   *
> -*  Copyright (c) 2018, ARM Limited. All rights reserved.
> +*  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.
>   *
>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
>   *
> @@ -9,6 +9,9 @@
>   #include <Library/AcpiLib.h>
>   #include <Library/DebugLib.h>
>   #include <Library/HobLib.h>
> +#include <Library/PL011UartLib.h>
> +
> +#include <IoVirtSoCExp.h>
>   #include <SgiPlatform.h>
>   
>   VOID
> @@ -16,6 +19,64 @@ InitVirtioDevices (
>     VOID
>     );
>   
> +/**
> +  Initialize UART controllers connected to IO Virtualization block.
> +
> +  Use PL011UartLib Library to initialize UART controllers that are present in
> +  the SoC expansion block. This SoC expansion block is connected to the IO
> +  virtualization block on Arm infrastructure reference design (RD) platforms.
> +
> +  @retval  None
> +**/
> +STATIC
> +VOID
> +InitIoVirtSocExpBlkUartControllers (VOID)
> +{
> +  EFI_STATUS                 Status;
> +  EFI_PARITY_TYPE            Parity;
> +  EFI_STOP_BITS_TYPE         StopBits;
> +  UINT64                     BaudRate;
> +  UINT32                     ReceiveFifoDepth;
> +  UINT8                      DataBits;
> +  UINT8                      UartIdx;
> +  UINT32                     ChipIdx;
> +  UINT64                     UartAddr;
> +
> +  if (FixedPcdGet32 (PcdIoVirtSocExpBlkUartEnable) == 0)
> +    return;
> +
> +  ReceiveFifoDepth = 0;
> +  Parity = 1;
> +  DataBits = 8;
> +  StopBits = 1;
> +  BaudRate = 115200;
> +
> +  for (ChipIdx = 0; ChipIdx < FixedPcdGet32 (PcdChipCount); ChipIdx++) {
> +    for (UartIdx = 0; UartIdx < 2; UartIdx++) {
> +      UartAddr = SGI_REMOTE_CHIP_MEM_OFFSET(ChipIdx) + UART_START(UartIdx);
> +
> +      Status = PL011UartInitializePort (
> +                 (UINTN)UartAddr,
> +                 FixedPcdGet32 (PcdSerialDbgUartClkInHz),
> +                 &BaudRate,
> +                 &ReceiveFifoDepth,
> +                 &Parity,
> +                 &DataBits,
> +                 &StopBits
> +                 );
> +
> +      if (EFI_ERROR (Status)) {
> +        DEBUG ((
> +          DEBUG_ERROR,
> +          "Failed to init PL011_UART%u on IO Virt Block port, status: %r\n",
> +          UartIdx,
> +          Status
> +          ));
> +      }
> +    }
> +  }
> +}
> +
>   EFI_STATUS
>   EFIAPI
>   ArmSgiPkgEntryPoint (
> @@ -32,6 +93,7 @@ ArmSgiPkgEntryPoint (
>     }
>   
>     InitVirtioDevices ();
> +  InitIoVirtSocExpBlkUartControllers ();
>   
>     return Status;
>   }
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
> index 8139b75d8ee4..08aa9bf64940 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
> @@ -1,6 +1,6 @@
>   /** @file
>   *
> -*  Copyright (c) 2018-2020, ARM Limited. All rights reserved.
> +*  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
>   *
>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
>   *
> @@ -13,11 +13,23 @@
>   #include <Library/IoLib.h>
>   #include <Library/MemoryAllocationLib.h>
>   
> +#include <IoVirtSoCExp.h>
>   #include <SgiPlatform.h>
>   
>   // Total number of descriptors, including the final "end-of-table" descriptor.
> -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS                 \
> -          (14 + (FixedPcdGet32 (PcdChipCount) * 2))
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS                                     \
> +          ((14 + (FixedPcdGet32 (PcdChipCount) * 2)) +                         \
> +           (FixedPcdGet32 (PcdIoVirtSocExpBlkUartEnable) *                     \
> +            FixedPcdGet32 (PcdChipCount) * 2))
> +
> +// Memory Map descriptor for IO Virtualization SoC Expansion Block UART
> +#define IO_VIRT_SOC_EXP_BLK_UART_MMAP(UartIdx, ChipIdx)                        \
> +  VirtualMemoryTable[++Index].PhysicalBase =                                   \
> +    SGI_REMOTE_CHIP_MEM_OFFSET(ChipIdx) + UART_START(UartIdx);                 \
> +  VirtualMemoryTable[Index].VirtualBase    =                                   \
> +    SGI_REMOTE_CHIP_MEM_OFFSET(ChipIdx) + UART_START(UartIdx);                 \
> +  VirtualMemoryTable[Index].Length         = SIZE_64KB;                        \
> +  VirtualMemoryTable[Index].Attributes     = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>   
>   /**
>     Returns the Virtual Memory Map of the platform.
> @@ -171,6 +183,31 @@ ArmPlatformGetVirtualMemoryMap (
>     VirtualMemoryTable[Index].Length          = SIZE_64KB;
>     VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>   
> +#if (FixedPcdGet32 (PcdIoVirtSocExpBlkUartEnable) == 1)
> +  // Chip-0 IO Virtualization SoC Expansion Block - UART0
> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 0)
> +  // Chip-0 IO Virtualization SoC Expansion Block - UART0

NIT: shouldn't it be UART1 ?

> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 0)
> +#if (FixedPcdGet32 (PcdChipCount) > 1)
> +  // Chip-1 IO Virtualization SoC Expansion Block - UART0
> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 1)
> +  // Chip-1 IO Virtualization SoC Expansion Block - UART1
> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 1)
> +#if (FixedPcdGet32 (PcdChipCount) > 2)
> +  // Chip-2 IO Virtualization SoC Expansion Block - UART0
> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 2)
> +  // Chip-2 IO Virtualization SoC Expansion Block - UART1
> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 2)
> +#if (FixedPcdGet32 (PcdChipCount) > 3)
> +  // Chip-3 IO Virtualization SoC Expansion Block - UART0
> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 3)
> +  // Chip-3 IO Virtualization SoC Expansion Block - UART1
> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 3)
> +#endif
> +#endif
> +#endif
> +#endif
> +
>     // DDR - (2GB - 16MB)
>     VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 (PcdSystemMemoryBase);
>     VirtualMemoryTable[Index].VirtualBase     = PcdGet64 (PcdSystemMemoryBase);
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
> index 407f03c1c3e8..43d350ec48bb 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dec
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
> @@ -102,6 +102,7 @@
>     gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base|0|UINT64|0x0000002B
>     gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset|0|UINT32|0x0000002C
>     gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkResourceSize|0|UINT32|0x0000002D
> +  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable|0|UINT32|0x0000002E

PcdIoVirtSocExpBlkUartEnable isn't set for any platform, is it normal ?

>   
>   [Ppis]
>     gNtFwConfigDtInfoPpiGuid     = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-platforms][PATCH V2 3/5] Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 3/5] Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block Vivek Kumar Gautam
@ 2023-02-03 15:56   ` PierreGondois
  2023-02-07  6:59     ` Vivek Kumar Gautam
  0 siblings, 1 reply; 14+ messages in thread
From: PierreGondois @ 2023-02-03 15:56 UTC (permalink / raw)
  To: Vivek Gautam, devel; +Cc: ardb+tianocore, leif, Sami.Mujawar

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 <vivek.gautam@arm.com>
> ---
>   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 } }

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 for RD-N2 variants
  2023-01-27  9:23 [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 for RD-N2 variants Vivek Kumar Gautam
                   ` (4 preceding siblings ...)
  2023-01-27  9:23 ` [edk2-platforms][PATCH V2 5/5] Platform/Sgi: Enable SoC expansion block for RD-N2 variants Vivek Kumar Gautam
@ 2023-02-03 15:58 ` PierreGondois
  2023-02-07  6:15   ` Vivek Kumar Gautam
  5 siblings, 1 reply; 14+ messages in thread
From: PierreGondois @ 2023-02-03 15:58 UTC (permalink / raw)
  To: Vivek Gautam, devel; +Cc: ardb+tianocore, leif, Sami.Mujawar

Hello Vivek,

On 1/27/23 10:23, Vivek Gautam wrote:
> Arm reference design Fixed Virtual Platforms (FVPs) such as the RD-N2
> platform variants 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 Arm SMMUv3, a
> GIC-ITS and a NCI (network chip interconnect).
> 
> SoC expansion blocks connect to the IO virtualization blocks via x4, x8
> or x16 ports exposed by the system. A SoC expansion block implementation
> includes 2 UARTs, 2 DMA devices and 2 Memory nodes.
> 
> In addition, Arm reference design platforms support Virtio-P9 device as
> part of the Rest of System (RoS). The Virtio-P9 device implements a
> subset of the Plan 9 file protocol over a virtio transport that enables
> accessing a shared directory on the host's filesystem from a running
> FVP platform.
> 
> This patch series adds SSDT tables for various RD-N2 platforms such as
> RD-N2, RD-N2-Cfg1, and RD-N2-Cfg2 to describe the SoC expansion block
> devices - UARTs, and DMAs and the Virtio-P9 devices present on the
> platforms. The patches also add support for platform DXE driver to
> initialize the UARTs that are present in SoC expansion blocks. By
> default these UARTs are kept disabled and can be enabled with a Pcd -
> PcdIoVirtSocExpBlkUartEnable.
> 
> This patch series is now a combination of two patch series [1] and [2]
> that added Virtio-P9 support and SoC expansion block (non-discoverable)
> IO block for RD-N2:
> [edk2-platforms][PATCH V1 0/2] Enable Virtio-P9 on RD-N2 platforms
> [edk2-platforms][PATCH V1 0/6] Add non-discoverable IO block for Rd-N2
> 
> [1] https://edk2.groups.io/g/devel/message/94936
> [2] https://edk2.groups.io/g/devel/message/86646
> 
> Changes since v1:
>   - Minor update to Virtio-P9 SSDT table:
>     - Name of the DefinitionBlock() is set to SsdtRosVirtioP9.aml rather
>       than SsdtRosVirtioP9Table.aml
>   - Updates to SoC expansion block:
>     - Removed IORT table for SoC expansion block and kept only the SSDT
>       table for devices.

Is it possible to know why the IORT table was removed ?

>     - SSDT table now uses arithmetic operations to calculate the start
>       and end addresses of the devices in QWordMemory() blocks.
>     - The number of PCDs for UARTs and DMAs are now reduced as the
>       addresses are now calculated within the SSDT table based on the
>       SoC expansion block base address and device offsets.
>     - Defined macros for Interrupt() block for various DMA nodes.
>     - Removed the first patch of the series that added PCDs for SMMU:
>       [PATCH V1 1/6] Platform/Sgi: add PCDs for SMMUv3 base address and interrupts
>     - Added support for SoC expansion block on RD-N2-Cfg2 platform as
>       well.
> 
> Shriram K (1):
>    Platform/Sgi: Initialize additional UART controllers
> 
> Vivek Gautam (4):
>    Platform/Sgi: Add SSDT table for Virtio-P9
>    Platform/Sgi: Enable virtio-p9 device on RD-N2 platform variants
>    Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block
>    Platform/Sgi: Enable SoC expansion block for RD-N2 variants

For the Virtio-P9 patches:
- Platform/Sgi: Add SSDT table for Virtio-P9
- Platform/Sgi: Enable virtio-p9 device on RD-N2 platform variants
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
I had some questions for the other patches.

Regards,
Pierre

> 
>   Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc                |  12 +-
>   Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf        |  15 +-
>   Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf    |  15 +-
>   Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf    |  11 +-
>   Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf  |  10 +-
>   Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf  |   7 +-
>   Platform/ARM/SgiPkg/Include/IoVirtSoCExp.h               | 189 ++++++++++++++++++++
>   Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c    |  64 ++++++-
>   Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c |  43 ++++-
>   Platform/ARM/SgiPkg/AcpiTables/SsdtIoVirtSocExp.asl      |  96 ++++++++++
>   Platform/ARM/SgiPkg/AcpiTables/SsdtRosVirtioP9.asl       |  42 +++++
>   Platform/ARM/SgiPkg/SgiPlatform.dec                      |  13 +-
>   12 files changed, 503 insertions(+), 14 deletions(-)
>   create mode 100644 Platform/ARM/SgiPkg/Include/IoVirtSoCExp.h
>   create mode 100644 Platform/ARM/SgiPkg/AcpiTables/SsdtIoVirtSocExp.asl
>   create mode 100644 Platform/ARM/SgiPkg/AcpiTables/SsdtRosVirtioP9.asl
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-devel] [edk2-platforms][PATCH V2 4/5] Platform/Sgi: Initialize additional UART controllers
  2023-02-03 15:55   ` PierreGondois
@ 2023-02-07  6:10     ` Vivek Kumar Gautam
  0 siblings, 0 replies; 14+ messages in thread
From: Vivek Kumar Gautam @ 2023-02-07  6:10 UTC (permalink / raw)
  To: devel, pierre.gondois; +Cc: ardb+tianocore, leif, Sami.Mujawar

Hi Pierre,

On 2/3/23 21:25, PierreGondois via groups.io wrote:
> Hello Vivek,

Thanks for your review. Please find my responses inline below.

> 
> On 1/27/23 10:23, Vivek Gautam wrote:
>> From: Shriram K <shriram.k@arm.com>
>>
>> The IO virtualization block on reference design platforms allow
>> connecting SoC expansion devices such as PL011 UART. On platforms
>> that support this, initialize the UART controller connected to the
>> IO virtualization block.
>>
>> Signed-off-by: Shriram K <shriram.k@arm.com>
>> Signed-off-by: Vivek Gautam <vivek.gautam@arm.com>
>> ---
>>   Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf  | 10 ++-
>>   Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf  |  7 ++-
>>   Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c    | 64 
>> +++++++++++++++++++-
>>   Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c | 43 
>> ++++++++++++-
>>   Platform/ARM/SgiPkg/SgiPlatform.dec                      |  1 +
>>   5 files changed, 118 insertions(+), 7 deletions(-)
>>
[snip]

>> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c 
>> b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
>> index 8139b75d8ee4..08aa9bf64940 100644
>> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
>> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
>> @@ -1,6 +1,6 @@
>>   /** @file
>>   *
>> -*  Copyright (c) 2018-2020, ARM Limited. All rights reserved.
>> +*  Copyright (c) 2018 - 2023, Arm Limited. All rights reserved.
>>   *
>>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
>>   *
>> @@ -13,11 +13,23 @@
>>   #include <Library/IoLib.h>
>>   #include <Library/MemoryAllocationLib.h>
>> +#include <IoVirtSoCExp.h>
>>   #include <SgiPlatform.h>
>>   // Total number of descriptors, including the final "end-of-table" 
>> descriptor.
>> -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS                 \
>> -          (14 + (FixedPcdGet32 (PcdChipCount) * 2))
>> +#define 
>> MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS                                     \
>> +          ((14 + (FixedPcdGet32 (PcdChipCount) * 2)) 
>> +                         \
>> +           (FixedPcdGet32 (PcdIoVirtSocExpBlkUartEnable) 
>> *                     \
>> +            FixedPcdGet32 (PcdChipCount) * 2))
>> +
>> +// Memory Map descriptor for IO Virtualization SoC Expansion Block UART
>> +#define IO_VIRT_SOC_EXP_BLK_UART_MMAP(UartIdx, 
>> ChipIdx)                        \
>> +  VirtualMemoryTable[++Index].PhysicalBase 
>> =                                   \
>> +    SGI_REMOTE_CHIP_MEM_OFFSET(ChipIdx) + 
>> UART_START(UartIdx);                 \
>> +  VirtualMemoryTable[Index].VirtualBase    
>> =                                   \
>> +    SGI_REMOTE_CHIP_MEM_OFFSET(ChipIdx) + 
>> UART_START(UartIdx);                 \
>> +  VirtualMemoryTable[Index].Length         = 
>> SIZE_64KB;                        \
>> +  VirtualMemoryTable[Index].Attributes     = 
>> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>>   /**
>>     Returns the Virtual Memory Map of the platform.
>> @@ -171,6 +183,31 @@ ArmPlatformGetVirtualMemoryMap (
>>     VirtualMemoryTable[Index].Length          = SIZE_64KB;
>>     VirtualMemoryTable[Index].Attributes      = 
>> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>> +#if (FixedPcdGet32 (PcdIoVirtSocExpBlkUartEnable) == 1)
>> +  // Chip-0 IO Virtualization SoC Expansion Block - UART0
>> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 0)
>> +  // Chip-0 IO Virtualization SoC Expansion Block - UART0
> 
> NIT: shouldn't it be UART1 ?

Yes, it should be UART1. Will correct it.

> 
>> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 0)
>> +#if (FixedPcdGet32 (PcdChipCount) > 1)
>> +  // Chip-1 IO Virtualization SoC Expansion Block - UART0
>> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 1)
>> +  // Chip-1 IO Virtualization SoC Expansion Block - UART1
>> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 1)
>> +#if (FixedPcdGet32 (PcdChipCount) > 2)
>> +  // Chip-2 IO Virtualization SoC Expansion Block - UART0
>> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 2)
>> +  // Chip-2 IO Virtualization SoC Expansion Block - UART1
>> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 2)
>> +#if (FixedPcdGet32 (PcdChipCount) > 3)
>> +  // Chip-3 IO Virtualization SoC Expansion Block - UART0
>> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(0, 3)
>> +  // Chip-3 IO Virtualization SoC Expansion Block - UART1
>> +  IO_VIRT_SOC_EXP_BLK_UART_MMAP(1, 3)
>> +#endif
>> +#endif
>> +#endif
>> +#endif
>> +
>>     // DDR - (2GB - 16MB)
>>     VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 
>> (PcdSystemMemoryBase);
>>     VirtualMemoryTable[Index].VirtualBase     = PcdGet64 
>> (PcdSystemMemoryBase);
>> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec 
>> b/Platform/ARM/SgiPkg/SgiPlatform.dec
>> index 407f03c1c3e8..43d350ec48bb 100644
>> --- a/Platform/ARM/SgiPkg/SgiPlatform.dec
>> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
>> @@ -102,6 +102,7 @@
>>     gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlk0Base|0|UINT64|0x0000002B
>>     
>> gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkPeriOffset|0|UINT32|0x0000002C
>>     
>> gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkResourceSize|0|UINT32|0x0000002D
>> +  gArmSgiTokenSpaceGuid.PcdIoVirtSocExpBlkUartEnable|0|UINT32|0x0000002E
> 
> PcdIoVirtSocExpBlkUartEnable isn't set for any platform, is it normal ?

Yes, by default we are keeping these Uarts disabled as these are not 
used for system debug. Platforms that plan to use them can enable these 
in debug/production as needed.


Best regards
Vivek

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 for RD-N2 variants
  2023-02-03 15:58 ` [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 " PierreGondois
@ 2023-02-07  6:15   ` Vivek Kumar Gautam
  0 siblings, 0 replies; 14+ messages in thread
From: Vivek Kumar Gautam @ 2023-02-07  6:15 UTC (permalink / raw)
  To: Pierre Gondois, devel; +Cc: ardb+tianocore, leif, Sami.Mujawar

Hi Pierre,


On 2/3/23 21:28, Pierre Gondois wrote:
> Hello Vivek,
> 
> On 1/27/23 10:23, Vivek Gautam wrote:
>> Arm reference design Fixed Virtual Platforms (FVPs) such as the RD-N2
>> platform variants 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 Arm SMMUv3, a
>> GIC-ITS and a NCI (network chip interconnect).
>>
>> SoC expansion blocks connect to the IO virtualization blocks via x4, x8
>> or x16 ports exposed by the system. A SoC expansion block implementation
>> includes 2 UARTs, 2 DMA devices and 2 Memory nodes.
>>
>> In addition, Arm reference design platforms support Virtio-P9 device as
>> part of the Rest of System (RoS). The Virtio-P9 device implements a
>> subset of the Plan 9 file protocol over a virtio transport that enables
>> accessing a shared directory on the host's filesystem from a running
>> FVP platform.
>>
>> This patch series adds SSDT tables for various RD-N2 platforms such as
>> RD-N2, RD-N2-Cfg1, and RD-N2-Cfg2 to describe the SoC expansion block
>> devices - UARTs, and DMAs and the Virtio-P9 devices present on the
>> platforms. The patches also add support for platform DXE driver to
>> initialize the UARTs that are present in SoC expansion blocks. By
>> default these UARTs are kept disabled and can be enabled with a Pcd -
>> PcdIoVirtSocExpBlkUartEnable.
>>
>> This patch series is now a combination of two patch series [1] and [2]
>> that added Virtio-P9 support and SoC expansion block (non-discoverable)
>> IO block for RD-N2:
>> [edk2-platforms][PATCH V1 0/2] Enable Virtio-P9 on RD-N2 platforms
>> [edk2-platforms][PATCH V1 0/6] Add non-discoverable IO block for Rd-N2
>>
>> [1] https://edk2.groups.io/g/devel/message/94936
>> [2] https://edk2.groups.io/g/devel/message/86646
>>
>> Changes since v1:
>>   - Minor update to Virtio-P9 SSDT table:
>>     - Name of the DefinitionBlock() is set to SsdtRosVirtioP9.aml rather
>>       than SsdtRosVirtioP9Table.aml
>>   - Updates to SoC expansion block:
>>     - Removed IORT table for SoC expansion block and kept only the SSDT
>>       table for devices.
> 
> Is it possible to know why the IORT table was removed ?

The IORT table is removed as that will come along with the common IORT 
table changes that includes all the GIT ITS nodes, SMMUv3 nodes, PCIe 
RCs, and the Named Component nodes for DMA PL330 devices.
I will update this in the cover letter.

> 
>>     - SSDT table now uses arithmetic operations to calculate the start
>>       and end addresses of the devices in QWordMemory() blocks.
>>     - The number of PCDs for UARTs and DMAs are now reduced as the
>>       addresses are now calculated within the SSDT table based on the
>>       SoC expansion block base address and device offsets.
>>     - Defined macros for Interrupt() block for various DMA nodes.
>>     - Removed the first patch of the series that added PCDs for SMMU:
>>       [PATCH V1 1/6] Platform/Sgi: add PCDs for SMMUv3 base address 
>> and interrupts
>>     - Added support for SoC expansion block on RD-N2-Cfg2 platform as
>>       well.
>>
>> Shriram K (1):
>>    Platform/Sgi: Initialize additional UART controllers
>>
>> Vivek Gautam (4):
>>    Platform/Sgi: Add SSDT table for Virtio-P9
>>    Platform/Sgi: Enable virtio-p9 device on RD-N2 platform variants
>>    Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block
>>    Platform/Sgi: Enable SoC expansion block for RD-N2 variants
> 
> For the Virtio-P9 patches:
> - Platform/Sgi: Add SSDT table for Virtio-P9
> - Platform/Sgi: Enable virtio-p9 device on RD-N2 platform variants
> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
> I had some questions for the other patches.

Thanks for your review. I will rework the other patches and re-post them.

Best regards
Vivek

> 
> Regards,
> Pierre
> 
>>
>>   Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc                |  12 +-
>>   Platform/ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf        |  15 +-
>>   Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf    |  15 +-
>>   Platform/ARM/SgiPkg/AcpiTables/RdN2Cfg2AcpiTables.inf    |  11 +-
>>   Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf  |  10 +-
>>   Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf  |   7 +-
>>   Platform/ARM/SgiPkg/Include/IoVirtSoCExp.h               | 189 
>> ++++++++++++++++++++
>>   Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c    |  64 ++++++-
>>   Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c |  43 ++++-
>>   Platform/ARM/SgiPkg/AcpiTables/SsdtIoVirtSocExp.asl      |  96 
>> ++++++++++
>>   Platform/ARM/SgiPkg/AcpiTables/SsdtRosVirtioP9.asl       |  42 +++++
>>   Platform/ARM/SgiPkg/SgiPlatform.dec                      |  13 +-
>>   12 files changed, 503 insertions(+), 14 deletions(-)
>>   create mode 100644 Platform/ARM/SgiPkg/Include/IoVirtSoCExp.h
>>   create mode 100644 Platform/ARM/SgiPkg/AcpiTables/SsdtIoVirtSocExp.asl
>>   create mode 100644 Platform/ARM/SgiPkg/AcpiTables/SsdtRosVirtioP9.asl
>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-platforms][PATCH V2 3/5] Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block
  2023-02-03 15:56   ` PierreGondois
@ 2023-02-07  6:59     ` Vivek Kumar Gautam
  2023-02-07  8:50       ` PierreGondois
  0 siblings, 1 reply; 14+ messages in thread
From: Vivek Kumar Gautam @ 2023-02-07  6:59 UTC (permalink / raw)
  To: Pierre Gondois, devel; +Cc: ardb+tianocore, leif, Sami.Mujawar

Hi Pierre

On 2/3/23 21:26, Pierre Gondois wrote:
> Hello Vivek,

Thanks for review the changes, please find my responses inline below.

> 
> 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 <vivek.gautam@arm.com>
>> ---
>>   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).

Right it should have been (2 * index * offset). I will correct this 
comment and the one below for DMA.

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

I agree that these PCDs are same for RDN2 {|Cfg1|Cfg2}. I think now that 
I can add local macros for PcdIoVirtSocExpBlkPeriOffset and 
PcdIoVirtSocExpBlkResourceSize.
But I plan to keep the PCD for base address - PcdIoVirtSocExpBlk0Base as 
that can be reused for other platforms considering that other platforms 
may map the IoVirtSocExpBlk in a different expansion base address. Let 
me know what do you think.

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

Sorry, I didn't understand your comment here.

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

We have not used the DynamicTablePkg maintain the readability and 
maintainability of the SSDT tables on these Arm reference design platforms.
I took a reference from RaspbarryPi platform [1] to use the 
CreateQWordField() and Add() methods to calculate the resultant start 
and end addresses whereever necessary. Let me know if this doesn't look 
right.

> 
> 
>> +                                                                               \
>> +      Return 
>> (RBUF)                                                            \
>> +    } /* end Method(_CRS) 
>> */                                                   \
>> +  }
>> +

[snip]

>> +-------------------------------------------------------------------------------+
>> +
>> +  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 (...'

Yes, it should be possible. I will update the patch with the suggested 
change.

[1] 
https://github.com/tianocore/edk2-platforms/blob/master/Platform/RaspberryPi/AcpiTables/Pci.asl#L116

Best regards
Vivek

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-platforms][PATCH V2 3/5] Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block
  2023-02-07  6:59     ` Vivek Kumar Gautam
@ 2023-02-07  8:50       ` PierreGondois
  2023-02-10  7:54         ` Vivek Kumar Gautam
  0 siblings, 1 reply; 14+ messages in thread
From: PierreGondois @ 2023-02-07  8:50 UTC (permalink / raw)
  To: Vivek Kumar Gautam, devel; +Cc: ardb+tianocore, leif, Sami.Mujawar

Hello Vivek,
Thanks for the answers,

On 2/7/23 07:59, Vivek Kumar Gautam wrote:
> Hi Pierre
> 
> On 2/3/23 21:26, Pierre Gondois wrote:
>> Hello Vivek,
> 
> Thanks for review the changes, please find my responses inline below.
> 
>>
>> 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 <vivek.gautam@arm.com>
>>> ---
>>>    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).
> 
> Right it should have been (2 * index * offset). I will correct this
> comment and the one below for DMA.
> 
>>
>>> +  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 ?
> 
> I agree that these PCDs are same for RDN2 {|Cfg1|Cfg2}. I think now that
> I can add local macros for PcdIoVirtSocExpBlkPeriOffset and
> PcdIoVirtSocExpBlkResourceSize.
> But I plan to keep the PCD for base address - PcdIoVirtSocExpBlk0Base as
> that can be reused for other platforms considering that other platforms
> may map the IoVirtSocExpBlk in a different expansion base address. Let
> me know what do you think.

Ok right, works for me.

> 
>>
>>> +
>>> +// 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
> 
> Sorry, I didn't understand your comment here.

Sorry I didn't finish the sentence. I was wondering what TDL files were for.

> 
>>> +
>>> +  @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 ?
> 
> We have not used the DynamicTablePkg maintain the readability and
> maintainability of the SSDT tables on these Arm reference design platforms.
> I took a reference from RaspbarryPi platform [1] to use the
> CreateQWordField() and Add() methods to calculate the resultant start
> and end addresses whereever necessary. Let me know if this doesn't look
> right.

This is the first time I see this, but it seems to be functional.

> 
>>
>>
>>> +                                                                               \
>>> +      Return
>>> (RBUF)                                                            \
>>> +    } /* end Method(_CRS)
>>> */                                                   \
>>> +  }
>>> +
> 
> [snip]
> 
>>> +-------------------------------------------------------------------------------+
>>> +
>>> +  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 (...'
> 
> Yes, it should be possible. I will update the patch with the suggested
> change.
> 
> [1]
> https://github.com/tianocore/edk2-platforms/blob/master/Platform/RaspberryPi/AcpiTables/Pci.asl#L116
> 
> Best regards
> Vivek
> 
>>
>>> +  } // 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 } }

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [edk2-platforms][PATCH V2 3/5] Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block
  2023-02-07  8:50       ` PierreGondois
@ 2023-02-10  7:54         ` Vivek Kumar Gautam
  0 siblings, 0 replies; 14+ messages in thread
From: Vivek Kumar Gautam @ 2023-02-10  7:54 UTC (permalink / raw)
  To: Pierre Gondois, devel; +Cc: ardb+tianocore, leif, Sami.Mujawar

Hi Pierre,


On 2/7/23 14:20, Pierre Gondois wrote:
> Hello Vivek,
> Thanks for the answers,
> 
> On 2/7/23 07:59, Vivek Kumar Gautam wrote:
>> Hi Pierre
>>
>> On 2/3/23 21:26, Pierre Gondois wrote:
>>> Hello Vivek,
>>
>> Thanks for review the changes, please find my responses inline below.
>>
>>>
>>> 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 <vivek.gautam@arm.com>
>>>> ---
>>>>    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).
>>
>> Right it should have been (2 * index * offset). I will correct this
>> comment and the one below for DMA.
>>
>>>
>>>> +  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 ?
>>
>> I agree that these PCDs are same for RDN2 {|Cfg1|Cfg2}. I think now that
>> I can add local macros for PcdIoVirtSocExpBlkPeriOffset and
>> PcdIoVirtSocExpBlkResourceSize.
>> But I plan to keep the PCD for base address - PcdIoVirtSocExpBlk0Base as
>> that can be reused for other platforms considering that other platforms
>> may map the IoVirtSocExpBlk in a different expansion base address. Let
>> me know what do you think.
> 
> Ok right, works for me.

Cool. Thanks.

> 
>>
>>>
>>>> +
>>>> +// 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
>>
>> Sorry, I didn't understand your comment here.
> 
> Sorry I didn't finish the sentence. I was wondering what TDL files were 
> for.

I think this is a stray comment. I will remove it.

> 
>>
>>>> +
>>>> +  @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 ?
>>
>> We have not used the DynamicTablePkg maintain the readability and
>> maintainability of the SSDT tables on these Arm reference design 
>> platforms.
>> I took a reference from RaspbarryPi platform [1] to use the
>> CreateQWordField() and Add() methods to calculate the resultant start
>> and end addresses whereever necessary. Let me know if this doesn't look
>> right.
> 
> This is the first time I see this, but it seems to be functional.

Yes, I also found it while looking for few references and it is 
functional :-)
Please let me know if you want me to change anything here. I can send 
the updated series soon after addressing your review comments.

[snip]

Best regards
Vivek

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-02-10  7:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-27  9:23 [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 for RD-N2 variants Vivek Kumar Gautam
2023-01-27  9:23 ` [edk2-platforms][PATCH V2 1/5] Platform/Sgi: Add SSDT table for Virtio-P9 Vivek Kumar Gautam
2023-01-27  9:23 ` [edk2-platforms][PATCH V2 2/5] Platform/Sgi: Enable virtio-p9 device on RD-N2 platform variants Vivek Kumar Gautam
2023-01-27  9:23 ` [edk2-platforms][PATCH V2 3/5] Platform/Sgi: Add SSDT table for IO virtualization SoC expansion block Vivek Kumar Gautam
2023-02-03 15:56   ` PierreGondois
2023-02-07  6:59     ` Vivek Kumar Gautam
2023-02-07  8:50       ` PierreGondois
2023-02-10  7:54         ` Vivek Kumar Gautam
2023-01-27  9:23 ` [edk2-platforms][PATCH V2 4/5] Platform/Sgi: Initialize additional UART controllers Vivek Kumar Gautam
2023-02-03 15:55   ` PierreGondois
2023-02-07  6:10     ` [edk2-devel] " Vivek Kumar Gautam
2023-01-27  9:23 ` [edk2-platforms][PATCH V2 5/5] Platform/Sgi: Enable SoC expansion block for RD-N2 variants Vivek Kumar Gautam
2023-02-03 15:58 ` [edk2-platforms][PATCH V2 0/5] Enable SoC expansion block and Virtio-P9 " PierreGondois
2023-02-07  6:15   ` Vivek Kumar Gautam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox