Hi Pierre, Thank you for the review feedback.
Please see my response inline marked [SAMI]. Regards, Sami Mujawar On 22/09/2023, 09:53, "Pierre Gondois" <pierre.gond...@arm.com <mailto:pierre.gond...@arm.com>> wrote: Hi Sami, On 9/13/23 14:49, Sami Mujawar wrote: > The ACPI 6.5 specification updates the MADT table to add > a new field to GICC for specifying the TRBE interrupt and > also adds support for Online Capable flag to the GICC flags. > > The Online Capable flags should be passed transparently > through as specified in the CM_ARM_GICC_INFO.Flags field > and only require the MADT table revision to be setup to > 6 to reflect the ACPI 6.5 specification. > > The TRBE field needs to be appropriately setup in the > GICC structure. > > Therefore, update the MADT generator to reflect the > above updates required for supporting ACPI 6.5 > > Signed-off-by: Sami Mujawar <sami.muja...@arm.com > <mailto:sami.muja...@arm.com>> > --- > > Notes: > v2: > - TRBE interrupt not set correctly for ACPI 6.4 [Jeshua] > - Fixed issue with setting TRBE interrupt [Sami] > Ref: https://edk2.groups.io/g/devel/message/107427 > <https://edk2.groups.io/g/devel/message/107427> > > DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c | 83 > +++++++++++--------- > 1 file changed, 46 insertions(+), 37 deletions(-) > > diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c > b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c > index > 2102a59faf498eaab7777c509443461ada999610..97be08b5f5b967944a351f834c3bc3f1ee5029b6 > 100644 > --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c > +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/MadtGenerator.c > @@ -1,11 +1,11 @@ > /** @file > MADT Table Generator > > - Copyright (c) 2017 - 2020, ARM Limited. All rights reserved. > + Copyright (c) 2017 - 2023, Arm Limited. All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent > > @par Reference(s): > - - ACPI 6.3 Specification - January 2019 > + - ACPI 6.5 Specification - Aug 29, 2022 > > **/ > > @@ -82,7 +82,7 @@ GET_OBJECT_LIST ( > ); > > /** This function updates the GIC CPU Interface Information in the > - EFI_ACPI_6_3_GIC_STRUCTURE structure. > + EFI_ACPI_6_5_GIC_STRUCTURE structure. > > @param [in] Gicc Pointer to GIC CPU Interface structure. > @param [in] GicCInfo Pointer to the GIC CPU Interface Information. > @@ -91,7 +91,7 @@ GET_OBJECT_LIST ( > STATIC > VOID > AddGICC ( > - IN EFI_ACPI_6_3_GIC_STRUCTURE *CONST Gicc, > + IN EFI_ACPI_6_5_GIC_STRUCTURE *CONST Gicc, > IN CONST CM_ARM_GICC_INFO *CONST GicCInfo, > IN CONST UINT8 MadtRev > ) > @@ -100,9 +100,9 @@ AddGICC ( > ASSERT (GicCInfo != NULL); > > // UINT8 Type > - Gicc->Type = EFI_ACPI_6_3_GIC; > + Gicc->Type = EFI_ACPI_6_5_GIC; > // UINT8 Length > - Gicc->Length = sizeof (EFI_ACPI_6_3_GIC_STRUCTURE); > + Gicc->Length = sizeof (EFI_ACPI_6_5_GIC_STRUCTURE); > // UINT16 Reserved > Gicc->Reserved = EFI_ACPI_RESERVED_WORD; > > @@ -148,6 +148,15 @@ AddGICC ( > // in EFI_ACPI_6_2_GIC_STRUCTURE. > Gicc->SpeOverflowInterrupt = 0; > } > + > + // UINT16 TrbeInterrupt > + if (MadtRev > EFI_ACPI_6_4_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION) { > + Gicc->TrbeInterrupt = GicCInfo->TrbeInterrupt; > + } else { > + // Setting TrbeInterrupt to 0 ensures backward compatibility with > + // ACPI 6.4 > + Gicc->TrbeInterrupt = 0; I'm not sure this is necessary as the Gicc struct should be 0-ed, [SAMI] Yes, I think we do not need to zero this field as the memory allocated for the MADT table in BuildMadtTable() is allocated using AllocateZeroPool(). I can drop the else condition. [/SAMI] Regards, Pierre -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108985): https://edk2.groups.io/g/devel/message/108985 Mute This Topic: https://groups.io/mt/101335844/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
