ACPI RSDT not zero

[samerhaj]

Acpiview shows an error indicating RSDT Address needs to be NULL on ARM platforms.

This is consistent with SBBR 1.2 rule (section 4.2.1.1)

 --------------- RSDP Table --------------- 

Address  : 0x33D30014
Length   : 36
  
00000000 : 52 53 44 20 50 54 52 20 - 97 4D 43 52 53 46 54 02   RSD PTR .MCRSFT.
00000010 : 74 00 D2 33 24 00 00 00 - E8 00 D2 33 00 00 00 00   t..3$......3....
00000020 : EF 00 00 00                                         ....

Table Checksum : OK

RSDP                                 :
  Signature                          : RSD PTR 
  Checksum                           : 0x97
  Oem ID                             : MCRSFT
  Revision                           : 2
  RSDT Address                       : 0x33D20074
ERROR: Rsdt Address = 0x33D20074. This must be NULL on ARM Platforms.
  Length                             : 36
  XSDT Address                       : 0x33D200E8
  Extended Checksum                  : 0xEF
  Reserved                           : 0 0 0

[pbatard]

Yeah, I've seen that error for some time. It looks to me like an edk2 bug, because, as far as I can tell, that table is generated automatically and we have little/no control about it in our platform files.

I'm willing to bet that we're far from the only ARM platform producing this error, and I've kind of been waiting for some other ARM platforms to be bothered enough with this to send a global edk2 fix... 😉

[samerhaj]

I think you simply need to set gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions to 0x20. This is what the other Arm platforms (Juno, FVP, Socionext DeveloperBox, etc..) did/

The default value of PcdAcpiExposedTableVersions is 0x3E, which sets multiple ACPI revisions supported, including EFI_ACPI_TABLE_VERSION_1_0B (bit 1)

The code in AcpiTableDxe populates the RSDT pointer only if PcdAcpiExposedTableVersions has the EFI_ACPI_TABLE_VERSION_1_0B bit set. Look for example at PublishTables ( ) and similar code. This is per ACPI spec section 5.2.7, which defines RSDT to be for compatability with ACPI 1.0 OSes.

[samerhaj]

I think you simply need to set gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions to 0x20.

Confirmed this fixes the issue. I will send a patch

[pbatard]

That's good to know (and I will test that too), but I would argue that, if the EDK2 code for ARM sets a default PCD value that produces an ACPI error, then EDK2 should be fixed, because I think one is entitled to expect that the default PCD values should not generate an ACPI error.

In other words, the default value for gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions should be set to 0x20 for ARM platforms in the EDK2, and that is the patch I think we should be aiming for.

[pbatard]

It's currently defined at https://github.com/tianocore/edk2/blob/01ce872739d2f0cd3a8917be2180381db5f0391e/MdeModulePkg/MdeModulePkg.dec#L1415

I think we probably want to have a [PcdsFixedAtBuild.ARM, PcdsFixedAtBuild.AARCH64, PcdsPatchableInModule.ARM, PcdsPatchableInModule.AARCH64] section to override this, since this is currently set under [PcdsFixedAtBuild, PcdsPatchableInModule]. If you want to send a patch for that, that would be great, otherwise I'll be happy to do it.

[samerhaj]

OK adding this in MdeModulePkg.dec does take care of the issue, and seems to keep the old value for non-Arm systems

[PcdsFixedAtBuild.ARM, PcdsFixedAtBuild.AARCH64, PcdsPatchableInModule.ARM, PcdsPatchableInModule.AARCH64]
  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20|UINT32|0x0001004c

However, I think we should probably do this only for Aarch64 and leave Arm alone. The SBBR requirement only applies to Aarch64.

So it should be:

[PcdsFixedAtBuild.AARCH64, PcdsPatchableInModule.AARCH64]
  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20|UINT32|0x0001004c

Thoughts?

[pbatard]

Sounds good to me. The people reviewing the patch on edk2-devel will most likely let us know if they think this should apply to more than AARCH64 anyway.

[andreiw]

Probably fixed even earlier, my bad.