public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Pete Batard" <pete@akeo.ie>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>,
	"Leif Lindholm" <leif.lindholm@linaro.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [edk2-platforms][PATCH 2/6] Platform/RPi4: Improve FADT ACPI table generation
Date: Wed, 18 Dec 2019 16:31:02 +0000	[thread overview]
Message-ID: <d75f22f2-071d-68ee-a37f-2fa77f221688@akeo.ie> (raw)
In-Reply-To: <CAKv+Gu-RAZo+SQXMsO1Cbj2gRofh7nzP5zyWdLj0kzdC773B-Q@mail.gmail.com>

Hi Ard,

On 2019.12.18 14:46, Ard Biesheuvel wrote:
> Hi Pete,
> 
> On Wed, 18 Dec 2019 at 13:42, Pete Batard <pete@akeo.ie> 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 <pete@akeo.ie>
>> ---
>>   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 <pete@akeo.ie>
>>    *  Copyright (c) 2018, Andrey Warkentin <andrey.warkentin@gmail.com>
>>    *  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 <IndustryStandard/Acpi.h>
>> +#include <Library/AcpiLib.h>
>> +#include <Library/PcdLib.h>
>> +
>> +#include "AcpiTables.h"
>> +
>> +#define RPI_DSDT_BASE_ADDRESS   0x33CD0000
> 
> 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      S4BiosReq
>> +  0,                                                                        // UINT8      PstateCnt
>> +  0,                                                                        // UINT32     Pm1aEvtBlk
>> +  0,                                                                        // UINT32     Pm1bEvtBlk
>> +  0,                                                                        // UINT32     Pm1aCntBlk
>> +  0,                                                                        // UINT32     Pm1bCntBlk
>> +  0,                                                                        // UINT32     Pm2CntBlk
>> +  0,                                                                        // UINT32     PmTmrBlk
>> +  0,                                                                        // UINT32     Gpe0Blk
>> +  0,                                                                        // UINT32     Gpe1Blk
>> +  0,                                                                        // UINT8      Pm1EvtLen
>> +  0,                                                                        // UINT8      Pm1CntLen
>> +  0,                                                                        // UINT8      Pm2CntLen
>> +  RPI_PM_TIMER_BLOCK_LEN,                                                   // UINT8      PmTmrLen
> 
> and here
> 
>> +  0,                                                                        // UINT8      Gpe0BlkLen
>> +  0,                                                                        // UINT8      Gpe1BlkLen
>> +  0,                                                                        // UINT8      Gpe1Base
>> +  RPI_CST_VALUE,                                                            // UINT8      CstCnt
> 
> and here
> 
>> +  0,                                                                        // UINT16     PLvl2Lat
>> +  0,                                                                        // UINT16     PLvl3Lat
>> +  0,                                                                        // UINT16     FlushSize
>> +  0,                                                                        // UINT16     FlushStride
>> +  0,                                                                        // UINT8      DutyOffset
>> +  0,                                                                        // UINT8      DutyWidth
>> +  0,                                                                        // UINT8      DayAlrm
>> +  0,                                                                        // UINT8      MonAlrm
>> +  0,                                                                        // UINT8      Century
>> +  EFI_ACPI_5_1_VGA_NOT_PRESENT | EFI_ACPI_5_1_MSI_NOT_SUPPORTED |           // UINT16     IaPcBootArch
>> +  EFI_ACPI_5_1_PCIE_ASPM_CONTROLS | EFI_ACPI_5_1_CMOS_RTC_NOT_PRESENT,
> 
> IaPcBootArch is RES0 on ARM, so put 0 here as well please
> 
>> +  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
>> +  0,                                                                        // UINT8      ResetValue
>> +  EFI_ACPI_5_1_ARM_PSCI_COMPLIANT,                                          // UINT16     ArmBootArchFlags
>> +  EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION,                 // UINT8      MinorRevision
>> +  0,                                                                        // UINT64     XFirmwareCtrl
>> +  RPI_DSDT_BASE_ADDRESS,                                                    // UINT64     XDsdt
> 
> Put 0 here
> 
>> +  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
>>   };
>>
>>   //
>> --
>> 2.21.0.windows.1
>>


  reply	other threads:[~2019-12-18 16:31 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-18 11:41 [edk2-platforms][PATCH 0/6] Platform/RPi4: ACPI improvements Pete Batard
2019-12-18 11:41 ` [edk2-platforms][PATCH 1/6] Platform/RPi4: Clean up ACPI definitions Pete Batard
2019-12-18 11:41 ` [edk2-platforms][PATCH 2/6] Platform/RPi4: Improve FADT ACPI table generation Pete Batard
2019-12-18 14:46   ` Ard Biesheuvel
2019-12-18 16:31     ` Pete Batard [this message]
2019-12-18 11:41 ` [edk2-platforms][PATCH 3/6] Platform/RPi4: Improve SPCR and DBG2 " Pete Batard
2019-12-18 15:57   ` Philippe Mathieu-Daudé
2019-12-18 16:36     ` Pete Batard
2019-12-18 17:00       ` Philippe Mathieu-Daudé
2019-12-18 17:10         ` Pete Batard
2019-12-18 11:41 ` [edk2-platforms][PATCH 4/6] Platform/RPi4: Add switch to select between PL011 and miniUART Pete Batard
2019-12-18 16:05   ` Philippe Mathieu-Daudé
2019-12-18 16:59     ` [edk2-devel] " Pete Batard
2019-12-18 17:05       ` Philippe Mathieu-Daudé
2019-12-18 11:41 ` [edk2-platforms][PATCH 5/6] Platform/RPi4: Add XHCI and MCFG ACPI tables Pete Batard
2019-12-18 14:55   ` Ard Biesheuvel
2019-12-18 16:31     ` Pete Batard
2019-12-18 11:41 ` [edk2-platforms][PATCH 6/6] Platform/RPi4: Add ACPI basic mode build option Pete Batard
2019-12-18 12:10   ` Leif Lindholm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d75f22f2-071d-68ee-a37f-2fa77f221688@akeo.ie \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox