Hi Sami, The patch is tested and looks good to me: Tested-by: Jagadeesh Ujja <jagadeesh.u...@arm.com>
On Wed, Jul 6, 2022 at 8:57 PM Sami Mujawar <sami.muja...@arm.com> 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: <pierre.gond...@arm.com> > > > > 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 <sami.muja...@arm.com> > >> --- > >> 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)) { > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91217): https://edk2.groups.io/g/devel/message/91217 Mute This Topic: https://groups.io/mt/92204479/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
