public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "PierreGondois" <pierre.gondois@arm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>,
	edk2-devel-groups-io <devel@edk2.groups.io>,
	"Feng, Bob C" <bob.c.feng@intel.com>,
	"Gao, Liming" <liming.gao@intel.com>, nd <nd@arm.com>,
	Matteo Carlini <Matteo.Carlini@arm.com>,
	Laura Moretta <Laura.Moretta@arm.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files
Date: Tue, 24 Dec 2019 16:47:45 +0000	[thread overview]
Message-ID: <DB7PR08MB3113DA7D36A368BA0FC9712D8B290@DB7PR08MB3113.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <CAKv+Gu8ZyEJkceivrPc6AD3sKcoqkVcfK6sOu32faEGJ9y_94Q@mail.gmail.com>

Hello Ard,

> So this is for making changes at runtime, right? And are you only
> using the char[] array in its entirety?
Yes, this is to make changes at runtime on SSDT tables.
If we consider that the SSDT table contained in the char[] array is in two parts: the header, and the AML blob; what we effectively need is the AML blob. However, the header is still very useful to have the size of the AML blob and other information as the name of the table, a checksum etc. Not having the SSDT header would mean that we would need get the size of the table by another mean.

> You could look at SynQuacer or Overdrive platforms: those also load
> SSDT tables from FFS sections at runtime.
I think the arm FVP and Juno platforms where also loading ACPI tables (DSDT/SSDT included) from FFS sections previous to the dynamic tables framework.
The FVP is loading its ACPI tables (DSDT/SSDT included) with the MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf module. The module uses this function to find the ACPI tables:
/**
  Locate the first instance of a protocol.  If the protocol requested is an
  FV protocol, then it will return the first FV that contains the ACPI table
  storage file.

  @param  Instance      Return pointer to the first instance of the protocol

  @return EFI_SUCCESS           The function completed successfully.
  @return EFI_NOT_FOUND         The protocol could not be located.
  @return EFI_OUT_OF_RESOURCES  There are not enough resources to find the protocol.

**/
EFI_STATUS
LocateFvInstanceWithTables (
  OUT EFI_FIRMWARE_VOLUME2_PROTOCOL **Instance
  )

The Juno is loading its ACPI tables (DSDT/SSDT included) with the edk2-platforms/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c module, using this function:
/**
  Locate and Install the ACPI tables from the Firmware Volume

  @param  AcpiFile              Guid of the ACPI file into the Firmware Volume

  @return EFI_SUCCESS           The function completed successfully.
  @return EFI_NOT_FOUND         The protocol could not be located.
  @return EFI_OUT_OF_RESOURCES  There are not enough resources to find the protocol.

**/
EFI_STATUS
LocateAndInstallAcpiFromFv (
  IN CONST EFI_GUID* AcpiFile
  )


About the platform you pointed, what I understood:
- Overdrive platform:
  This platform loads ACPI tables from FFS sections. Depending on PCD values, the driver at edk2-platforms/Silicon/AMD/Styx/Drivers/AcpiPlatformDxe/AcpiPlatformDxe decides which tables to install.
  The code I am describing in the following paragraph is available at edk2-platforms/Silicon/AMD/Styx/Drivers/AcpiPlatformDxe/AcpiPlatform.c:

  When finding the SSDT table generated from SsdtXgbe.asl, the driver patches the two MAC addresses described in the SSDT table.
  To do so, it parses the AML bytecode, looking for the pattern mDefaultMacPackageA (resp mDefaultMacPackageB for the second MAC address).
  It then updates the value with a mix of the pattern (mDefaultMacPackageA) and a PCD value (PcdEthMacA).

- SynQuacer platform:
  This platform installs ACPI tables (DSDT/SSDT included) with the MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf module.
  I think the FVP was using the same module before using the dynamic tables.
  The only particularity that I can find is for the SSDT table generated from edk2-platforms/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.asl.
  This table is fetched from FFS, like the other tables of the platform available at edk2-platforms/Silicon/Socionext/SynQuacer/AcpiTables. It is however installed
  under certain conditions, and by sending an event to the gEfiAcpiTableProtocolGuid. When the gEfiAcpiTableProtocolGuid is installed, the emmc table is installed aswell.

To answer your remark, I think this is effectively possible to retrieve our SSDT templates from the FFS. However, these templates are not meant to be installed as such. Each SSDT template needs to be patched by a piece of '.c' code before being installed. As each piece of '.c' code is specific to an ASL template (and vice versa), they form a couple. I think it would be better bind this couple in a same unique module.

To compare with what is done on the Overdrive platform, in order to install the SsdtXgbe SSDT table, the platform needs to:
- Compile all its ASL files
- Pack them in a Firmware Volume
- Look in the FFS for the SsdtXgbe SSDT table
- Once found, update the MAC addresses
- Install the SsdtXgbe SSDT table

Except if other modules want to access the SsdtXgbe SSDT table, it doesn't seem to me that the table needs to go through the FFS to end in the source code updating the MAC address. Effectively, the code making the modification and the table are in the same module. Plus, making the table available to other modules could be a threat.

What we would like to do is:
- Compile each template/generator couple (the generator is the '.c' file) as a module
- For each couple, when executing the generator, update some values in the corresponding SSDT table. As the SSDT table is available in the generator as an array (included from the '.hex' file), there is no need to look in the FFS
- For each couple, install the template SSDT table

Regards,
Pierre

      reply	other threads:[~2019-12-24 16:47 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-30 13:50 [PATCH v1 1/1] BaseTools: Build ASL files before C files PierreGondois
2019-10-30 13:52 ` [edk2-devel] " PierreGondois
2019-10-31  0:41 ` Liming Gao
2019-10-31 11:12   ` PierreGondois
2019-11-04  7:49     ` Bob Feng
2019-11-04 10:32       ` PierreGondois
2019-11-12 13:33       ` PierreGondois
2019-11-13  1:15         ` Liming Gao
2019-11-13  1:40         ` Bob Feng
2019-11-15 16:27           ` PierreGondois
2019-12-04 17:32             ` PierreGondois
2019-12-11 11:23               ` PierreGondois
2019-12-12  9:14                 ` Bob Feng
2019-12-16  0:33                   ` PierreGondois
2019-12-17 11:41                     ` Bob Feng
2019-12-18 10:43                       ` PierreGondois
2019-12-18 17:50 ` Ard Biesheuvel
2019-12-19 11:18   ` PierreGondois
2019-12-19 16:58     ` Ard Biesheuvel
2019-12-19 18:15       ` Sami Mujawar
2019-12-23 16:09         ` Ard Biesheuvel
2019-12-24 16:47           ` PierreGondois [this message]

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=DB7PR08MB3113DA7D36A368BA0FC9712D8B290@DB7PR08MB3113.eurprd08.prod.outlook.com \
    --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