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.gond...@arm.com <mailto:pierre.gond...@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.muja...@arm.com > <mailto:sami.muja...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
