[edk2-devel] [PATCH edk2-platforms v2 0/3] Platform/ARM: Add dynamic CPU node, TRBE & ETE support to FVP

[edk2-devel] [PATCH edk2-platforms v2 0/3] Platform/ARM: Add dynamic CPU node, TRBE & ETE support to FVP

[Sami Mujawar]

This patch series updates the FVP platform firmware
to add support for dynamically generating CPU nodes
in AML and also reports the TRBE interrupt in the
MADT GICC structure. The last patch in the series
enables support for adding an ETE device to the CPU
device nodes in AML.

This patch series depends on the edk2 patch series
at: https://edk2.groups.io/g/devel/message/108591

Updates from v1 patch series:
  - Fixed comment in patch 2/3 to specify the TRBE
    interrupt as PPI 15.

The changes can be seen at:
https://github.com/samimujawar/edk2-platforms/tree/2620_ete_dev_fvp_v2

Sami Mujawar (3):
  Platform/ARM: Add dynamic CPU node generation for FVP
  Platform/ARM: FVP: Specify TRBE interrupt in MADT GICC
  Platform/ARM: FVP: Add ETE device if supported by FVP

 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/AslTables/Dsdt.asl          | 38 +----------
 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c      | 72 +++++++++++++++++---
 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h      |  4 +-
 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf |  3 +-
 4 files changed, 69 insertions(+), 48 deletions(-)

-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108595): https://edk2.groups.io/g/devel/message/108595
Mute This Topic: https://groups.io/mt/101335887/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms v2 1/3] Platform/ARM: Add dynamic CPU node generation for FVP

[Sami Mujawar]

Add support to dynamically generate the CPU nodes in SSDT.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
    - No code change from v1 patch series.      [SAMI]

 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/AslTables/Dsdt.asl     | 38 +-------------------
 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c |  8 ++++-
 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h |  2 +-
 3 files changed, 9 insertions(+), 39 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/AslTables/Dsdt.asl b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/AslTables/Dsdt.asl
index 3acc0c6bc1b48b747e011022f2300b2fe3fa994b..f60430b94c867ee9744a3355b0ea377aff42b1c4 100644
--- a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/AslTables/Dsdt.asl
+++ b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/AslTables/Dsdt.asl
@@ -1,7 +1,7 @@
 /** @file
   Differentiated System Description Table Fields (DSDT)
 
-  Copyright (c) 2014-2021, ARM Ltd. All rights reserved.<BR>
+  Copyright (c) 2014-2023, ARM Ltd. All rights reserved.<BR>
   Copyright (c) 2013, Al Stone <al.stone@linaro.org>
   All rights reserved.
 
@@ -11,42 +11,6 @@
 
 DefinitionBlock("DsdtTable.aml", "DSDT", 2, "ARMLTD", "ARM-VEXP", 1) {
   Scope(_SB) {
-    //
-    // Processor
-    //
-    Device(CPU0) {
-      Name(_HID, "ACPI0007")
-      Name(_UID, Zero)
-    }
-    Device(CPU1) {
-      Name(_HID, "ACPI0007")
-      Name(_UID, One)
-    }
-    Device(CPU2) {
-      Name(_HID, "ACPI0007")
-      Name(_UID, 2)
-    }
-    Device(CPU3) {
-      Name(_HID, "ACPI0007")
-      Name(_UID, 3)
-    }
-    Device(CPU4) {
-      Name(_HID, "ACPI0007")
-      Name(_UID, 4)
-    }
-    Device(CPU5) {
-      Name(_HID, "ACPI0007")
-      Name(_UID, 5)
-    }
-    Device(CPU6) {
-      Name(_HID, "ACPI0007")
-      Name(_UID, 6)
-    }
-    Device(CPU7) {
-      Name(_HID, "ACPI0007")
-      Name(_UID, 7)
-    }
-
     // SMC91X
     Device (NET0) {
       Name (_HID, "LNRO0003")
diff --git a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
index 58a4bf9890bc2a701dab558a1987f9a51662481a..4df2d6cdae58df344804a8b41208a3adb8ee0110 100644
--- a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
+++ b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
@@ -77,7 +77,13 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
       CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdDbg2),
       NULL
     },
-
+    // SSDT Cpu Hierarchy Table
+    {
+      EFI_ACPI_6_3_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+      0, // Unused
+      CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSsdtCpuTopology),
+      NULL
+    },
     // Note: The last 3 tables in this list are for FVP RevC only.
     // IORT Table - FVP RevC
     {
diff --git a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
index aebf0a355291df5df5f588e8b7076e21eda9a152..1b52c2ebc7efb633c748f7316606e3dbe4e0b21c 100644
--- a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
+++ b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
@@ -89,7 +89,7 @@ typedef EFI_STATUS (*CM_OBJECT_HANDLER_PROC) (
 
 /** The number of ACPI tables to install
 */
-#define PLAT_ACPI_TABLE_COUNT       9
+#define PLAT_ACPI_TABLE_COUNT       10
 
 /** The number of platform generic timer blocks
 */
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108594): https://edk2.groups.io/g/devel/message/108594
Mute This Topic: https://groups.io/mt/101335886/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms v2 2/3] Platform/ARM: FVP: Specify TRBE interrupt in MADT GICC

[Sami Mujawar]

When TRBE is enabled the FVP model uses the PPI 15
(i.e. INT ID 31) as the TRBE interrupt.
Ref: https://www.kernel.org/doc/Documentation/
devicetree/bindings/arm/arm,trace-buffer-extension.yaml

Therefore, check the debug feature register
ID_AA64DFR0_EL1.TraceBuffer field to see if TRBE is
enabled and configure the TRBE interrupt in the GICC
structure in the MADT ACPI table.

Note: To enable TRBE support in the FVP REvC model
1. Build TF-A with the CTX_INCLUDE_AARCH32_REGS=0
   build flag set, otherwise this results in an
   exception when booting TF-A.
2. Set the model parameters to enable TRBE
    -C cluster0.has_trbe=1 -C cluster1.has_trbe=1

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    V2:
    - Incorrect comment for TRBE interrupt number   [Jeshua]
    - Fixed comment to specify TRBE interrupt as    [Sami]
      PPI 15
      Ref: https://edk2.groups.io/g/devel/message/107426

 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c      | 39 ++++++++++++++++----
 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf |  3 +-
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
index 4df2d6cdae58df344804a8b41208a3adb8ee0110..221ccd44ca419edf030a0b37a6bbe64a1ab11273 100644
--- a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
+++ b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
@@ -1,7 +1,7 @@
 /** @file
   Configuration Manager Dxe
 
-  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2017 - 2023, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -15,6 +15,7 @@
 #include <IndustryStandard/IoRemappingTable.h>
 #include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
 #include <Library/ArmLib.h>
+#include <Library/ArmLib/AArch64/AArch64Lib.h>
 #include <Library/DebugLib.h>
 #include <Library/IoLib.h>
 #include <Library/PcdLib.h>
@@ -37,8 +38,8 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
   {
     // FADT Table
     {
-      EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
-      EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,
+      EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+      EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,
       CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt),
       NULL
     },
@@ -51,8 +52,8 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
     },
     // MADT Table
     {
-      EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
-      EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
+      EFI_ACPI_6_5_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
+      EFI_ACPI_6_5_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
       CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMadt),
       NULL
     },
@@ -109,15 +110,15 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
   },
 
   // Boot architecture information
-  { EFI_ACPI_6_3_ARM_PSCI_COMPLIANT },              // BootArchFlags
+  { EFI_ACPI_6_5_ARM_PSCI_COMPLIANT },              // BootArchFlags
 
 #ifdef HEADLESS_PLATFORM
   // Fixed feature flag information
-  { EFI_ACPI_6_3_HEADLESS },                        // Fixed feature flags
+  { EFI_ACPI_6_5_HEADLESS },                        // Fixed feature flags
 #endif
 
   // Power management profile information
-  { EFI_ACPI_6_3_PM_PROFILE_ENTERPRISE_SERVER },    // PowerManagement Profile
+  { EFI_ACPI_6_5_PM_PROFILE_ENTERPRISE_SERVER },    // PowerManagement Profile
 
   /* GIC CPU Interface information
      GIC_ENTRY (CPUInterfaceNumber, Mpidr, PmuIrq, VGicIrq, EnergyEfficiency)
@@ -474,6 +475,9 @@ InitializePlatformRepository (
   )
 {
   EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
+  UINT64 DbgFeatures;
+  UINTN  Index;
+  UINT16 TrbeInterrupt;
 
   PlatformRepo = This->PlatRepoInfo;
 
@@ -491,6 +495,25 @@ InitializePlatformRepository (
     PlatformRepo->GicCInfo[6].MPIDR = GET_MPID_MT (1, 2, 0);
     PlatformRepo->GicCInfo[7].MPIDR = GET_MPID_MT (1, 3, 0);
   }
+
+  TrbeInterrupt = 0;
+  DbgFeatures = ArmReadIdAA64Dfr0 ();
+  DEBUG ((
+    DEBUG_INFO,
+    "Debug Feature Register 0 - ID_AA64DFR0_EL1 = 0x%lx\n",
+    DbgFeatures
+    ));
+
+  // The ID_AA64DFR0_EL1.TraceBuffer field identifies support for FEAT_TRBE.
+  if (((DbgFeatures >> 44) & 0xF) != 0) {
+    // TRBE Interrupt is PPI 15 on FVP model.
+    TrbeInterrupt = 31;
+  }
+
+  for (Index = 0; Index < PLAT_CPU_COUNT; Index++) {
+    PlatformRepo->GicCInfo[Index].TrbeInterrupt = TrbeInterrupt;
+  }
+
   return EFI_SUCCESS;
 }
 
diff --git a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
index b53daf51d4b1afd45e41d0debb0b9f084f135f6a..dd08f8597768d4de62941c5fd74e329c431582b0 100644
--- a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
+++ b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
@@ -1,7 +1,7 @@
 ## @file
 #  Configuration Manager Dxe
 #
-#  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
+#  Copyright (c) 2017 - 2023, Arm Limited. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
@@ -34,6 +34,7 @@ [Packages]
   Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
 
 [LibraryClasses]
+  ArmLib
   ArmPlatformLib
   PrintLib
   UefiBootServicesTableLib
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108597): https://edk2.groups.io/g/devel/message/108597
Mute This Topic: https://groups.io/mt/101335889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH edk2-platforms v2 3/3] Platform/ARM: FVP: Add ETE device if supported by FVP

[Sami Mujawar]

When ETE is enabled in the FVP model the firmware can check
the debug feature register ID_AA64DFR0_EL1.TraceVer field
to identify the presence of FEAT_ETE and add an ETE device
to the CPU node in the AML CPU hierarchy. This enables the
Operating System driver to probe and enable ETE support.

 Note: To enable ETE support in the FVP REvC model
    1. Build TF-A with the CTX_INCLUDE_AARCH32_REGS=0
       build flag set, otherwise this results in an
       exception when booting TF-A
    2. Set the model parameters to enable TRBE as this
       is required for ETE
        -C cluster0.has_trbe=1 -C cluster1.has_trbe=1
    3. Set the ETE plugin for the model
       --plugin <PLUGIN_PATH>\libete-plugin.[so|dll]

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v2:
    - No code change from v1 patch series.      [SAMI]

 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c | 25 ++++++++++++++++++++
 Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
index 221ccd44ca419edf030a0b37a6bbe64a1ab11273..b4855c6b6fdca0093744cad65cfabbb785a6c62c 100644
--- a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
+++ b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
@@ -366,6 +366,11 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
     FixedPcdGet32 (PcdPciBusMin),
     FixedPcdGet32 (PcdPciBusMax)
   },
+
+  // Embedded Trace device info
+  {
+    ArmEtTypeEte
+  }
 };
 
 /** A helper function for returning the Configuration Manager Objects.
@@ -478,6 +483,7 @@ InitializePlatformRepository (
   UINT64 DbgFeatures;
   UINTN  Index;
   UINT16 TrbeInterrupt;
+  CM_OBJECT_TOKEN EtToken;
 
   PlatformRepo = This->PlatRepoInfo;
 
@@ -497,6 +503,7 @@ InitializePlatformRepository (
   }
 
   TrbeInterrupt = 0;
+  EtToken = CM_NULL_TOKEN;
   DbgFeatures = ArmReadIdAA64Dfr0 ();
   DEBUG ((
     DEBUG_INFO,
@@ -510,8 +517,14 @@ InitializePlatformRepository (
     TrbeInterrupt = 31;
   }
 
+  // The ID_AA64DFR0_EL1.TraceVer field identifies the presence of FEAT_ETE.
+  if (((DbgFeatures >> 4) & 0xF) != 0) {
+    EtToken = (CM_OBJECT_TOKEN)&PlatformRepo->EtInfo;
+  }
+
   for (Index = 0; Index < PLAT_CPU_COUNT; Index++) {
     PlatformRepo->GicCInfo[Index].TrbeInterrupt = TrbeInterrupt;
+    PlatformRepo->GicCInfo[Index].EtToken = EtToken;
   }
 
   return EFI_SUCCESS;
@@ -983,6 +996,18 @@ GetArmNameSpaceObject (
                  );
       break;
 
+    case EArmObjEtInfo:
+      if (Token == (CM_OBJECT_TOKEN)&PlatformRepo->EtInfo) {
+        Status = HandleCmObject (
+                  CmObjectId,
+                  &PlatformRepo->EtInfo,
+                  sizeof (PlatformRepo->EtInfo),
+                  1,
+                  CmObject
+                  );
+      }
+      break;
+
     default: {
       Status = EFI_NOT_FOUND;
       DEBUG ((
diff --git a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
index 1b52c2ebc7efb633c748f7316606e3dbe4e0b21c..be2b512911f897dc57328673ae4f4a2014ec20fb 100644
--- a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
+++ b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.h
@@ -171,6 +171,8 @@ typedef struct PlatformRepositoryInfo {
   /// PCI configuration space information
   CM_ARM_PCI_CONFIG_SPACE_INFO          PciConfigInfo;
 
+  CM_ARM_ET_INFO                        EtInfo;
+
   /// System ID
   UINT32                                SysId;
 } EDKII_PLATFORM_REPOSITORY_INFO;
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108596): https://edk2.groups.io/g/devel/message/108596
Mute This Topic: https://groups.io/mt/101335888/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v2 2/3] Platform/ARM: FVP: Specify TRBE interrupt in MADT GICC

[PierreGondois]

Hi Sami,

On 9/13/23 14:52, Sami Mujawar wrote:
> When TRBE is enabled the FVP model uses the PPI 15
> (i.e. INT ID 31) as the TRBE interrupt.
> Ref: https://www.kernel.org/doc/Documentation/
> devicetree/bindings/arm/arm,trace-buffer-extension.yaml
> 
> Therefore, check the debug feature register
> ID_AA64DFR0_EL1.TraceBuffer field to see if TRBE is
> enabled and configure the TRBE interrupt in the GICC
> structure in the MADT ACPI table.
> 
> Note: To enable TRBE support in the FVP REvC model
> 1. Build TF-A with the CTX_INCLUDE_AARCH32_REGS=0
>     build flag set, otherwise this results in an
>     exception when booting TF-A.
> 2. Set the model parameters to enable TRBE
>      -C cluster0.has_trbe=1 -C cluster1.has_trbe=1
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> 
> Notes:
>      V2:
>      - Incorrect comment for TRBE interrupt number   [Jeshua]
>      - Fixed comment to specify TRBE interrupt as    [Sami]
>        PPI 15
>        Ref: https://edk2.groups.io/g/devel/message/107426
> 
>   Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c      | 39 ++++++++++++++++----
>   Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf |  3 +-
>   2 files changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> index 4df2d6cdae58df344804a8b41208a3adb8ee0110..221ccd44ca419edf030a0b37a6bbe64a1ab11273 100644
> --- a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> +++ b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> @@ -1,7 +1,7 @@
>   /** @file
>     Configuration Manager Dxe
>   
> -  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
> +  Copyright (c) 2017 - 2023, Arm Limited. All rights reserved.<BR>
>   
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>   
> @@ -15,6 +15,7 @@
>   #include <IndustryStandard/IoRemappingTable.h>
>   #include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
>   #include <Library/ArmLib.h>
> +#include <Library/ArmLib/AArch64/AArch64Lib.h>
>   #include <Library/DebugLib.h>
>   #include <Library/IoLib.h>
>   #include <Library/PcdLib.h>
> @@ -37,8 +38,8 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
>     {
>       // FADT Table
>       {
> -      EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> -      EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,
> +      EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> +      EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,
>         CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt),
>         NULL
>       },
> @@ -51,8 +52,8 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
>       },
>       // MADT Table
>       {
> -      EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
> -      EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
> +      EFI_ACPI_6_5_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
> +      EFI_ACPI_6_5_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
>         CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMadt),
>         NULL
>       },
> @@ -109,15 +110,15 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
>     },
>   
>     // Boot architecture information
> -  { EFI_ACPI_6_3_ARM_PSCI_COMPLIANT },              // BootArchFlags
> +  { EFI_ACPI_6_5_ARM_PSCI_COMPLIANT },              // BootArchFlags
>   
>   #ifdef HEADLESS_PLATFORM
>     // Fixed feature flag information
> -  { EFI_ACPI_6_3_HEADLESS },                        // Fixed feature flags
> +  { EFI_ACPI_6_5_HEADLESS },                        // Fixed feature flags
>   #endif
>   
>     // Power management profile information
> -  { EFI_ACPI_6_3_PM_PROFILE_ENTERPRISE_SERVER },    // PowerManagement Profile
> +  { EFI_ACPI_6_5_PM_PROFILE_ENTERPRISE_SERVER },    // PowerManagement Profile
>   
>     /* GIC CPU Interface information
>        GIC_ENTRY (CPUInterfaceNumber, Mpidr, PmuIrq, VGicIrq, EnergyEfficiency)
> @@ -474,6 +475,9 @@ InitializePlatformRepository (
>     )
>   {
>     EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
> +  UINT64 DbgFeatures;
> +  UINTN  Index;
> +  UINT16 TrbeInterrupt;
>   
>     PlatformRepo = This->PlatRepoInfo;
>   
> @@ -491,6 +495,25 @@ InitializePlatformRepository (
>       PlatformRepo->GicCInfo[6].MPIDR = GET_MPID_MT (1, 2, 0);
>       PlatformRepo->GicCInfo[7].MPIDR = GET_MPID_MT (1, 3, 0);
>     }
> +
> +  TrbeInterrupt = 0;
> +  DbgFeatures = ArmReadIdAA64Dfr0 ();
> +  DEBUG ((
> +    DEBUG_INFO,
> +    "Debug Feature Register 0 - ID_AA64DFR0_EL1 = 0x%lx\n",
> +    DbgFeatures
> +    ));
> +
> +  // The ID_AA64DFR0_EL1.TraceBuffer field identifies support for FEAT_TRBE.
> +  if (((DbgFeatures >> 44) & 0xF) != 0) {

Ideally I think we should add macros for these register flags instead of hard-coded
values like 44/0xF, same comment for:
- [PATCH edk2-platforms v2 3/3] Platform/ARM: FVP: Add ETE device if supported by FVP

Regards,
Pierre


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108984): https://edk2.groups.io/g/devel/message/108984
Mute This Topic: https://groups.io/mt/101335889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-platforms v2 2/3] Platform/ARM: FVP: Specify TRBE interrupt in MADT GICC

[Sami Mujawar]

Hi Pierre,

Thank you for the feedback.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 22/09/2023, 09:56, "Pierre Gondois" <pierre.gondois@arm.com <mailto:pierre.gondois@arm.com>> wrote:


Hi Sami,


On 9/13/23 14:52, Sami Mujawar wrote:
> When TRBE is enabled the FVP model uses the PPI 15
> (i.e. INT ID 31) as the TRBE interrupt.
> Ref: https://www.kernel.org/doc/Documentation/ <https://www.kernel.org/doc/Documentation/>
> devicetree/bindings/arm/arm,trace-buffer-extension.yaml
> 
> Therefore, check the debug feature register
> ID_AA64DFR0_EL1.TraceBuffer field to see if TRBE is
> enabled and configure the TRBE interrupt in the GICC
> structure in the MADT ACPI table.
> 
> Note: To enable TRBE support in the FVP REvC model
> 1. Build TF-A with the CTX_INCLUDE_AARCH32_REGS=0
> build flag set, otherwise this results in an
> exception when booting TF-A.
> 2. Set the model parameters to enable TRBE
> -C cluster0.has_trbe=1 -C cluster1.has_trbe=1
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com>>
> ---
> 
> Notes:
> V2:
> - Incorrect comment for TRBE interrupt number [Jeshua]
> - Fixed comment to specify TRBE interrupt as [Sami]
> PPI 15
> Ref: https://edk2.groups.io/g/devel/message/107426 <https://edk2.groups.io/g/devel/message/107426>
> 
> Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c | 39 ++++++++++++++++----
> Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf | 3 +-
> 2 files changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> index 4df2d6cdae58df344804a8b41208a3adb8ee0110..221ccd44ca419edf030a0b37a6bbe64a1ab11273 100644
> --- a/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> +++ b/Platform/ARM/VExpressPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> @@ -1,7 +1,7 @@
> /** @file
> Configuration Manager Dxe
> 
> - Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
> + Copyright (c) 2017 - 2023, Arm Limited. All rights reserved.<BR>
> 
> SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -15,6 +15,7 @@
> #include <IndustryStandard/IoRemappingTable.h>
> #include <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
> #include <Library/ArmLib.h>
> +#include <Library/ArmLib/AArch64/AArch64Lib.h>
> #include <Library/DebugLib.h>
> #include <Library/IoLib.h>
> #include <Library/PcdLib.h>
> @@ -37,8 +38,8 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
> {
> // FADT Table
> {
> - EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> - EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,
> + EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> + EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,
> CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt),
> NULL
> },
> @@ -51,8 +52,8 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
> },
> // MADT Table
> {
> - EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
> - EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
> + EFI_ACPI_6_5_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
> + EFI_ACPI_6_5_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
> CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMadt),
> NULL
> },
> @@ -109,15 +110,15 @@ EDKII_PLATFORM_REPOSITORY_INFO VExpressPlatRepositoryInfo = {
> },
> 
> // Boot architecture information
> - { EFI_ACPI_6_3_ARM_PSCI_COMPLIANT }, // BootArchFlags
> + { EFI_ACPI_6_5_ARM_PSCI_COMPLIANT }, // BootArchFlags
> 
> #ifdef HEADLESS_PLATFORM
> // Fixed feature flag information
> - { EFI_ACPI_6_3_HEADLESS }, // Fixed feature flags
> + { EFI_ACPI_6_5_HEADLESS }, // Fixed feature flags
> #endif
> 
> // Power management profile information
> - { EFI_ACPI_6_3_PM_PROFILE_ENTERPRISE_SERVER }, // PowerManagement Profile
> + { EFI_ACPI_6_5_PM_PROFILE_ENTERPRISE_SERVER }, // PowerManagement Profile
> 
> /* GIC CPU Interface information
> GIC_ENTRY (CPUInterfaceNumber, Mpidr, PmuIrq, VGicIrq, EnergyEfficiency)
> @@ -474,6 +475,9 @@ InitializePlatformRepository (
> )
> {
> EDKII_PLATFORM_REPOSITORY_INFO * PlatformRepo;
> + UINT64 DbgFeatures;
> + UINTN Index;
> + UINT16 TrbeInterrupt;
> 
> PlatformRepo = This->PlatRepoInfo;
> 
> @@ -491,6 +495,25 @@ InitializePlatformRepository (
> PlatformRepo->GicCInfo[6].MPIDR = GET_MPID_MT (1, 2, 0);
> PlatformRepo->GicCInfo[7].MPIDR = GET_MPID_MT (1, 3, 0);
> }
> +
> + TrbeInterrupt = 0;
> + DbgFeatures = ArmReadIdAA64Dfr0 ();
> + DEBUG ((
> + DEBUG_INFO,
> + "Debug Feature Register 0 - ID_AA64DFR0_EL1 = 0x%lx\n",
> + DbgFeatures
> + ));
> +
> + // The ID_AA64DFR0_EL1.TraceBuffer field identifies support for FEAT_TRBE.
> + if (((DbgFeatures >> 44) & 0xF) != 0) {


Ideally I think we should add macros for these register flags instead of hard-coded
values like 44/0xF, same comment for:
- [PATCH edk2-platforms v2 3/3] Platform/ARM: FVP: Add ETE device if supported by FVP

[SAMI] I will check if I can add helper functions like ArmHasTrbe() in ArmLib.

Regards,
Pierre





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108986): https://edk2.groups.io/g/devel/message/108986
Mute This Topic: https://groups.io/mt/101335889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-