[Qemu-devel] [PATCH] tcg/aarch64: Fix tcg_out_qemu_{ld, st} for guest_base == 0

[Qemu-devel] [PATCH] tcg/aarch64: Fix tcg_out_qemu_{ld, st} for guest_base == 0

[Richard Henderson]

In ffc6372851d8631a9f9fa56ec613b3244dc635b9, we swapped the guest
base to the address base register from the address index register.
Except that 31 in the base slot is SP not XZR, so we need to be
more intelligent about which reg gets placed in which slot.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Richard Henderson <rth@twiddle.net>
Cc: qemu-stable@nongnu.org (v2.4.0)
Reported-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/aarch64/tcg-target.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
index 01ae610..0ed10a9 100644
--- a/tcg/aarch64/tcg-target.c
+++ b/tcg/aarch64/tcg-target.c
@@ -56,6 +56,11 @@ static const int tcg_target_call_oarg_regs[1] = {
 #define TCG_REG_TMP TCG_REG_X30
 
 #ifndef CONFIG_SOFTMMU
+/* Note that XZR cannot be encoded in the address base register slot,
+   as that actaully encodes SP.  So if we need to zero-extend the guest
+   address, via the address index register slot, we need to load even
+   a zero guest base into a register.  */
+#define USE_GUEST_BASE     (guest_base != 0 || TARGET_LONG_BITS == 32)
 #define TCG_REG_GUEST_BASE TCG_REG_X28
 #endif
 
@@ -1224,9 +1229,13 @@ static void tcg_out_qemu_ld(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
     add_qemu_ldst_label(s, true, oi, ext, data_reg, addr_reg,
                         s->code_ptr, label_ptr);
 #else /* !CONFIG_SOFTMMU */
-    tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
-                           guest_base ? TCG_REG_GUEST_BASE : TCG_REG_XZR,
-                           otype, addr_reg);
+    if (USE_GUEST_BASE) {
+        tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
+                               TCG_REG_GUEST_BASE, otype, addr_reg);
+    } else {
+        tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
+                               addr_reg, TCG_TYPE_I64, TCG_REG_XZR);
+    }
 #endif /* CONFIG_SOFTMMU */
 }
 
@@ -1245,9 +1254,13 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
     add_qemu_ldst_label(s, false, oi, (memop & MO_SIZE)== MO_64,
                         data_reg, addr_reg, s->code_ptr, label_ptr);
 #else /* !CONFIG_SOFTMMU */
-    tcg_out_qemu_st_direct(s, memop, data_reg,
-                           guest_base ? TCG_REG_GUEST_BASE : TCG_REG_XZR,
-                           otype, addr_reg);
+    if (USE_GUEST_BASE) {
+        tcg_out_qemu_st_direct(s, memop, data_reg,
+                               TCG_REG_GUEST_BASE, otype, addr_reg);
+    } else {
+        tcg_out_qemu_st_direct(s, memop, data_reg,
+                               addr_reg, TCG_TYPE_I64, TCG_REG_XZR);
+    }
 #endif /* CONFIG_SOFTMMU */
 }
 
@@ -1806,7 +1819,7 @@ static void tcg_target_qemu_prologue(TCGContext *s)
                   CPU_TEMP_BUF_NLONGS * sizeof(long));
 
 #if !defined(CONFIG_SOFTMMU)
-    if (guest_base) {
+    if (USE_GUEST_BASE) {
         tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_GUEST_BASE, guest_base);
         tcg_regset_set_reg(s->reserved_regs, TCG_REG_GUEST_BASE);
     }
-- 
2.4.3

Re: [Qemu-devel] [PATCH] tcg/aarch64: Fix tcg_out_qemu_{ld, st} for guest_base == 0

[Paolo Bonzini]

On 01/09/2015 22:07, Richard Henderson wrote:
> In ffc6372851d8631a9f9fa56ec613b3244dc635b9, we swapped the guest
> base to the address base register from the address index register.
> Except that 31 in the base slot is SP not XZR, so we need to be
> more intelligent about which reg gets placed in which slot.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: qemu-stable@nongnu.org (v2.4.0)
> Reported-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/aarch64/tcg-target.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
> index 01ae610..0ed10a9 100644
> --- a/tcg/aarch64/tcg-target.c
> +++ b/tcg/aarch64/tcg-target.c
> @@ -56,6 +56,11 @@ static const int tcg_target_call_oarg_regs[1] = {
>  #define TCG_REG_TMP TCG_REG_X30
>  
>  #ifndef CONFIG_SOFTMMU
> +/* Note that XZR cannot be encoded in the address base register slot,
> +   as that actaully encodes SP.  So if we need to zero-extend the guest
> +   address, via the address index register slot, we need to load even
> +   a zero guest base into a register.  */
> +#define USE_GUEST_BASE     (guest_base != 0 || TARGET_LONG_BITS == 32)
>  #define TCG_REG_GUEST_BASE TCG_REG_X28
>  #endif
>  
> @@ -1224,9 +1229,13 @@ static void tcg_out_qemu_ld(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
>      add_qemu_ldst_label(s, true, oi, ext, data_reg, addr_reg,
>                          s->code_ptr, label_ptr);
>  #else /* !CONFIG_SOFTMMU */
> -    tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
> -                           guest_base ? TCG_REG_GUEST_BASE : TCG_REG_XZR,
> -                           otype, addr_reg);
> +    if (USE_GUEST_BASE) {
> +        tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
> +                               TCG_REG_GUEST_BASE, otype, addr_reg);
> +    } else {
> +        tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
> +                               addr_reg, TCG_TYPE_I64, TCG_REG_XZR);
> +    }
>  #endif /* CONFIG_SOFTMMU */
>  }
>  
> @@ -1245,9 +1254,13 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
>      add_qemu_ldst_label(s, false, oi, (memop & MO_SIZE)== MO_64,
>                          data_reg, addr_reg, s->code_ptr, label_ptr);
>  #else /* !CONFIG_SOFTMMU */
> -    tcg_out_qemu_st_direct(s, memop, data_reg,
> -                           guest_base ? TCG_REG_GUEST_BASE : TCG_REG_XZR,
> -                           otype, addr_reg);
> +    if (USE_GUEST_BASE) {
> +        tcg_out_qemu_st_direct(s, memop, data_reg,
> +                               TCG_REG_GUEST_BASE, otype, addr_reg);
> +    } else {
> +        tcg_out_qemu_st_direct(s, memop, data_reg,
> +                               addr_reg, TCG_TYPE_I64, TCG_REG_XZR);
> +    }
>  #endif /* CONFIG_SOFTMMU */
>  }
>  
> @@ -1806,7 +1819,7 @@ static void tcg_target_qemu_prologue(TCGContext *s)
>                    CPU_TEMP_BUF_NLONGS * sizeof(long));
>  
>  #if !defined(CONFIG_SOFTMMU)
> -    if (guest_base) {
> +    if (USE_GUEST_BASE) {
>          tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_GUEST_BASE, guest_base);
>          tcg_regset_set_reg(s->reserved_regs, TCG_REG_GUEST_BASE);
>      }
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH] tcg/aarch64: Fix tcg_out_qemu_{ld, st} for guest_base == 0

[Andreas Färber]

Am 01.09.2015 um 22:07 schrieb Richard Henderson:
> In ffc6372851d8631a9f9fa56ec613b3244dc635b9, we swapped the guest
> base to the address base register from the address index register.
> Except that 31 in the base slot is SP not XZR, so we need to be
> more intelligent about which reg gets placed in which slot.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: qemu-stable@nongnu.org (v2.4.0)
> Reported-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/aarch64/tcg-target.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)

Tested-by: Andreas Färber <afaerber@suse.de>

Fixes my test case, thanks.

Backporting it to v2.4.0 was a little complicated due to the guest base
changes though, still waiting on results from our build service there.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)