Re: [edk2-devel] [edk2-platforms][PATCH 2/6] Platform/RPi4: Improve FADT ACPI table generation

2019-12-18 Thread Pete Batard

Hi Ard,

On 2019.12.18 14:46, Ard Biesheuvel wrote:

Hi Pete,

On Wed, 18 Dec 2019 at 13:42, Pete Batard  wrote:


Use a proper aslc source to build the table.

Note that we use ACPI 5.1 for this table to match the MADT
constraints.

Signed-off-by: Pete Batard 
---
  Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc | 104 ++--
  1 file changed, 76 insertions(+), 28 deletions(-)

diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc 
b/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc
index 3ef877fde5f4..2d851794b989 100644
--- a/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc
+++ b/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc
@@ -2,6 +2,7 @@
   *
   *  Fixed ACPI Description Table (FADT)
   *
+ *  Copyright (c) 2019, Pete Batard 
   *  Copyright (c) 2018, Andrey Warkentin 
   *  Copyright (c) Microsoft Corporation. All rights reserved.
   *
@@ -9,34 +10,81 @@
   *
   **/

-UINT8 Fadt[268] = {
-  0x46, 0x41, 0x43, 0x50, 0x0C, 0x01, 0x00, 0x00, 0x05, 0x00, /*   0 */
-  0x42, 0x43, 0x32, 0x38, 0x33, 0x36, 0x45, 0x44, 0x4B, 0x32, /*  10 */
-  0x20, 0x20, 0x20, 0x20, 0x01, 0x00, 0x00, 0x00, 0x4D, 0x53, /*  20 */
-  0x46, 0x54, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  30 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, /*  40 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  50 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  60 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  70 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  80 */
-  0x00, 0x04, 0x00, 0x00, 0x00, 0xE3, 0x00, 0x00, 0x00, 0x00, /*  90 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3C, /* 100 */
-  0x00, 0x00, 0x21, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, /* 110 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, /* 120 */
-  0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 130 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 140 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 150 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 160 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 170 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 180 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 190 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 200 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 210 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 220 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 230 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 240 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 250 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00  /* 268 */
+#include 
+#include 
+#include 
+
+#include "AcpiTables.h"
+
+#define RPI_DSDT_BASE_ADDRESS   0x33CD


DSDTs have no compile time base addresses - they end up wherever the
AcpiPlatformDxe driver puts them.


Makes sense. I was a bit too blind in trying to match the output of 
acpiview.



+#define RPI_PM_TIMER_BLOCK_LEN  0x04
+#define RPI_CST_VALUE   0xE3


Any idea what these mean?


Not really. I just picked those values from the acpiview output in the 
Shell with the assumption that these and the IaPcBootArch flags (such as 
lack of CMOS RTC which I'm hoping Microsoft can pick through different 
means on ARM) were static values that needed to be preserved especially 
as the blobs we are replacing here came from Microsoft.


I'll validate that it works with zero/default as you suggest below and 
submit a v2.


Regards,

/Pete




+
+/*
+ * Note: Use ACPI 5.1 since we need to match MADT ACPI requirements
+ */
+EFI_ACPI_5_1_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
+  ),
+  0,// 
UINT32 FirmwareCtrl
+  RPI_DSDT_BASE_ADDRESS,// 
UINT32 Dsdt


Just put 0 here


+  EFI_ACPI_RESERVED_BYTE,   // 
UINT8  Reserved0
+  EFI_ACPI_5_1_PM_PROFILE_APPLIANCE_PC, // 
UINT8  PreferredPmProfile
+  0,// 
UINT16 SciInt
+  0,// 
UINT32 SmiCmd
+  0,// 
UINT8  AcpiEnable
+  0,// 
UINT8  AcpiDisable
+  0,// 
UINT8

Re: [edk2-devel] [edk2-platforms][PATCH 2/6] Platform/RPi4: Improve FADT ACPI table generation

2019-12-18 Thread Ard Biesheuvel
Hi Pete,

On Wed, 18 Dec 2019 at 13:42, Pete Batard  wrote:
>
> Use a proper aslc source to build the table.
>
> Note that we use ACPI 5.1 for this table to match the MADT
> constraints.
>
> Signed-off-by: Pete Batard 
> ---
>  Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc | 104 ++--
>  1 file changed, 76 insertions(+), 28 deletions(-)
>
> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc 
> b/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc
> index 3ef877fde5f4..2d851794b989 100644
> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc
> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc
> @@ -2,6 +2,7 @@
>   *
>   *  Fixed ACPI Description Table (FADT)
>   *
> + *  Copyright (c) 2019, Pete Batard 
>   *  Copyright (c) 2018, Andrey Warkentin 
>   *  Copyright (c) Microsoft Corporation. All rights reserved.
>   *
> @@ -9,34 +10,81 @@
>   *
>   **/
>
> -UINT8 Fadt[268] = {
> -  0x46, 0x41, 0x43, 0x50, 0x0C, 0x01, 0x00, 0x00, 0x05, 0x00, /*   0 */
> -  0x42, 0x43, 0x32, 0x38, 0x33, 0x36, 0x45, 0x44, 0x4B, 0x32, /*  10 */
> -  0x20, 0x20, 0x20, 0x20, 0x01, 0x00, 0x00, 0x00, 0x4D, 0x53, /*  20 */
> -  0x46, 0x54, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  30 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, /*  40 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  50 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  60 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  70 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  80 */
> -  0x00, 0x04, 0x00, 0x00, 0x00, 0xE3, 0x00, 0x00, 0x00, 0x00, /*  90 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3C, /* 100 */
> -  0x00, 0x00, 0x21, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, /* 110 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, /* 120 */
> -  0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 130 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 140 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 150 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 160 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 170 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 180 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 190 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 200 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 210 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 220 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 230 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 240 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 250 */
> -  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00  /* 268 */
> +#include 
> +#include 
> +#include 
> +
> +#include "AcpiTables.h"
> +
> +#define RPI_DSDT_BASE_ADDRESS   0x33CD

DSDTs have no compile time base addresses - they end up wherever the
AcpiPlatformDxe driver puts them.

> +#define RPI_PM_TIMER_BLOCK_LEN  0x04
> +#define RPI_CST_VALUE   0xE3

Any idea what these mean?

> +
> +/*
> + * Note: Use ACPI 5.1 since we need to match MADT ACPI requirements
> + */
> +EFI_ACPI_5_1_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
> +  ),
> +  0,
> // UINT32 FirmwareCtrl
> +  RPI_DSDT_BASE_ADDRESS,
> // UINT32 Dsdt

Just put 0 here

> +  EFI_ACPI_RESERVED_BYTE,   
> // UINT8  Reserved0
> +  EFI_ACPI_5_1_PM_PROFILE_APPLIANCE_PC, 
> // UINT8  PreferredPmProfile
> +  0,
> // UINT16 SciInt
> +  0,
> // UINT32 SmiCmd
> +  0,
> // UINT8  AcpiEnable
> +  0,
> // UINT8  AcpiDisable
> +  0,
> // UINT8  S4BiosReq
> +  0,
> // UINT8  PstateCnt
> +  0,
> // UINT32 Pm1aEvtBlk
> +  0,
> // UINT32 Pm1bEvtBlk
> +  0,
> // UINT32 

[edk2-devel] [edk2-platforms][PATCH 2/6] Platform/RPi4: Improve FADT ACPI table generation

2019-12-18 Thread Pete Batard
Use a proper aslc source to build the table.

Note that we use ACPI 5.1 for this table to match the MADT
constraints.

Signed-off-by: Pete Batard 
---
 Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc | 104 ++--
 1 file changed, 76 insertions(+), 28 deletions(-)

diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc 
b/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc
index 3ef877fde5f4..2d851794b989 100644
--- a/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc
+++ b/Platform/RaspberryPi/RPi4/AcpiTables/Fadt.aslc
@@ -2,6 +2,7 @@
  *
  *  Fixed ACPI Description Table (FADT)
  *
+ *  Copyright (c) 2019, Pete Batard 
  *  Copyright (c) 2018, Andrey Warkentin 
  *  Copyright (c) Microsoft Corporation. All rights reserved.
  *
@@ -9,34 +10,81 @@
  *
  **/
 
-UINT8 Fadt[268] = {
-  0x46, 0x41, 0x43, 0x50, 0x0C, 0x01, 0x00, 0x00, 0x05, 0x00, /*   0 */
-  0x42, 0x43, 0x32, 0x38, 0x33, 0x36, 0x45, 0x44, 0x4B, 0x32, /*  10 */
-  0x20, 0x20, 0x20, 0x20, 0x01, 0x00, 0x00, 0x00, 0x4D, 0x53, /*  20 */
-  0x46, 0x54, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  30 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, /*  40 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  50 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  60 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  70 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*  80 */
-  0x00, 0x04, 0x00, 0x00, 0x00, 0xE3, 0x00, 0x00, 0x00, 0x00, /*  90 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3C, /* 100 */
-  0x00, 0x00, 0x21, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, /* 110 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, /* 120 */
-  0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 130 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 140 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 150 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 160 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 170 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 180 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 190 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 200 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 210 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 220 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 230 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 240 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 250 */
-  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00  /* 268 */
+#include 
+#include 
+#include 
+
+#include "AcpiTables.h"
+
+#define RPI_DSDT_BASE_ADDRESS   0x33CD
+#define RPI_PM_TIMER_BLOCK_LEN  0x04
+#define RPI_CST_VALUE   0xE3
+
+/*
+ * Note: Use ACPI 5.1 since we need to match MADT ACPI requirements
+ */
+EFI_ACPI_5_1_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
+  ),
+  0,// 
UINT32 FirmwareCtrl
+  RPI_DSDT_BASE_ADDRESS,// 
UINT32 Dsdt
+  EFI_ACPI_RESERVED_BYTE,   // 
UINT8  Reserved0
+  EFI_ACPI_5_1_PM_PROFILE_APPLIANCE_PC, // 
UINT8  PreferredPmProfile
+  0,// 
UINT16 SciInt
+  0,// 
UINT32 SmiCmd
+  0,// 
UINT8  AcpiEnable
+  0,// 
UINT8  AcpiDisable
+  0,// 
UINT8  S4BiosReq
+  0,// 
UINT8  PstateCnt
+  0,// 
UINT32 Pm1aEvtBlk
+  0,// 
UINT32 Pm1bEvtBlk
+  0,// 
UINT32 Pm1aCntBlk
+  0,// 
UINT32 Pm1bCntBlk
+  0,// 
UINT32 Pm2CntBlk
+  0,// 
UINT32 PmTmrBlk
+  0,// 
UINT32