[Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Marcel Apfelbaum]

There is an issue with iasl on big endian machines: It
cannot disassemble acpi tables taken from little endian
machines, so we cannot check the expected tables.

Do not run iasl on those machines until this
problem is solved by the acpica community.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
V2 -> V3:
 Addressed Michael S. Tsirkin's review:
 - tests don't need to re-run detection, use configure
   to figure out if it is an LE machine.

V1 -> V2:
 Addressed an offline tip for a much cleaner
 macro line, thanks!

 configure | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index aae617e..2c0a3b2 100755
--- a/configure
+++ b/configure
@@ -4656,7 +4656,10 @@ else
 fi
 echo "PYTHON=$python" >> $config_host_mak
 echo "CC=$cc" >> $config_host_mak
-if $iasl -h > /dev/null 2>&1; then
+# All known versions of iasl on BE machines are broken.
+# TODO: add detection code once a non-broken version makes an appearance.
+if ($iasl -h > /dev/null 2>&1) &&
+   (lscpu | grep "Byte Order" | grep --quiet "Little Endian" ); then
   echo "IASL=$iasl" >> $config_host_mak
 fi
 echo "CC_I386=$cc_i386" >> $config_host_mak
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Paolo Bonzini]

Il 20/03/2014 22:14, Marcel Apfelbaum ha scritto:
> +# All known versions of iasl on BE machines are broken.
> +# TODO: add detection code once a non-broken version makes an appearance.
> +if ($iasl -h > /dev/null 2>&1) &&
> +   (lscpu | grep "Byte Order" | grep --quiet "Little Endian" ); then

lscpu is not portable.

Paolo

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Marcel Apfelbaum]

On Thu, 2014-03-20 at 22:57 +0100, Paolo Bonzini wrote:
> Il 20/03/2014 22:14, Marcel Apfelbaum ha scritto:
> > +# All known versions of iasl on BE machines are broken.
> > +# TODO: add detection code once a non-broken version makes an appearance.
> > +if ($iasl -h > /dev/null 2>&1) &&
> > +   (lscpu | grep "Byte Order" | grep --quiet "Little Endian" ); then
> 
> lscpu is not portable.
I am open to suggestions...
I'll try to come up with something else.

Thanks,
Marcel
> 
> Paolo

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Marcel Apfelbaum]

On Thu, 2014-03-20 at 22:57 +0100, Paolo Bonzini wrote:
> Il 20/03/2014 22:14, Marcel Apfelbaum ha scritto:
> > +# All known versions of iasl on BE machines are broken.
> > +# TODO: add detection code once a non-broken version makes an appearance.
> > +if ($iasl -h > /dev/null 2>&1) &&
> > +   (lscpu | grep "Byte Order" | grep --quiet "Little Endian" ); then
> 
> lscpu is not portable.
I am open to suggestions...
I'll try to come up with something else.

Thanks,
Marcel

> 
> Paolo

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Peter Maydell]

On 20 March 2014 22:06, Marcel Apfelbaum <marcel.a@redhat.com> wrote:
> On Thu, 2014-03-20 at 22:57 +0100, Paolo Bonzini wrote:
>> Il 20/03/2014 22:14, Marcel Apfelbaum ha scritto:
>> > +# All known versions of iasl on BE machines are broken.
>> > +# TODO: add detection code once a non-broken version makes an appearance.
>> > +if ($iasl -h > /dev/null 2>&1) &&
>> > +   (lscpu | grep "Byte Order" | grep --quiet "Little Endian" ); then
>>
>> lscpu is not portable.
> I am open to suggestions...

echo "trivial iasl source" | iasl --compile-options | iasl
--disassemble-options | grep "error"

Fill in the handwaving with actual syntax ;-)

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Marcel Apfelbaum]

On Thu, 2014-03-20 at 22:17 +0000, Peter Maydell wrote:
> On 20 March 2014 22:06, Marcel Apfelbaum <marcel.a@redhat.com> wrote:
> > On Thu, 2014-03-20 at 22:57 +0100, Paolo Bonzini wrote:
> >> Il 20/03/2014 22:14, Marcel Apfelbaum ha scritto:
> >> > +# All known versions of iasl on BE machines are broken.
> >> > +# TODO: add detection code once a non-broken version makes an appearance.
> >> > +if ($iasl -h > /dev/null 2>&1) &&
> >> > +   (lscpu | grep "Byte Order" | grep --quiet "Little Endian" ); then
> >>
> >> lscpu is not portable.
> > I am open to suggestions...
> 
> echo "trivial iasl source" | iasl --compile-options | iasl
> --disassemble-options | grep "error"
> 
> Fill in the handwaving with actual syntax ;-)
Thanks Peter!

Problem with this solution is that if we start from the source,
it will compile into a *wrong* AML (e.g header length will be BE and not LE),
then the disassemble will *succeed* (!!!).
However, the expected AML file will be in the right format and fail :(.

I can use one of the expected AML files, but it would not be elegant
to use a test file in the configuration script.
What about Laszlo's idea? Would something like:
X=$(printf '\336\255\276\357' | od -A n -t x4) be acceptable ?

Thanks,
Marcel

> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Peter Maydell]

On 20 March 2014 22:41, Marcel Apfelbaum <marcel.a@redhat.com> wrote:
> On Thu, 2014-03-20 at 22:17 +0000, Peter Maydell wrote:
>> echo "trivial iasl source" | iasl --compile-options | iasl
>> --disassemble-options | grep "error"
>>
>> Fill in the handwaving with actual syntax ;-)

> Problem with this solution is that if we start from the source,
> it will compile into a *wrong* AML (e.g header length will be BE and not LE),
> then the disassemble will *succeed* (!!!).
> However, the expected AML file will be in the right format and fail :(.

Well, grep for 'some integer that comes out wrong due
to the iasl bug', if it doesn't always result in an error
like the case you reported to the iasl mailing list.

You need to write this code at some point because we do
need to be able to configure-time detect whether we have
a working iasl or a broken one, once fixed ones get out
into the wild. It doesn't seem like it ought to be that hard
to just do it right to start with...

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Michael S. Tsirkin]

On Thu, Mar 20, 2014 at 11:03:04PM +0000, Peter Maydell wrote:
> On 20 March 2014 22:41, Marcel Apfelbaum <marcel.a@redhat.com> wrote:
> > On Thu, 2014-03-20 at 22:17 +0000, Peter Maydell wrote:
> >> echo "trivial iasl source" | iasl --compile-options | iasl
> >> --disassemble-options | grep "error"
> >>
> >> Fill in the handwaving with actual syntax ;-)
> 
> > Problem with this solution is that if we start from the source,
> > it will compile into a *wrong* AML (e.g header length will be BE and not LE),
> > then the disassemble will *succeed* (!!!).
> > However, the expected AML file will be in the right format and fail :(.
> 
> Well, grep for 'some integer that comes out wrong due
> to the iasl bug', if it doesn't always result in an error
> like the case you reported to the iasl mailing list.
> 
> You need to write this code at some point because we do
> need to be able to configure-time detect whether we have
> a working iasl or a broken one, once fixed ones get out
> into the wild. It doesn't seem like it ought to be that hard
> to just do it right to start with...
> 
> thanks
> -- PMM

Hard to predict the future: it's possible that some distros
will ship partially broken iasl on BE.
We really need to see a working one to be sure ...

-- 
MST

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Peter Maydell]

On 23 March 2014 09:52, Michael S. Tsirkin <mst@redhat.com> wrote:
> Hard to predict the future: it's possible that some distros
> will ship partially broken iasl on BE.
> We really need to see a working one to be sure ...

Well at the moment we have a known broken iasl,
and we have a known OK iasl (in the sense that we
know what the correct behaviour is, it should match
what it does on LE hosts). So we can write a test that
distinguishes the two. In the future if somebody fixes
half the bugs for some reason we will then be able
to tighten up the test if we have to.

But I think a test that correctly distinguishes the
two cases we have currently is better than one
which marks them both as "broken".

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Michael S. Tsirkin]

On Thu, Mar 20, 2014 at 10:17:14PM +0000, Peter Maydell wrote:
> On 20 March 2014 22:06, Marcel Apfelbaum <marcel.a@redhat.com> wrote:
> > On Thu, 2014-03-20 at 22:57 +0100, Paolo Bonzini wrote:
> >> Il 20/03/2014 22:14, Marcel Apfelbaum ha scritto:
> >> > +# All known versions of iasl on BE machines are broken.
> >> > +# TODO: add detection code once a non-broken version makes an appearance.
> >> > +if ($iasl -h > /dev/null 2>&1) &&
> >> > +   (lscpu | grep "Byte Order" | grep --quiet "Little Endian" ); then
> >>
> >> lscpu is not portable.
> > I am open to suggestions...
> 
> echo "trivial iasl source" | iasl --compile-options | iasl
> --disassemble-options | grep "error"
> 
> Fill in the handwaving with actual syntax ;-)
> 
> thanks
> -- PMM

True but whatever test we write, it's possible that a buggy iasl
passes it by luck.
Let's wait for a working iasl to appear on some BE machines.
When that happens, we'll be able to write a robust detection script.


-- 
MST

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Laszlo Ersek]

On 03/20/14 23:06, Marcel Apfelbaum wrote:
> On Thu, 2014-03-20 at 22:57 +0100, Paolo Bonzini wrote:
>> Il 20/03/2014 22:14, Marcel Apfelbaum ha scritto:
>>> +# All known versions of iasl on BE machines are broken.
>>> +# TODO: add detection code once a non-broken version makes an appearance.
>>> +if ($iasl -h > /dev/null 2>&1) &&
>>> +   (lscpu | grep "Byte Order" | grep --quiet "Little Endian" ); then
>>
>> lscpu is not portable.
> I am open to suggestions...
> I'll try to come up with something else.

The printf and od utilities are portable. You can use printf to print a
character string, and use od to group that character string into
multibyte integers in the native byte order.

Example:

  X=$(printf '\336\255\276\357' | od -A n -t x4)

This sets X to " efbeadde" on little endian, and " deadbeef" on big endian.

Laszlo

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Marcel Apfelbaum]

On Thu, 2014-03-20 at 23:26 +0100, Laszlo Ersek wrote:
> On 03/20/14 23:06, Marcel Apfelbaum wrote:
> > On Thu, 2014-03-20 at 22:57 +0100, Paolo Bonzini wrote:
> >> Il 20/03/2014 22:14, Marcel Apfelbaum ha scritto:
> >>> +# All known versions of iasl on BE machines are broken.
> >>> +# TODO: add detection code once a non-broken version makes an appearance.
> >>> +if ($iasl -h > /dev/null 2>&1) &&
> >>> +   (lscpu | grep "Byte Order" | grep --quiet "Little Endian" ); then
> >>
> >> lscpu is not portable.
> > I am open to suggestions...
> > I'll try to come up with something else.
> 
> The printf and od utilities are portable. You can use printf to print a
> character string, and use od to group that character string into
> multibyte integers in the native byte order.
> 
> Example:
> 
>   X=$(printf '\336\255\276\357' | od -A n -t x4)
Hi Laszlo,
Thanks for the help!

I've seen something like that somewhere, but I didn't quite like it.
I was looking for something more elegant as I was *almost* sure
this kind of solution will not pass the reviews :)

But maybe I'll try this, let's see what happens,
Thanks!
Marcel

> 
> This sets X to " efbeadde" on little endian, and " deadbeef" on big endian.
> 
> Laszlo

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Laszlo Ersek]

On 03/20/14 23:33, Marcel Apfelbaum wrote:
> On Thu, 2014-03-20 at 23:26 +0100, Laszlo Ersek wrote:
>> On 03/20/14 23:06, Marcel Apfelbaum wrote:
>>> On Thu, 2014-03-20 at 22:57 +0100, Paolo Bonzini wrote:
>>>> Il 20/03/2014 22:14, Marcel Apfelbaum ha scritto:
>>>>> +# All known versions of iasl on BE machines are broken.
>>>>> +# TODO: add detection code once a non-broken version makes an appearance.
>>>>> +if ($iasl -h > /dev/null 2>&1) &&
>>>>> +   (lscpu | grep "Byte Order" | grep --quiet "Little Endian" ); then
>>>>
>>>> lscpu is not portable.
>>> I am open to suggestions...
>>> I'll try to come up with something else.
>>
>> The printf and od utilities are portable. You can use printf to print a
>> character string, and use od to group that character string into
>> multibyte integers in the native byte order.
>>
>> Example:
>>
>>   X=$(printf '\336\255\276\357' | od -A n -t x4)
> Hi Laszlo,
> Thanks for the help!
> 
> I've seen something like that somewhere, but I didn't quite like it.
> I was looking for something more elegant as I was *almost* sure
> this kind of solution will not pass the reviews :)
> 
> But maybe I'll try this, let's see what happens,

For ultimate pedantry, don't depend on the number and kind of blanks on
the left hand side of the output. If you set LC_ALL to POSIX, then only
<space> and <tab> count as blank; but even that way you should be
prepared to see any number of them on the LHS. Probably just use grep:

if printf '\336\255\276\357' | od -A n -t x4 | grep -q deadbeef; then
  printf 'big endian\n' >&2
fi

Laszlo

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Paolo Bonzini]

Il 20/03/2014 23:33, Marcel Apfelbaum ha scritto:
> I've seen something like that somewhere, but I didn't quite like it.
> I was looking for something more elegant as I was *almost* sure
> this kind of solution will not pass the reviews :)
>
> But maybe I'll try this, let's see what happens,

If all you're looking for is bigendian (disabling iasl disassembly on 
bigendian makes sense), your patch v2 is fine.

Assembling ASL on bigendian is supported by at least Fedora and Debian 
(and hence Ubuntu).

Paolo

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Michael S. Tsirkin]

On Fri, Mar 21, 2014 at 12:16:53AM +0100, Paolo Bonzini wrote:
> Il 20/03/2014 23:33, Marcel Apfelbaum ha scritto:
> >I've seen something like that somewhere, but I didn't quite like it.
> >I was looking for something more elegant as I was *almost* sure
> >this kind of solution will not pass the reviews :)
> >
> >But maybe I'll try this, let's see what happens,
> 
> If all you're looking for is bigendian (disabling iasl disassembly
> on bigendian makes sense), your patch v2 is fine.
> 
> Assembling ASL on bigendian is supported by at least Fedora and
> Debian (and hence Ubuntu).
> 
> Paolo


At this point I'm confused.
If iasl compiler is broken, we should detect and fix that.
It might be ok to just detect endian-ness as a quick work-around.
BTW configure already has code to detect endian-ness:
if test "$bigendian" = "yes" ; then
  echo "HOST_WORDS_BIGENDIAN=y" >> $config_host_mak
fi


If only the dis-assembler is broken, we should only detect that
when running tests. Using expected files for this should be fine.


-- 
MST

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Peter Maydell]

On 23 March 2014 09:49, Michael S. Tsirkin <mst@redhat.com> wrote:
> At this point I'm confused.
> If iasl compiler is broken, we should detect and fix that.
> It might be ok to just detect endian-ness as a quick work-around.
> BTW configure already has code to detect endian-ness:
> if test "$bigendian" = "yes" ; then
>   echo "HOST_WORDS_BIGENDIAN=y" >> $config_host_mak
> fi

That's the endianness of the machine we're compiling QEMU
for, not the endianness of the machine we're compiling QEMU
on. If for instance you're on x86_64 cross-compiling for PPC
then HOST_WORDS_BIGENDIAN is true, but the iasl you use
in the build process will be running on a little endian machine.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Marcel Apfelbaum]

On Sun, 2014-03-23 at 12:14 +0000, Peter Maydell wrote:
> On 23 March 2014 09:49, Michael S. Tsirkin <mst@redhat.com> wrote:
> > At this point I'm confused.
> > If iasl compiler is broken, we should detect and fix that.
> > It might be ok to just detect endian-ness as a quick work-around.
> > BTW configure already has code to detect endian-ness:
> > if test "$bigendian" = "yes" ; then
> >   echo "HOST_WORDS_BIGENDIAN=y" >> $config_host_mak
> > fi
> 
> That's the endianness of the machine we're compiling QEMU
> for, not the endianness of the machine we're compiling QEMU
> on. If for instance you're on x86_64 cross-compiling for PPC
> then HOST_WORDS_BIGENDIAN is true, but the iasl you use
> in the build process will be running on a little endian machine.

Hi Peter, are you sure about this?
I saw the 'target_bigendian' that does what you described above.
$bigendian is the result of a little C program that checks *host's* endian-ness.
Of course I might have missed something.

By the way, considering what Paolo said that the iasl compiler does work
for BE machines (the dis-assembler has the problem), leads me to see this issue
as the test's problem.
So I am going to leave the 'configure' with no modifications
and check inside the test itself that the expected files can be disassembled.

Thanks,
Marcel


> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Peter Maydell]

On 23 March 2014 12:32, Marcel Apfelbaum <marcel.a@redhat.com> wrote:
> On Sun, 2014-03-23 at 12:14 +0000, Peter Maydell wrote:
>> On 23 March 2014 09:49, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > At this point I'm confused.
>> > If iasl compiler is broken, we should detect and fix that.
>> > It might be ok to just detect endian-ness as a quick work-around.
>> > BTW configure already has code to detect endian-ness:
>> > if test "$bigendian" = "yes" ; then
>> >   echo "HOST_WORDS_BIGENDIAN=y" >> $config_host_mak
>> > fi
>>
>> That's the endianness of the machine we're compiling QEMU
>> for, not the endianness of the machine we're compiling QEMU
>> on. If for instance you're on x86_64 cross-compiling for PPC
>> then HOST_WORDS_BIGENDIAN is true, but the iasl you use
>> in the build process will be running on a little endian machine.
>
> Hi Peter, are you sure about this?
> I saw the 'target_bigendian' that does what you described above.
> $bigendian is the result of a little C program that checks *host's* endian-ness.
> Of course I might have missed something.

"host" for QEMU means "the machine QEMU will run on"
(as opposed to "target" meaning "the machine QEMU is emulating").
Those can both be different from the machine you're building on.
Example: you can be on an x86_64 machine cross-compiling a
qemu-system-mips intended to run on PPC hosts:
 build system: x86_64 (we don't currently try to identify its endianness)
 host system: PPC ("$bigendian" is set to endianness)
 target system: mips ("$target_bigendian" is set to endianness)

bigendian is set by cross-compiling a test object, which produces
a PPC object file that we then examine to see which endianness
the PPC system we're building for is.
target_bigendian is set for the target machine by just hard-coding it --
in this case it would be set because 'mips' is a bigendian target.
The iasl we use in the build process is the x86_64 native binary,
so neither $bigendian nor $target_bigendian are correct.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Marcel Apfelbaum]

On Sun, 2014-03-23 at 12:48 +0000, Peter Maydell wrote:
> On 23 March 2014 12:32, Marcel Apfelbaum <marcel.a@redhat.com> wrote:
> > On Sun, 2014-03-23 at 12:14 +0000, Peter Maydell wrote:
> >> On 23 March 2014 09:49, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > At this point I'm confused.
> >> > If iasl compiler is broken, we should detect and fix that.
> >> > It might be ok to just detect endian-ness as a quick work-around.
> >> > BTW configure already has code to detect endian-ness:
> >> > if test "$bigendian" = "yes" ; then
> >> >   echo "HOST_WORDS_BIGENDIAN=y" >> $config_host_mak
> >> > fi
> >>
> >> That's the endianness of the machine we're compiling QEMU
> >> for, not the endianness of the machine we're compiling QEMU
> >> on. If for instance you're on x86_64 cross-compiling for PPC
> >> then HOST_WORDS_BIGENDIAN is true, but the iasl you use
> >> in the build process will be running on a little endian machine.
> >
> > Hi Peter, are you sure about this?
> > I saw the 'target_bigendian' that does what you described above.
> > $bigendian is the result of a little C program that checks *host's* endian-ness.
> > Of course I might have missed something.
> 
> "host" for QEMU means "the machine QEMU will run on"
> (as opposed to "target" meaning "the machine QEMU is emulating").
> Those can both be different from the machine you're building on.
> Example: you can be on an x86_64 machine cross-compiling a
> qemu-system-mips intended to run on PPC hosts:
>  build system: x86_64 (we don't currently try to identify its endianness)
>  host system: PPC ("$bigendian" is set to endianness)
>  target system: mips ("$target_bigendian" is set to endianness)
> 
> bigendian is set by cross-compiling a test object, which produces
> a PPC object file that we then examine to see which endianness
> the PPC system we're building for is.
> target_bigendian is set for the target machine by just hard-coding it --
> in this case it would be set because 'mips' is a bigendian target.
> The iasl we use in the build process is the x86_64 native binary,
> so neither $bigendian nor $target_bigendian are correct.

Thanks for the detailed answer! It really helped!
I missed the build machine != host machine != target machine :(

Marcel

> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Paolo Bonzini]

Il 23/03/2014 13:32, Marcel Apfelbaum ha scritto:
>> > That's the endianness of the machine we're compiling QEMU
>> > for, not the endianness of the machine we're compiling QEMU
>> > on. If for instance you're on x86_64 cross-compiling for PPC
>> > then HOST_WORDS_BIGENDIAN is true, but the iasl you use
>> > in the build process will be running on a little endian machine.
> Hi Peter, are you sure about this?
> I saw the 'target_bigendian' that does what you described above.
> $bigendian is the result of a little C program that checks *host's* endian-ness.
> Of course I might have missed something.

Peter is right.  There are three systems: build (what you run on), host 
(what you compile for), target (always x86-64 for now for acpi-test). 
$bigendian and G_BYTE_ORDER check the host system.  If you wanted to 
test the build system's endianness, Laszlo's suggestion would be fine.

However, Michael is also right if we assume that iasl on bigendian can 
assemble, and that the problems are only on disassembling.  This is 
because in this particular case you'll be running an executable compiled 
for the host and you know that host == build.

I think the assumption is sane.  As far as I remember, upstream iasl 
doesn't compile at all on big-endian machines.  You need specific 
patches such as those shared by Fedora and Debian's.  Unfortunately, the 
patches are incomplete, they only fix assembling because that's what was 
needed so far to cross-compile SeaBIOS.

Paolo

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Marcel Apfelbaum]

On Sun, 2014-03-23 at 13:51 +0100, Paolo Bonzini wrote:
> Il 23/03/2014 13:32, Marcel Apfelbaum ha scritto:
> >> > That's the endianness of the machine we're compiling QEMU
> >> > for, not the endianness of the machine we're compiling QEMU
> >> > on. If for instance you're on x86_64 cross-compiling for PPC
> >> > then HOST_WORDS_BIGENDIAN is true, but the iasl you use
> >> > in the build process will be running on a little endian machine.
> > Hi Peter, are you sure about this?
> > I saw the 'target_bigendian' that does what you described above.
> > $bigendian is the result of a little C program that checks *host's* endian-ness.
> > Of course I might have missed something.
> 
> Peter is right.  There are three systems: build (what you run on), host 
> (what you compile for), target (always x86-64 for now for acpi-test). 
> $bigendian and G_BYTE_ORDER check the host system.  If you wanted to 
> test the build system's endianness, Laszlo's suggestion would be fine.
> 
> However, Michael is also right if we assume that iasl on bigendian can 
> assemble, and that the problems are only on disassembling.  This is 
> because in this particular case you'll be running an executable compiled 
> for the host and you know that host == build.
> 
> I think the assumption is sane.  As far as I remember, upstream iasl 
> doesn't compile at all on big-endian machines.  You need specific 
> patches such as those shared by Fedora and Debian's.  Unfortunately, the 
> patches are incomplete, they only fix assembling because that's what was 
> needed so far to cross-compile SeaBIOS.

Hi Paolo, thanks for the help!
I need a decision here :)

1.
If iasl *does* work for the scenarios used by Qemu until now,
I can conclude that the only part that suffers from it is the
test itself, because it dis-assembles AML code. In this case
I can handle it in the test itself by checking the endian-ness
(V2 already posted), or trying to dis-assemble the expected AML
file and not failing the test if this fails.

2. If iasl causes problems to run a x86_64 guest if one or both build/host
machines are BE, we need to tweak the configuration file.

After this discussion it seems that (2.) is not the case, I think maybe re-sending:
 [PATCH for-2.0 V2] tests/acpi-test: do not run iasl on big endian machines

Is everyone OK with this?
Thanks,
Marcel

> Paolo

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Michael S. Tsirkin]

On Sun, Mar 23, 2014 at 03:17:32PM +0200, Marcel Apfelbaum wrote:
> On Sun, 2014-03-23 at 13:51 +0100, Paolo Bonzini wrote:
> > Il 23/03/2014 13:32, Marcel Apfelbaum ha scritto:
> > >> > That's the endianness of the machine we're compiling QEMU
> > >> > for, not the endianness of the machine we're compiling QEMU
> > >> > on. If for instance you're on x86_64 cross-compiling for PPC
> > >> > then HOST_WORDS_BIGENDIAN is true, but the iasl you use
> > >> > in the build process will be running on a little endian machine.
> > > Hi Peter, are you sure about this?
> > > I saw the 'target_bigendian' that does what you described above.
> > > $bigendian is the result of a little C program that checks *host's* endian-ness.
> > > Of course I might have missed something.
> > 
> > Peter is right.  There are three systems: build (what you run on), host 
> > (what you compile for), target (always x86-64 for now for acpi-test). 
> > $bigendian and G_BYTE_ORDER check the host system.  If you wanted to 
> > test the build system's endianness, Laszlo's suggestion would be fine.
> > 
> > However, Michael is also right if we assume that iasl on bigendian can 
> > assemble, and that the problems are only on disassembling.  This is 
> > because in this particular case you'll be running an executable compiled 
> > for the host and you know that host == build.
> > 
> > I think the assumption is sane.  As far as I remember, upstream iasl 
> > doesn't compile at all on big-endian machines.  You need specific 
> > patches such as those shared by Fedora and Debian's.  Unfortunately, the 
> > patches are incomplete, they only fix assembling because that's what was 
> > needed so far to cross-compile SeaBIOS.
> 
> Hi Paolo, thanks for the help!
> I need a decision here :)
> 
> 1.
> If iasl *does* work for the scenarios used by Qemu until now,
> I can conclude that the only part that suffers from it is the
> test itself, because it dis-assembles AML code. In this case
> I can handle it in the test itself by checking the endian-ness
> (V2 already posted), or trying to dis-assemble the expected AML
> file and not failing the test if this fails.

The last option seems best to me.
It will automatically start working when iasl
disassembler is fixed and will handle any other
case of breakage, not just BE.


> 2. If iasl causes problems to run a x86_64 guest if one or both build/host
> machines are BE, we need to tweak the configuration file.
> 
> After this discussion it seems that (2.) is not the case, I think maybe re-sending:
>  [PATCH for-2.0 V2] tests/acpi-test: do not run iasl on big endian machines
> 
> Is everyone OK with this?
> Thanks,
> Marcel
> 
> > Paolo
> 
> 

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Andreas Färber]

Am 23.03.2014 10:49, schrieb Michael S. Tsirkin:
> On Fri, Mar 21, 2014 at 12:16:53AM +0100, Paolo Bonzini wrote:
>> Il 20/03/2014 23:33, Marcel Apfelbaum ha scritto:
>>> I've seen something like that somewhere, but I didn't quite like it.
>>> I was looking for something more elegant as I was *almost* sure
>>> this kind of solution will not pass the reviews :)
>>>
>>> But maybe I'll try this, let's see what happens,
>>
>> If all you're looking for is bigendian (disabling iasl disassembly
>> on bigendian makes sense), your patch v2 is fine.
>>
>> Assembling ASL on bigendian is supported by at least Fedora and
>> Debian (and hence Ubuntu).
>>
>> Paolo
> 
> 
> At this point I'm confused.
> If iasl compiler is broken, we should detect and fix that.
> It might be ok to just detect endian-ness as a quick work-around.
> BTW configure already has code to detect endian-ness:
> if test "$bigendian" = "yes" ; then
>   echo "HOST_WORDS_BIGENDIAN=y" >> $config_host_mak
> fi

Careful, this is about the endianness of the built target binary, which
may be different from the endianness of the build system. However I
would hope the tests will not be executed for cross-builds.

Alexey, you're using cross-builds for ppc, right? Have you ever tested
running make check there?

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Alexey Kardashevskiy]

On 03/24/2014 10:02 PM, Andreas Färber wrote:
> Am 23.03.2014 10:49, schrieb Michael S. Tsirkin:
>> On Fri, Mar 21, 2014 at 12:16:53AM +0100, Paolo Bonzini wrote:
>>> Il 20/03/2014 23:33, Marcel Apfelbaum ha scritto:
>>>> I've seen something like that somewhere, but I didn't quite like it.
>>>> I was looking for something more elegant as I was *almost* sure
>>>> this kind of solution will not pass the reviews :)
>>>>
>>>> But maybe I'll try this, let's see what happens,
>>>
>>> If all you're looking for is bigendian (disabling iasl disassembly
>>> on bigendian makes sense), your patch v2 is fine.
>>>
>>> Assembling ASL on bigendian is supported by at least Fedora and
>>> Debian (and hence Ubuntu).
>>>
>>> Paolo
>>
>>
>> At this point I'm confused.
>> If iasl compiler is broken, we should detect and fix that.
>> It might be ok to just detect endian-ness as a quick work-around.
>> BTW configure already has code to detect endian-ness:
>> if test "$bigendian" = "yes" ; then
>>   echo "HOST_WORDS_BIGENDIAN=y" >> $config_host_mak
>> fi
> 
> Careful, this is about the endianness of the built target binary, which
> may be different from the endianness of the build system. However I
> would hope the tests will not be executed for cross-builds.
> 
> Alexey, you're using cross-builds for ppc, right? Have you ever tested
> running make check there?


"make check" on what? ppc64-softmmu target compiled on x86_64? That fails
because of errors like this:

tests/check-qjson: tests/check-qjson: cannot execute binary file
make: *** [check-tests/check-qstring] Error 1



-- 
Alexey

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Marcel Apfelbaum]

On Fri, 2014-03-21 at 00:16 +0100, Paolo Bonzini wrote:
> Il 20/03/2014 23:33, Marcel Apfelbaum ha scritto:
> > I've seen something like that somewhere, but I didn't quite like it.
> > I was looking for something more elegant as I was *almost* sure
> > this kind of solution will not pass the reviews :)
> >
> > But maybe I'll try this, let's see what happens,
> 
> If all you're looking for is bigendian (disabling iasl disassembly on 
> bigendian makes sense), your patch v2 is fine.

Thanks Paolo, I thought so too, but Michael NACKED it having a valid point:
We should check this at configure time... .

Anyway, if only iasl dis-assembler has issues, this can be seen as acpi test's
problem and handled inside the test.

Thanks,
Marcel
> 
> Assembling ASL on bigendian is supported by at least Fedora and Debian 
> (and hence Ubuntu).
> 
> Paolo