[PATCH v1 1/1] BaseTools: Build ASL files before C files

[PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

From: Pierre Gondois <pierre.gondois@arm.com>

The '-tc' option of the Intel iASL compiler facilitates
generation of AML code in a C array. This AML code is
output to a .hex file. The .hex file can be included
from a C source file, thereby allowing one to run a
fix-up code in C.

For example, this technique can be used to patch the
resource data elements that describe the base address
or interrupt number for a device, before installing
the ACPI table.

To implement this feature two conditions need to be
satisfied:
 - The ASL and C files must be included in the sources
   section of the same .inf file for the module.
 - The ASL files are pre-processed and compiled before
   the C files (so that the .hex file include dependency
   is satisfied).

This patch resolves the dependency by sorting the
CODA_TARGET list for the module being built and
placing the .aml files at the very beginning of
the list.

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---

The changes can be seen at https://github.com/PierreARM/edk2/commits/676_build_asl_first_v1

Notes:
    v1:
    - Sort .aml files first in the CODA_TARGET to build      [Pierre]
      them before other files.

 BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
index f0812b6887be6f9fbdb14003f2efff229633fb34..a59ed1d1952c23d0d3de83353a0566fb8a7dab77 100755
--- a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
@@ -2,6 +2,7 @@
 # Create makefile for MS nmake and GNU make
 #
 # Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2019, ARM Limited. All rights reserved.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 from __future__ import absolute_import
@@ -931,7 +932,14 @@ class ModuleAutoGen(AutoGen):
     @cached_property
     def CodaTargetList(self):
         self.Targets
-        return self._FinalBuildTargetList
+
+        # To resolve dependencies on compiled ASL files (.aml files) in modules,
+        # build them first by putting them as the first targets in the
+        # CodaTargetList.
+        OrderedList = list(self._FinalBuildTargetList)
+        OrderedList.sort(key=lambda T: (T.Target.Ext.lower() != '.aml'))
+
+        return OrderedList
 
     @cached_property
     def FileTypes(self):
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 52 bytes --]

The first one was not sent to devel@edk2.groups.io

[-- Attachment #2: Type: text/html, Size: 52 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[Liming Gao]

Pierre:
  Can you give the whole solution for this usage model? Does it mean C source file depends on ASL file? This is related to the priority of source file type. Now, there is no method to let user configure them. I suggest to introduce the generic way instead of the specific handle. 

Thanks
Liming
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>PierreGondois
>Sent: Wednesday, October 30, 2019 9:51 PM
>To: devel@edk2.groups.io
>Cc: Pierre Gondois <Pierre.Gondois@arm.com>; Feng, Bob C
><bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>;
>Sami.Mujawar@arm.com; nd@arm.com
>Subject: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files
>
>From: Pierre Gondois <pierre.gondois@arm.com>
>
>The '-tc' option of the Intel iASL compiler facilitates
>generation of AML code in a C array. This AML code is
>output to a .hex file. The .hex file can be included
>from a C source file, thereby allowing one to run a
>fix-up code in C.
>
>For example, this technique can be used to patch the
>resource data elements that describe the base address
>or interrupt number for a device, before installing
>the ACPI table.
>
>To implement this feature two conditions need to be
>satisfied:
> - The ASL and C files must be included in the sources
>   section of the same .inf file for the module.
> - The ASL files are pre-processed and compiled before
>   the C files (so that the .hex file include dependency
>   is satisfied).
>
>This patch resolves the dependency by sorting the
>CODA_TARGET list for the module being built and
>placing the .aml files at the very beginning of
>the list.
>
>Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>---
>
>The changes can be seen at
>https://github.com/PierreARM/edk2/commits/676_build_asl_first_v1
>
>Notes:
>    v1:
>    - Sort .aml files first in the CODA_TARGET to build      [Pierre]
>      them before other files.
>
> BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
>b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
>index
>f0812b6887be6f9fbdb14003f2efff229633fb34..a59ed1d1952c23d0d3de83353a0
>566fb8a7dab77 100755
>--- a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
>+++ b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
>@@ -2,6 +2,7 @@
> # Create makefile for MS nmake and GNU make
> #
> # Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
>+# Copyright (c) 2019, ARM Limited. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> from __future__ import absolute_import
>@@ -931,7 +932,14 @@ class ModuleAutoGen(AutoGen):
>     @cached_property
>     def CodaTargetList(self):
>         self.Targets
>-        return self._FinalBuildTargetList
>+
>+        # To resolve dependencies on compiled ASL files (.aml files) in modules,
>+        # build them first by putting them as the first targets in the
>+        # CodaTargetList.
>+        OrderedList = list(self._FinalBuildTargetList)
>+        OrderedList.sort(key=lambda T: (T.Target.Ext.lower() != '.aml'))
>+
>+        return OrderedList
>
>     @cached_property
>     def FileTypes(self):
>--
>'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
>
>

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 2438 bytes --]

Hello Liming,
Please find my answers marked [PIERRE] inline.

>> Can you give the whole solution for this usage model?
[Pierre]
We are planning to modify specific values in pre-compiled AML definition blocks at run-time, and then install these AML definition blocks as SSDT tables.
Instead of having multiple ASL describing the same kind of device (e.g.: a serial port), we would have one template describing this kind of device (e.g.: an ASL description of a serial port).
This template would be generic for the type of device it is describing, and compiled as one standalone AML definition block.
Platform specific values of this template would then be modified at run-time.

>> Does it mean C source file depends on ASL file?
[Pierre]
Yes. To do this, we need to embed these pre-compiled AML definition blocks. The .c file doing the fix-up needs to include this template. Thus this template needs to be compiled first.

The '.inf' file responsible of the fix-up looks like:
[SOURCES]
SsdtSerialPortTemplate.asl
SerialPortFixup.c

Compilation of 'SsdtSerialPortTemplate.asl' with the '-tc' option outputs a 'SsdtSerialPortTemplate.hex' file. '.hex' file's content looks like:
unsigned char ssdtserialtemplate_aml_code[] =
{
0x53,0x53,0x44,0x54,0xC8,0x00,0x00,0x00,  /* 00000000    "SSDT...." */
...
}

The 'SerialPortFixup.c' includes the '.hex' file as shown below:
#include "SsdtSerialPortTemplate.hex"

>> This is related to the priority of source file type.
[Pierre]
Yes, the 'SsdtSerialPortTemplate.asl' needs to be compiled (so that the 'SsdtSerialPortTemplate.hex' is generated) prior to the compilation of 'SerialPortFixup.c'.

>> Now, there is no method to let user configure them. I suggest to introduce the generic way instead of the specific handle.
[Pierre]
I agree that we should have a generic way to configure the order in which the files should be compiled.
A new "SourceOrder" section could be introduced. This section would describe file extensions to compile first, and their order. Unreferenced file extensions would be compiled at the end, unordered.
e.g.:
[SourceOrder]
asl
c

Here, '.asl' files would be compiled first, then '.c' files. '.asl' files would remain unordered in among all '.asl' files. '.c' files would remain unordered in among all '.c' files. Any remaining files in the "Sources" section would be compiled at the end, unordered.

Regards,
Pierre

[-- Attachment #2: Type: text/html, Size: 2755 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[Bob Feng]

[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]

Hi Pierre,

How do you compile 'SsdtSerialPortTemplate.asl' with the '-tc' to the .HEX file? , based on the build_rule.txt, I think the .asl output file is a .aml file.

[Acpi-Source-Language-File]
    <InputFile>
        ?.asl, ?.Asl, ?.ASL

    <OutputFile>
        $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml

Thanks,
Bob

From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of PierreGondois
Sent: Thursday, October 31, 2019 7:12 PM
To: Gao; Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hello Liming,
Please find my answers marked [PIERRE] inline.

>> Can you give the whole solution for this usage model?
[Pierre]
We are planning to modify specific values in pre-compiled AML definition blocks at run-time, and then install these AML definition blocks as SSDT tables.
Instead of having multiple ASL describing the same kind of device (e.g.: a serial port), we would have one template describing this kind of device (e.g.: an ASL description of a serial port).
This template would be generic for the type of device it is describing, and compiled as one standalone AML definition block.
Platform specific values of this template would then be modified at run-time.

>> Does it mean C source file depends on ASL file?
[Pierre]
Yes. To do this, we need to embed these pre-compiled AML definition blocks. The .c file doing the fix-up needs to include this template. Thus this template needs to be compiled first.

The '.inf' file responsible of the fix-up looks like:
[SOURCES]
  SsdtSerialPortTemplate.asl
  SerialPortFixup.c

Compilation of 'SsdtSerialPortTemplate.asl' with the '-tc' option outputs a 'SsdtSerialPortTemplate.hex' file. '.hex' file's content looks like:
unsigned char ssdtserialtemplate_aml_code[] =
{
    0x53,0x53,0x44,0x54,0xC8,0x00,0x00,0x00,  /* 00000000    "SSDT...." */
    ...
}

The 'SerialPortFixup.c' includes the '.hex' file as shown below:
#include "SsdtSerialPortTemplate.hex"

>> This is related to the priority of source file type.
[Pierre]
Yes, the 'SsdtSerialPortTemplate.asl' needs to be compiled (so that the 'SsdtSerialPortTemplate.hex' is generated) prior to the compilation of 'SerialPortFixup.c'.

>> Now, there is no method to let user configure them. I suggest to introduce the generic way instead of the specific handle.
[Pierre]
I agree that we should have a generic way to configure the order in which the files should be compiled.
A new "SourceOrder" section could be introduced. This section would describe file extensions to compile first, and their order. Unreferenced file extensions would be compiled at the end, unordered.
e.g.:
  [SourceOrder]
    asl
    c

Here, '.asl' files would be compiled first, then '.c' files. '.asl' files would remain unordered in among all '.asl' files. '.c' files would remain unordered in among all '.c' files. Any remaining files in the "Sources" section would be compiled at the end, unordered.

Regards,
Pierre


[-- Attachment #2: Type: text/html, Size: 8003 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 508 bytes --]

Hello Bob,

>> How do you compile 'SsdtSerialPortTemplate.asl' with the '-tc' to the .HEX file? , based on the build_rule.txt, I think the .asl output file is a .aml file.

[Pierre]
Our platform has the following flag:
[BuildOptions]
*_*_*_ASL_FLAGS          = -tc

This flag is used in the build_rule.txt as shown below:
[Acpi-Source-Language-File]
...

<Command.XXX>
...
"$(ASL)" $( *ASL_FLAGS* ) $(ASL_OUTFLAGS)${dst} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii

Regards,
Pierre

[-- Attachment #2: Type: text/html, Size: 804 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 4339 bytes --]

Hello Bob and Liming,
Do you think the solution suggested earlier could be acceptable? I refer to the solution below, i.e. having a new section in the ‘.inf’ files describing an extension build order.
E.g.:
  [SourceOrder]
    asl
    c
In this example, ‘.asl’ files would be built first, then ‘.c’ files, then any other files present in the modules. Among ‘.asl’ files, the order would be preserved. Same among ‘.c’ files.

If this solution suits you, we can begin to do a first implementation.

Regards,
Pierre

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bob Feng via Groups.Io
Sent: 04 November 2019 07:50
To: devel@edk2.groups.io; Pierre Gondois <Pierre.Gondois@arm.com>; Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hi Pierre,

How do you compile 'SsdtSerialPortTemplate.asl' with the '-tc' to the .HEX file? , based on the build_rule.txt, I think the .asl output file is a .aml file.

[Acpi-Source-Language-File]
    <InputFile>
        ?.asl, ?.Asl, ?.ASL

    <OutputFile>
        $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml

Thanks,
Bob

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of PierreGondois
Sent: Thursday, October 31, 2019 7:12 PM
To: Gao; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hello Liming,
Please find my answers marked [PIERRE] inline.

>> Can you give the whole solution for this usage model?
[Pierre]
We are planning to modify specific values in pre-compiled AML definition blocks at run-time, and then install these AML definition blocks as SSDT tables.
Instead of having multiple ASL describing the same kind of device (e.g.: a serial port), we would have one template describing this kind of device (e.g.: an ASL description of a serial port).
This template would be generic for the type of device it is describing, and compiled as one standalone AML definition block.
Platform specific values of this template would then be modified at run-time.

>> Does it mean C source file depends on ASL file?
[Pierre]
Yes. To do this, we need to embed these pre-compiled AML definition blocks. The .c file doing the fix-up needs to include this template. Thus this template needs to be compiled first.

The '.inf' file responsible of the fix-up looks like:
[SOURCES]
  SsdtSerialPortTemplate.asl
  SerialPortFixup.c

Compilation of 'SsdtSerialPortTemplate.asl' with the '-tc' option outputs a 'SsdtSerialPortTemplate.hex' file. '.hex' file's content looks like:
unsigned char ssdtserialtemplate_aml_code[] =
{
    0x53,0x53,0x44,0x54,0xC8,0x00,0x00,0x00,  /* 00000000    "SSDT...." */
    ...
}

The 'SerialPortFixup.c' includes the '.hex' file as shown below:
#include "SsdtSerialPortTemplate.hex"

>> This is related to the priority of source file type.
[Pierre]
Yes, the 'SsdtSerialPortTemplate.asl' needs to be compiled (so that the 'SsdtSerialPortTemplate.hex' is generated) prior to the compilation of 'SerialPortFixup.c'.

>> Now, there is no method to let user configure them. I suggest to introduce the generic way instead of the specific handle.
[Pierre]
I agree that we should have a generic way to configure the order in which the files should be compiled.
A new "SourceOrder" section could be introduced. This section would describe file extensions to compile first, and their order. Unreferenced file extensions would be compiled at the end, unordered.
e.g.:
  [SourceOrder]
    asl
    c

Here, '.asl' files would be compiled first, then '.c' files. '.asl' files would remain unordered in among all '.asl' files. '.c' files would remain unordered in among all '.c' files. Any remaining files in the "Sources" section would be compiled at the end, unordered.

Regards,
Pierre

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[-- Attachment #2: Type: text/html, Size: 11003 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[Liming Gao]

[-- Attachment #1: Type: text/plain, Size: 5143 bytes --]

Pierre:
  Order may not resolve the problem. Build tool needs to know their relationship, and generate the correct dependency between ASL and C source file.
  If there is no dependency between them in Makefile, Makefile multiple thread build (make -jn) may break it.

Thanks
Liming
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of PierreGondois
Sent: Tuesday, November 12, 2019 9:34 PM
To: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; nd <nd@arm.com>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hello Bob and Liming,
Do you think the solution suggested earlier could be acceptable? I refer to the solution below, i.e. having a new section in the ‘.inf’ files describing an extension build order.
E.g.:
  [SourceOrder]
    asl
    c
In this example, ‘.asl’ files would be built first, then ‘.c’ files, then any other files present in the modules. Among ‘.asl’ files, the order would be preserved. Same among ‘.c’ files.

If this solution suits you, we can begin to do a first implementation.

Regards,
Pierre

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Bob Feng via Groups.Io
Sent: 04 November 2019 07:50
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Pierre Gondois <Pierre.Gondois@arm.com<mailto:Pierre.Gondois@arm.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hi Pierre,

How do you compile 'SsdtSerialPortTemplate.asl' with the '-tc' to the .HEX file? , based on the build_rule.txt, I think the .asl output file is a .aml file.

[Acpi-Source-Language-File]
    <InputFile>
        ?.asl, ?.Asl, ?.ASL

    <OutputFile>
        $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml

Thanks,
Bob

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of PierreGondois
Sent: Thursday, October 31, 2019 7:12 PM
To: Gao; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hello Liming,
Please find my answers marked [PIERRE] inline.

>> Can you give the whole solution for this usage model?
[Pierre]
We are planning to modify specific values in pre-compiled AML definition blocks at run-time, and then install these AML definition blocks as SSDT tables.
Instead of having multiple ASL describing the same kind of device (e.g.: a serial port), we would have one template describing this kind of device (e.g.: an ASL description of a serial port).
This template would be generic for the type of device it is describing, and compiled as one standalone AML definition block.
Platform specific values of this template would then be modified at run-time.

>> Does it mean C source file depends on ASL file?
[Pierre]
Yes. To do this, we need to embed these pre-compiled AML definition blocks. The .c file doing the fix-up needs to include this template. Thus this template needs to be compiled first.

The '.inf' file responsible of the fix-up looks like:
[SOURCES]
  SsdtSerialPortTemplate.asl
  SerialPortFixup.c

Compilation of 'SsdtSerialPortTemplate.asl' with the '-tc' option outputs a 'SsdtSerialPortTemplate.hex' file. '.hex' file's content looks like:
unsigned char ssdtserialtemplate_aml_code[] =
{
    0x53,0x53,0x44,0x54,0xC8,0x00,0x00,0x00,  /* 00000000    "SSDT...." */
    ...
}

The 'SerialPortFixup.c' includes the '.hex' file as shown below:
#include "SsdtSerialPortTemplate.hex"

>> This is related to the priority of source file type.
[Pierre]
Yes, the 'SsdtSerialPortTemplate.asl' needs to be compiled (so that the 'SsdtSerialPortTemplate.hex' is generated) prior to the compilation of 'SerialPortFixup.c'.

>> Now, there is no method to let user configure them. I suggest to introduce the generic way instead of the specific handle.
[Pierre]
I agree that we should have a generic way to configure the order in which the files should be compiled.
A new "SourceOrder" section could be introduced. This section would describe file extensions to compile first, and their order. Unreferenced file extensions would be compiled at the end, unordered.
e.g.:
  [SourceOrder]
    asl
    c

Here, '.asl' files would be compiled first, then '.c' files. '.asl' files would remain unordered in among all '.asl' files. '.c' files would remain unordered in among all '.c' files. Any remaining files in the "Sources" section would be compiled at the end, unordered.

Regards,
Pierre
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


[-- Attachment #2: Type: text/html, Size: 13546 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[Bob Feng]

[-- Attachment #1: Type: text/plain, Size: 5608 bytes --]

Hi Pierre,

I meet another problem related to .asl file. In one of platforms build process, there is a step which need to compile a .asl file a .h file firstly and that .h file will be included by a module’s source code. That step cannot be handled in edk2 build since the compilation command use different build option, so it’s done in a pre-build script. I want to make edk2 build be able to handle such case. So together with your case,  I think we may need a solution that can do:

1.       Create a module which contains the .asl file and the build option to compile that .asl file.

2.       Find a way to describe the dependency relation between the module contains .asl and the corresponding module contains .c file

Thanks,
Bob

From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of PierreGondois
Sent: Tuesday, November 12, 2019 9:34 PM
To: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; nd <nd@arm.com>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hello Bob and Liming,
Do you think the solution suggested earlier could be acceptable? I refer to the solution below, i.e. having a new section in the ‘.inf’ files describing an extension build order.
E.g.:
  [SourceOrder]
    asl
    c
In this example, ‘.asl’ files would be built first, then ‘.c’ files, then any other files present in the modules. Among ‘.asl’ files, the order would be preserved. Same among ‘.c’ files.

If this solution suits you, we can begin to do a first implementation.

Regards,
Pierre

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Bob Feng via Groups.Io
Sent: 04 November 2019 07:50
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Pierre Gondois <Pierre.Gondois@arm.com<mailto:Pierre.Gondois@arm.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hi Pierre,

How do you compile 'SsdtSerialPortTemplate.asl' with the '-tc' to the .HEX file? , based on the build_rule.txt, I think the .asl output file is a .aml file.

[Acpi-Source-Language-File]
    <InputFile>
        ?.asl, ?.Asl, ?.ASL

    <OutputFile>
        $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml

Thanks,
Bob

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of PierreGondois
Sent: Thursday, October 31, 2019 7:12 PM
To: Gao; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hello Liming,
Please find my answers marked [PIERRE] inline.

>> Can you give the whole solution for this usage model?
[Pierre]
We are planning to modify specific values in pre-compiled AML definition blocks at run-time, and then install these AML definition blocks as SSDT tables.
Instead of having multiple ASL describing the same kind of device (e.g.: a serial port), we would have one template describing this kind of device (e.g.: an ASL description of a serial port).
This template would be generic for the type of device it is describing, and compiled as one standalone AML definition block.
Platform specific values of this template would then be modified at run-time.

>> Does it mean C source file depends on ASL file?
[Pierre]
Yes. To do this, we need to embed these pre-compiled AML definition blocks. The .c file doing the fix-up needs to include this template. Thus this template needs to be compiled first.

The '.inf' file responsible of the fix-up looks like:
[SOURCES]
  SsdtSerialPortTemplate.asl
  SerialPortFixup.c

Compilation of 'SsdtSerialPortTemplate.asl' with the '-tc' option outputs a 'SsdtSerialPortTemplate.hex' file. '.hex' file's content looks like:
unsigned char ssdtserialtemplate_aml_code[] =
{
    0x53,0x53,0x44,0x54,0xC8,0x00,0x00,0x00,  /* 00000000    "SSDT...." */
    ...
}

The 'SerialPortFixup.c' includes the '.hex' file as shown below:
#include "SsdtSerialPortTemplate.hex"

>> This is related to the priority of source file type.
[Pierre]
Yes, the 'SsdtSerialPortTemplate.asl' needs to be compiled (so that the 'SsdtSerialPortTemplate.hex' is generated) prior to the compilation of 'SerialPortFixup.c'.

>> Now, there is no method to let user configure them. I suggest to introduce the generic way instead of the specific handle.
[Pierre]
I agree that we should have a generic way to configure the order in which the files should be compiled.
A new "SourceOrder" section could be introduced. This section would describe file extensions to compile first, and their order. Unreferenced file extensions would be compiled at the end, unordered.
e.g.:
  [SourceOrder]
    asl
    c

Here, '.asl' files would be compiled first, then '.c' files. '.asl' files would remain unordered in among all '.asl' files. '.c' files would remain unordered in among all '.c' files. Any remaining files in the "Sources" section would be compiled at the end, unordered.

Regards,
Pierre
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


[-- Attachment #2: Type: text/html, Size: 16703 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 10310 bytes --]

Hello Bob and Liming,

Liming:
> Order may not resolve the problem. Build tool needs to know their relationship, and generate the correct dependency between ASL and C source file.
> If there is no dependency between them in Makefile, Makefile multiple thread build (make -jn) may break it.

I agree. The patch I sent is just re-ordering the files in the "CODA_TARGET" makfefile target, putting the '.aml' file before the '.c' file. The outcome is as below in the generated Makefile:
CODA_TARGET = [AbsolutePathTo]\OUTPUT\SsdtSerialTemplate.aml \
                               [AbsolutePathTo]\OUTPUT\AcpiSsdtSerialLibArm.lib \

For a multiple thread build, the dependency would not be respected. The patch I sent cannot work.

Bob:
> I meet another problem related to .asl file. In one of platforms build process, there is a step which need to compile a .asl file a .h file firstly and that .h file will be included by a module’s source code.
> That step cannot be handled in edk2 build since the compilation command use different build option, so it’s done in a pre-build script. I want to make edk2 build be able to handle such case. So
> together with your case,  I think we may need a solution that can do:
> 1.       Create a module which contains the .asl file and the build option to compile that .asl file.
> 2.       Find a way to describe the dependency relation between the module contains .asl and the corresponding module contains .c file

We have tested the solution you suggested and the build is working. We split our module in two modules:
* a first one containing the '.asl' files to compile. It looks like:
[Sources]
   SsdtSerialTemplate.asl

* a second one containing the '.c' and '.h' files. It resolves the dependency on the generated '.hex' file via the 'TemplateLibClass' library class. It looks like:
AcpiSsdtSerialLibArm.inf:
  [Sources]
    SourceFile1.h
    SourceFile1.c
    SourceFile2.h
    SourceFile2.c
  [LibraryClasses]
    TemplateLibClass
  [BuildOptions]
    *_*_*_PLATFORM_FLAGS = -I$(OUTPUT_DIR)
    *_*_*_PLATFORM_FLAGS = -I$(OUTPUT_DIR)$(MODULE_RELATIVE_DIR)Template/OUTPUT

Our '.dsc' file build both modules like:
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtSerialLibArm/AcpiSsdtSerialLibArm.inf
TemplateLibClass|DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtSerialLibArm/Template.inf

This solution is working. However we have to create two modules for one single feature. Thus we need to include output files from two modules. These modules need to be correctly configured to make the build work, which I feel is not straightforward.
You say you have a similar issue. Having a more intuitive and standard solution to impose a build order within a module would potentially help other people. It would also simplify their build configuration. I think having a new [SourceOrder] section in '.inf' files could avoid us to do this.
We just need to create an intermediate target in the Makefile for the CODA_TARGET target. For instance, for the following order in a '.inf' file:
[SourceOrder]
  .c        # Build '.c' files first
  .asl      # Build '.asl' second

This would create the following Makefile tagets:
# CODA_TARGET needs to build FIRST_TARGET, SECOND_TARGET and UNORDERED_TARGET.
# We create as many targets as the number of elements in the [SourceOrder] section.
CODA_TARGET = $(FIRST_TARGET) $(SECOND_TARGET) $(UNORDERED_TARGET)

# Build SECOND_TARGET before UNORDERED_TARGET
UNORDERED_TARGET: SECOND_TARGET
# Build FIRST_TARGET before SECOND_TARGET
SECOND_TARGET: FIRST_TARGET

# Build the .aml first, then the .c files, then the unordered files (none here).
FIRST_TARGET = $(OUTPUT_DIR)\SsdtSerialTemplate.aml
SECOND_TARGET = $(OUTPUT_DIR)\AcpiSsdtSerialLibArm.lib
UNORDERED_TARGET =

Again, I am ok to write a prototype if this solution is acceptable to you.

Regards,
Pierre

From: Feng, Bob C <bob.c.feng@intel.com>
Sent: 13 November 2019 01:40
To: devel@edk2.groups.io; Pierre Gondois <Pierre.Gondois@arm.com>; Gao, Liming <liming.gao@intel.com>; nd <nd@arm.com>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Subject: RE: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hi Pierre,

I meet another problem related to .asl file. In one of platforms build process, there is a step which need to compile a .asl file a .h file firstly and that .h file will be included by a module’s source code. That step cannot be handled in edk2 build since the compilation command use different build option, so it’s done in a pre-build script. I want to make edk2 build be able to handle such case. So together with your case,  I think we may need a solution that can do:

  1.  Create a module which contains the .asl file and the build option to compile that .asl file.
  2.  Find a way to describe the dependency relation between the module contains .asl and the corresponding module contains .c file

Thanks,
Bob

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of PierreGondois
Sent: Tuesday, November 12, 2019 9:34 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Feng, Bob C <bob.c.feng@intel.com<mailto:bob.c.feng@intel.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Cc: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hello Bob and Liming,
Do you think the solution suggested earlier could be acceptable? I refer to the solution below, i.e. having a new section in the ‘.inf’ files describing an extension build order.
E.g.:
  [SourceOrder]
    asl
    c
In this example, ‘.asl’ files would be built first, then ‘.c’ files, then any other files present in the modules. Among ‘.asl’ files, the order would be preserved. Same among ‘.c’ files.

If this solution suits you, we can begin to do a first implementation.

Regards,
Pierre

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Bob Feng via Groups.Io
Sent: 04 November 2019 07:50
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Pierre Gondois <Pierre.Gondois@arm.com<mailto:Pierre.Gondois@arm.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hi Pierre,

How do you compile 'SsdtSerialPortTemplate.asl' with the '-tc' to the .HEX file? , based on the build_rule.txt, I think the .asl output file is a .aml file.

[Acpi-Source-Language-File]
    <InputFile>
        ?.asl, ?.Asl, ?.ASL

    <OutputFile>
        $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml

Thanks,
Bob

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of PierreGondois
Sent: Thursday, October 31, 2019 7:12 PM
To: Gao; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hello Liming,
Please find my answers marked [PIERRE] inline.

>> Can you give the whole solution for this usage model?
[Pierre]
We are planning to modify specific values in pre-compiled AML definition blocks at run-time, and then install these AML definition blocks as SSDT tables.
Instead of having multiple ASL describing the same kind of device (e.g.: a serial port), we would have one template describing this kind of device (e.g.: an ASL description of a serial port).
This template would be generic for the type of device it is describing, and compiled as one standalone AML definition block.
Platform specific values of this template would then be modified at run-time.

>> Does it mean C source file depends on ASL file?
[Pierre]
Yes. To do this, we need to embed these pre-compiled AML definition blocks. The .c file doing the fix-up needs to include this template. Thus this template needs to be compiled first.

The '.inf' file responsible of the fix-up looks like:
[SOURCES]
  SsdtSerialPortTemplate.asl
  SerialPortFixup.c

Compilation of 'SsdtSerialPortTemplate.asl' with the '-tc' option outputs a 'SsdtSerialPortTemplate.hex' file. '.hex' file's content looks like:
unsigned char ssdtserialtemplate_aml_code[] =
{
    0x53,0x53,0x44,0x54,0xC8,0x00,0x00,0x00,  /* 00000000    "SSDT...." */
    ...
}

The 'SerialPortFixup.c' includes the '.hex' file as shown below:
#include "SsdtSerialPortTemplate.hex"

>> This is related to the priority of source file type.
[Pierre]
Yes, the 'SsdtSerialPortTemplate.asl' needs to be compiled (so that the 'SsdtSerialPortTemplate.hex' is generated) prior to the compilation of 'SerialPortFixup.c'.

>> Now, there is no method to let user configure them. I suggest to introduce the generic way instead of the specific handle.
[Pierre]
I agree that we should have a generic way to configure the order in which the files should be compiled.
A new "SourceOrder" section could be introduced. This section would describe file extensions to compile first, and their order. Unreferenced file extensions would be compiled at the end, unordered.
e.g.:
  [SourceOrder]
    asl
    c

Here, '.asl' files would be compiled first, then '.c' files. '.asl' files would remain unordered in among all '.asl' files. '.c' files would remain unordered in among all '.c' files. Any remaining files in the "Sources" section would be compiled at the end, unordered.

Regards,
Pierre
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[-- Attachment #2: Type: text/html, Size: 30346 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 338 bytes --]

Hello,
As said previously, the solution Bob is suggesting is working. However it seems this is a common issue and that we could do it in a simpler way.
Do you see a future in having a way to configure the order in which files are built according to their extension? If yes, we can discuss on how you want to do it.

Regards,
Pierre

[-- Attachment #2: Type: text/html, Size: 358 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

Hello Bob and Liming,
I am coming back to you after re-reading Bob's answer:
> I meet another problem related to .asl file. In one of platforms build process, there is a step which need to compile a .asl file a .h file firstly and that .h file will be included by a module’s source code.
> That step cannot be handled in edk2 build since the compilation command use different build option, so it’s done in a pre-build script. I want to make edk2 build be able to handle such case. So together with your case,  I think we may need a solution that can do:
> 1.       Create a module which contains the .asl file and the build option to compile that .asl file.
> 2.       Find a way to describe the dependency relation between the module contains .asl and the corresponding module contains .c file

I split my module into one module containing the .asl file, and a second module containing .c source files. However I could not find a way to describe a dependency between the two modules (as suggested in the second step).
This part of Bob's answer makes me think it's not possible using current edk2 build system:
> That step cannot be handled in edk2 build since the compilation command use different build option, so it’s done in a pre-build script.

I will have to reproduce the solution to multiple folders and for multiple platforms. Using a pre-build script would not be a scalable solution.
I would need to create a dependency between file extensions (or between modules which are edk2 library classes) using the build system.
Would you have an idea of how to do it, or if this is currently not possible and this implies modification in the build system, is there a way I could help?

Regards,
Pierre

[-- Attachment #2: Type: text/html, Size: 1884 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[Bob Feng]

[-- Attachment #1: Type: text/plain, Size: 2690 bytes --]

Hi Pierre,

Currently, I don’t have good solution for this issue. I considered the solution that I mentioned before, I don’t think that is a good solution either. As you mentioned that to split one feature into 2 modules is not good, and now there is no way to describe the dependency of those two modules.

I think if we can describe the dependency between source files in INF file. For example,

[Sources.IA32]
  asl_source.asl
  c_source.c | asl_source.asl

The build tool will generate the dependency that says c_source.c depend on the output of asl_source.asl in Makefile?

What do you think?

Would you submit a BZ to track this issue?

Thanks,
Bob

From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of PierreGondois
Sent: Wednesday, December 11, 2019 7:24 PM
To: PierreGondois <pierre.gondois@arm.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hello Bob and Liming,
I am coming back to you after re-reading Bob's answer:
> I meet another problem related to .asl file. In one of platforms build process, there is a step which need to compile a .asl file a .h file firstly and that .h file will be included by a module’s source code.
> That step cannot be handled in edk2 build since the compilation command use different build option, so it’s done in a pre-build script. I want to make edk2 build be able to handle such case. So together with your case,  I think we may need a solution that can do:
> 1.       Create a module which contains the .asl file and the build option to compile that .asl file.
> 2.       Find a way to describe the dependency relation between the module contains .asl and the corresponding module contains .c file

I split my module into one module containing the .asl file, and a second module containing .c source files. However I could not find a way to describe a dependency between the two modules (as suggested in the second step).
This part of Bob's answer makes me think it's not possible using current edk2 build system:
> That step cannot be handled in edk2 build since the compilation command use different build option, so it’s done in a pre-build script.

I will have to reproduce the solution to multiple folders and for multiple platforms. Using a pre-build script would not be a scalable solution.
I would need to create a dependency between file extensions (or between modules which are edk2 library classes) using the build system.
Would you have an idea of how to do it, or if this is currently not possible and this implies modification in the build system, is there a way I could help?

Regards,
Pierre


[-- Attachment #2: Type: text/html, Size: 7909 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]

Hello Bob,

> I think if we can describe the dependency between source files in INF file. For example,
> [Sources.IA32]
>   asl_source.asl
>   c_source.c | asl_source.asl
This looks good to me! Would it be possible to have multiple dependencies on one file (1), or would it be limited to one file only (2)?
Maybe having multiple dependencies like in (1) would make it easier to read:

Example (1):
[Sources.IA32]
asl_source_1.asl
asl_source_2.asl
c_source.c | asl_source_1.asl | asl_source_2.asl

Example (2):
[Sources.IA32]
asl_source_1.asl
asl_source_2.asl | asl_source_1.asl
c_source.c | asl_source_2

> The build tool will generate the dependency that says c_source.c depend on the output of asl_source.asl in Makefile?
> What do you think?
Would there be a rule in the Makefile on c_source.c or on c_source.obj? And how can we describe a dependency on the output of asl_source.asl?

What I thought was that the resulting Makefile dependencies would look like this:
(a) c_source.obj: asl_source.aml

The dependencies between the source files would be converted to Makefile dependencies between their corresponding output format (cf edk2/BaseTools/Conf/build_rule.template).
E.g.: a dependency on a '.dts' file would be converted to a dependency on a '.dtb' file.

Dependencies on file extensions without output format would trigger an error.
E.g.: no dependency on '.h' would be allowed.

The issue with what I am suggesting is that in my case, I am compiling a '.asl' file into a '.hex' file using the iASL compiler with the '-tc' option. This '.hex' file is included in a '.c' file. This means that the dependency is actually between the output of the compiled '.asl' file (so the '.hex' file), and the '.c' file.
I.e.: the 'true' dependency is
(b) c_source.c: asl_source.hex
However, making edk2 know that the asl_source.asl will produce asl_source.hex will lead to modification to the Conf/build_rule.txt, wich is not a good idea neither I believe.

Having (a) instead of (b) should be simpler to obtain, even though less correct.
I don't know what you think about what format should have/
* the target of the Makefile rule (c_source.c or c_source.obj?)
* the prerequisites of the Makefile rule (asl_source.?)

> Would you submit a BZ to track this issue?
Yes, I will do it at the beginning of the week.

Please tell me what do you think,

Regards,
Pierre

[-- Attachment #2: Type: text/html, Size: 3157 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[Bob Feng]

[-- Attachment #1: Type: text/plain, Size: 3062 bytes --]

Hi Pierre,

I add comments inline

Thanks,
Bob

From: PierreGondois [mailto:pierre.gondois@arm.com]
Sent: Monday, December 16, 2019 8:34 AM
To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hello Bob,

> I think if we can describe the dependency between source files in INF file. For example,
> [Sources.IA32]
>   asl_source.asl
>   c_source.c | asl_source.asl
This looks good to me! Would it be possible to have multiple dependencies on one file (1), or would it be limited to one file only (2)?
Maybe having multiple dependencies like in (1) would make it easier to read:

[Bob] I think we need to support multiple files, but we need to use other character as separator.

Example (1):
[Sources.IA32]
  asl_source_1.asl
  asl_source_2.asl
  c_source.c | asl_source_1.asl | asl_source_2.asl

Example (2):
  [Sources.IA32]
  asl_source_1.asl
  asl_source_2.asl | asl_source_1.asl
  c_source.c | asl_source_2


> The build tool will generate the dependency that says c_source.c depend on the output of asl_source.asl in Makefile?
> What do you think?
Would there be a rule in the Makefile on c_source.c or on c_source.obj? And how can we describe a dependency on the output of asl_source.asl?


What I thought was that the resulting Makefile dependencies would look like this:
(a) c_source.obj: asl_source.aml

The dependencies between the source files would be converted to Makefile dependencies between their corresponding output format (cf edk2/BaseTools/Conf/build_rule.template).
E.g.: a dependency on a '.dts' file would be converted to a dependency on a '.dtb' file.

Dependencies on file extensions without output format would trigger an error.
E.g.: no dependency on '.h' would be allowed.

The issue with what I am suggesting is that in my case, I am compiling a '.asl' file into a '.hex' file using the iASL compiler with the '-tc' option. This '.hex' file is included in a '.c' file. This means that the dependency is actually between the output of the compiled '.asl' file (so the '.hex' file), and the '.c' file.
I.e.: the 'true' dependency is
(b) c_source.c: asl_source.hex
However, making edk2 know that the asl_source.asl will produce asl_source.hex will lead to modification to the Conf/build_rule.txt, wich is not a good idea neither I believe.

Having (a) instead of (b) should be simpler to obtain, even though less correct.
I don't know what you think about what format should have/
* the target of the Makefile rule (c_source.c or c_source.obj?)
* the prerequisites of the Makefile rule (asl_source.?)

I think we may modify the build rule of .asl file to have the build tool always generate both *.aml and *.hex files for asl type source file. And in Makefile we just use c_source.obj: asl_source.aml format. What do you think?


> Would you submit a BZ to track this issue?
Yes, I will do it at the beginning of the week.

Please tell me what do you think,

Regards,
Pierre

[-- Attachment #2: Type: text/html, Size: 9657 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

[-- Attachment #1: Type: text/plain, Size: 4439 bytes --]

Hello Bob,

Please find my answers inline.

I created a Bugzilla ticket here: https://bugzilla.tianocore.org/show_bug.cgi?id=2425

I have started to look at the implementation,

Regards,
Pierre

From: Feng, Bob C <bob.c.feng@intel.com>
Sent: 17 December 2019 11:42
To: Pierre Gondois <Pierre.Gondois@arm.com>; devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>
Subject: RE: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hi Pierre,

I add comments inline

Thanks,
Bob

From: PierreGondois [mailto:pierre.gondois@arm.com]
Sent: Monday, December 16, 2019 8:34 AM
To: Feng, Bob C <bob.c.feng@intel.com<mailto:bob.c.feng@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

Hello Bob,

> I think if we can describe the dependency between source files in INF file. For example,
> [Sources.IA32]
>   asl_source.asl
>   c_source.c | asl_source.asl
This looks good to me! Would it be possible to have multiple dependencies on one file (1), or would it be limited to one file only (2)?
Maybe having multiple dependencies like in (1) would make it easier to read:

[Bob] I think we need to support multiple files, but we need to use other character as separator.
[Pierre]
I agree. I suggest to use a colon ':' as in Makefiles. We can aslo use '<' or '>' depending on the interpretation. I don't really mind.
It is currently possible to add options when adding a source file in the ‘[Source]’ section: Famliy | TagName | CommandCode | FeatureFlagExpress
Maybe the additional dependencies should be at the end, such as:
  FileName | Famliy | TagName | CommandCode | FeatureFlagExpress : dependency_1 : dependency_2 : dependency_3

Example (1):
[Sources.IA32]
  asl_source_1.asl
  asl_source_2.asl
  c_source.c | asl_source_1.asl | asl_source_2.asl

Example (2):
  [Sources.IA32]
  asl_source_1.asl
  asl_source_2.asl | asl_source_1.asl
  c_source.c | asl_source_2


> The build tool will generate the dependency that says c_source.c depend on the output of asl_source.asl in Makefile?
> What do you think?
Would there be a rule in the Makefile on c_source.c or on c_source.obj? And how can we describe a dependency on the output of asl_source.asl?


What I thought was that the resulting Makefile dependencies would look like this:
(a) c_source.obj: asl_source.aml

The dependencies between the source files would be converted to Makefile dependencies between their corresponding output format (cf edk2/BaseTools/Conf/build_rule.template).
E.g.: a dependency on a '.dts' file would be converted to a dependency on a '.dtb' file.

Dependencies on file extensions without output format would trigger an error.
E.g.: no dependency on '.h' would be allowed.

The issue with what I am suggesting is that in my case, I am compiling a '.asl' file into a '.hex' file using the iASL compiler with the '-tc' option. This '.hex' file is included in a '.c' file. This means that the dependency is actually between the output of the compiled '.asl' file (so the '.hex' file), and the '.c' file.
I.e.: the 'true' dependency is
(b) c_source.c: asl_source.hex
However, making edk2 know that the asl_source.asl will produce asl_source.hex will lead to modification to the Conf/build_rule.txt, wich is not a good idea neither I believe.

Having (a) instead of (b) should be simpler to obtain, even though less correct.
I don't know what you think about what format should have/
* the target of the Makefile rule (c_source.c or c_source.obj?)
* the prerequisites of the Makefile rule (asl_source.?)

I think we may modify the build rule of .asl file to have the build tool always generate both *.aml and *.hex files for asl type source file. And in Makefile we just use c_source.obj: asl_source.aml format. What do you think?
[Pierre]

  *   About modifying the build rule for ‘.asl’ files in order to generate a ‘.hex’ by default:
The ‘.hex’ file is exactly what I want to generate, so I am totally ok with it. It might just be pollution for other people who don’t need it.
  *   About having a Makefile rule looking like: “c_source.obj: asl_source.aml”:
I am also ok with it.

> Would you submit a BZ to track this issue?
Yes, I will do it at the beginning of the week.

Please tell me what do you think,

Regards,
Pierre

[-- Attachment #2: Type: text/html, Size: 20694 bytes --]

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[Ard Biesheuvel]

On Wed, 30 Oct 2019 at 15:50, PierreGondois <pierre.gondois@arm.com> wrote:
>
> From: Pierre Gondois <pierre.gondois@arm.com>
>
> The '-tc' option of the Intel iASL compiler facilitates
> generation of AML code in a C array. This AML code is
> output to a .hex file. The .hex file can be included
> from a C source file, thereby allowing one to run a
> fix-up code in C.
>
> For example, this technique can be used to patch the
> resource data elements that describe the base address
> or interrupt number for a device, before installing
> the ACPI table.
>
> To implement this feature two conditions need to be
> satisfied:
>  - The ASL and C files must be included in the sources
>    section of the same .inf file for the module.
>  - The ASL files are pre-processed and compiled before
>    the C files (so that the .hex file include dependency
>    is satisfied).
>
> This patch resolves the dependency by sorting the
> CODA_TARGET list for the module being built and
> placing the .aml files at the very beginning of
> the list.
>
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>
> The changes can be seen at https://github.com/PierreARM/edk2/commits/676_build_asl_first_v1
>

Hello Pierre,

I am not convinced it is a good idea to depend on what amounts to an
implementation detail of the ASL compiler. Is the .hex format
documented somewhere?


> Notes:
>     v1:
>     - Sort .aml files first in the CODA_TARGET to build      [Pierre]
>       them before other files.
>
>  BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> index f0812b6887be6f9fbdb14003f2efff229633fb34..a59ed1d1952c23d0d3de83353a0566fb8a7dab77 100755
> --- a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> @@ -2,6 +2,7 @@
>  # Create makefile for MS nmake and GNU make
>  #
>  # Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2019, ARM Limited. All rights reserved.<BR>
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>  from __future__ import absolute_import
> @@ -931,7 +932,14 @@ class ModuleAutoGen(AutoGen):
>      @cached_property
>      def CodaTargetList(self):
>          self.Targets
> -        return self._FinalBuildTargetList
> +
> +        # To resolve dependencies on compiled ASL files (.aml files) in modules,
> +        # build them first by putting them as the first targets in the
> +        # CodaTargetList.
> +        OrderedList = list(self._FinalBuildTargetList)
> +        OrderedList.sort(key=lambda T: (T.Target.Ext.lower() != '.aml'))
> +
> +        return OrderedList
>
>      @cached_property
>      def FileTypes(self):
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
>
> 
>

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

Hello Ard,
I cannot find any documentation more than what the iASL compiler does with the '-tc' argument:
 -tc                 Create hex AML table in C (*.hex)

The '.hex' file looks like this:

#ifndef __ASL_SOURCE_FILE_HEX__
#define __ASL_SOURCE_FILE_HEX__

unsigned char ASL_SOURCE_FILE[] =
{
    0x53,0x53,0x44,0x54,0xAD,0x00,0x00,0x00,  /* 00000000    "SSDT...." */
    0x01,0xF8,0x41,0x52,0x4D,0x4C,0x54,0x44,  /* 00000008    "..ARMLTD" */
...
}

#endif __ASL_SOURCE_FILE_HEX__

I can make a python script to create a similar '.hex' file in case the iASL compiler changes its ouput. However this script would have to be included in Conf/build_rule.txt.

Regards,
Pierre

-----Original Message-----
From: Ard Biesheuvel <ard.biesheuvel@linaro.org> 
Sent: 18 December 2019 17:50
To: edk2-devel-groups-io <devel@edk2.groups.io>; Pierre Gondois <Pierre.Gondois@arm.com>
Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sami Mujawar <Sami.Mujawar@arm.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

On Wed, 30 Oct 2019 at 15:50, PierreGondois <pierre.gondois@arm.com> wrote:
>
> From: Pierre Gondois <pierre.gondois@arm.com>
>
> The '-tc' option of the Intel iASL compiler facilitates generation of 
> AML code in a C array. This AML code is output to a .hex file. The 
> .hex file can be included from a C source file, thereby allowing one 
> to run a fix-up code in C.
>
> For example, this technique can be used to patch the resource data 
> elements that describe the base address or interrupt number for a 
> device, before installing the ACPI table.
>
> To implement this feature two conditions need to be
> satisfied:
>  - The ASL and C files must be included in the sources
>    section of the same .inf file for the module.
>  - The ASL files are pre-processed and compiled before
>    the C files (so that the .hex file include dependency
>    is satisfied).
>
> This patch resolves the dependency by sorting the CODA_TARGET list for 
> the module being built and placing the .aml files at the very 
> beginning of the list.
>
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>
> The changes can be seen at 
> https://github.com/PierreARM/edk2/commits/676_build_asl_first_v1
>

Hello Pierre,

I am not convinced it is a good idea to depend on what amounts to an implementation detail of the ASL compiler. Is the .hex format documented somewhere?


> Notes:
>     v1:
>     - Sort .aml files first in the CODA_TARGET to build      [Pierre]
>       them before other files.
>
>  BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py 
> b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> index 
> f0812b6887be6f9fbdb14003f2efff229633fb34..a59ed1d1952c23d0d3de83353a05
> 66fb8a7dab77 100755
> --- a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> @@ -2,6 +2,7 @@
>  # Create makefile for MS nmake and GNU make  #  # Copyright (c) 2019, 
> Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2019, ARM Limited. All rights reserved.<BR>
>  # SPDX-License-Identifier: BSD-2-Clause-Patent  #  from __future__ 
> import absolute_import @@ -931,7 +932,14 @@ class 
> ModuleAutoGen(AutoGen):
>      @cached_property
>      def CodaTargetList(self):
>          self.Targets
> -        return self._FinalBuildTargetList
> +
> +        # To resolve dependencies on compiled ASL files (.aml files) in modules,
> +        # build them first by putting them as the first targets in the
> +        # CodaTargetList.
> +        OrderedList = list(self._FinalBuildTargetList)
> +        OrderedList.sort(key=lambda T: (T.Target.Ext.lower() != 
> + '.aml'))
> +
> +        return OrderedList
>
>      @cached_property
>      def FileTypes(self):
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
>
> 
>

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[Ard Biesheuvel]

(+ Leif)

On Thu, 19 Dec 2019 at 13:19, Pierre Gondois <Pierre.Gondois@arm.com> wrote:
>
> Hello Ard,
> I cannot find any documentation more than what the iASL compiler does with the '-tc' argument:
>  -tc                 Create hex AML table in C (*.hex)
>
> The '.hex' file looks like this:
>
> #ifndef __ASL_SOURCE_FILE_HEX__
> #define __ASL_SOURCE_FILE_HEX__
>
> unsigned char ASL_SOURCE_FILE[] =
> {
>     0x53,0x53,0x44,0x54,0xAD,0x00,0x00,0x00,  /* 00000000    "SSDT...." */
>     0x01,0xF8,0x41,0x52,0x4D,0x4C,0x54,0x44,  /* 00000008    "..ARMLTD" */
> ...
> }
>
> #endif __ASL_SOURCE_FILE_HEX__
>
> I can make a python script to create a similar '.hex' file in case the iASL compiler changes its ouput. However this script would have to be included in Conf/build_rule.txt.
>

I understand the appeal of using intermediate output of the iasl
compiler, but I don't think we should rely on this unless it is a
clear stable interface. Managing the complexity of all the various
toolchains and build tools is already proving to be too difficult, and
adding another degree of freedom is not a good idea.

Can you explain why exactly you need this?



> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: 18 December 2019 17:50
> To: edk2-devel-groups-io <devel@edk2.groups.io>; Pierre Gondois <Pierre.Gondois@arm.com>
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sami Mujawar <Sami.Mujawar@arm.com>; nd <nd@arm.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files
>
> On Wed, 30 Oct 2019 at 15:50, PierreGondois <pierre.gondois@arm.com> wrote:
> >
> > From: Pierre Gondois <pierre.gondois@arm.com>
> >
> > The '-tc' option of the Intel iASL compiler facilitates generation of
> > AML code in a C array. This AML code is output to a .hex file. The
> > .hex file can be included from a C source file, thereby allowing one
> > to run a fix-up code in C.
> >
> > For example, this technique can be used to patch the resource data
> > elements that describe the base address or interrupt number for a
> > device, before installing the ACPI table.
> >
> > To implement this feature two conditions need to be
> > satisfied:
> >  - The ASL and C files must be included in the sources
> >    section of the same .inf file for the module.
> >  - The ASL files are pre-processed and compiled before
> >    the C files (so that the .hex file include dependency
> >    is satisfied).
> >
> > This patch resolves the dependency by sorting the CODA_TARGET list for
> > the module being built and placing the .aml files at the very
> > beginning of the list.
> >
> > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> > ---
> >
> > The changes can be seen at
> > https://github.com/PierreARM/edk2/commits/676_build_asl_first_v1
> >
>
> Hello Pierre,
>
> I am not convinced it is a good idea to depend on what amounts to an implementation detail of the ASL compiler. Is the .hex format documented somewhere?
>
>
> > Notes:
> >     v1:
> >     - Sort .aml files first in the CODA_TARGET to build      [Pierre]
> >       them before other files.
> >
> >  BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> > b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> > index
> > f0812b6887be6f9fbdb14003f2efff229633fb34..a59ed1d1952c23d0d3de83353a05
> > 66fb8a7dab77 100755
> > --- a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> > +++ b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> > @@ -2,6 +2,7 @@
> >  # Create makefile for MS nmake and GNU make  #  # Copyright (c) 2019,
> > Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2019, ARM Limited. All rights reserved.<BR>
> >  # SPDX-License-Identifier: BSD-2-Clause-Patent  #  from __future__
> > import absolute_import @@ -931,7 +932,14 @@ class
> > ModuleAutoGen(AutoGen):
> >      @cached_property
> >      def CodaTargetList(self):
> >          self.Targets
> > -        return self._FinalBuildTargetList
> > +
> > +        # To resolve dependencies on compiled ASL files (.aml files) in modules,
> > +        # build them first by putting them as the first targets in the
> > +        # CodaTargetList.
> > +        OrderedList = list(self._FinalBuildTargetList)
> > +        OrderedList.sort(key=lambda T: (T.Target.Ext.lower() !=
> > + '.aml'))
> > +
> > +        return OrderedList
> >
> >      @cached_property
> >      def FileTypes(self):
> > --
> > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >
> >
> >
> > 
> >

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[Sami Mujawar]

Hi Ard,

This is required for loading the AML blob in memory so that we can parse and fixup tables.

However, I understand your concern regarding dependency/relying on asl compilers for the .hex file.
The .hex file can be easily generated by a post-processing python script (20-30 lines of code) that reads the AML file and dumps the blob in a C array.
If preferred, we can post a patch that adds this support so that we don’t have to rely on intermediate outputs of asl compilers.

Alternatively, we would be grateful if you can any suggest another option.

Regards,

Sami Mujawar

-----Original Message-----
From: Ard Biesheuvel <ard.biesheuvel@linaro.org> 
Sent: 19 December 2019 04:58 PM
To: Pierre Gondois <Pierre.Gondois@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sami Mujawar <Sami.Mujawar@arm.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

(+ Leif)

On Thu, 19 Dec 2019 at 13:19, Pierre Gondois <Pierre.Gondois@arm.com> wrote:
>
> Hello Ard,
> I cannot find any documentation more than what the iASL compiler does with the '-tc' argument:
>  -tc                 Create hex AML table in C (*.hex)
>
> The '.hex' file looks like this:
>
> #ifndef __ASL_SOURCE_FILE_HEX__
> #define __ASL_SOURCE_FILE_HEX__
>
> unsigned char ASL_SOURCE_FILE[] =
> {
>     0x53,0x53,0x44,0x54,0xAD,0x00,0x00,0x00,  /* 00000000    "SSDT...." */
>     0x01,0xF8,0x41,0x52,0x4D,0x4C,0x54,0x44,  /* 00000008    "..ARMLTD" */
> ...
> }
>
> #endif __ASL_SOURCE_FILE_HEX__
>
> I can make a python script to create a similar '.hex' file in case the iASL compiler changes its ouput. However this script would have to be included in Conf/build_rule.txt.
>

I understand the appeal of using intermediate output of the iasl compiler, but I don't think we should rely on this unless it is a clear stable interface. Managing the complexity of all the various toolchains and build tools is already proving to be too difficult, and adding another degree of freedom is not a good idea.

Can you explain why exactly you need this?



> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: 18 December 2019 17:50
> To: edk2-devel-groups-io <devel@edk2.groups.io>; Pierre Gondois 
> <Pierre.Gondois@arm.com>
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming 
> <liming.gao@intel.com>; Sami Mujawar <Sami.Mujawar@arm.com>; nd 
> <nd@arm.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files 
> before C files
>
> On Wed, 30 Oct 2019 at 15:50, PierreGondois <pierre.gondois@arm.com> wrote:
> >
> > From: Pierre Gondois <pierre.gondois@arm.com>
> >
> > The '-tc' option of the Intel iASL compiler facilitates generation 
> > of AML code in a C array. This AML code is output to a .hex file. 
> > The .hex file can be included from a C source file, thereby allowing 
> > one to run a fix-up code in C.
> >
> > For example, this technique can be used to patch the resource data 
> > elements that describe the base address or interrupt number for a 
> > device, before installing the ACPI table.
> >
> > To implement this feature two conditions need to be
> > satisfied:
> >  - The ASL and C files must be included in the sources
> >    section of the same .inf file for the module.
> >  - The ASL files are pre-processed and compiled before
> >    the C files (so that the .hex file include dependency
> >    is satisfied).
> >
> > This patch resolves the dependency by sorting the CODA_TARGET list 
> > for the module being built and placing the .aml files at the very 
> > beginning of the list.
> >
> > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> > ---
> >
> > The changes can be seen at
> > https://github.com/PierreARM/edk2/commits/676_build_asl_first_v1
> >
>
> Hello Pierre,
>
> I am not convinced it is a good idea to depend on what amounts to an implementation detail of the ASL compiler. Is the .hex format documented somewhere?
>
>
> > Notes:
> >     v1:
> >     - Sort .aml files first in the CODA_TARGET to build      [Pierre]
> >       them before other files.
> >
> >  BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> > b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> > index
> > f0812b6887be6f9fbdb14003f2efff229633fb34..a59ed1d1952c23d0d3de83353a
> > 05
> > 66fb8a7dab77 100755
> > --- a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> > +++ b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
> > @@ -2,6 +2,7 @@
> >  # Create makefile for MS nmake and GNU make  #  # Copyright (c) 
> > 2019, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2019, ARM Limited. All rights reserved.<BR>
> >  # SPDX-License-Identifier: BSD-2-Clause-Patent  #  from __future__ 
> > import absolute_import @@ -931,7 +932,14 @@ class
> > ModuleAutoGen(AutoGen):
> >      @cached_property
> >      def CodaTargetList(self):
> >          self.Targets
> > -        return self._FinalBuildTargetList
> > +
> > +        # To resolve dependencies on compiled ASL files (.aml files) in modules,
> > +        # build them first by putting them as the first targets in the
> > +        # CodaTargetList.
> > +        OrderedList = list(self._FinalBuildTargetList)
> > +        OrderedList.sort(key=lambda T: (T.Target.Ext.lower() !=
> > + '.aml'))
> > +
> > +        return OrderedList
> >
> >      @cached_property
> >      def FileTypes(self):
> > --
> > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >
> >
> >
> > 
> >

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[Ard Biesheuvel]

On Thu, 19 Dec 2019 at 19:15, Sami Mujawar <Sami.Mujawar@arm.com> wrote:
>
> Hi Ard,
>
> This is required for loading the AML blob in memory so that we can parse and fixup tables.
>
> However, I understand your concern regarding dependency/relying on asl compilers for the .hex file.
> The .hex file can be easily generated by a post-processing python script (20-30 lines of code) that reads the AML file and dumps the blob in a C array.
> If preferred, we can post a patch that adds this support so that we don’t have to rely on intermediate outputs of asl compilers.
>
> Alternatively, we would be grateful if you can any suggest another option.
>

So this is for making changes at runtime, right? And are you only
using the char[] array in its entirety?

You could look at SynQuacer or Overdrive platforms: those also load
SSDT tables from FFS sections at runtime.

Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Build ASL files before C files

[PierreGondois]

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