On Tue, Oct 19, 2021 at 01:14 AM, PierreGondois wrote:
Hi Khasim,
2 minor comments:
On 10/10/21 19:29, Khasim Mohammed via groups.io wrote:
This patch creates Dsdt.asl, SsdtPci.asl and SsdtRemotePci.asl files
to provide the platform specific APCI table entries.
Three PCI root ports are available on N1Sdp, PCI0 is the default root port
PCI1 is the CCIX root port and PCI2 is the Remote host root port.
The Remote host specific entries are defined in a separate file
SsdtRemotePci.asl to avoid confusions with other PCI entries
and for better readability and understanding of interfaces.
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Khasim Syed Mohammed <khasim.mohammed@arm.com>
Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
Signed-off-by: anukou01 <anurag.koul@arm.com>
Signed-off-by: Manoj Kumar <manoj.kumar3@arm.com>
---
.../AslTables/Dsdt.asl | 477 ++++++++++++++++++
.../AslTables/SsdtPci.asl | 247 +++++++++
.../AslTables/SsdtRemotePci.asl | 156 ++++++
3 files changed, 880 insertions(+)
create mode 100644 Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/AslTables/Dsdt.asl
create mode 100644 Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/AslTables/SsdtPci.asl
create mode 100644 Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/AslTables/SsdtRemotePci.asl
diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/AslTables/Dsdt.asl b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/AslTables/Dsdt.asl
new file mode 100644
index 0000000000..0d7dde1a73
--- /dev/null
+++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/AslTables/Dsdt.asl
@@ -0,0 +1,477 @@
+/** @file
+ Differentiated System Description Table Fields (DSDT)
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
Is it possible to add a '@par Reference(s):' section in the header
(e.g.:
edk2/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c)
I believe you used these documents:
-ACPI for CoreSight 1.1, Platform Design Document
-ACPI for Arm Components 1.0, Platform Design Document
+
+#include "N1SdpAcpiHeader.h"
+#include "NeoverseN1Soc.h"
+
+#define ACPI_GRAPH_REV 0
+#define ACPI_GRAPH_UUID "ab02a46b-74c7-45a2-bd68-f7d344ef2153"
+
+#define CORESIGHT_GRAPH_UUID "3ecbc8b6-1d0e-4fb3-8107-e627f805c6cd"
+
+#define CS_LINK_MASTER 1
+#define CS_LINK_SLAVE 0
+
[snip]
diff --git a/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/AslTables/SsdtPci.asl b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/AslTables/SsdtPci.asl
new file mode 100644
index 0000000000..87a4fdaf88
--- /dev/null
+++ b/Platform/ARM/N1Sdp/ConfigurationManager/ConfigurationManagerDxe/AslTables/SsdtPci.asl
@@ -0,0 +1,247 @@
+/** @file
+ Secondary System Description Table (SSDT)
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include "N1SdpAcpiHeader.h"
+
+/*
+ See ACPI 6.4 Section 6.2.13
+
+ There are two ways that _PRT can be used.
+
+ In the first model, a PCI Link device is used to provide additional
+ configuration information such as whether the interrupt is Level or
+ Edge triggered, it is active High or Low, Shared or Exclusive, etc.
+
+ In the second model, the PCI interrupts are hardwired to specific
+ interrupt inputs on the interrupt controller and are not
+ configurable. In this case, the Source field in _PRT does not
+ reference a device, but instead contains the value zero, and the
+ Source Index field contains the global system interrupt to which the
+ PCI interrupt is hardwired.
+
+ We use the first model with link indirection to set the correct
+ interrupt type as PCI defaults (Level Triggered, Active Low) are not
+ compatible with GICv2.
+*/
+#define LNK_DEVICE(Unique_Id, Link_Name, irq) \
+ Device(Link_Name) { \
+ Name(_HID, EISAID("PNP0C0F")) \
+ Name(_UID, Unique_Id) \
+ Name(_PRS, ResourceTemplate() { \
+ Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { irq } \
+ }) \
+ Method (_CRS, 0) { Return (_PRS) } \
+ Method (_SRS, 1) { } \
+ Method (_DIS) { } \
+}
+
+#define PRT_ENTRY(Address, Pin, Link) \
+ Package (4) { \
+ Address, /* uses the same format as _ADR */ \
+ Pin, /* The PCI pin number of the device (0-INTA, 1-INTB, 2-INTC, 3-INTD) */ \
+ Link, /* Interrupt allocated via Link device */ \
+ Zero /* global system interrupt number (no used) */ \
+}
+
+/*
+ See Reference [1] 6.1.1
+ "High word-Device #, Low word-Function #. (for example, device 3,
+ function 2 is 0x00030002). To refer to all the functions on a device #,
+ use a function number of FFFF)."
+*/
+#define ROOT_PRT_ENTRY(Pin, Link) PRT_ENTRY(0x0000FFFF, Pin, Link) // Device 0 for Bridge.
+
+DefinitionBlock("SsdtPci.aml", "SSDT", 1, "ARMLTD", "N1Sdp",
+ EFI_ACPI_ARM_OEM_REVISION)
+{
+ Scope (_SB) {
+
+ // PCI Root Complex
+ LNK_DEVICE(1, LNKA, 201)
+ LNK_DEVICE(2, LNKB, 202)
+ LNK_DEVICE(3, LNKC, 203)
+ LNK_DEVICE(4, LNKD, 204)
+ LNK_DEVICE(5, LNKE, 233)
+ LNK_DEVICE(6, LNKF, 234)
+ LNK_DEVICE(7, LNKG, 235)
+ LNK_DEVICE(8, LNKH, 236)
+
+ // PCI Root Complex
+ Device(PCI0) {
+ Name (_HID, EISAID("PNP0A08")) // PCI Express Root Bridge
+ Name (_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge
+ Name (_SEG, Zero) // PCI Segment Group number
+ Name (_BBN, Zero) // PCI Base Bus Number
+ Name (_CCA, 1) // Cache Coherency Attribute
+
+ // Root Complex 0
+ Device (RP0) {
+ Name(_ADR, 0xF0000000) // Dev 0, Func 0
+ }
Isn't it necessary to have an _OSC object as in
edk2-platforms/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/AslTables/SsdtPci.asl
?
Same question for SsdtRemotePci.asl
[snip]
Hi Pierre,
Yes, the _OSC object should have been implemented but unfortunately there are few PCIe Quirks and we are managing with additional patches to kernel. I am not sure if any of the _OSC functionality would result in exporting undesirable functionality due to these hardware errors.
I have a made a note of this suggestion, we will experiment the _OSC object and functionality and if everything works as expected then I will submit a follow on patch later.
Thanks.
Regards,
Khasim
Regards,
Pierre