Re: [Qemu-devel] Shifts, ppc[64], xtensa

[Peter Maydell]

On 18 September 2012 20:52, malc <address@hidden> wrote:
>
> Looks like PPC/PPC64 is also hit by shift issues, on top of that xtensa
> exposed another bug in power's tcg - gototb's target was expected to be
> always filled via tb_set_jmp_target (even though it's clearly not what
> tcg/README prescribes, sorry about that).
>
> Thanks to Max Filippov for pointing to xtensa test suite that helped to
> narrow the search to gototb.
>
> Testing of the following with other targets on ppc flavours is welcome..
>
> P.S. Xtensa does mighty weird things with shifts i must say...
>
> diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
> index 26c4b33..08f62fa 100644
> --- a/tcg/ppc/tcg-target.c
> +++ b/tcg/ppc/tcg-target.c
> @@ -409,6 +409,7 @@ static int tcg_target_const_match(tcg_target_long val,
>
>  #define TW     XO31(4)
>  #define TRAP   (TW | TO (31))
> +#define NOP    0x60000000
>
>  #define RT(r) ((r)<<21)
>  #define RS(r) ((r)<<21)
> @@ -1306,10 +1307,10 @@ void ppc_tb_set_jmp_target (unsigned long jmp_addr, 
> unsigned long addr)
>              *ptr = 0x48000000 | (disp & 0x03fffffc); /* b disp */
>              patch_size = 4;
>          } else {
> -            ptr[0] = 0x60000000; /* nop */
> -            ptr[1] = 0x60000000;
> -            ptr[2] = 0x60000000;
> -            ptr[3] = 0x60000000;
> +            ptr[0] = NOP;
> +            ptr[1] = NOP;
> +            ptr[2] = NOP;
> +            ptr[3] = NOP;
>              patch_size = 16;
>          }
>      }
> @@ -1330,7 +1331,10 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, 
> const TCGArg *args,
>              /* direct jump method */
>
>              s->tb_jmp_offset[args[0]] = s->code_ptr - s->code_buf;
> -            s->code_ptr += 16;
> +            tcg_out32 (s, NOP);
> +            tcg_out32 (s, NOP);
> +            tcg_out32 (s, NOP);
> +            tcg_out32 (s, NOP);

Not too familiar with the PPC backend, but doesn't this mean that
in the retranslation case we will overwrite a correct jump destination
with these NOP words and then rewrite it again with the correct
destination? That can cause problems with cache incoherency;
compare the fix applied in commit c69806ab8276 for ARM.

thanks
-- PMM