From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.27286.1657538656256821169 for ; Mon, 11 Jul 2022 04:24:16 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: jagadeesh.ujja@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0162A1A00 for ; Mon, 11 Jul 2022 04:24:16 -0700 (PDT) Received: from mail-qk1-f179.google.com (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7B8653F792 for ; Mon, 11 Jul 2022 04:24:15 -0700 (PDT) Received: by mail-qk1-f179.google.com with SMTP id z11so2076736qkz.13 for ; Mon, 11 Jul 2022 04:24:15 -0700 (PDT) X-Gm-Message-State: AJIora/JkozCnYgzFny7J3SBMvnAYTKCgqXrjDF3yksA5eY/HoANoQC4 95dTBRhL06JPGgU4Bp+4kbb6bBPM7EfOg3fh+VA= X-Google-Smtp-Source: AGRyM1vacnEhjnP3Ropyrx2AR5V4HNuPYjyNSr4zNYtpniPxby6acJnXNNp80v2DkcMddsX+6ph9+yldorDeJGuZiwY= X-Received: by 2002:a05:620a:1242:b0:6b5:77b1:8449 with SMTP id a2-20020a05620a124200b006b577b18449mr5908690qkl.180.1657538650872; Mon, 11 Jul 2022 04:24:10 -0700 (PDT) MIME-Version: 1.0 References: <20220706114009.26492-1-sami.mujawar@arm.com> <6a6a750c-8daa-c383-6972-306bce254a23@arm.com> <42f974ce-0afb-4e07-7f34-60b02a6cd791@arm.com> In-Reply-To: <42f974ce-0afb-4e07-7f34-60b02a6cd791@arm.com> From: "Jagadeesh Ujja" Date: Mon, 11 Jul 2022 16:53:59 +0530 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v1 1/1] DynamicTablesPkg: Add support to specify FADT minor revision To: devel@edk2.groups.io, Sami Mujawar Cc: Pierre Gondois , meenakshi.aggarwal@nxp.com, Leif Lindholm , Alexei.Fedorov@arm.com, jbrasen@nvidia.com, Deepak.Pandey@arm.com, Chandni.Cherukuri@arm.com, Matteo.Carlini@arm.com, Akanksha.Jain2@arm.com, Ben.Adderson@arm.com, nd@arm.com Content-Type: text/plain; charset="UTF-8" Hi Sami, The patch is tested and looks good to me: Tested-by: Jagadeesh Ujja On Wed, Jul 6, 2022 at 8:57 PM Sami Mujawar wrote: > > Hi Pierre, > > On 06/07/2022 04:03 pm, Pierre Gondois wrote: > > Hello Sami, > > The only configuration manager not using ACPI 6.4 tables is at: > > Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c > > > > > > I think the minor version of its FADT table needs to be updated along > > with > > this patch. Otherwise: > > > Thank you for bringing this up. > > As it stands the NXP platform would be using ACPI 6.4 which is the > default case, and this patch will not change that. So, downgrading this > to ACPI 6.2 may not be expected. > > I have copied the NXP package maintainers for further inputs. If needed, > I could submit a edk2-platforms patch as required. > > Regards, > > Sami Mujawar > > > > Reviewed-by: > > > > Regards, > > Pierre > > > > On 7/6/22 13:40, Sami Mujawar wrote: > >> The CM_STD_OBJ_ACPI_TABLE_INFO.AcpiTableRevision can be used to specify > >> the major revision number of the ACPI table that the generator must use. > >> Although most ACPI tables only have a major revision number, the FADT > >> table additionally has a minor revision number. > >> > >> The FADT generator currently defaults to setting the latest supported > >> ACPI revision for the FADT table i.e. ACPI 6.4. This means that the > >> minor > >> revision for the FADT table is always set to 4 and there is no provision > >> for a user to specify the minor revision to be selected. > >> > >> Therefore, update CM_STD_OBJ_ACPI_TABLE_INFO to introduce a new field > >> MinorRevision which can be used to specify the minor revision for an > >> ACPI table. Also update the FADT generator to validate the supported > >> FADT revisions ans use the specified minor revision for the FADT table > >> if supported. If an unsupported minor revision is specified the FADT > >> generator defaults to the latest supported minor revision. > >> > >> Since the CM_STD_OBJ_ACPI_TABLE_INFO.MinorRevision field is added to > >> the end of the structure, it should not break existing platform code. > >> > >> Signed-off-by: Sami Mujawar > >> --- > >> The changes can be seen at: > >> https://github.com/samimujawar/edk2/tree/2221_support_fadt_minor_rev_v1 > >> > >> DynamicTablesPkg/Include/StandardNameSpaceObjects.h | 10 ++++++- > >> DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c | 29 > >> ++++++++++++++++++-- > >> 2 files changed, 36 insertions(+), 3 deletions(-) > >> > >> diff --git a/DynamicTablesPkg/Include/StandardNameSpaceObjects.h > >> b/DynamicTablesPkg/Include/StandardNameSpaceObjects.h > >> index > >> 8d0c7da15a73e4910f9099c68f6e5cc2f06c0ecb..8ec3238225abe4fc16a7337c29ecd655590b408f > >> 100644 > >> --- a/DynamicTablesPkg/Include/StandardNameSpaceObjects.h > >> +++ b/DynamicTablesPkg/Include/StandardNameSpaceObjects.h > >> @@ -1,6 +1,6 @@ > >> /** @file > >> - Copyright (c) 2017 - 2019, ARM Limited. All rights reserved. > >> + Copyright (c) 2017 - 2022, Arm Limited. All rights reserved. > >> SPDX-License-Identifier: BSD-2-Clause-Patent > >> @@ -105,6 +105,14 @@ typedef struct CmAStdObjAcpiTableInfo { > >> /// Generators shall populate this information using the revision > >> of the > >> /// Configuration Manager > >> (CM_STD_OBJ_CONFIGURATION_MANAGER_INFO.Revision). > >> UINT32 OemRevision; > >> + > >> + /// The minor revision of an ACPI table if required by the table. > >> + /// Note: If this field is not populated (has value of Zero), then > >> the > >> + /// Generators shall populate this information based on the latest > >> minor > >> + /// revision of the table that is supported by the generator. > >> + /// e.g. This field can be used to specify the minor revision to > >> be set > >> + /// for the FADT table. > >> + UINT8 MinorRevision; > >> } CM_STD_OBJ_ACPI_TABLE_INFO; > >> /** A structure used to describe the SMBIOS table generators to > >> be invoked. > >> diff --git > >> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c > >> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c > >> index > >> 96295f539fb0505378e862edeef898be40257cdd..1d10ea55e2395c55291faa3c247e5c59e345650c > >> 100644 > >> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c > >> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/FadtGenerator.c > >> @@ -1,7 +1,7 @@ > >> /** @file > >> FADT Table Generator > >> - Copyright (c) 2017 - 2021, ARM Limited. All rights reserved. > >> + Copyright (c) 2017 - 2022, Arm Limited. All rights reserved. > >> SPDX-License-Identifier: BSD-2-Clause-Patent > >> @par Reference(s): > >> @@ -167,7 +167,7 @@ EFI_ACPI_6_4_FIXED_ACPI_DESCRIPTION_TABLE > >> AcpiFadt = { > >> // UINT16 ArmBootArch > >> EFI_ACPI_6_4_ARM_PSCI_COMPLIANT, // {Template}: ARM Boot > >> Architecture Flags > >> // UINT8 MinorRevision > >> - EFI_ACPI_6_4_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION, > >> + EFI_ACPI_6_4_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION, // > >> {Template} > >> // UINT64 XFirmwareCtrl > >> 0, > >> // UINT64 XDsdt > >> @@ -546,6 +546,31 @@ BuildFadtTable ( > >> goto error_handler; > >> } > >> + // Update the MinorRevision for the FADT table if it has been > >> specified > >> + // otherwise default to the latest FADT minor revision supported. > >> + // Note: > >> + // Bits 0-3 - The low order bits correspond to the minor version > >> of the > >> + // specification version. > >> + // Bits 4-7 - The high order bits correspond to the version of the > >> ACPI > >> + // specification errata. > >> + if (AcpiTableInfo->MinorRevision != 0) { > >> + if (((AcpiTableInfo->MinorRevision & 0xF) >= > >> + EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION) && > >> + ((AcpiTableInfo->MinorRevision & 0xF) <= > >> + EFI_ACPI_6_4_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION)) > >> + { > >> + AcpiFadt.MinorVersion = AcpiTableInfo->MinorRevision; > >> + } else { > >> + DEBUG (( > >> + DEBUG_WARN, > >> + "WARNING: FADT: Unsupported FADT Minor Revision 0x%x > >> specified, " \ > >> + "defaulting to FADT Minor Revision 0x%x\n", > >> + AcpiTableInfo->MinorRevision, > >> + EFI_ACPI_6_4_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION > >> + )); > >> + } > >> + } > >> + > >> // Update PmProfile Info > >> Status = FadtAddPmProfileInfo (CfgMgrProtocol); > >> if (EFI_ERROR (Status)) { > > > > >