Re: [edk2-devel] [PATCH v2 05/11] DynamicTablesPkg: Update MADT generator for ACPI 6.5

["Sami Mujawar" <sami.mujawar@arm.com>]

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.gondois@arm.com <mailto:pierre.gondois@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.mujawar@arm.com <mailto:sami.mujawar@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-