On 11/5/18 8:09 PM, wanghuiqiang wrote:
Something wrong with my mailbox. On 2018/11/3 4:41, Al Stone wrote:
On 11/1/18 8:32 PM, wanghuiqiang wrote:
Hi Al Stone, On 2018/11/1 23:31, Al Stone wrote:
On 11/1/18 3:17 AM, Ming Huang wrote:
Hi Leif & Al Stone,
Some questions about this fixes: 1 Is need modify [0001] to [0004] in proximmity Domain? +[0001] Proximity Domain : 00000001
Any field enclosed in '[]' is ignored by iasl. The user's guide can be found here:
https://www.acpica.org/documentation
Follow the links in the section "iASL Compiler/Disassembler User Guide".
2 Is need modify Revision to 1 for type 2 node?
The specification requires a value of 1. The full and most current IORT specification can be found here:
Look for the link to IORT.
3 Is need add [0001] and [0003] for this two line? + Memory Size Limit : 00 + Reserved : 00000000
No. Fields enclosed in '[]' are ignored, and are not required. They are added by the iasl disassembler when it disassemblers a table into text form and are used to indicate the length of the data field; in some cases, they are used to show the offset to the binary data in the file. In all cases, they are ignored when compiling the table.
4 Acpica 20181003 can compile ERP D03/D05 without modify IORT, because D03/D05 IORT don't enable SMMU. For plinth project with enable SMMU IORT, D05 will compile failed with 20181003 regardless modify IORT like blow or not.
plinth source IORT: IORT: OpenPlatformPkg/Chips/Hisilicon/Hi1616/D05AcpiTables/D05IortSmmu.asl https://github.com/hisilicon/OpenPlatformPkg.git (master)
Compile fail info:
"iasl" -p/home/huangming/source/plinth/Build/D05/RELEASE_GCC49/AARCH64/OpenPlatformPkg/Chips/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616/OUTPUT/./D05IortSmmu.aml
/home/huangming/source/plinth/Build/D05/RELEASE_GCC49/AARCH64/OpenPlatformPkg/Chips/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616/OUTPUT/./D05IortSmmu.iiii
make: Nothing to be done for 'tbuild'. Error 6126 - Could not compile input file
/home/huangming/source/plinth/Build/D05/RELEASE_GCC49/AARCH64/OpenPlatformPkg/Chips/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616/OUTPUT/./D05IortSmmu.iiii
141: [0004] ID Count : 00000800
Error 6303
Integer too large for target ^ (0000000000000800 - max 1 bytes)
Unfortunately, iasl does not produce very good error messages. When you see a message like this, the actual error probably occurred several lines before this. In this case, the error is reported at line 141, but the actual error is line 135: there is no 'Memory Size Limit' field for a Type 1 subtable in an IORT. I did not do the arithmetic, but please double check the 'Padding' field on line 137, also; it may not always be exactly four bytes long.
I would recommend reading the latest version of the IORT spec in detail and double checking all of the entries in the ASL.
When we developed IORT for D05,the IORT spec Rev D not release yet. Do you think that IORT table in BIOS always be upgraded following IORT spec update, especially for sustaining product? is there any possible that iasl check each IORT node revision information to see which format should be OK for the node? In this way, we may avoid such failure in future.
Unfortunately, this version of IORT is a special case. Normally, you would not have to upgrade the IORT table in BIOS, and iasl would handle it properly. But, some mistakes were made that needed to be corrected, and that forced a change to IORT that broke older versions.
In fact, we found this issue after added SMMUv3 node and NUMA PXM function. It is ugly that we need change iasl back and forth to build different project's code with IORT table. We need find a better way to solve this issue.
For now, unfortunately, the only way to handle this is to use only the newest IORT definition. This issue is only because of the changes made to add SMMUv3 to the IORT and the changes that forced in the IORT spec.
The ACPI specification tries really hard to be backward compatible, and it is, almost always. However, there are two types of table specifications: (1) tables that are completely defined *in* the specification, and (2) tables that are only *referenced* by the specification. For (1), the standards committee can control changes and does a really good job of it because no one wants to upgrade their BIOS; in my five years on the committee, this has not been a problem.
Yes, I agree, ACPI spec is more mature and robust in backward compatibility.
IORT is of type (2), though: it is controlled and defined by ARM, and is only referenced by the ACPI specification. This is done so that ARM can change the table when and how it sees fit, since it only applies to ARM SoCs. The problem is that the original IORT definition had an error that required ARM to change it in such a way that it was not backwards compatible; it was decided to break things now _before_ too many machines had been shipped instead of waiting until later when it would be a much, much larger problem.
It is impossible that any ACPI table can use one version to cover all case forever,to make the table upgrade impact as narrow limits as possible, defined table revision is very helpful. IORT defined table revision as the member in IORT HEAD. In addition, ARM also defined revision field for each node, not sure why ARM defined this for each IORT node initially, but seems it is very useful if we want to solve this IORT compatibility issue.If BIOS developer create the IORT table follow IORT spec strictly and iasl developer/other software check this node revision field, the IORT compatibility issue will disappear.
Correct. This is just a short term glitch. Future revisions -- in theory -- should not have this problem. And, this is exactly why the revisions are in almost every table in the ACPI spec.
So, in the future, iasl will be able to use an IORT revision number; this is just an unfortunate special case.
Suggest iasl also check node revision field. We could use this node revision information to decide which node format is used.
That's not the way iasl works; iasl always uses the most recent revision. It would require a major rewrite to change that. The iasl compiler/disassembler only reads and writes tables -- it is up to drivers and OSs to interpret the content in the table. If the driver or OS needs a certain revision, it needs to check for that; iasl has no way of knowing what the driver or OS needs, and is written so that it does not have to know.
In this case, iasl will check that the revision is less than or equal to the most recent, and then use a data structure that looks like the most recent revision only. It works on the assumption that the data structures -- like for a node format -- only get larger over time, and that any changes to the format or structure occur at the end of the structure, or when a field changes from being reserved to a named data field. For IORT, when SMMUv3 was added, it was necessary to correct an error in the definition that violated that assumption, and hence caused the problem we see today.
Intel ACPI Component Architecture
ASL+ Optimizing Compiler/Disassembler version 20181003 Copyright (c) 2000 - 2018 Intel Corporation Building ... /home/huangming/source/plinth/MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf
[AARCH64] /home/huangming/source/plinth/Build/D05/RELEASE_GCC49/AARCH64/OpenPlatformPkg/Chips/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616/OUTPUT/./D05IortSmmu.iiii
156: [0008] Base Address : 700a0040000
Error 6303
Integer too large for target ^ (00000700A0040000 - max 4 bytes)
Table Input: /home/huangming/source/plinth/Build/D05/RELEASE_GCC49/AARCH64/OpenPlatformPkg/Chips/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616/OUTPUT/./D05IortSmmu.iiii
- 987 lines, 29849 bytes, 166 fields
/home/huangming/source/plinth/Build/D05/RELEASE_GCC49/AARCH64/OpenPlatformPkg/Chips/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616/OUTPUT/./D05IortSmmu.iiii
177: [0004] Input base : 00013000
Error 6303
Integer too large for target ^ (0000000000013000 - max 1 bytes) Compilation complete. 5 Errors, 0 Warnings, 0 Remarks
/home/huangming/source/plinth/Build/D05/RELEASE_GCC49/AARCH64/OpenPlatformPkg/Chips/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616/OUTPUT/./D05IortSmmu.iiii
192: [098h 0152 8] Base Address : 00000000C0040000 GNUmakefile:358: recipe for target '/home/huangming/source/plinth/Build/D05/RELEASE_GCC49/AARCH64/OpenPlatformPkg/Chips/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616/OUTPUT/D05IortSmmu.aml'
failed Error 6303
Integer too large for target ^ (00000000C0040000 - max 1 bytes)
make: *** [/home/huangming/source/plinth/Build/D05/RELEASE_GCC49/AARCH64/OpenPlatformPkg/Chips/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616/OUTPUT/D05IortSmmu.aml]
Error 255
IORT changes:
--- a/Chips/Hisilicon/Hi1616/D05AcpiTables/D05IortSmmu.asl +++ b/Chips/Hisilicon/Hi1616/D05AcpiTables/D05IortSmmu.asl @@ -133,9 +133,7 @@ [0004] PRI Interrupt : 00000000 [0004] GERR Interrupt : 00000000 [0004] Sync Interrupt : 00000000 -[0001] Proximity domain: 00 -[0001] Reserved1: 00 -[0002] Reserved2: 0000 +[0001] Proximity domain: 00000000^M /* this is the map for PCIe2 in 1P NA */ [0004] Input base : 0002f800 [0004] ID Count : 00000800 @@ -165,9 +163,7 @@ [0004] PRI Interrupt : 00000000 [0004] GERR Interrupt : 00000000 [0004] Sync Interrupt : 00000000 -[0001] Proximity domain: 03 -[0001] Reserved1: 00 -[0002] Reserved2: 0000 +[0001] Proximity domain: 00000003^M /* this is the map for pcie0 in 2p nb */ [0004] Input base : 00002000 [0004] Id count : 00001000 @@ -203,9 +199,7 @@ [0B8h 0184 4] PRI GSIV : 00000000 [0BCh 0188 4] GERR GSIV : 00000000 [0C0h 0192 4] Sync GSIV : 00000000 -[0001] Proximity domain: 00 -[0001] Reserved1: 00 -[0002] Reserved2: 0000 +[0001] Proximity domain: 00000000^M
//1F0 /* 1P NB PCIe SMMU */ @@ -228,9 +222,7 @@ [0004] PRI Interrupt : 00000000 [0004] GERR Interrupt : 00000000 [0004] Sync Interrupt : 00000000 -[0001] Proximity domain: 01 -[0001] Reserved1: 00 -[0002] Reserved2: 0000 +[0001] Proximity domain: 00000001^M /* this is the map for PCIe1 in 1P NB */ [0004] Input base : 00017800 [0004] ID Count : 00000800 @@ -267,9 +259,7 @@ [0004] PRI Interrupt : 00000000 [0004] GERR Interrupt : 00000000 [0004] Sync Interrupt : 00000000 -[0001] Proximity domain: 02 -[0001] Reserved1: 00 -[0002] Reserved2: 0000 +[0001] Proximity domain: 00000002^M /* this is the map for PCIe2 in 2P NA */ [0004] Input base : 00021000 [0004] ID Count : 00001000 @@ -563,7 +553,7 @@ [0001] Revision : 00 [0004] Reserved : 00000000 [0004] Mapping Count : 00000001 -[0004] Mapping Offset : 00000020 +[0004] Mapping Offset : 00000024^M
[0008] Memory Properties : [IORT Memory Access Properties] [0004] Cache Coherency : 00000001 @@ -578,6 +568,8 @@ Device Attribute : 0 [0004] ATS Attribute : 00000000 [0004] PCI Segment Number : 00000002 + Memory Size Limit : 00^M + Reserved : 00000000^M
[0004] Input base : 0000f800 [0004] ID Count : 00000800 @@ -591,7 +583,7 @@ [0001] Revision : 00 [0004] Reserved : 00000000 [0004] Mapping Count : 00000001 -[0004] Mapping Offset : 00000020 +[0004] Mapping Offset : 00000024^M
[0008] Memory Properties : [IORT Memory Access Properties] [0004] Cache Coherency : 00000001 @@ -606,6 +598,8 @@ Device Attribute : 0 [0004] ATS Attribute : 00000000 [0004] PCI Segment Number : 00000004 + Memory Size Limit : 00^M + Reserved : 00000000^M
[0004] Input base : 00008800 [0004] ID Count : 00000800 @@ -620,7 +614,7 @@ [0001] Revision : 00 [0004] Reserved : 00000000 [0004] Mapping Count : 00000001 -[0004] Mapping Offset : 00000020 +[0004] Mapping Offset : 00000024^M
[0008] Memory Properties : [IORT Memory Access Properties] [0004] Cache Coherency : 00000001 @@ -635,6 +629,8 @@ Device Attribute : 0 [0004] ATS Attribute : 00000000 [0004] PCI Segment Number : 00000005 + Memory Size Limit : 00^M + Reserved : 00000000^M
[0004] Input base : 00007800 [0004] ID Count : 00000800 @@ -649,7 +645,7 @@ [0001] Revision : 00 [0004] Reserved : 00000000 [0004] Mapping Count : 00000001 -[0004] Mapping Offset : 00000020 +[0004] Mapping Offset : 00000024^M
[0008] Memory Properties : [IORT Memory Access Properties] [0004] Cache Coherency : 00000001 @@ -664,6 +660,8 @@ Device Attribute : 0 [0004] ATS Attribute : 00000000 [0004] PCI Segment Number : 00000006 + Memory Size Limit : 00^M + Reserved : 00000000^M
[0004] Input base : 0000c000 [0004] ID Count : 00000800 @@ -677,7 +675,7 @@ [0001] Revision : 00 [0004] Reserved : 00000000 [0004] Mapping Count : 00000001 -[0004] Mapping Offset : 00000020 +[0004] Mapping Offset : 00000024^M
[0008] Memory Properties : [IORT Memory Access Properties] [0004] Cache Coherency : 00000001 @@ -692,6 +690,8 @@ Device Attribute : 0 [0004] ATS Attribute : 00000000 [0004] PCI Segment Number : 00000007 + Memory Size Limit : 00^M + Reserved : 00000000^M
[0004] Input base : 00009000 [0004] ID Count : 00000800 @@ -705,7 +705,7 @@ [0001] Revision : 00 [0004] Reserved : 00000000 [0004] Mapping Count : 00000001 -[0004] Mapping Offset : 00000020 +[0004] Mapping Offset : 00000024^M
[0008] Memory Properties : [IORT Memory Access Properties] [0004] Cache Coherency : 00000001 @@ -720,6 +720,8 @@ Device Attribute : 0 [0004] ATS Attribute : 00000000 [0004] PCI Segment Number : 0000000a + Memory Size Limit : 00^M + Reserved : 00000000^M
[0004] Input base : 00001000 [0004] ID Count : 00001000 @@ -734,7 +736,7 @@ [0001] Revision : 00 [0004] Reserved : 00000000 [0004] Mapping Count : 00000001 -[0004] Mapping Offset : 00000020 +[0004] Mapping Offset : 00000024^M
[0008] Memory Properties : [IORT Memory Access Properties] [0004] Cache Coherency : 00000001 @@ -749,6 +751,8 @@ Device Attribute : 0 [0004] ATS Attribute : 00000000 [0004] PCI Segment Number : 0000000c + Memory Size Limit : 00^M + Reserved : 00000000^M
[0004] Input base : 00002000 [0004] ID Count : 00001000 @@ -763,7 +767,7 @@ [0001] Revision : 00 [0004] Reserved : 00000000 [0004] Mapping Count : 00000001 -[0004] Mapping Offset : 00000020 +[0004] Mapping Offset : 00000024^M
[0008] Memory Properties : [IORT Memory Access Properties] [0004] Cache Coherency : 00000001 @@ -778,6 +782,8 @@ Device Attribute : 0 [0004] ATS Attribute : 00000000 [0004] PCI Segment Number : 0000000d + Memory Size Limit : 00^M + Reserved : 00000000^M
[0004] Input base : 00003000 [0004] ID Count : 00001000
On 10/31/2018 7:06 PM, Leif Lindholm wrote:
(On further thinking, and testing ...)
Could you also make sure D03/D05 ACPI is up to date and builds with acpica 20181003? I think anything that compiles with 20180629 should also work with 20181003.
Best Regards,
Leif
On Mon, Oct 29, 2018 at 06:58:46PM +0000, Leif Lindholm wrote: > Hi Ming, > > A while back, I forwarded to Al Stone (on cc) the issues we were > having with different versions of acpica-tools and the D06 ACPI table > generation. > > He's come back with a few bugs he found in the .asl, as well as a > weird bug in iasl ... apparently only reproducible under Fedora. > Since the latter has not been fully investigated yet, I'll ignore that > until someone flags this as an issue for them. > > I'm attaching a simple patch that resolves all the issues on current > master. Can you please turn this into a proper patch and send out > (giving Al a Reported-by:)? > > Main gist is reformatting some of the IORT into a form the current > acpica-tools can handle, but there are also some bugfixes and closing > of comment blocks. > > Once I have given feedback for v1 of your edk2-platforms series, > please rebase that onto a version that incorporates these fixes for > v2. > > Best Regards, > > Leif > > From 5651683495f3d36c886f97db17976c9a6eac7b47 Mon Sep 17 00:00:00 2001 > From: Leif Lindholm leif.lindholm@linaro.org > Date: Mon, 29 Oct 2018 16:34:23 +0000 > Subject: [PATCH edk2-platforms] ahs3-fixes > > --- > Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Iort.asl | 32 > +++++++------------- > Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620IortNoSmmu.asl | 2 ++ > 2 files changed, 13 insertions(+), 21 deletions(-) > > diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Iort.asl > b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Iort.asl > index 33b5d5250b..4037ea4f1b 100644 > --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Iort.asl > +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Iort.asl > @@ -53,9 +53,7 @@ > [0004] PRI Interrupt : 00000000 > [0004] GERR Interrupt : 00000000 > [0004] Sync Interrupt : 00000000 > -[0001] Proximity Domain : 01 > -[0001] Reserved : 00 > -[0002] Reserved : 0000 > +[0001] Proximity Domain : 00000001 > [0004] DeviceID mapping index : 00000002 > > [0004] Input base : 00000000 > @@ -99,9 +97,7 @@ > [0004] PRI Interrupt : 00000000 > [0004] GERR Interrupt : 00000000 > [0004] Sync Interrupt : 00000000 > -[0001] Proximity Domain : 01 > -[0001] Reserved : 00 > -[0002] Reserved : 0000 > +[0001] Proximity Domain : 00000001 > [0004] DeviceID mapping index : 0001 > > [0004] Input base : 00007c00 > @@ -139,9 +135,7 @@ > [0004] PRI Interrupt : 00000000 > [0004] GERR Interrupt : 00000000 > [0004] Sync Interrupt : 00000000 > -[0001] Proximity Domain : 01 > -[0001] Reserved : 00 > -[0002] Reserved : 0000 > +[0001] Proximity Domain : 00000001 > [0004] DeviceID mapping index : 00000001 > > [0004] Input base : 00007400 > @@ -179,9 +173,7 @@ > [0004] PRI Interrupt : 00000000 > [0004] GERR Interrupt : 00000000 > [0004] Sync Interrupt : 00000000 > -[0001] Proximity Domain : 03 > -[0001] Reserved : 00 > -[0002] Reserved : 0000 > +[0001] Proximity Domain : 00000003 > [0004] DeviceID mapping index : 00000002 > > [0004] Input base : 00008000 > @@ -225,9 +217,7 @@ > [0004] PRI Interrupt : 00000000 > [0004] GERR Interrupt : 00000000 > [0004] Sync Interrupt : 00000000 > -[0001] Proximity Domain : 03 > -[0001] Reserved : 00 > -[0002] Reserved : 0000 > +[0001] Proximity Domain : 00000003 > [0004] DeviceID mapping index : 0001 > > [0004] Input base : 0000BC00 > @@ -265,9 +255,7 @@ > [0004] PRI Interrupt : 00000000 > [0004] GERR Interrupt : 00000000 > [0004] Sync Interrupt : 00000000 > -[0001] Proximity Domain : 03 > -[0001] Reserved : 00 > -[0002] Reserved : 0000 > +[0001] Proximity Domain : 00000003 > [0004] DeviceID mapping index : 00000001 > > [0004] Input base : 0000B400 > @@ -290,7 +278,7 @@ > [0001] Revision : 00 > [0004] Reserved : 00000000 > [0004] Mapping Count : 0000000C > -[0004] Mapping Offset : 00000028 > +[0004] Mapping Offset : 00000024 > > [0008] Memory Properties : [IORT Memory Access Properties] > [0004] Cache Coherency : 00000001 > @@ -305,6 +293,8 @@ > Device Attribute : 0 > [0004] ATS Attribute : 00000000 > [0004] PCI Segment Number : 00000000 // should > match with above MCFG > + Memory Size Limit : 00 > + Reserved : 00000000 > > /* BDF of pcie host 0 -> stream ID of pcie 0/1 SMMU */ > [0004] Input base : 00000000 > @@ -322,7 +312,7 @@ > [0004] Flags (decoded below) : 00000000 > Single Mapping : 1 > > -/* host2 and host3 should no open smmu for chips smmu bug * > +/* host2 and host3 should no open smmu for chips smmu bug */ > /* BDF of pcie host 2 -> stream ID of pcie 0/1 ITS */ > [0004] Input base : 00007a00 > [0004] ID Count : 00000100 // the > number of IDs in range > @@ -371,7 +361,7 @@ > [0004] Flags (decoded below) : 00000000 > Single Mapping : 1 > > -/* host8 and host9 should no open smmu for chips smmu bug * > +/* host8 and host9 should no open smmu for chips smmu bug */ > /* BDF of pcie host 8 -> stream ID of pcie ITS */ > [0004] Input base : 0000BA00 > [0004] ID Count : 00000100 // the > number of IDs in range > diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620IortNoSmmu.asl > b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620IortNoSmmu.asl > index 63d11b83eb..f4bef6ec89 100644 > --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620IortNoSmmu.asl > +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620IortNoSmmu.asl > @@ -54,6 +54,8 @@ > Device Attribute : 0 > [0004] ATS Attribute : 00000000 > [0004] PCI Segment Number : 00000000 // should > match with above MCFG > + Memory Size Limit : 00 > + Reserved : 00000000 > > /* BDF of pcie host 0 -> stream ID of pcie 0/1 SMMU */ > [0004] Input base : 00000000 > -- > 2.11.0 >