public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms][PATCH 1/1] Platform/RPi/AcpiTables: Update all tables to latest ACPI
@ 2020-05-06 11:37 Pete Batard
  2020-05-06 12:26 ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: Pete Batard @ 2020-05-06 11:37 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif, awarkentin

Because of its widespread availability and low price, we expect the
Raspberry Pi source to be used by platform developers as a starting
point to create their own platform implementation.

As such, it makes a lot of sense to want to use the most up to date
underlying standards, even if the pay off is limited in this case,
or may even be seen as a liability in terms of ensuring greater
compatibility with older OSes, as it may help others benefit from
the latest improvements and features brought by modern ACPI.

We also happen to already be using ACPI 6.3 constructs, such as
PPTT (which was only introduced in ACPI 6.2 and further extended
in 6.3), and have some reliance on 6.x GIC, as the original MADT
binary blobs from Microsoft were 6.0 and abuse the GICR Base
Address field in order for Windows to boot properly (for reasons
that the proprietary nature of the Windows kernel makes difficult
to determine). So in effect, we did apply a potential breaking
change to Windows when we downgraded MADT to ACPI 5.1, though we
did validate at the time that the downgrade in ACPI version didn't
*seem* to break OS functionality. Still, because we are in the
dark as to what the ACPI fields we removed when downgrading ACPI
version were being used for, we obviously want to add them back.

Therefore, since we do see a need for ACPI 6.x features, and
effectively have a 6.3 table with PPTT, and also since we have
tested that we are not seeing ill effects from doing so for the
most common OSes we support, we bring all of the relevant ACPI
tables to version 6.3.

This is mostly accomplished by simply altering the version of
ACPI being references in the macros, except for the new fields
being initialized in the MADT table where we:
* Use 1 for GICR Base Address in GICC_STRUCTURE_INIT because, even
  as this field is not supposed to apply to any of our platforms,
  this is what Microsoft used in the original Pi 3 hardcoded MADT
  blobs and Windows 10 doesn't boot on the Pi 3 if set to 0.
* Use 2 for GIC version in GIC_DISTRIBUTOR_INIT, since the Pi 4 GIC
  is v2 only.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/AcpiTables/AcpiTables.h |  8 ++--
 Platform/RaspberryPi/AcpiTables/Csrt.aslc    | 14 +++----
 Platform/RaspberryPi/AcpiTables/Dbg2.aslc    |  6 +--
 Platform/RaspberryPi/AcpiTables/Fadt.aslc    | 43 +++++++++-----------
 Platform/RaspberryPi/AcpiTables/Gtdt.aslc    | 14 +++----
 Platform/RaspberryPi/AcpiTables/Madt.aslc    | 41 +++++++++++--------
 Platform/RaspberryPi/AcpiTables/Spcr.aslc    |  4 +-
 7 files changed, 66 insertions(+), 64 deletions(-)

diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.h b/Platform/RaspberryPi/AcpiTables/AcpiTables.h
index dfae763d8107..37e2a6bdf409 100644
--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.h
+++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.h
@@ -32,7 +32,7 @@
 #endif
 #define EFI_ACPI_OEM_REVISION                 0x00000200
 #define EFI_ACPI_CREATOR_ID                   SIGNATURE_32 ('E','D','K','2')
-#define EFI_ACPI_CREATOR_REVISION             0x00000200
+#define EFI_ACPI_CREATOR_REVISION             0x00000300
 
 #define EFI_ACPI_VENDOR_ID                    SIGNATURE_32 ('R','P','I','F')
 
@@ -63,7 +63,7 @@
 #define RPI_SYSTEM_TIMER_BASE_ADDRESS         0xFF80001C
 #endif
 
-#define EFI_ACPI_5_1_CSRT_REVISION            0x00000000
+#define EFI_ACPI_6_3_CSRT_REVISION            0x00000000
 
 typedef enum
 {
@@ -95,7 +95,7 @@ typedef struct
   UINT16 Revision;                // 2 bytes
   UINT16 Reserved;                // 2 bytes
   UINT32 SharedInfoLength;        // 4 bytes
-} EFI_ACPI_5_1_CSRT_RESOURCE_GROUP_HEADER;
+} EFI_ACPI_6_3_CSRT_RESOURCE_GROUP_HEADER;
 
 //------------------------------------------------------------------------
 // CSRT Resource Descriptor 12 bytes total
@@ -106,7 +106,7 @@ typedef struct
   UINT16 ResourceType;            // 2 bytes
   UINT16 ResourceSubType;         // 2 bytes
   UINT32 UID;                     // 4 bytes
-} EFI_ACPI_5_1_CSRT_RESOURCE_DESCRIPTOR_HEADER;
+} EFI_ACPI_6_3_CSRT_RESOURCE_DESCRIPTOR_HEADER;
 
 //------------------------------------------------------------------------
 // Interrupts. These are specific to each platform
diff --git a/Platform/RaspberryPi/AcpiTables/Csrt.aslc b/Platform/RaspberryPi/AcpiTables/Csrt.aslc
index 03d888fffb8b..76dbdfafba2c 100644
--- a/Platform/RaspberryPi/AcpiTables/Csrt.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Csrt.aslc
@@ -41,7 +41,7 @@ typedef struct
 //------------------------------------------------------------------------
 typedef struct
 {
-  EFI_ACPI_5_1_CSRT_RESOURCE_DESCRIPTOR_HEADER DmaControllerHeader;
+  EFI_ACPI_6_3_CSRT_RESOURCE_DESCRIPTOR_HEADER DmaControllerHeader;
   DMA_CONTROLLER_VENDOR_DATA ControllerVendorData;
 } RD_DMA_CONTROLLER;
 
@@ -61,7 +61,7 @@ typedef struct
 //------------------------------------------------------------------------
 typedef struct
 {
-  EFI_ACPI_5_1_CSRT_RESOURCE_DESCRIPTOR_HEADER DmaChannelHeader;
+  EFI_ACPI_6_3_CSRT_RESOURCE_DESCRIPTOR_HEADER DmaChannelHeader;
   DMA_CHANNEL_VENDOR_DATA ChannelVendorData;
 } RD_DMA_CHANNEL;
 
@@ -71,7 +71,7 @@ typedef struct
 
 typedef struct
 {
-  EFI_ACPI_5_1_CSRT_RESOURCE_GROUP_HEADER ResGroupHeader;
+  EFI_ACPI_6_3_CSRT_RESOURCE_GROUP_HEADER ResGroupHeader;
   RD_DMA_CONTROLLER DmaController;
   RD_DMA_CHANNEL DmaChannels[RPI_DMA_CHANNEL_COUNT];
 } RG_DMA;
@@ -87,17 +87,17 @@ typedef struct
 // DMA Resource Group
   RG_DMA DmaResourceGroup;
 
-} EFI_ACPI_5_1_CSRT_TABLE;
+} EFI_ACPI_6_3_CSRT_TABLE;
 
-EFI_ACPI_5_1_CSRT_TABLE Csrt =
+EFI_ACPI_6_3_CSRT_TABLE Csrt =
 {
   //------------------------------------------------------------------------
   // ACPI Table Header
   //------------------------------------------------------------------------
   {
-    EFI_ACPI_5_1_CORE_SYSTEM_RESOURCE_TABLE_SIGNATURE,       // Signature "CSRT"
+    EFI_ACPI_6_3_CORE_SYSTEM_RESOURCE_TABLE_SIGNATURE,       // Signature "CSRT"
     sizeof (EFI_ACPI_DESCRIPTION_HEADER) + sizeof (RG_DMA),  // Length
-    EFI_ACPI_5_1_CSRT_REVISION,     // Revision
+    EFI_ACPI_6_3_CSRT_REVISION,     // Revision
     0x00,                           // Checksum calculated at runtime.
     EFI_ACPI_OEM_ID,                // OEMID is a 6 bytes long field
     EFI_ACPI_OEM_TABLE_ID,          // OEM table identification (8 bytes long)
diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc b/Platform/RaspberryPi/AcpiTables/Dbg2.aslc
index c3d5994f8e97..c35b15693f5a 100644
--- a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Dbg2.aslc
@@ -40,7 +40,7 @@
 
 typedef struct {
   EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
-  EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
+  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
   UINT32                                                AddressSize;
   UINT8                                                 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE];
 } DBG2_DEBUG_DEVICE_INFORMATION;
@@ -66,7 +66,7 @@ typedef struct {
       OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
       OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
     },                                                                                                                \
-    ARM_GAS32 (UartBase),                            /* EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
+    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
     UartAddrLen,                                     /* UINT32  AddressSize */                                        \
     UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
   }
@@ -75,7 +75,7 @@ typedef struct {
 STATIC DBG2_TABLE Dbg2 = {
   {
     ACPI_HEADER (
-      EFI_ACPI_5_1_DEBUG_PORT_2_TABLE_SIGNATURE,
+      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
       DBG2_TABLE,
       EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
     ),
diff --git a/Platform/RaspberryPi/AcpiTables/Fadt.aslc b/Platform/RaspberryPi/AcpiTables/Fadt.aslc
index 3e3d68703298..b625c38016ec 100644
--- a/Platform/RaspberryPi/AcpiTables/Fadt.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Fadt.aslc
@@ -26,19 +26,16 @@
 #define EFI_ACPI_OEM_ID         {'B','C','2','8','3','6'}
 #endif
 
-/*
- * Note: Use ACPI 5.1 since we need to match MADT ACPI requirements
- */
-EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE Fadt = {
+EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE Fadt = {
   ACPI_HEADER (
-    EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
-    EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE,
-    EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE_REVISION
+    EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+    EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE,
+    EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_REVISION
   ),
   0,                                                                        // UINT32     FirmwareCtrl
   0,                                                                        // UINT32     Dsdt
   EFI_ACPI_RESERVED_BYTE,                                                   // UINT8      Reserved0
-  EFI_ACPI_5_1_PM_PROFILE_APPLIANCE_PC,                                     // UINT8      PreferredPmProfile
+  EFI_ACPI_6_3_PM_PROFILE_APPLIANCE_PC,                                     // UINT8      PreferredPmProfile
   0,                                                                        // UINT16     SciInt
   0,                                                                        // UINT32     SmiCmd
   0,                                                                        // UINT8      AcpiEnable
@@ -72,24 +69,24 @@ EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE Fadt = {
   0,                                                                        // UINT8      Century
   EFI_ACPI_RESERVED_WORD,                                                   // UINT16     IaPcBootArch (Reserved on ARM)
   EFI_ACPI_RESERVED_BYTE,                                                   // UINT8      Reserved1
-  EFI_ACPI_5_1_WBINVD | EFI_ACPI_5_1_SLP_BUTTON |                           // UINT32     Flags
-  EFI_ACPI_5_1_HW_REDUCED_ACPI,
-  NULL_GAS,                                                                 // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  ResetReg
+  EFI_ACPI_6_3_WBINVD | EFI_ACPI_6_3_SLP_BUTTON |                           // UINT32     Flags
+  EFI_ACPI_6_3_HW_REDUCED_ACPI,
+  NULL_GAS,                                                                 // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  ResetReg
   0,                                                                        // UINT8      ResetValue
-  EFI_ACPI_5_1_ARM_PSCI_COMPLIANT,                                          // UINT16     ArmBootArchFlags
-  EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION,                 // UINT8      MinorRevision
+  EFI_ACPI_6_3_ARM_PSCI_COMPLIANT,                                          // UINT16     ArmBootArchFlags
+  EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION,                 // UINT8      MinorRevision
   0,                                                                        // UINT64     XFirmwareCtrl
   0,                                                                        // UINT64     XDsdt
-  NULL_GAS,                                                                 // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  XPm1aEvtBlk
-  NULL_GAS,                                                                 // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  XPm1bEvtBlk
-  NULL_GAS,                                                                 // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  XPm1aCntBlk
-  NULL_GAS,                                                                 // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  XPm1bCntBlk
-  NULL_GAS,                                                                 // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  XPm2CntBlk
-  NULL_GAS,                                                                 // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  XPmTmrBlk
-  NULL_GAS,                                                                 // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  XGpe0Blk
-  NULL_GAS,                                                                 // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  XGpe1Blk
-  NULL_GAS,                                                                 // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  SleepControlReg
-  NULL_GAS                                                                  // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  SleepStatusReg
+  NULL_GAS,                                                                 // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  XPm1aEvtBlk
+  NULL_GAS,                                                                 // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  XPm1bEvtBlk
+  NULL_GAS,                                                                 // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  XPm1aCntBlk
+  NULL_GAS,                                                                 // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  XPm1bCntBlk
+  NULL_GAS,                                                                 // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  XPm2CntBlk
+  NULL_GAS,                                                                 // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  XPmTmrBlk
+  NULL_GAS,                                                                 // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  XGpe0Blk
+  NULL_GAS,                                                                 // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  XGpe1Blk
+  NULL_GAS,                                                                 // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  SleepControlReg
+  NULL_GAS                                                                  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  SleepStatusReg
 };
 
 //
diff --git a/Platform/RaspberryPi/AcpiTables/Gtdt.aslc b/Platform/RaspberryPi/AcpiTables/Gtdt.aslc
index 8453e487fb63..a29ad69b9ae0 100644
--- a/Platform/RaspberryPi/AcpiTables/Gtdt.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Gtdt.aslc
@@ -15,22 +15,22 @@
 #include "AcpiTables.h"
 
 #define RPI_GTDT_GLOBAL_FLAGS           0
-#define RPI_GTDT_GTIMER_FLAGS           EFI_ACPI_5_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_POLARITY
+#define RPI_GTDT_GTIMER_FLAGS           EFI_ACPI_6_3_GTDT_TIMER_FLAG_TIMER_INTERRUPT_POLARITY
 
 #pragma pack (1)
 
 typedef struct {
-  EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE Gtdt;
-} EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES;
+  EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE Gtdt;
+} EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLES;
 
 #pragma pack ()
 
-EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
+EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
   {
     ACPI_HEADER(
-      EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
-      EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES,
-      EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION
+      EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
+      EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLES,
+      EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION
     ),
     RPI_SYSTEM_TIMER_BASE_ADDRESS,                // UINT64  PhysicalAddress
     0,                                            // UINT32  Reserved
diff --git a/Platform/RaspberryPi/AcpiTables/Madt.aslc b/Platform/RaspberryPi/AcpiTables/Madt.aslc
index faf461814536..cd84fec39fd5 100644
--- a/Platform/RaspberryPi/AcpiTables/Madt.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Madt.aslc
@@ -21,10 +21,10 @@
 #pragma pack (1)
 
 typedef struct {
-  EFI_ACPI_5_1_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER   Header;
-  EFI_ACPI_5_1_GIC_STRUCTURE                            GicInterfaces[4];
+  EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER   Header;
+  EFI_ACPI_6_3_GIC_STRUCTURE                            GicInterfaces[4];
 #if (RPI_MODEL != 3)
-  EFI_ACPI_5_1_GIC_DISTRIBUTOR_STRUCTURE                GicDistributor;
+  EFI_ACPI_6_3_GIC_DISTRIBUTOR_STRUCTURE                GicDistributor;
 #endif
 } PI_MULTIPLE_APIC_DESCRIPTION_TABLE;
 
@@ -33,9 +33,9 @@ typedef struct {
 PI_MULTIPLE_APIC_DESCRIPTION_TABLE Madt = {
   {
     ACPI_HEADER (
-      EFI_ACPI_5_1_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
+      EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
       PI_MULTIPLE_APIC_DESCRIPTION_TABLE,
-      EFI_ACPI_5_1_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION
+      EFI_ACPI_6_3_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION
     ),
     //
     // MADT specific fields
@@ -44,25 +44,30 @@ PI_MULTIPLE_APIC_DESCRIPTION_TABLE Madt = {
     0, // Flags
   },
   {
-    EFI_ACPI_5_1_GICC_STRUCTURE_INIT(
-      0, 0, GET_MPID(0, 0), EFI_ACPI_5_1_GIC_ENABLED, FixedPcdGet32 (PcdGicPmuIrq0),
+    EFI_ACPI_6_3_GICC_STRUCTURE_INIT (
+      0, 0, GET_MPID(0, 0), EFI_ACPI_6_0_GIC_ENABLED, FixedPcdGet32 (PcdGicPmuIrq0),
       FixedPcdGet64 (PcdGicInterruptInterfaceBase), FixedPcdGet64 (PcdGicInterruptInterfaceVBase),
-      FixedPcdGet64 (PcdGicInterruptInterfaceHBase), FixedPcdGet32 (PcdGicGsivId), 0),
-    EFI_ACPI_5_1_GICC_STRUCTURE_INIT(
-      1, 1, GET_MPID(0, 1), EFI_ACPI_5_1_GIC_ENABLED, FixedPcdGet32 (PcdGicPmuIrq1),
+      //
+      // Use 1 for GICR Base Address below, since Windows 10 on Raspberry Pi 3 does not
+      // boot otherwise, and this is the value that Microsoft had in their IoT blobs.
+      // Kept to 1 for GICv2-based Pi 4, since this field only matters for GICv3.
+      //
+      FixedPcdGet64 (PcdGicInterruptInterfaceHBase), FixedPcdGet32 (PcdGicGsivId), 0, 1, 0),
+    EFI_ACPI_6_3_GICC_STRUCTURE_INIT (
+      1, 1, GET_MPID(0, 1), EFI_ACPI_6_0_GIC_ENABLED, FixedPcdGet32 (PcdGicPmuIrq1),
       FixedPcdGet64 (PcdGicInterruptInterfaceBase), FixedPcdGet64 (PcdGicInterruptInterfaceVBase),
-      FixedPcdGet64 (PcdGicInterruptInterfaceHBase), FixedPcdGet32 (PcdGicGsivId), 0),
-    EFI_ACPI_5_1_GICC_STRUCTURE_INIT(
-      2, 2, GET_MPID(0, 2), EFI_ACPI_5_1_GIC_ENABLED, FixedPcdGet32 (PcdGicPmuIrq2),
+      FixedPcdGet64 (PcdGicInterruptInterfaceHBase), FixedPcdGet32 (PcdGicGsivId), 0, 1, 0),
+    EFI_ACPI_6_3_GICC_STRUCTURE_INIT (
+      2, 2, GET_MPID(0, 2), EFI_ACPI_6_0_GIC_ENABLED, FixedPcdGet32 (PcdGicPmuIrq2),
       FixedPcdGet64 (PcdGicInterruptInterfaceBase), FixedPcdGet64 (PcdGicInterruptInterfaceVBase),
-      FixedPcdGet64 (PcdGicInterruptInterfaceHBase), FixedPcdGet32 (PcdGicGsivId), 0),
-    EFI_ACPI_5_1_GICC_STRUCTURE_INIT(
-      3, 3, GET_MPID(0, 3), EFI_ACPI_5_1_GIC_ENABLED, FixedPcdGet32 (PcdGicPmuIrq3),
+      FixedPcdGet64 (PcdGicInterruptInterfaceHBase), FixedPcdGet32 (PcdGicGsivId), 0, 1, 0),
+    EFI_ACPI_6_3_GICC_STRUCTURE_INIT (
+      3, 3, GET_MPID(0, 3), EFI_ACPI_6_0_GIC_ENABLED, FixedPcdGet32 (PcdGicPmuIrq3),
       FixedPcdGet64 (PcdGicInterruptInterfaceBase), FixedPcdGet64 (PcdGicInterruptInterfaceVBase),
-      FixedPcdGet64 (PcdGicInterruptInterfaceHBase), FixedPcdGet32 (PcdGicGsivId), 0),
+      FixedPcdGet64 (PcdGicInterruptInterfaceHBase), FixedPcdGet32 (PcdGicGsivId), 0, 1, 0),
   },
 #if (RPI_MODEL != 3)
-  EFI_ACPI_5_0_GIC_DISTRIBUTOR_INIT(0, FixedPcdGet64 (PcdGicDistributorBase), 0)
+  EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT (0, FixedPcdGet64 (PcdGicDistributorBase), 0, 2)
 #endif
 };
 
diff --git a/Platform/RaspberryPi/AcpiTables/Spcr.aslc b/Platform/RaspberryPi/AcpiTables/Spcr.aslc
index bec4ad660ec9..07df3a718d52 100644
--- a/Platform/RaspberryPi/AcpiTables/Spcr.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Spcr.aslc
@@ -30,7 +30,7 @@
 #endif
 STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
   ACPI_HEADER (
-    EFI_ACPI_5_1_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
+    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
     EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
     EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
   ),
@@ -42,7 +42,7 @@ STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
     EFI_ACPI_RESERVED_BYTE,
     EFI_ACPI_RESERVED_BYTE
   },
-  // EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
+  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
   ARM_GAS32 (RPI_UART_BASE_ADDRESS),
   // UINT8                                   InterruptType;
   EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
-- 
2.21.0.windows.1


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

* Re: [edk2-platforms][PATCH 1/1] Platform/RPi/AcpiTables: Update all tables to latest ACPI
  2020-05-06 11:37 [edk2-platforms][PATCH 1/1] Platform/RPi/AcpiTables: Update all tables to latest ACPI Pete Batard
@ 2020-05-06 12:26 ` Ard Biesheuvel
  2020-05-06 12:45   ` Pete Batard
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2020-05-06 12:26 UTC (permalink / raw)
  To: Pete Batard, devel; +Cc: leif, awarkentin

On 5/6/20 1:37 PM, Pete Batard wrote:
...
> Therefore, since we do see a need for ACPI 6.x features, and
> effectively have a 6.3 table with PPTT, and also since we have
> tested that we are not seeing ill effects from doing so for the
> most common OSes we support, we bring all of the relevant ACPI
> tables to version 6.3.
> 
> This is mostly accomplished by simply altering the version of
> ACPI being references in the macros,

OK, so even the macros that resolve to the exact same code are renamed. 
I suppose this means a blanket rename of everything once ACPI 6.4 comes out?

> except for the new fields
> being initialized in the MADT table where we:
> * Use 1 for GICR Base Address in GICC_STRUCTURE_INIT because, even
>    as this field is not supposed to apply to any of our platforms,
>    this is what Microsoft used in the original Pi 3 hardcoded MADT
>    blobs and Windows 10 doesn't boot on the Pi 3 if set to 0.

Fair enough.

> * Use 2 for GIC version in GIC_DISTRIBUTOR_INIT, since the Pi 4 GIC
>    is v2 only.
> 

OK. But you are introducing new references to ACPI_6_0 macros here. If 
the point is to rename everything for cosmetic reasons, shouldn't you 
use 6.3 there as well?

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

* Re: [edk2-platforms][PATCH 1/1] Platform/RPi/AcpiTables: Update all tables to latest ACPI
  2020-05-06 12:26 ` Ard Biesheuvel
@ 2020-05-06 12:45   ` Pete Batard
  2020-05-06 12:50     ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: Pete Batard @ 2020-05-06 12:45 UTC (permalink / raw)
  To: Ard Biesheuvel, devel; +Cc: leif, awarkentin

On 2020.05.06 13:26, Ard Biesheuvel wrote:
> On 5/6/20 1:37 PM, Pete Batard wrote:
> ...
>> Therefore, since we do see a need for ACPI 6.x features, and
>> effectively have a 6.3 table with PPTT, and also since we have
>> tested that we are not seeing ill effects from doing so for the
>> most common OSes we support, we bring all of the relevant ACPI
>> tables to version 6.3.
>>
>> This is mostly accomplished by simply altering the version of
>> ACPI being references in the macros,
> 
> OK, so even the macros that resolve to the exact same code are renamed. 
> I suppose this means a blanket rename of everything once ACPI 6.4 comes 
> out?

No.

I am considering that we are still in the process of introducing the 
platform, therefore we introduce it with the most up to date version of 
ACPI at that time.

Though I could see some point in doing so, I have no plan to update out 
ACPI for the Pi's every time a new version is released, unless there is 
an actual need to do so for a specific table or there is a blanket 
request (from seeing lots of folks using our code as a starting point) 
that warrants it.

In other words, even as RPi3 was introduced a year ago, I consider that 
we are still in the process of adding RPi3/RPi4 as a brand new platform, 
and therefore want to use the most up to date ACPI at the time of 
introduction. But I'm definitely not making a point of trying to follow 
ACPI updates past that this.

> 
>> except for the new fields
>> being initialized in the MADT table where we:
>> * Use 1 for GICR Base Address in GICC_STRUCTURE_INIT because, even
>>    as this field is not supposed to apply to any of our platforms,
>>    this is what Microsoft used in the original Pi 3 hardcoded MADT
>>    blobs and Windows 10 doesn't boot on the Pi 3 if set to 0.
> 
> Fair enough.
> 
>> * Use 2 for GIC version in GIC_DISTRIBUTOR_INIT, since the Pi 4 GIC
>>    is v2 only.
>>
> 
> OK. But you are introducing new references to ACPI_6_0 macros here. If 
> the point is to rename everything for cosmetic reasons, shouldn't you 
> use 6.3 there as well?

Well, EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT was not updated for 6_1, 6_2, 
6_3 (most likely because the macro, which is defined in 
EmbeddedPkg/Include/Library/AcpiLib.h applies the same for all 6.x 
versions), so I just used the highest version I could find, which is 
6_0. This is based on what I saw other platforms do.

I don't mind sending an EDK2 patch to add the various 
EFI_ACPI_6_x_GIC_DISTRIBUTOR_INIT if that's what you request, but that 
seems a bit like overkill and may break existing platforms, because it 
will most likely require moving EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT to a 
different header (MdePkg/Include/IndustryStandard/Acpi60.h).

Regards,

/Pete

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

* Re: [edk2-platforms][PATCH 1/1] Platform/RPi/AcpiTables: Update all tables to latest ACPI
  2020-05-06 12:45   ` Pete Batard
@ 2020-05-06 12:50     ` Ard Biesheuvel
  2020-05-06 12:56       ` Pete Batard
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2020-05-06 12:50 UTC (permalink / raw)
  To: Pete Batard, devel; +Cc: leif, awarkentin

On 5/6/20 2:45 PM, Pete Batard wrote:
> On 2020.05.06 13:26, Ard Biesheuvel wrote:
>> On 5/6/20 1:37 PM, Pete Batard wrote:
>> ...
>>> Therefore, since we do see a need for ACPI 6.x features, and
>>> effectively have a 6.3 table with PPTT, and also since we have
>>> tested that we are not seeing ill effects from doing so for the
>>> most common OSes we support, we bring all of the relevant ACPI
>>> tables to version 6.3.
>>>
>>> This is mostly accomplished by simply altering the version of
>>> ACPI being references in the macros,
>>
>> OK, so even the macros that resolve to the exact same code are 
>> renamed. I suppose this means a blanket rename of everything once ACPI 
>> 6.4 comes out?
> 
> No.
> 
> I am considering that we are still in the process of introducing the 
> platform, therefore we introduce it with the most up to date version of 
> ACPI at that time.
> 
> Though I could see some point in doing so, I have no plan to update out 
> ACPI for the Pi's every time a new version is released, unless there is 
> an actual need to do so for a specific table or there is a blanket 
> request (from seeing lots of folks using our code as a starting point) 
> that warrants it.
> 
> In other words, even as RPi3 was introduced a year ago, I consider that 
> we are still in the process of adding RPi3/RPi4 as a brand new platform, 
> and therefore want to use the most up to date ACPI at the time of 
> introduction. But I'm definitely not making a point of trying to follow 
> ACPI updates past that this.
> 

Good.

>>> except for the new fields
>>> being initialized in the MADT table where we:
>>> * Use 1 for GICR Base Address in GICC_STRUCTURE_INIT because, even
>>>    as this field is not supposed to apply to any of our platforms,
>>>    this is what Microsoft used in the original Pi 3 hardcoded MADT
>>>    blobs and Windows 10 doesn't boot on the Pi 3 if set to 0.
>>
>> Fair enough.
>>
>>> * Use 2 for GIC version in GIC_DISTRIBUTOR_INIT, since the Pi 4 GIC
>>>    is v2 only.
>>>
>>
>> OK. But you are introducing new references to ACPI_6_0 macros here. If 
>> the point is to rename everything for cosmetic reasons, shouldn't you 
>> use 6.3 there as well?
> 
> Well, EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT was not updated for 6_1, 6_2, 
> 6_3 (most likely because the macro, which is defined in 
> EmbeddedPkg/Include/Library/AcpiLib.h applies the same for all 6.x 
> versions), so I just used the highest version I could find, which is 
> 6_0. This is based on what I saw other platforms do.
> 
> I don't mind sending an EDK2 patch to add the various 
> EFI_ACPI_6_x_GIC_DISTRIBUTOR_INIT if that's what you request, but that 
> seems a bit like overkill and may break existing platforms, because it 
> will most likely require moving EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT to a 
> different header (MdePkg/Include/IndustryStandard/Acpi60.h).
> 

Fair enough.

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Pushed as 66d1b02429fa..11189124fbc2




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

* Re: [edk2-platforms][PATCH 1/1] Platform/RPi/AcpiTables: Update all tables to latest ACPI
  2020-05-06 12:50     ` Ard Biesheuvel
@ 2020-05-06 12:56       ` Pete Batard
  0 siblings, 0 replies; 5+ messages in thread
From: Pete Batard @ 2020-05-06 12:56 UTC (permalink / raw)
  To: Ard Biesheuvel, devel; +Cc: leif, awarkentin

On 2020.05.06 13:50, Ard Biesheuvel wrote:
> On 5/6/20 2:45 PM, Pete Batard wrote:
>> On 2020.05.06 13:26, Ard Biesheuvel wrote:
>>> On 5/6/20 1:37 PM, Pete Batard wrote:
>>> ...
>>>> Therefore, since we do see a need for ACPI 6.x features, and
>>>> effectively have a 6.3 table with PPTT, and also since we have
>>>> tested that we are not seeing ill effects from doing so for the
>>>> most common OSes we support, we bring all of the relevant ACPI
>>>> tables to version 6.3.
>>>>
>>>> This is mostly accomplished by simply altering the version of
>>>> ACPI being references in the macros,
>>>
>>> OK, so even the macros that resolve to the exact same code are 
>>> renamed. I suppose this means a blanket rename of everything once 
>>> ACPI 6.4 comes out?
>>
>> No.
>>
>> I am considering that we are still in the process of introducing the 
>> platform, therefore we introduce it with the most up to date version 
>> of ACPI at that time.
>>
>> Though I could see some point in doing so, I have no plan to update 
>> out ACPI for the Pi's every time a new version is released, unless 
>> there is an actual need to do so for a specific table or there is a 
>> blanket request (from seeing lots of folks using our code as a 
>> starting point) that warrants it.
>>
>> In other words, even as RPi3 was introduced a year ago, I consider 
>> that we are still in the process of adding RPi3/RPi4 as a brand new 
>> platform, and therefore want to use the most up to date ACPI at the 
>> time of introduction. But I'm definitely not making a point of trying 
>> to follow ACPI updates past that this.
>>
> 
> Good.
> 
>>>> except for the new fields
>>>> being initialized in the MADT table where we:
>>>> * Use 1 for GICR Base Address in GICC_STRUCTURE_INIT because, even
>>>>    as this field is not supposed to apply to any of our platforms,
>>>>    this is what Microsoft used in the original Pi 3 hardcoded MADT
>>>>    blobs and Windows 10 doesn't boot on the Pi 3 if set to 0.
>>>
>>> Fair enough.
>>>
>>>> * Use 2 for GIC version in GIC_DISTRIBUTOR_INIT, since the Pi 4 GIC
>>>>    is v2 only.
>>>>
>>>
>>> OK. But you are introducing new references to ACPI_6_0 macros here. 
>>> If the point is to rename everything for cosmetic reasons, shouldn't 
>>> you use 6.3 there as well?
>>
>> Well, EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT was not updated for 6_1, 6_2, 
>> 6_3 (most likely because the macro, which is defined in 
>> EmbeddedPkg/Include/Library/AcpiLib.h applies the same for all 6.x 
>> versions), so I just used the highest version I could find, which is 
>> 6_0. This is based on what I saw other platforms do.
>>
>> I don't mind sending an EDK2 patch to add the various 
>> EFI_ACPI_6_x_GIC_DISTRIBUTOR_INIT if that's what you request, but that 
>> seems a bit like overkill and may break existing platforms, because it 
>> will most likely require moving EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT to a 
>> different header (MdePkg/Include/IndustryStandard/Acpi60.h).
>>
> 
> Fair enough.
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> 
> Pushed as 66d1b02429fa..11189124fbc2

Thanks!

/Pete

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

end of thread, other threads:[~2020-05-06 12:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-06 11:37 [edk2-platforms][PATCH 1/1] Platform/RPi/AcpiTables: Update all tables to latest ACPI Pete Batard
2020-05-06 12:26 ` Ard Biesheuvel
2020-05-06 12:45   ` Pete Batard
2020-05-06 12:50     ` Ard Biesheuvel
2020-05-06 12:56       ` Pete Batard

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