Bug 1332449 - iasl couldn't parse the expected aml
Summary: iasl couldn't parse the expected aml
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: acpica-tools
Version: rawhide
Hardware: s390x
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Al Stone
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords: Reopened
Depends On:
Blocks: ZedoraTracker PPCTracker
TreeView+ depends on / blocked
 
Reported: 2016-05-03 08:49 UTC by Dan Horák
Modified: 2017-10-18 13:10 UTC (History)
4 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2017-10-18 13:10:36 UTC


Attachments (Terms of Use)
Example AML file (120 bytes, application/octet-stream)
2017-10-18 12:17 UTC, Daniel Berrange
no flags Details

Description Dan Horák 2016-05-03 08:49:20 UTC
We are seeing some warnings during qemu builds when running test-suite for x86 system emulator on big endian arches (ppc64, s390x). The qemu guys point to some big endian related issue in bug 1330174 and resulting fix in http://patchwork.ozlabs.org/patch/616730/

from qemu build.log (http://s390.koji.fedoraproject.org/koji/taskinfo?taskID=2209619 or similar issue in qemu 2.5)
...
TEST: tests/bios-tables-test... (pid=27094)
  /i386/acpi/piix4/tcg:                                                Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml
OK
  /i386/acpi/piix4/tcg/bridge:                                         Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml
OK
  /i386/acpi/q35/tcg:                                                  Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml
OK
  /i386/acpi/q35/tcg/bridge:                                           Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml
OK
PASS: tests/bios-tables-test
...

Version-Release number of selected component (if applicable):
acpica-tools-20160318-1.fc24

Comment 1 Al Stone 2016-10-04 22:27:29 UTC
Is this still an issue?

Comment 2 Dan Horák 2016-10-05 06:16:34 UTC
It is, please see build.log of qemu-2.7.0-1.fc26 that uses acpica-tools-20160831-1.fc26 at http://s390.koji.fedoraproject.org/koji/buildinfo?buildID=429201

Comment 3 Al Stone 2016-10-06 00:37:54 UTC
(In reply to Dan Horák from comment #2)
> It is, please see build.log of qemu-2.7.0-1.fc26 that uses
> acpica-tools-20160831-1.fc26 at
> http://s390.koji.fedoraproject.org/koji/buildinfo?buildID=429201

I looked at the upstream qemu source to see if I could figure out exactly what AML is being compared and how it is being compared.  I started with this output from the build log:

TEST: tests/bios-tables-test... (pid=55920)
  /i386/acpi/piix4/tcg:                                                
Looking for expected file 'tests/acpi-test-data/pc/DSDT'
Using expected file 'tests/acpi-test-data/pc/DSDT'
Looking for expected file 'tests/acpi-test-data/pc/APIC'
Using expected file 'tests/acpi-test-data/pc/APIC'
Looking for expected file 'tests/acpi-test-data/pc/HPET'
Using expected file 'tests/acpi-test-data/pc/HPET'
Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml
Warning! iasl couldn't parse the expected aml

The first problem I see is that the error message from the test is not correct; if I look at the code in test_acpi_asl() in bios-tables-test.c, where the message is printed out, it is actually doing a string compare of the ASL resulting from decompiling the AML, not the AML itself.  The message is misleading and should probably be corrected.

The second problem is that the build log does not contain the result of the call to dump_aml_files() that shows up early in test_acpi_asl().  Unfortunately, what I need to debug this further is to patch up the test case something like this:

--- bios-tables-test.c	2016-10-05 18:30:48.144393043 -0600
+++ bios-tables-test-patched.c	2016-10-05 18:33:28.585536556 -0600
@@ -523,6 +523,7 @@
     memset(&exp_data, 0, sizeof(exp_data));
     exp_data.tables = load_expected_aml(data);
     dump_aml_files(data, false);
+    dump_aml_files(exp_data, false);
     for (i = 0; i < data->tables->len; ++i) {
         GString *asl, *exp_asl;
 
@@ -531,9 +532,11 @@
 
         err = load_asl(data->tables, sdt);
         asl = normalize_asl(sdt->asl);
+	fprintf(stderr, "actual asl ==>\n%s\n", asl)
 
         exp_err = load_asl(exp_data.tables, exp_sdt);
         exp_asl = normalize_asl(exp_sdt->asl);
+	fprintf(stderr, "expected asl ==>\n%s\n", exp_asl)
 
         /* TODO: check for warnings */
         g_assert(!err || exp_err);

Or something like it; I need to have the binary AML for the actual and expected cases, and the resulting decompiled ASL from the actual and expected cases to be able to see what's actually wrong.  The DSDT being used is over 2000 lines; this info will help narrow it down.

Comment 4 Al Stone 2016-10-11 15:22:52 UTC
Just an FYI: I've already found and fixed five different big-endian problems at this point.  There are clearly still more (e.g., a disassembly of the qemu test DSDT still is not working correctly).  Stay tuned.

Comment 5 Al Stone 2016-10-11 15:23:10 UTC
Just an FYI: I've already found and fixed five different big-endian problems at this point.  There are clearly still more (e.g., a disassembly of the qemu test DSDT still is not working correctly).  Stay tuned.

Comment 6 Al Stone 2017-03-02 22:09:18 UTC
A quick update: the rawhide version is doing fairly well at this point; it is however still has some big-endian issues, especially in the disassembler part of iasl.  Rawhide has been brought up to 20170119, and I will soon update it to 20170224 as a base version.

Comment 7 Dan Horák 2017-03-03 10:51:46 UTC
Thanks for keeping working on that.

Comment 8 Fedora End Of Life 2017-07-25 20:39:46 UTC
This message is a reminder that Fedora 24 is nearing its end of life.
Approximately 2 (two) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 24. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '24'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version'
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not
able to fix it before Fedora 24 is end of life. If you would still like
to see this bug fixed and are able to reproduce it against a later version
of Fedora, you are encouraged  change the 'version' to a later Fedora
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's
lifetime, sometimes those efforts are overtaken by events. Often a
more recent Fedora release includes newer upstream software that fixes
bugs or makes them obsolete.

Comment 9 Fedora End Of Life 2017-08-08 14:23:59 UTC
Fedora 24 changed to end-of-life (EOL) status on 2017-08-08. Fedora 24 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.

Comment 10 Daniel Berrange 2017-10-12 08:32:16 UTC
Re-opening, because QEMU tests are still failing with iasl on big endian arches with current rawhide.

Comment 11 Daniel Berrange 2017-10-12 12:16:58 UTC
I added some debugging in QEMU to get the stdout + stderr from iasl when it fails:

The ARGV were

iasl -p /tmp/asl-AM4U7Y.dsl -d /tmp/aml-2F4U7Y 


Stdout is

Intel ACPI Component Architecture
ASL+ Optimizing Compiler/Disassembler version 20170831
Copyright (c) 2000 - 2017 Intel Corporation
Could not get ACPI tables from /tmp/aml-2F4U7Y, AE_BAD_HEADER


Stderr is

Input file /tmp/aml-2F4U7Y, Length 0x13EA (5098) bytes
Table [TDSD] is too long for file - needs: 0xEA130000, remaining in file: 0x13EA

So that looks like an endianess bug in reading the length here

Comment 12 Al Stone 2017-10-17 21:57:40 UTC
(In reply to Daniel Berrange from comment #11)
> I added some debugging in QEMU to get the stdout + stderr from iasl when it
> fails:
> 
> The ARGV were
> 
> iasl -p /tmp/asl-AM4U7Y.dsl -d /tmp/aml-2F4U7Y 
> 
> 
> Stdout is
> 
> Intel ACPI Component Architecture
> ASL+ Optimizing Compiler/Disassembler version 20170831
> Copyright (c) 2000 - 2017 Intel Corporation
> Could not get ACPI tables from /tmp/aml-2F4U7Y, AE_BAD_HEADER
> 
> 
> Stderr is
> 
> Input file /tmp/aml-2F4U7Y, Length 0x13EA (5098) bytes
> Table [TDSD] is too long for file - needs: 0xEA130000, remaining in file:
> 0x13EA
> 
> So that looks like an endianess bug in reading the length here

The code performing the check that results in the message above is this chunk:

    FileSize = CmGetFileSize (File);
    ACPI_MOVE_32_TO_32(&TableLength, &TableHeader.Length);
    if (TableLength > (UINT32) (FileSize - TableOffset))
    {
        fprintf (stderr, "Table [%4.4s] is too long for file - "
            "needs: 0x%.2X, remaining in file: 0x%.2X\n",
            TableHeader.Signature, TableLength,
            (UINT32) (FileSize - TableOffset));
        return (AE_BAD_HEADER);
    }

I think this is actually correct since I have a couple of hundred examples of DSDTs that seem to compile and disassemble just fine on s390x.

But, this is why I need a copy of the input and output files to iasl; based on what the code is doing, it looks like the AML file being disassembled has the length stored in the file in big-endian form, which would be incorrect -- all AML files must be little-endian.  So, at this point, I can't tell if the AML was compiled incorrectly from source or not.

Comment 13 Daniel Berrange 2017-10-18 12:17 UTC
Created attachment 1340162 [details]
Example AML file

I think you're onto something there. I finally managed to exfiltrate one of the AML files from the PPC64 build root and run it from iasl on an x86_64 host. I get the same error message about bogus length.

So it looks like this remaining problem may well be in the way QEMU generates the AML file in the first place.

Comment 14 Daniel Berrange 2017-10-18 13:10:36 UTC
I've checked the QEMU AML files further, comparing their content from an ppc64 build and an x86_64 build and they're clearly different. So I think the remaining blame here is firmly on the QEMU side.

https://bugs.launchpad.net/qemu/+bug/1724570

I'll re-close this on the assumption that iasl is working fine in rawhide unless I find further info to the contrary.


Note You need to log in before you can comment on or make changes to this bug.