public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v1 1/1] DynamicTablesPkg: Add support to specify FADT minor revision
@ 2022-07-06 11:40 Sami Mujawar
  2022-07-06 15:03 ` PierreGondois
  0 siblings, 1 reply; 4+ messages in thread
From: Sami Mujawar @ 2022-07-06 11:40 UTC (permalink / raw)
  To: devel
  Cc: Sami Mujawar, Alexei.Fedorov, Pierre.Gondois, jbrasen,
	Jagadeesh.Ujja, Deepak.Pandey, Chandni.Cherukuri, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

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.mujawar@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)) {
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v1 1/1] DynamicTablesPkg: Add support to specify FADT minor revision
  2022-07-06 11:40 [PATCH v1 1/1] DynamicTablesPkg: Add support to specify FADT minor revision Sami Mujawar
@ 2022-07-06 15:03 ` PierreGondois
  2022-07-06 15:27   ` Sami Mujawar
  0 siblings, 1 reply; 4+ messages in thread
From: PierreGondois @ 2022-07-06 15:03 UTC (permalink / raw)
  To: Sami Mujawar, devel
  Cc: Alexei.Fedorov, jbrasen, Jagadeesh.Ujja, Deepak.Pandey,
	Chandni.Cherukuri, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson,
	nd

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:

Reviewed-by: <pierre.gondois@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.mujawar@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)) {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1 1/1] DynamicTablesPkg: Add support to specify FADT minor revision
  2022-07-06 15:03 ` PierreGondois
@ 2022-07-06 15:27   ` Sami Mujawar
  2022-07-11 11:23     ` [edk2-devel] " Jagadeesh Ujja
  0 siblings, 1 reply; 4+ messages in thread
From: Sami Mujawar @ 2022-07-06 15:27 UTC (permalink / raw)
  To: Pierre Gondois, devel, meenakshi.aggarwal, Leif Lindholm
  Cc: Alexei.Fedorov, jbrasen, Jagadeesh.Ujja, Deepak.Pandey,
	Chandni.Cherukuri, Matteo.Carlini, Akanksha.Jain2, Ben.Adderson,
	nd

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.gondois@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.mujawar@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)) {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH v1 1/1] DynamicTablesPkg: Add support to specify FADT minor revision
  2022-07-06 15:27   ` Sami Mujawar
@ 2022-07-11 11:23     ` Jagadeesh Ujja
  0 siblings, 0 replies; 4+ messages in thread
From: Jagadeesh Ujja @ 2022-07-11 11:23 UTC (permalink / raw)
  To: devel, Sami Mujawar
  Cc: Pierre Gondois, meenakshi.aggarwal, Leif Lindholm, Alexei.Fedorov,
	jbrasen, Deepak.Pandey, Chandni.Cherukuri, Matteo.Carlini,
	Akanksha.Jain2, Ben.Adderson, nd

Hi Sami,
The patch is tested and looks good to me:

Tested-by: Jagadeesh Ujja <Jagadeesh.Ujja@arm.com>

On Wed, Jul 6, 2022 at 8:57 PM Sami Mujawar <sami.mujawar@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.gondois@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.mujawar@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)) {
>
>
> 
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-07-11 11:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-06 11:40 [PATCH v1 1/1] DynamicTablesPkg: Add support to specify FADT minor revision Sami Mujawar
2022-07-06 15:03 ` PierreGondois
2022-07-06 15:27   ` Sami Mujawar
2022-07-11 11:23     ` [edk2-devel] " Jagadeesh Ujja

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox