1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH qemu REPOST] spapr/rtas: Force big endian compile for rtas

Alexey Kardashevskiy posted 1 patch 4 years, 10 months ago
Test s390x passed
Test checkpatch passed
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190612020723.96802-1-aik@ozlabs.ru
Maintainers: David Gibson <david@gibson.dropbear.id.au>
pc-bios/spapr-rtas/Makefile | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH qemu REPOST] spapr/rtas: Force big endian compile for rtas
Posted by Alexey Kardashevskiy 4 years, 10 months ago 
At the moment the rtas's Makefile uses generic QEMU rules which means
that when QEMU is compiled on a little endian system, the spapr-rtas.bin
is compiled as little endian too which is incorrect as it is always
executed in big endian mode.

This enforces -mbig by defining %.o:%.S rule as spapr-rtas.bin is
a standalone guest binary which should not depend on QEMU flags anyway.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 pc-bios/spapr-rtas/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/pc-bios/spapr-rtas/Makefile b/pc-bios/spapr-rtas/Makefile
index f26dd428b79e..4b9bb1230658 100644
--- a/pc-bios/spapr-rtas/Makefile
+++ b/pc-bios/spapr-rtas/Makefile
@@ -14,8 +14,11 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/spapr-rtas)
 
 build-all: spapr-rtas.bin
 
+%.o: %.S
+	$(call quiet-command,$(CCAS) -mbig -c -o $@ $<,"CCAS","$(TARGET_DIR)$@")
+
 %.img: %.o
-	$(call quiet-command,$(CC) -nostdlib -o $@ $<,"Building","$(TARGET_DIR)$@")
+	$(call quiet-command,$(CC) -nostdlib -mbig -o $@ $<,"Building","$(TARGET_DIR)$@")
 
 %.bin: %.img
 	$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"Building","$(TARGET_DIR)$@")
-- 
2.17.1
Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu REPOST] spapr/rtas: Force big endian compile for rtas
Posted by Greg Kurz 4 years, 10 months ago 
On Wed, 12 Jun 2019 12:07:23 +1000
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> At the moment the rtas's Makefile uses generic QEMU rules which means
> that when QEMU is compiled on a little endian system, the spapr-rtas.bin
> is compiled as little endian too which is incorrect as it is always
> executed in big endian mode.
> 

I'm naively thinking that executing code compiled as little endian
in big endian mode would result in an exception... Can you explain
how/why this ever worked ?

> This enforces -mbig by defining %.o:%.S rule as spapr-rtas.bin is
> a standalone guest binary which should not depend on QEMU flags anyway.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  pc-bios/spapr-rtas/Makefile | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/spapr-rtas/Makefile b/pc-bios/spapr-rtas/Makefile
> index f26dd428b79e..4b9bb1230658 100644
> --- a/pc-bios/spapr-rtas/Makefile
> +++ b/pc-bios/spapr-rtas/Makefile
> @@ -14,8 +14,11 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/spapr-rtas)
>  
>  build-all: spapr-rtas.bin
>  
> +%.o: %.S
> +	$(call quiet-command,$(CCAS) -mbig -c -o $@ $<,"CCAS","$(TARGET_DIR)$@")
> +
>  %.img: %.o
> -	$(call quiet-command,$(CC) -nostdlib -o $@ $<,"Building","$(TARGET_DIR)$@")
> +	$(call quiet-command,$(CC) -nostdlib -mbig -o $@ $<,"Building","$(TARGET_DIR)$@")
>  
>  %.bin: %.img
>  	$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"Building","$(TARGET_DIR)$@")
Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu REPOST] spapr/rtas: Force big endian compile for rtas
Posted by David Gibson 4 years, 10 months ago 
On Mon, Jun 17, 2019 at 10:25:10AM +0200, Greg Kurz wrote:
65;5603;1c> On Wed, 12 Jun 2019 12:07:23 +1000
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
> > At the moment the rtas's Makefile uses generic QEMU rules which means
> > that when QEMU is compiled on a little endian system, the spapr-rtas.bin
> > is compiled as little endian too which is incorrect as it is always
> > executed in big endian mode.
> 
> I'm naively thinking that executing code compiled as little endian
> in big endian mode would result in an exception... Can you explain
> how/why this ever worked ?

Because basically nobody actually built the rtas blob from the
sources, they just used the pre-compiled blob, which is correctly
built BE.

That said executing LE code in BE mode won't necessarily result in an
exception - it'll just execute whatever the instructions are you get
when you byte reverse the ones you inteded, which may or may not be
valid.  It's *likely* to cause an exception fairly soon, but the
opcode space is densely populated enough that there's a good chance it
won't cause an immediate illegal instruction.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu REPOST] spapr/rtas: Force big endian compile for rtas
Posted by Greg Kurz 4 years, 10 months ago 
On Mon, 17 Jun 2019 21:12:05 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Jun 17, 2019 at 10:25:10AM +0200, Greg Kurz wrote:
> 65;5603;1c> On Wed, 12 Jun 2019 12:07:23 +1000
> > Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >   
> > > At the moment the rtas's Makefile uses generic QEMU rules which means
> > > that when QEMU is compiled on a little endian system, the spapr-rtas.bin
> > > is compiled as little endian too which is incorrect as it is always
> > > executed in big endian mode.  
> > 
> > I'm naively thinking that executing code compiled as little endian
> > in big endian mode would result in an exception... Can you explain
> > how/why this ever worked ?  
> 
> Because basically nobody actually built the rtas blob from the
> sources, they just used the pre-compiled blob, which is correctly
> built BE.
> 

Ah ! Everyone has been using blob from this pre-ppc64le commit:

commit d818bfc5c34c59e9c6d03b3b9983bb5435967292
Author: Aurelien Jarno <aurelien@aurel32.net>
Date:   Fri Apr 1 20:04:24 2011 +0200

    pc-bios/spapr-rtas.bin: remove executable flag
    
    Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

> That said executing LE code in BE mode won't necessarily result in an
> exception - it'll just execute whatever the instructions are you get
> when you byte reverse the ones you inteded, which may or may not be
> valid.  It's *likely* to cause an exception fairly soon, but the
> opcode space is densely populated enough that there's a good chance it
> won't cause an immediate illegal instruction.
> 

In theory yes, but in this precise case, the first instruction of the
rtas blob is 7c641b78 ('mr r4, r3') and I've manually checked that
781b647c raises an exception on both POWER8 and POWER9.
Re: [Qemu-devel] [PATCH qemu REPOST] spapr/rtas: Force big endian compile for rtas
Posted by David Gibson 4 years, 10 months ago 
On Wed, Jun 12, 2019 at 12:07:23PM +1000, Alexey Kardashevskiy wrote:
> At the moment the rtas's Makefile uses generic QEMU rules which means
> that when QEMU is compiled on a little endian system, the spapr-rtas.bin
> is compiled as little endian too which is incorrect as it is always
> executed in big endian mode.
> 
> This enforces -mbig by defining %.o:%.S rule as spapr-rtas.bin is
> a standalone guest binary which should not depend on QEMU flags anyway.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Applied to ppc-for-4.1, thanks.

> ---
>  pc-bios/spapr-rtas/Makefile | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/pc-bios/spapr-rtas/Makefile b/pc-bios/spapr-rtas/Makefile
> index f26dd428b79e..4b9bb1230658 100644
> --- a/pc-bios/spapr-rtas/Makefile
> +++ b/pc-bios/spapr-rtas/Makefile
> @@ -14,8 +14,11 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/spapr-rtas)
>  
>  build-all: spapr-rtas.bin
>  
> +%.o: %.S
> +	$(call quiet-command,$(CCAS) -mbig -c -o $@ $<,"CCAS","$(TARGET_DIR)$@")
> +
>  %.img: %.o
> -	$(call quiet-command,$(CC) -nostdlib -o $@ $<,"Building","$(TARGET_DIR)$@")
> +	$(call quiet-command,$(CC) -nostdlib -mbig -o $@ $<,"Building","$(TARGET_DIR)$@")
>  
>  %.bin: %.img
>  	$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"Building","$(TARGET_DIR)$@")

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.