Bug #40
closedThe IASL update to 20160318 generates wrong bytecode on samsung/lumpy
0%
Files
Updated by Idwer Vollering about 8 years ago
The specific change to AML I ran into involves _S5:
--- dsdt.dsl_6911219 2016-04-11 03:59:18.610400722 +0200
+++ dsdt.dsl_64b35f3 2016-04-11 03:59:18.546402408 +0200
@@ -1,34 +1,36 @@
/*
* Intel ACPI Component Architecture
- * AML/ASL+ Disassembler version 20150619-64
- * Copyright (c) 2000 - 2015 Intel Corporation
+ * AML/ASL+ Disassembler version 20160318-64
+ * Copyright (c) 2000 - 2016 Intel Corporation
*
* Disassembling to symbolic ASL+ operators
*
- * Disassembly of dsdt.aml, Mon Apr 11 03:59:56 2016
+ * Disassembly of dsdt.aml, Mon Apr 11 04:02:42 2016
*
* Original Table Header:
* Signature "DSDT"
- * Length 0x000034F6 (13558)
+ * Length 0x0000359C (13724)
* Revision 0x02
- * Checksum 0x66 **** Incorrect checksum, should be 0x77
+ * Checksum 0xCC
* OEM ID "COREv4"
* OEM Table ID "COREBOOT"
* OEM Revision 0x20110725 (537986853)
* Compiler ID "INTL"
- * Compiler Version 0x20150619 (538248729)
+ * Compiler Version 0x20160318 (538313496)
*/
-DefinitionBlock ("dsdt.aml", "DSDT", 2, "COREv4", "COREBOOT", 0x20110725)
+DefinitionBlock ("", "DSDT", 2, "COREv4", "COREBOOT", 0x20110725)
{
/*
- * iASL Warning: There were 3 external control methods found during
- * disassembly, but additional ACPI tables to resolve these externals
- * were not specified. This resulting disassembler output file may not
- * compile because the disassembler did not know how many arguments
- * to assign to these methods. To specify the tables needed to resolve
- * external control method references, the -e option can be used to
- * specify the filenames. Note: SSDTs can be dynamically loaded at
+ * iASL Warning: There were 2 external control methods found during
+ * disassembly, but only 0 were resolved (2 unresolved). Additional
+ * ACPI tables may be required to properly disassemble the code. This
+ * resulting disassembler output file may not compile because the
+ * disassembler did not know how many arguments to assign to the
+ * unresolved methods. Note: SSDTs can be dynamically loaded at
* runtime and may or may not be available via the host OS.
+ *
+ * To specify the tables needed to resolve external control method
+ * references, the -e option can be used to specify the filenames.
* Example iASL invocations:
* iasl -e ssdt1.aml ssdt2.aml ssdt3.aml -d dsdt.aml
* iasl -e dsdt.aml ssdt2.aml -d ssdt1.aml
@@ -45,19 +47,15 @@
* because the disassembler had to guess at the number of arguments
* required for each:
*/
- External (G***, MethodObj) // Warning: Unresolved method, guessing 4 arguments
- External (NVSA, MethodObj) // Warning: Unresolved method, guessing 1 arguments
- External (RS**, MethodObj) // Warning: Unresolved method, guessing 6 arguments
-
- External (_PR_.CP00, UnknownObj)
+ External (_PR_.CP00, DeviceObj)
External (_PR_.CP00._PSS, UnknownObj)
- External (_PR_.CP01, UnknownObj)
- External (_PR_.CP02, UnknownObj)
- External (_PR_.CP03, UnknownObj)
- External (_PR_.CP04, UnknownObj)
- External (_PR_.CP05, UnknownObj)
- External (_PR_.CP06, UnknownObj)
- External (_PR_.CP07, UnknownObj)
+ External (_PR_.CP01, DeviceObj)
+ External (_PR_.CP02, DeviceObj)
+ External (_PR_.CP03, DeviceObj)
+ External (_PR_.CP04, DeviceObj)
+ External (_PR_.CP05, DeviceObj)
+ External (_PR_.CP06, DeviceObj)
+ External (_PR_.CP07, DeviceObj)
External (_SB_.PCI0.LPCB.SIO_.SMBX.LPCB.LNKA, UnknownObj)
External (_SB_.PCI0.LPCB.SIO_.SMBX.LPCB.LNKB, UnknownObj)
External (_SB_.PCI0.LPCB.SIO_.SMBX.LPCB.LNKC, UnknownObj)
@@ -68,7 +66,11 @@
External (_SB_.PCI0.LPCB.SIO_.SMBX.LPCB.LNKH, UnknownObj)
External (_SB_.PCI0.LPCB.SIO_.SMBX.XHC_.POSC, UnknownObj)
External (_TZ_.SKIN, UnknownObj)
- External (LCD0, UnknownObj)
+ External (_TZ_.THRM, UnknownObj)
+ External (G***, MethodObj) // Warning: Unknown method, guessing 4 arguments
+ External (LCD0, DeviceObj)
+ External (NVSA, UnknownObj)
+ External (RS**, MethodObj) // Warning: Unknown method, guessing 6 arguments
OperationRegion (APMP, SystemIO, 0xB2, 0x02)
Field (APMP, ByteAcc, NoLock, Preserve)
@@ -164,152 +166,154 @@
Name (PICM, Zero)
Name (DSEN, One)
- OperationRegion (GNVS, SystemMemory, NVSA (0x0F00), Field (GNVS, ByteAcc, NoLock, Preserve)
- {
- OSYS, 16,
- SMIF, 8,
- PRM0, 8,
- PRM1, 8,
- SCIF, 8,
- PRM2, 8,
- PRM3, 8,
- LCKF, 8,
- PRM4, 8,
- PRM5, 8,
- P80D, 32,
- LIDS, 8,
- PWRS, 8,
- TLVL, 8,
- FLVL, 8,
- TCRT, 8,
- TPSV, 8,
- TMAX, 8,
- F0OF, 8,
- F0ON, 8,
- F0PW, 8,
- F1OF, 8,
- F1ON, 8,
- F1PW, 8,
- F2OF, 8,
- F2ON, 8,
- F2PW, 8,
- F3OF, 8,
- F3ON, 8,
- F3PW, 8,
- F4OF, 8,
- F4ON, 8,
- F4PW, 8,
- TMPS, 8,
- Offset (0x28),
- APIC, 8,
- MPEN, 8,
- PCP0, 8,
- PCP1, 8,
- PPCM, 8,
- PCNT, 8,
- Offset (0x32),
- NATP, 8,
- S5U0, 8,
- S5U1, 8,
- S3U0, 8,
- S3U1, 8,
- S33G, 8,
- CMEM, 32,
- IGDS, 8,
- TLST, 8,
- CADL, 8,
- PADL, 8,
- CSTE, 16,
- NSTE, 16,
- SSTE, 16,
- NDID, 8,
- DID1, 32,
- DID2, 32,
- DID3, 32,
- DID4, 32,
- DID5, 32,
- Offset (0x64),
- BLCS, 8,
- BRTL, 8,
- ODDS, 8,
- Offset (0x6E),
- ALSE, 8,
- ALAF, 8,
- LLOW, 8,
- LHIH, 8,
- Offset (0x78),
- EMAE, 8,
- EMAP, 16,
- EMAL, 16,
- Offset (0x82),
- MEFE, 8,
- Offset (0x8C),
- TPMP, 8,
- TPME, 8,
- Offset (0x96),
- GTF0, 56,
- GTF1, 56,
- GTF2, 56,
- IDEM, 8,
- IDET, 8,
- Offset (0xB2),
- XHCI, 8,
- Offset (0xB4),
- ASLB, 32,
- IBTT, 8,
- IPAT, 8,
- ITVF, 8,
- ITVM, 8,
- IPSC, 8,
- IBLC, 8,
- IBIA, 8,
- ISSC, 8,
- I409, 8,
- I509, 8,
- I609, 8,
- I709, 8,
- IDMM, 8,
- IDMS, 8,
- IF1E, 8,
- HVCO, 8,
- NXD1, 32,
- NXD2, 32,
- NXD3, 32,
- NXD4, 32,
- NXD5, 32,
- NXD6, 32,
- NXD7, 32,
- NXD8, 32,
- ISCI, 8,
- PAVP, 8,
- Offset (0xEB),
- OSCC, 8,
- NPCE, 8,
- PLFL, 8,
- BREV, 8,
- DPBM, 8,
- DPCM, 8,
- DPDM, 8,
- ALFP, 8,
- IMON, 8,
- MMIO, 8,
- Offset (0x100),
- VBT0, 32,
- VBT1, 32,
- VBT2, 32,
- VBT3, 16,
- VBT4, 2048,
- VBT5, 512,
- VBT6, 512,
- VBT7, 32,
- VBT8, 32,
- VBT9, 32,
- CHVD, 24576,
- VBTA, 32,
- MEHH, 256,
- RMOB, 32,
- RMOL, 32
- })
+ OperationRegion (GNVS, SystemMemory, NVSA, 0x0F00)
+ Field (GNVS, ByteAcc, NoLock, Preserve)
+ {
+ OSYS, 16,
+ SMIF, 8,
+ PRM0, 8,
+ PRM1, 8,
+ SCIF, 8,
+ PRM2, 8,
+ PRM3, 8,
+ LCKF, 8,
+ PRM4, 8,
+ PRM5, 8,
+ P80D, 32,
+ LIDS, 8,
+ PWRS, 8,
+ TLVL, 8,
+ FLVL, 8,
+ TCRT, 8,
+ TPSV, 8,
+ TMAX, 8,
+ F0OF, 8,
+ F0ON, 8,
+ F0PW, 8,
+ F1OF, 8,
+ F1ON, 8,
+ F1PW, 8,
+ F2OF, 8,
+ F2ON, 8,
+ F2PW, 8,
+ F3OF, 8,
+ F3ON, 8,
+ F3PW, 8,
+ F4OF, 8,
+ F4ON, 8,
+ F4PW, 8,
+ TMPS, 8,
+ Offset (0x28),
+ APIC, 8,
+ MPEN, 8,
+ PCP0, 8,
+ PCP1, 8,
+ PPCM, 8,
+ PCNT, 8,
+ Offset (0x32),
+ NATP, 8,
+ S5U0, 8,
+ S5U1, 8,
+ S3U0, 8,
+ S3U1, 8,
+ S33G, 8,
+ CMEM, 32,
+ IGDS, 8,
+ TLST, 8,
+ CADL, 8,
+ PADL, 8,
+ CSTE, 16,
+ NSTE, 16,
+ SSTE, 16,
+ NDID, 8,
+ DID1, 32,
+ DID2, 32,
+ DID3, 32,
+ DID4, 32,
+ DID5, 32,
+ Offset (0x64),
+ BLCS, 8,
+ BRTL, 8,
+ ODDS, 8,
+ Offset (0x6E),
+ ALSE, 8,
+ ALAF, 8,
+ LLOW, 8,
+ LHIH, 8,
+ Offset (0x78),
+ EMAE, 8,
+ EMAP, 16,
+ EMAL, 16,
+ Offset (0x82),
+ MEFE, 8,
+ Offset (0x8C),
+ TPMP, 8,
+ TPME, 8,
+ Offset (0x96),
+ GTF0, 56,
+ GTF1, 56,
+ GTF2, 56,
+ IDEM, 8,
+ IDET, 8,
+ Offset (0xB2),
+ XHCI, 8,
+ Offset (0xB4),
+ ASLB, 32,
+ IBTT, 8,
+ IPAT, 8,
+ ITVF, 8,
+ ITVM, 8,
+ IPSC, 8,
+ IBLC, 8,
+ IBIA, 8,
+ ISSC, 8,
+ I409, 8,
+ I509, 8,
+ I609, 8,
+ I709, 8,
+ IDMM, 8,
+ IDMS, 8,
+ IF1E, 8,
+ HVCO, 8,
+ NXD1, 32,
+ NXD2, 32,
+ NXD3, 32,
+ NXD4, 32,
+ NXD5, 32,
+ NXD6, 32,
+ NXD7, 32,
+ NXD8, 32,
+ ISCI, 8,
+ PAVP, 8,
+ Offset (0xEB),
+ OSCC, 8,
+ NPCE, 8,
+ PLFL, 8,
+ BREV, 8,
+ DPBM, 8,
+ DPCM, 8,
+ DPDM, 8,
+ ALFP, 8,
+ IMON, 8,
+ MMIO, 8,
+ Offset (0x100),
+ VBT0, 32,
+ VBT1, 32,
+ VBT2, 32,
+ VBT3, 16,
+ VBT4, 2048,
+ VBT5, 512,
+ VBT6, 512,
+ VBT7, 32,
+ VBT8, 32,
+ VBT9, 32,
+ CHVD, 24576,
+ VBTA, 32,
+ MEHH, 256,
+ RMOB, 32,
+ RMOL, 32
+ }
+
Method (S3UE, 0, NotSerialized)
{
S3U0 = One
@@ -557,36 +561,30 @@
\_PR.CP07
})
}
- Else
+ ElseIf ((PCNT >= 0x04))
{
- If ((PCNT >= 0x04))
+ Return (Package (0x04)
{
- Return (Package (0x04)
- {
- \_PR.CP00,
- \_PR.CP01,
- \_PR.CP02,
- \_PR.CP03
- })
- }
- Else
+ \_PR.CP00,
+ \_PR.CP01,
+ \_PR.CP02,
+ \_PR.CP03
+ })
+ }
+ ElseIf ((PCNT >= 0x02))
+ {
+ Return (Package (0x02)
{
- If ((PCNT >= 0x02))
- {
- Return (Package (0x02)
- {
- \_PR.CP00,
- \_PR.CP01
- })
- }
- Else
- {
- Return (Package (0x01)
- {
- \_PR.CP00
- })
- }
- }
+ \_PR.CP00,
+ \_PR.CP01
+ })
+ }
+ Else
+ {
+ Return (Package (0x01)
+ {
+ \_PR.CP00
+ })
}
}
@@ -842,7 +840,7 @@
Local1 = SizeOf (\_PR.CP00._PSS)
While ((Local0 < Local1))
{
- Local2 = (DerefOf (Index (DerefOf (Index (\_PR.CP00._PSS, Local0)), 0x04)) >> 0x08)
+ Local2 = (DerefOf (DerefOf (\_PR.CP00._PSS [Local0]) [0x04]) >> 0x08)
If ((Local2 == Arg0))
{
Return ((Local0 - One))
@@ -1149,17 +1147,17 @@
Local2 = 0x02
While ((Local2 < (SizeOf (BRIG) - One)))
{
- Local3 = DerefOf (Index (BRIG, Local2))
+ Local3 = DerefOf (BRIG [Local2])
Local3 = ((Local3 * Local1) / 0x64)
If ((Local0 <= Local3))
{
- Return (DerefOf (Index (BRIG, Local2)))
+ Return (DerefOf (BRIG [Local2]))
}
Local2 += One
}
- Return (DerefOf (Index (BRIG, Local2)))
+ Return (DerefOf (BRIG [Local2]))
}
Name (BRCT, Zero)
@@ -1199,7 +1197,7 @@
Local0--
}
- XBCM (DerefOf (Index (BRIG, Local0)))
+ XBCM (DerefOf (BRIG [Local0]))
}
}
@@ -1217,7 +1215,7 @@
Local0++
}
- XBCM (DerefOf (Index (BRIG, Local0)))
+ XBCM (DerefOf (BRIG [Local0]))
}
}
@@ -1760,71 +1758,59 @@
Return (IQAP) /* \_SB_.PCI0.IRQM.IQAP */
}
}
- Else
+ ElseIf ((Match (Package (0x02)
+ {
+ 0x02,
+ 0x06
+ }, MEQ, _T_0, MTR, Zero, Zero) != Ones))
{
- If ((Match (Package (0x02)
- {
- 0x02,
- 0x06
- }, MEQ, _T_0, MTR, Zero, Zero) != Ones))
+ If (PICM)
{
- If (PICM)
- {
- Return (IQBA) /* \_SB_.PCI0.IRQM.IQBA */
- }
- Else
- {
- Return (IQBP) /* \_SB_.PCI0.IRQM.IQBP */
- }
+ Return (IQBA) /* \_SB_.PCI0.IRQM.IQBA */
}
Else
{
- If ((Match (Package (0x02)
- {
- 0x03,
- 0x07
- }, MEQ, _T_0, MTR, Zero, Zero) != Ones))
- {
- If (PICM)
- {
- Return (IQCA) /* \_SB_.PCI0.IRQM.IQCA */
- }
- Else
- {
- Return (IQCP) /* \_SB_.PCI0.IRQM.IQCP */
- }
- }
- Else
- {
- If ((Match (Package (0x02)
- {
- 0x04,
- 0x08
- }, MEQ, _T_0, MTR, Zero, Zero) != Ones))
- {
- If (PICM)
- {
- Return (IQDA) /* \_SB_.PCI0.IRQM.IQDA */
- }
- Else
- {
- Return (IQDP) /* \_SB_.PCI0.IRQM.IQDP */
- }
- }
- Else
- {
- If (PICM)
+ Return (IQBP) /* \_SB_.PCI0.IRQM.IQBP */
+ }
+ }
+ ElseIf ((Match (Package (0x02)
{
- Return (IQDA) /* \_SB_.PCI0.IRQM.IQDA */
- }
- Else
+ 0x03,
+ 0x07
+ }, MEQ, _T_0, MTR, Zero, Zero) != Ones))
+ {
+ If (PICM)
+ {
+ Return (IQCA) /* \_SB_.PCI0.IRQM.IQCA */
+ }
+ Else
+ {
+ Return (IQCP) /* \_SB_.PCI0.IRQM.IQCP */
+ }
+ }
+ ElseIf ((Match (Package (0x02)
{
- Return (IQDP) /* \_SB_.PCI0.IRQM.IQDP */
- }
- }
- }
+ 0x04,
+ 0x08
+ }, MEQ, _T_0, MTR, Zero, Zero) != Ones))
+ {
+ If (PICM)
+ {
+ Return (IQDA) /* \_SB_.PCI0.IRQM.IQDA */
+ }
+ Else
+ {
+ Return (IQDP) /* \_SB_.PCI0.IRQM.IQDP */
}
}
+ ElseIf (PICM)
+ {
+ Return (IQDA) /* \_SB_.PCI0.IRQM.IQDA */
+ }
+ Else
+ {
+ Return (IQDP) /* \_SB_.PCI0.IRQM.IQDP */
+ }
Break
}
@@ -2162,19 +2148,13 @@
{
Local1 = 0x0F
}
- Else
+ ElseIf ((Local0 == 0x02))
{
- If ((Local0 == 0x02))
- {
- Local1 = 0x03
- }
- Else
- {
- If ((Local0 == 0x03))
- {
- Local1 = Zero
- }
- }
+ Local1 = 0x03
+ }
+ ElseIf ((Local0 == 0x03))
+ {
+ Local1 = Zero
}
Local0 = (RPM3 & 0xFFFFFFF0)
@@ -3018,13 +2998,13 @@
Method (_BIF, 0, Serialized) // _BIF: Battery Information
{
- Index (PBIF, One) = SWAB (BTDA)
- Index (PBIF, 0x02) = SWAB (BTDF)
- Index (PBIF, 0x04) = SWAB (BTDV)
- Index (PBIF, 0x06) = SWAB (BTDL)
- Index (PBIF, 0x09) = BATM /* \BATM */
- Index (PBIF, 0x0A) = BATS /* \BATS */
- Index (PBIF, 0x0C) = BATV /* \BATV */
+ PBIF [One] = SWAB (BTDA)
+ PBIF [0x02] = SWAB (BTDF)
+ PBIF [0x04] = SWAB (BTDV)
+ PBIF [0x06] = SWAB (BTDL)
+ PBIF [0x09] = BATM /* \BATM */
+ PBIF [0x0A] = BATS /* \BATS */
+ PBIF [0x0C] = BATV /* \BATV */
Return (PBIF) /* \_SB_.PCI0.LPCB.EC0_.BAT0.PBIF */
}
@@ -3043,7 +3023,7 @@
Local4 = (Local0 & 0x04)
Local1 |= Local4
- Index (PBST, Zero) = Local1
+ PBST [Zero] = Local1
If ((Local1 != BSTP))
{
BSTP = Local1
@@ -3057,7 +3037,7 @@
Local1++
}
- Index (PBST, One) = Local1
+ PBST [One] = Local1
Local1 = SWAB (BTRA)
If (((Local1 != 0xFFFFFFFF) && (Local1 >= 0x8000)))
{
@@ -3076,8 +3056,8 @@
}
}
- Index (PBST, 0x02) = Local1
- Index (PBST, 0x03) = SWAB (BTVO)
+ PBST [0x02] = Local1
+ PBST [0x03] = SWAB (BTVO)
Return (PBST) /* \_SB_.PCI0.LPCB.EC0_.BAT0.PBST */
}
}
@@ -4054,7 +4034,8 @@
Name (_S5, Package (0x04) // _S5_: S5 System State
{
0x07,
- 0x07,
+ Zero,
+ Zero,
Zero,
Zero
})
Updated by Idwer Vollering about 8 years ago
Added the author and reviewers from #12817 to the watchers list.
Updated by Idwer Vollering about 8 years ago
This is the offending commit: https://github.com/acpica/acpica/commit/5be7dc4d0d69b2953d156f5bc4d3e8a65a390837
Updated by Martin Roth about 8 years ago
Interestingly, ALL of the differences are at the top of the file. Byte 0xCA in the new file corresponds to byte 0x24 in the old file, and there are NO differences beyond that point.
Updated by Martin Roth about 8 years ago
The difference in output is the new IASL version is outputting some 'External' instructions - hints for disassembly. These are supposed to be wrapped in an 'if 0' statement so that they have no effect on runtime parsing.
But something is getting really confused, and I'm not sure that it's related to the above.
When doing the disassembly, we see:
coreboot toolchain v1.38 April 3rd, 2016
Input file build/dsdt.aml, Length 0x35A0 (13728) bytes
ACPI: DSDT 0x0000000000000000 00359C (v02 COREv4 COREBOOT 20110725 INTL 20160318)
Pass 1 parse of [DSDT]
ACPI Warning: Invalid character(s) in name (0x0D115352), repaired: RS**
ACPI Warning: Invalid character(s) in name (0x0A000147), repaired: G***
These names are actually the second half of _CRS and the first half of the following instruction (52 53 11 0D):
2390: Name (_CRS, ResourceTemplate()
00003081: 08 5F 43 52 53 11 0D 0A "._CRS..."
and a portion of an IO decode instruction (47 01 00 0A):
dsdt.aml 2392: IO (Decode16, 0xa00, 0xa00 + 0x34, 0x01, 0x34)
Optimize 1015 - ^ Constant expression evaluated and reduced (ADD)
0000308A: 47 01 00 0A 00 00 01 34 "G......4"
This same error happens with code compiled with either the old or new compiler.
Updated by Martin Roth about 8 years ago
The issue is caused by src/superio/smsc/mec1308/acpi/superio.asl
Change:
Name (_CRS, ResourceTemplate()
{
IO (Decode16, SIO_SMBX_IO0, SIO_SMBX_IO0 + 0x34, 0x01, 0x34)
})
Name (_PRS, ResourceTemplate()
{
IO (Decode16, SIO_SMBX_IO0, SIO_SMBX_IO0 + 0x34, 0x01, 0x34)
})
to
#define SIO_SMBX_IO034 0xa34
Name (_CRS, ResourceTemplate()
{
IO (Decode16, SIO_SMBX_IO0, SIO_SMBX_IO034, 0x01, 0x34)
})
Name (_PRS, ResourceTemplate()
{
IO (Decode16, SIO_SMBX_IO0, SIO_SMBX_IO034, 0x01, 0x34)
})
And everything's happy.
I'll submit a patch.
Updated by Martin Roth about 8 years ago
Duncan pointed out that the AddressMax value is actually "The maximum acceptable starting address for the I/O range." Since this is a fixed address, AddressMin and AddressMax should be the same.
This fix has been submitted as https://review.coreboot.org/#/c/14343/
A check to detect these sorts of issues has been submitted as https://review.coreboot.org/#/c/14340/
This was demonstrated to catch the error (Lumpy was actually the only victim), and demonstrates that the fix for lumpy works.
There is still the outstanding question of why this ASL code created a problem. IASL does support addition inside ASL, but obviously it failed in this case for some reason, without generating an error. This should be debugged further to help fix the issue there.
The good ASL, then the bad: (Note that this is not the "Good" code that we ended up using, just the code similar to the initial code)
IO (Decode16, SIOSMBX_IO0, 0xA34, 0x01, 0x34)
IO (Decode16, SIOSMBX_IO0, 0xA00 + 0x34, 0x01, 0x34)
The good byte code, then the bad
0A 0A 47 01 00 0A 34 0A 01 34 79 00
0A 0A 47 01 00 0A 00 00 01 34 79 00 34 0A