| Message ID | 1395350099-14664-1-git-send-email-marcel.a@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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
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
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
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
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
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
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.
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.
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 ...
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
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
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
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
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
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
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
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
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 > >
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
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
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
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(-)