Re: [Qemu-devel] [PATCH] tcg/mips: Fix clobbering of qemu_ld inputs

[Richard Henderson <rth@twiddle.net>]

On 09/14/2015 03:34 AM, James Hogan wrote:
> The MIPS TCG backend implements qemu_ld with 64-bit targets using the v0
> register (base) as a temporary to load the upper half of the QEMU TLB
> comparator (see line 5 below), however this happens before the input
> address is used (line 8 to mask off the low bits for the TLB
> comparison, and line 12 to add the host-guest offset). If the input
> address (addrl) also happens to have been placed in v0 (as in the second
> column below), it gets clobbered before it is used.
> 
>      addrl in t2              addrl in v0
> 
>  1 srl     a0,t2,0x7        srl     a0,v0,0x7
>  2 andi    a0,a0,0x1fe0     andi    a0,a0,0x1fe0
>  3 addu    a0,a0,s0         addu    a0,a0,s0
>  4 lw      at,9136(a0)      lw      at,9136(a0)      set TCG_TMP0 (at)
>  5 lw      v0,9140(a0)      lw      v0,9140(a0)      set base (v0)
>  6 li      t9,-4093         li      t9,-4093
>  7 lw      a0,9160(a0)      lw      a0,9160(a0)      set addend (a0)
>  8 and     t9,t9,t2         and     t9,t9,v0         use addrl
>  9 bne     at,t9,0x836d8c8  bne     at,t9,0x836d838  use TCG_TMP0
> 10  nop                      nop
> 11 bne     v0,t8,0x836d8c8  bne     v0,a1,0x836d838  use base
> 12  addu   v0,a0,t2          addu   v0,a0,v0         use addrl, addend
> 13 lw      t0,0(v0)         lw      t0,0(v0)
> 
> Fix by using TCG_TMP0 (at) as the temporary instead of v0 (base),
> pushing the load on line 5 forward into the delay slot of the low
> comparison (line 10). The early load of the addend on line 7 also needs
> pushing even further for 64-bit targets, or it will clobber a0 before
> we're done with it. The output for 32-bit targets is unaffected.
> 
>  srl     a0,v0,0x7
>  andi    a0,a0,0x1fe0
>  addu    a0,a0,s0
>  lw      at,9136(a0)
> -lw      v0,9140(a0)      load high comparator
>  li      t9,-4093
> -lw      a0,9160(a0)      load addend
>  and     t9,t9,v0
>  bne     at,t9,0x836d838
> - nop
> + lw     at,9140(a0)      load high comparator
> +lw      a0,9160(a0)      load addend
> -bne     v0,a1,0x836d838
> +bne     at,a1,0x836d838
>   addu   v0,a0,v0
>  lw      t0,0(v0)
> 
> Suggested-by: Richard Henderson <rth@twiddle.net>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~