diff mbox

[20/22] ppc: move load and store helpers, switch to AREG0 free mode

Message ID CAAu8pHsxreCwtRftLOaw8LYwjoUquODcQqDigkytMME6Jz7+7Q@mail.gmail.com
State New
Headers show

Commit Message

Blue Swirl April 22, 2012, 1:26 p.m. UTC
Add an explicit CPUPPCState parameter instead of relying on AREG0
and rename op_helper.c (which only contains load and store helpers)
to mem_helper.c. Remove AREG0 swapping in
tlb_fill().

Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation
and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 Makefile.target                          |    6 +-
 configure                                |    2 +-
 cpu-all.h                                |    9 +++
 target-ppc/excp_helper.c                 |    3 +-
 target-ppc/helper.h                      |   30 ++++----
 target-ppc/{op_helper.c => mem_helper.c} |  109 +++++++++++++++---------------
 target-ppc/translate.c                   |   30 ++++----
 7 files changed, 101 insertions(+), 88 deletions(-)
 rename target-ppc/{op_helper.c => mem_helper.c} (68%)

     }
@@ -6405,7 +6405,7 @@ static void gen_stve##name(DisasContext *ctx)
                       \
         EA = tcg_temp_new();                                            \
         gen_addr_reg_index(ctx, EA);                                    \
         rs = gen_avr_ptr(rS(ctx->opcode));                              \
-        gen_helper_stve##name (rs, EA);                                 \
+        gen_helper_stve##name(cpu_env, rs, EA);                         \
         tcg_temp_free(EA);                                              \
         tcg_temp_free_ptr(rs);                                          \
     }
@@ -9683,9 +9683,9 @@ static inline void
gen_intermediate_code_internal(CPUPPCState *env,
         if (num_insns + 1 == max_insns && (tb->cflags & CF_LAST_IO))
             gen_io_start();
         if (unlikely(ctx.le_mode)) {
-            ctx.opcode = bswap32(ldl_code(ctx.nip));
+            ctx.opcode = bswap32(cpu_ldl_code(env, ctx.nip));
         } else {
-            ctx.opcode = ldl_code(ctx.nip);
+            ctx.opcode = cpu_ldl_code(env, ctx.nip);
         }
         LOG_DISAS("translate opcode %08x (%02x %02x %02x) (%s)\n",
                     ctx.opcode, opc1(ctx.opcode), opc2(ctx.opcode),

Comments

Alexander Graf April 30, 2012, 10:45 a.m. UTC | #1
On 22.04.2012, at 15:26, Blue Swirl wrote:

> Add an explicit CPUPPCState parameter instead of relying on AREG0
> and rename op_helper.c (which only contains load and store helpers)
> to mem_helper.c. Remove AREG0 swapping in
> tlb_fill().
> 
> Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation
> and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores.

This patch breaks qemu-system-ppc64 on ppc32 host user space for me. I'm trying to debug it down, but worst case I'll omit this patch set for 1.1.


Alex
Alexander Graf April 30, 2012, 11:51 a.m. UTC | #2
On 30.04.2012, at 12:45, Alexander Graf wrote:

> 
> On 22.04.2012, at 15:26, Blue Swirl wrote:
> 
>> Add an explicit CPUPPCState parameter instead of relying on AREG0
>> and rename op_helper.c (which only contains load and store helpers)
>> to mem_helper.c. Remove AREG0 swapping in
>> tlb_fill().
>> 
>> Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation
>> and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores.
> 
> This patch breaks qemu-system-ppc64 on ppc32 host user space for me. I'm trying to debug it down, but worst case I'll omit this patch set for 1.1.

Ok, so apparently nobody ever tested TCG_AREG0 mode with the ppc tcg target. It looks as if the 64-bit-guest-registers-in-32-bit-host-registers code path is missing completely.

This actually makes me less confident that this is a change we want for 1.1. I'll remove the patches from the queue.


Alex


TCG register swizzling code:

#ifdef CONFIG_TCG_PASS_AREG0
    /* XXX/FIXME: suboptimal */
    tcg_out_mov(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[3],
                tcg_target_call_iarg_regs[2]);
    tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2],
                tcg_target_call_iarg_regs[1]);
    tcg_out_mov(s, TCG_TYPE_TL, tcg_target_call_iarg_regs[1],
                tcg_target_call_iarg_regs[0]);
    tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0],
                TCG_AREG0);
#endif
    tcg_out_call (s, (tcg_target_long) qemu_st_helpers[opc], 1);

Log output:

NIP 00000000fff024e4   LR 0000000000000000 CTR 0000000000000000 XER 0000000000000000
MSR 0000000000000000 HID0 0000000060000000  HF 0000000000000000 idx 1
TB 00000000 01083771 DECR 4293883502
GPR00 0000000000000000 0000000000000000 0000000000000000 fffffffffff00000
GPR04 0000000000000000 00000000000024b0 0000000000000000 0000000000000000
GPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
CR 80000000  [ L  -  -  -  -  -  -  -  ]             RES ffffffffffffffff
FPR00 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPSCR 00000000
 SRR0 0000000000000000  SRR1 0000000000000000    PVR 00000000003c0301 VRSAVE 0000000000000000
SPRG0 0000000000000000 SPRG1 0000000000000000  SPRG2 0000000000000000  SPRG3 0000000000000000
SPRG4 0000000000000000 SPRG5 0000000000000000  SPRG6 0000000000000000  SPRG7 0000000000000000
 SDR1 0000000000000000
IN: 
0x00000000fff024e4:  stw     r6,0(r4)

OP:
 ---- 0xfff024e4
 movi_i32 access_type,$0x20
 mov_i32 tmp0,r4_0
 movi_i32 tmp1,$0x0
 qemu_st32 r6_0,tmp0,tmp1,$0x1
 goto_tb $0x0
 movi_i32 nip_0,$0xfff024e8
 movi_i32 nip_1,$0x0
 exit_tb $0xf4bae508

OUT: [size=180]
0xf5faf7a0:  lwz     r14,36(r27)
0xf5faf7a4:  lwz     r15,52(r27)
0xf5faf7a8:  li      r16,0
0xf5faf7ac:  li      r17,32
0xf5faf7b0:  stw     r17,672(r27)
0xf5faf7b4:  rlwinm  r3,r14,25,19,26
0xf5faf7b8:  add     r3,r3,r27
0xf5faf7bc:  lwzu    r4,8912(r3)
0xf5faf7c0:  rlwinm  r0,r14,0,30,19
0xf5faf7c4:  cmpw    cr7,r0,r4
0xf5faf7c8:  lwz     r4,4(r3)
0xf5faf7cc:  cmpw    cr6,r16,r4
0xf5faf7d0:  crand   4*cr7+eq,4*cr6+eq,4*cr7+eq
0xf5faf7d4:  beq-    cr7,0xf5faf80c
0xf5faf7d8:  mr      r3,r16
0xf5faf7dc:  mr      r4,r14
0xf5faf7e0:  mr      r5,r15
0xf5faf7e4:  li      r6,1
0xf5faf7e8:  mr      r6,r5
0xf5faf7ec:  mr      r5,r4
0xf5faf7f0:  mr      r4,r3
0xf5faf7f4:  mr      r3,r27
0xf5faf7f8:  lis     r0,4123
0xf5faf7fc:  ori     r0,r0,27696
0xf5faf800:  mtctr   r0
0xf5faf804:  bctrl
0xf5faf808:  b       0xf5faf818
0xf5faf80c:  lwz     r3,16(r3)
0xf5faf810:  add     r3,r3,r14
0xf5faf814:  stwx    r15,0,r3
0xf5faf818:  .long 0x0
0xf5faf81c:  .long 0x0
0xf5faf820:  .long 0x0
0xf5faf824:  .long 0x0
0xf5faf828:  lis     r14,-16
0xf5faf82c:  ori     r14,r14,9448
0xf5faf830:  stw     r14,668(r27)
0xf5faf834:  li      r14,0
0xf5faf838:  stw     r14,664(r27)
0xf5faf83c:  lis     r3,-2886
0xf5faf840:  ori     r3,r3,58632
0xf5faf844:  lis     r0,4264
0xf5faf848:  ori     r0,r0,20192
0xf5faf84c:  mtctr   r0
0xf5faf850:  bctr

Register state at bctr into helper_stl_mmu (0xf5faf804)

Breakpoint 1, helper_stl_mmu (env=0x10ab1cb0, addr=0, val=4294967295, mmu_idx=279465600)
    at /home/agraf/release/qemu/softmmu_template.h:266
266	    index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
(gdb) info registers 
r0             0xf5faf808	4126865416
r1             0xf4bac950	4105881936
r2             0xf4bb4900	4105914624
r3             0x10ab1cb0	279649456
r4             0x0	0
r5             0x0	0
r6             0x0	0
r7             0xffffffff	4294967295
r8             0x10a84e80	279465600
r9             0xf4bae4b8	4105888952
r10            0x80	128
r11            0x10ab1cb0	279649456
r12            0xfff024e7	4293928167
r13            0x10450748	272959304
r14            0x0	0
r15            0x0	0
r16            0x0	0
r17            0x20	32
r18            0xfb7	4023
r19            0x10ad4eb8	279793336
r20            0xf5faf808	4126865416
r21            0xfbf7150	264204624
r22            0x3	3
r23            0x939	2361
r24            0x0	0
r25            0xf4bae4b8	4105888952
r26            0x0	0
r27            0x10ab1cb0	279649456
r28            0xf4bae4e8	4105889000
r29            0x0	0
r30            0xf4bae4b8	4105888952
r31            0x10a84e80	279465600
pc             0x101b6c4c	0x101b6c4c <helper_stl_mmu+28>
msr            0x2d032	184370
cr             0x28004440	671106112
lr             0xf5faf808	0xf5faf808
ctr            0x101b6c30	270232624
xer            0x0	0
malc April 30, 2012, 3:34 p.m. UTC | #3
On Mon, 30 Apr 2012, Alexander Graf wrote:

> 
> On 30.04.2012, at 12:45, Alexander Graf wrote:
> 
> > 
> > On 22.04.2012, at 15:26, Blue Swirl wrote:
> > 
> >> Add an explicit CPUPPCState parameter instead of relying on AREG0
> >> and rename op_helper.c (which only contains load and store helpers)
> >> to mem_helper.c. Remove AREG0 swapping in
> >> tlb_fill().
> >> 
> >> Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation
> >> and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores.
> > 
> > This patch breaks qemu-system-ppc64 on ppc32 host user space for me. I'm trying to debug it down, but worst case I'll omit this patch set for 1.1.
> 
> Ok, so apparently nobody ever tested TCG_AREG0 mode with the ppc tcg 
> target. It looks as if the 
> 64-bit-guest-registers-in-32-bit-host-registers code path is missing 
> completely.
> 
> This actually makes me less confident that this is a change we want for 
> 1.1. I'll remove the patches from the queue.
> 
> 
> Alex
> 
> 
> TCG register swizzling code:
> 
> #ifdef CONFIG_TCG_PASS_AREG0
>     /* XXX/FIXME: suboptimal */
>     tcg_out_mov(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[3],
>                 tcg_target_call_iarg_regs[2]);
>     tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2],
>                 tcg_target_call_iarg_regs[1]);
>     tcg_out_mov(s, TCG_TYPE_TL, tcg_target_call_iarg_regs[1],
>                 tcg_target_call_iarg_regs[0]);
>     tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0],
>                 TCG_AREG0);
> #endif
>     tcg_out_call (s, (tcg_target_long) qemu_st_helpers[opc], 1);
> 

The above snippet is incorrect for SysV ppc32 ABI, due to misalignment
of long long argument in register file.

[..snip..]
Blue Swirl May 1, 2012, 9:15 a.m. UTC | #4
On Mon, Apr 30, 2012 at 11:51, Alexander Graf <agraf@suse.de> wrote:
>
> On 30.04.2012, at 12:45, Alexander Graf wrote:
>
>>
>> On 22.04.2012, at 15:26, Blue Swirl wrote:
>>
>>> Add an explicit CPUPPCState parameter instead of relying on AREG0
>>> and rename op_helper.c (which only contains load and store helpers)
>>> to mem_helper.c. Remove AREG0 swapping in
>>> tlb_fill().
>>>
>>> Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation
>>> and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores.
>>
>> This patch breaks qemu-system-ppc64 on ppc32 host user space for me. I'm trying to debug it down, but worst case I'll omit this patch set for 1.1.
>
> Ok, so apparently nobody ever tested TCG_AREG0 mode with the ppc tcg target. It looks as if the 64-bit-guest-registers-in-32-bit-host-registers code path is missing completely.
>
> This actually makes me less confident that this is a change we want for 1.1. I'll remove the patches from the queue.

Meh. It should be perfectly OK to apply all patches except the last
one which enables the AREG0 free mode. Also the problem with last
patch is not in the patch itself but PPC TCG host support, which by
the way is probably also broken for AREG0 free Sparc and Alpha, so I'd
really like to see them in 1.1. There should be plenty of time to fix
bugs in PPC TCG support during the freeze.

>
>
> Alex
>
>
> TCG register swizzling code:
>
> #ifdef CONFIG_TCG_PASS_AREG0
>    /* XXX/FIXME: suboptimal */
>    tcg_out_mov(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[3],
>                tcg_target_call_iarg_regs[2]);
>    tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2],
>                tcg_target_call_iarg_regs[1]);
>    tcg_out_mov(s, TCG_TYPE_TL, tcg_target_call_iarg_regs[1],
>                tcg_target_call_iarg_regs[0]);
>    tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0],
>                TCG_AREG0);
> #endif
>    tcg_out_call (s, (tcg_target_long) qemu_st_helpers[opc], 1);
>
> Log output:
>
> NIP 00000000fff024e4   LR 0000000000000000 CTR 0000000000000000 XER 0000000000000000
> MSR 0000000000000000 HID0 0000000060000000  HF 0000000000000000 idx 1
> TB 00000000 01083771 DECR 4293883502
> GPR00 0000000000000000 0000000000000000 0000000000000000 fffffffffff00000
> GPR04 0000000000000000 00000000000024b0 0000000000000000 0000000000000000
> GPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> CR 80000000  [ L  -  -  -  -  -  -  -  ]             RES ffffffffffffffff
> FPR00 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPSCR 00000000
>  SRR0 0000000000000000  SRR1 0000000000000000    PVR 00000000003c0301 VRSAVE 0000000000000000
> SPRG0 0000000000000000 SPRG1 0000000000000000  SPRG2 0000000000000000  SPRG3 0000000000000000
> SPRG4 0000000000000000 SPRG5 0000000000000000  SPRG6 0000000000000000  SPRG7 0000000000000000
>  SDR1 0000000000000000
> IN:
> 0x00000000fff024e4:  stw     r6,0(r4)
>
> OP:
>  ---- 0xfff024e4
>  movi_i32 access_type,$0x20
>  mov_i32 tmp0,r4_0
>  movi_i32 tmp1,$0x0
>  qemu_st32 r6_0,tmp0,tmp1,$0x1
>  goto_tb $0x0
>  movi_i32 nip_0,$0xfff024e8
>  movi_i32 nip_1,$0x0
>  exit_tb $0xf4bae508
>
> OUT: [size=180]
> 0xf5faf7a0:  lwz     r14,36(r27)
> 0xf5faf7a4:  lwz     r15,52(r27)
> 0xf5faf7a8:  li      r16,0
> 0xf5faf7ac:  li      r17,32
> 0xf5faf7b0:  stw     r17,672(r27)
> 0xf5faf7b4:  rlwinm  r3,r14,25,19,26
> 0xf5faf7b8:  add     r3,r3,r27
> 0xf5faf7bc:  lwzu    r4,8912(r3)
> 0xf5faf7c0:  rlwinm  r0,r14,0,30,19
> 0xf5faf7c4:  cmpw    cr7,r0,r4
> 0xf5faf7c8:  lwz     r4,4(r3)
> 0xf5faf7cc:  cmpw    cr6,r16,r4
> 0xf5faf7d0:  crand   4*cr7+eq,4*cr6+eq,4*cr7+eq
> 0xf5faf7d4:  beq-    cr7,0xf5faf80c
> 0xf5faf7d8:  mr      r3,r16
> 0xf5faf7dc:  mr      r4,r14
> 0xf5faf7e0:  mr      r5,r15
> 0xf5faf7e4:  li      r6,1
> 0xf5faf7e8:  mr      r6,r5
> 0xf5faf7ec:  mr      r5,r4
> 0xf5faf7f0:  mr      r4,r3
> 0xf5faf7f4:  mr      r3,r27
> 0xf5faf7f8:  lis     r0,4123
> 0xf5faf7fc:  ori     r0,r0,27696
> 0xf5faf800:  mtctr   r0
> 0xf5faf804:  bctrl
> 0xf5faf808:  b       0xf5faf818
> 0xf5faf80c:  lwz     r3,16(r3)
> 0xf5faf810:  add     r3,r3,r14
> 0xf5faf814:  stwx    r15,0,r3
> 0xf5faf818:  .long 0x0
> 0xf5faf81c:  .long 0x0
> 0xf5faf820:  .long 0x0
> 0xf5faf824:  .long 0x0
> 0xf5faf828:  lis     r14,-16
> 0xf5faf82c:  ori     r14,r14,9448
> 0xf5faf830:  stw     r14,668(r27)
> 0xf5faf834:  li      r14,0
> 0xf5faf838:  stw     r14,664(r27)
> 0xf5faf83c:  lis     r3,-2886
> 0xf5faf840:  ori     r3,r3,58632
> 0xf5faf844:  lis     r0,4264
> 0xf5faf848:  ori     r0,r0,20192
> 0xf5faf84c:  mtctr   r0
> 0xf5faf850:  bctr
>
> Register state at bctr into helper_stl_mmu (0xf5faf804)
>
> Breakpoint 1, helper_stl_mmu (env=0x10ab1cb0, addr=0, val=4294967295, mmu_idx=279465600)
>    at /home/agraf/release/qemu/softmmu_template.h:266
> 266         index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
> (gdb) info registers
> r0             0xf5faf808       4126865416
> r1             0xf4bac950       4105881936
> r2             0xf4bb4900       4105914624
> r3             0x10ab1cb0       279649456
> r4             0x0      0
> r5             0x0      0
> r6             0x0      0
> r7             0xffffffff       4294967295
> r8             0x10a84e80       279465600
> r9             0xf4bae4b8       4105888952
> r10            0x80     128
> r11            0x10ab1cb0       279649456
> r12            0xfff024e7       4293928167
> r13            0x10450748       272959304
> r14            0x0      0
> r15            0x0      0
> r16            0x0      0
> r17            0x20     32
> r18            0xfb7    4023
> r19            0x10ad4eb8       279793336
> r20            0xf5faf808       4126865416
> r21            0xfbf7150        264204624
> r22            0x3      3
> r23            0x939    2361
> r24            0x0      0
> r25            0xf4bae4b8       4105888952
> r26            0x0      0
> r27            0x10ab1cb0       279649456
> r28            0xf4bae4e8       4105889000
> r29            0x0      0
> r30            0xf4bae4b8       4105888952
> r31            0x10a84e80       279465600
> pc             0x101b6c4c       0x101b6c4c <helper_stl_mmu+28>
> msr            0x2d032  184370
> cr             0x28004440       671106112
> lr             0xf5faf808       0xf5faf808
> ctr            0x101b6c30       270232624
> xer            0x0      0
Alexander Graf May 1, 2012, 2:25 p.m. UTC | #5
On 01.05.2012, at 11:15, Blue Swirl <blauwirbel@gmail.com> wrote:

> On Mon, Apr 30, 2012 at 11:51, Alexander Graf <agraf@suse.de> wrote:
>> 
>> On 30.04.2012, at 12:45, Alexander Graf wrote:
>> 
>>> 
>>> On 22.04.2012, at 15:26, Blue Swirl wrote:
>>> 
>>>> Add an explicit CPUPPCState parameter instead of relying on AREG0
>>>> and rename op_helper.c (which only contains load and store helpers)
>>>> to mem_helper.c. Remove AREG0 swapping in
>>>> tlb_fill().
>>>> 
>>>> Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation
>>>> and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores.
>>> 
>>> This patch breaks qemu-system-ppc64 on ppc32 host user space for me. I'm trying to debug it down, but worst case I'll omit this patch set for 1.1.
>> 
>> Ok, so apparently nobody ever tested TCG_AREG0 mode with the ppc tcg target. It looks as if the 64-bit-guest-registers-in-32-bit-host-registers code path is missing completely.
>> 
>> This actually makes me less confident that this is a change we want for 1.1. I'll remove the patches from the queue.
> 
> Meh. It should be perfectly OK to apply all patches except the last
> one which enables the AREG0 free mode.

Yeah, that's what I did at first, but that still didn't get me into usable user space inside a ppc64 guest. Right now I don't have the time to track down if it's due to your patches and if so, why.

> Also the problem with last
> patch is not in the patch itself but PPC TCG host support, which by
> the way is probably also broken for AREG0 free Sparc and Alpha, so I'd
> really like to see them in 1.1.

I do agree on the first part. We need to make sure to test sparc and alpha targets on unusual host platforms and fix them there. But why do we need to also break ppc along the way?

> There should be plenty of time to fix
> bugs in PPC TCG support during the freeze.

Since this is a non user visible feature (in fact, looking at the emitted asm code it'll be more of a slowdown than anything), I'd rather like to keep 1.1 stable and get this into git right after the release split.

It's really not going against your patches. In fact, I really do like them - especially the cleanups. But this feature is pretty invasive and at least I do run ppc-on-ppc tcg, so we should be able to hammer out all bugs until the next release :). The whole AREG0 thing could also use some optimization love...


Alex
Blue Swirl May 1, 2012, 4:58 p.m. UTC | #6
On Tue, May 1, 2012 at 14:25, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 01.05.2012, at 11:15, Blue Swirl <blauwirbel@gmail.com> wrote:
>
>> On Mon, Apr 30, 2012 at 11:51, Alexander Graf <agraf@suse.de> wrote:
>>>
>>> On 30.04.2012, at 12:45, Alexander Graf wrote:
>>>
>>>>
>>>> On 22.04.2012, at 15:26, Blue Swirl wrote:
>>>>
>>>>> Add an explicit CPUPPCState parameter instead of relying on AREG0
>>>>> and rename op_helper.c (which only contains load and store helpers)
>>>>> to mem_helper.c. Remove AREG0 swapping in
>>>>> tlb_fill().
>>>>>
>>>>> Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation
>>>>> and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores.
>>>>
>>>> This patch breaks qemu-system-ppc64 on ppc32 host user space for me. I'm trying to debug it down, but worst case I'll omit this patch set for 1.1.
>>>
>>> Ok, so apparently nobody ever tested TCG_AREG0 mode with the ppc tcg target. It looks as if the 64-bit-guest-registers-in-32-bit-host-registers code path is missing completely.
>>>
>>> This actually makes me less confident that this is a change we want for 1.1. I'll remove the patches from the queue.
>>
>> Meh. It should be perfectly OK to apply all patches except the last
>> one which enables the AREG0 free mode.
>
> Yeah, that's what I did at first, but that still didn't get me into usable user space inside a ppc64 guest. Right now I don't have the time to track down if it's due to your patches and if so, why.

On second thought, I probably tested the set too lightly, so
'perfectly OK' was way too bold a claim.

>> Also the problem with last
>> patch is not in the patch itself but PPC TCG host support, which by
>> the way is probably also broken for AREG0 free Sparc and Alpha, so I'd
>> really like to see them in 1.1.
>
> I do agree on the first part. We need to make sure to test sparc and alpha targets on unusual host platforms and fix them there. But why do we need to also break ppc along the way?
>
>> There should be plenty of time to fix
>> bugs in PPC TCG support during the freeze.
>
> Since this is a non user visible feature (in fact, looking at the emitted asm code it'll be more of a slowdown than anything), I'd rather like to keep 1.1 stable and get this into git right after the release split.
>
> It's really not going against your patches. In fact, I really do like them - especially the cleanups. But this feature is pretty invasive and at least I do run ppc-on-ppc tcg, so we should be able to hammer out all bugs until the next release :). The whole AREG0 thing could also use some optimization love...

OK.

>
>
> Alex
>
Alexander Graf May 2, 2012, 1 p.m. UTC | #7
On 04/30/2012 05:34 PM, malc wrote:
> On Mon, 30 Apr 2012, Alexander Graf wrote:
>
>> On 30.04.2012, at 12:45, Alexander Graf wrote:
>>
>>> On 22.04.2012, at 15:26, Blue Swirl wrote:
>>>
>>>> Add an explicit CPUPPCState parameter instead of relying on AREG0
>>>> and rename op_helper.c (which only contains load and store helpers)
>>>> to mem_helper.c. Remove AREG0 swapping in
>>>> tlb_fill().
>>>>
>>>> Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation
>>>> and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores.
>>> This patch breaks qemu-system-ppc64 on ppc32 host user space for me. I'm trying to debug it down, but worst case I'll omit this patch set for 1.1.
>> Ok, so apparently nobody ever tested TCG_AREG0 mode with the ppc tcg
>> target. It looks as if the
>> 64-bit-guest-registers-in-32-bit-host-registers code path is missing
>> completely.
>>
>> This actually makes me less confident that this is a change we want for
>> 1.1. I'll remove the patches from the queue.
>>
>>
>> Alex
>>
>>
>> TCG register swizzling code:
>>
>> #ifdef CONFIG_TCG_PASS_AREG0
>>      /* XXX/FIXME: suboptimal */
>>      tcg_out_mov(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[3],
>>                  tcg_target_call_iarg_regs[2]);
>>      tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2],
>>                  tcg_target_call_iarg_regs[1]);
>>      tcg_out_mov(s, TCG_TYPE_TL, tcg_target_call_iarg_regs[1],
>>                  tcg_target_call_iarg_regs[0]);
>>      tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0],
>>                  TCG_AREG0);
>> #endif
>>      tcg_out_call (s, (tcg_target_long) qemu_st_helpers[opc], 1);
>>
> The above snippet is incorrect for SysV ppc32 ABI, due to misalignment
> of long long argument in register file.

Hmm - so what would be the correct version? :)


Alex
diff mbox

Patch

diff --git a/Makefile.target b/Makefile.target
index 68a4cb5..bac7656 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -82,10 +82,12 @@  libobj-y += tcg/tcg.o tcg/optimize.o
 libobj-$(CONFIG_TCG_INTERPRETER) += tci.o
 libobj-y += fpu/softfloat.o
 ifneq ($(TARGET_BASE_ARCH), sparc)
+ifneq ($(TARGET_BASE_ARCH), ppc)
 ifneq ($(TARGET_BASE_ARCH), alpha)
 libobj-y += op_helper.o
 endif
 endif
+endif
 libobj-y += helper.o
 ifeq ($(TARGET_BASE_ARCH), i386)
 libobj-y += cpu.o
@@ -108,7 +110,7 @@  libobj-$(TARGET_ALPHA) += int_helper.o
fpu_helper.o sys_helper.o mem_helper.o
 libobj-$(TARGET_ALPHA) += cpu.o
 ifeq ($(TARGET_BASE_ARCH), ppc)
 libobj-y += fpu_helper.o int_helper.o mmu_helper.o excp_helper.o
-libobj-y += timebase_helper.o misc_helper.o
+libobj-y += timebase_helper.o misc_helper.o mem_helper.o
 endif

 libobj-y += disas.o
@@ -120,7 +122,7 @@  $(libobj-y): $(GENERATED_HEADERS)

 # HELPER_CFLAGS is used for all the legacy code compiled with static register
 # variables
-ifneq ($(TARGET_BASE_ARCH), sparc)
+ifndef CONFIG_TCG_PASS_AREG0
 op_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 endif
 user-exec.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
diff --git a/configure b/configure
index 2d62d12..993d90e 100755
--- a/configure
+++ b/configure
@@ -3633,7 +3633,7 @@  case "$target_arch2" in
 esac

 case "$target_arch2" in
-  alpha | sparc*)
+  alpha | ppc* | sparc*)
     echo "CONFIG_TCG_PASS_AREG0=y" >> $config_target_mak
   ;;
 esac
diff --git a/cpu-all.h b/cpu-all.h
index f7d6867..9d4db0a 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -291,6 +291,15 @@  extern unsigned long reserved_va;
 #define stfl_kernel(p, v) stfl_raw(p, v)
 #define stfq_kernel(p, vt) stfq_raw(p, v)

+#ifdef CONFIG_TCG_PASS_AREG0
+#define cpu_ldub_data(env, addr) ldub_raw(addr)
+#define cpu_lduw_data(env, addr) lduw_raw(addr)
+#define cpu_ldl_data(env, addr) ldl_raw(addr)
+
+#define cpu_stb_data(env, addr, data) stb_raw(addr, data)
+#define cpu_stw_data(env, addr, data) stw_raw(addr, data)
+#define cpu_stl_data(env, addr, data) stl_raw(addr, data)
+#endif
 #endif /* defined(CONFIG_USER_ONLY) */

 /* page related stuff */
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 7fa7a59..c7762b9 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -179,7 +179,8 @@  static inline void powerpc_excp(CPUPPCState *env,
int excp_model, int excp)
         }
         /* XXX: this is false */
         /* Get rS/rD and rA from faulting opcode */
-        env->spr[SPR_DSISR] |= (ldl_code((env->nip - 4)) & 0x03FF0000) >> 16;
+        env->spr[SPR_DSISR] |= (cpu_ldl_code(env, (env->nip - 4))
+                                & 0x03FF0000) >> 16;
         goto store_current;
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index b7a157e..ddab97b 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -20,15 +20,15 @@  DEF_HELPER_1(hrfid, void, env)
 #endif
 #endif

-DEF_HELPER_2(lmw, void, tl, i32)
-DEF_HELPER_2(stmw, void, tl, i32)
-DEF_HELPER_3(lsw, void, tl, i32, i32)
-DEF_HELPER_4(lswx, void, tl, i32, i32, i32)
-DEF_HELPER_3(stsw, void, tl, i32, i32)
-DEF_HELPER_1(dcbz, void, tl)
-DEF_HELPER_1(dcbz_970, void, tl)
-DEF_HELPER_1(icbi, void, tl)
-DEF_HELPER_4(lscbx, tl, tl, i32, i32, i32)
+DEF_HELPER_3(lmw, void, env, tl, i32)
+DEF_HELPER_3(stmw, void, env, tl, i32)
+DEF_HELPER_4(lsw, void, env, tl, i32, i32)
+DEF_HELPER_5(lswx, void, env, tl, i32, i32, i32)
+DEF_HELPER_4(stsw, void, env, tl, i32, i32)
+DEF_HELPER_2(dcbz, void, env, tl)
+DEF_HELPER_2(dcbz_970, void, env, tl)
+DEF_HELPER_2(icbi, void, env, tl)
+DEF_HELPER_5(lscbx, tl, env, tl, i32, i32, i32)

 #if defined(TARGET_PPC64)
 DEF_HELPER_FLAGS_2(mulhd, TCG_CALL_CONST | TCG_CALL_PURE, i64, i64, i64)
@@ -226,12 +226,12 @@  DEF_HELPER_5(vmsumshm, void, env, avr, avr, avr, avr)
 DEF_HELPER_5(vmsumshs, void, env, avr, avr, avr, avr)
 DEF_HELPER_4(vmladduhm, void, avr, avr, avr, avr)
 DEF_HELPER_2(mtvscr, void, env, avr);
-DEF_HELPER_2(lvebx, void, avr, tl)
-DEF_HELPER_2(lvehx, void, avr, tl)
-DEF_HELPER_2(lvewx, void, avr, tl)
-DEF_HELPER_2(stvebx, void, avr, tl)
-DEF_HELPER_2(stvehx, void, avr, tl)
-DEF_HELPER_2(stvewx, void, avr, tl)
+DEF_HELPER_3(lvebx, void, env, avr, tl)
+DEF_HELPER_3(lvehx, void, env, avr, tl)
+DEF_HELPER_3(lvewx, void, env, avr, tl)
+DEF_HELPER_3(stvebx, void, env, avr, tl)
+DEF_HELPER_3(stvehx, void, env, avr, tl)
+DEF_HELPER_3(stvewx, void, env, avr, tl)
 DEF_HELPER_4(vsumsws, void, env, avr, avr, avr)
 DEF_HELPER_4(vsum2sws, void, env, avr, avr, avr)
 DEF_HELPER_4(vsum4sbs, void, env, avr, avr, avr)
diff --git a/target-ppc/op_helper.c b/target-ppc/mem_helper.c
similarity index 68%
rename from target-ppc/op_helper.c
rename to target-ppc/mem_helper.c
index cd1a533..ebcd7b2 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/mem_helper.c
@@ -1,5 +1,5 @@ 
 /*
- *  PowerPC emulation helpers for QEMU.
+ *  PowerPC memory access emulation helpers for QEMU.
  *
  *  Copyright (c) 2003-2007 Jocelyn Mayer
  *
@@ -16,9 +16,7 @@ 
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
-#include <string.h>
 #include "cpu.h"
-#include "dyngen-exec.h"
 #include "host-utils.h"
 #include "helper.h"

@@ -33,7 +31,8 @@ 
 /*****************************************************************************/
 /* Memory load and stores */

-static inline target_ulong addr_add(target_ulong addr, target_long arg)
+static inline target_ulong addr_add(CPUPPCState *env, target_ulong addr,
+                                    target_long arg)
 {
 #if defined(TARGET_PPC64)
     if (!msr_sf) {
@@ -45,44 +44,44 @@  static inline target_ulong addr_add(target_ulong
addr, target_long arg)
     }
 }

-void helper_lmw(target_ulong addr, uint32_t reg)
+void helper_lmw(CPUPPCState *env, target_ulong addr, uint32_t reg)
 {
     for (; reg < 32; reg++) {
         if (msr_le) {
-            env->gpr[reg] = bswap32(ldl(addr));
+            env->gpr[reg] = bswap32(cpu_ldl_data(env, addr));
         } else {
-            env->gpr[reg] = ldl(addr);
+            env->gpr[reg] = cpu_ldl_data(env, addr);
         }
-        addr = addr_add(addr, 4);
+        addr = addr_add(env, addr, 4);
     }
 }

-void helper_stmw(target_ulong addr, uint32_t reg)
+void helper_stmw(CPUPPCState *env, target_ulong addr, uint32_t reg)
 {
     for (; reg < 32; reg++) {
         if (msr_le) {
-            stl(addr, bswap32((uint32_t)env->gpr[reg]));
+            cpu_stl_data(env, addr, bswap32((uint32_t)env->gpr[reg]));
         } else {
-            stl(addr, (uint32_t)env->gpr[reg]);
+            cpu_stl_data(env, addr, (uint32_t)env->gpr[reg]);
         }
-        addr = addr_add(addr, 4);
+        addr = addr_add(env, addr, 4);
     }
 }

-void helper_lsw(target_ulong addr, uint32_t nb, uint32_t reg)
+void helper_lsw(CPUPPCState *env, target_ulong addr, uint32_t nb, uint32_t reg)
 {
     int sh;

     for (; nb > 3; nb -= 4) {
-        env->gpr[reg] = ldl(addr);
+        env->gpr[reg] = cpu_ldl_data(env, addr);
         reg = (reg + 1) % 32;
-        addr = addr_add(addr, 4);
+        addr = addr_add(env, addr, 4);
     }
     if (unlikely(nb > 0)) {
         env->gpr[reg] = 0;
         for (sh = 24; nb > 0; nb--, sh -= 8) {
-            env->gpr[reg] |= ldub(addr) << sh;
-            addr = addr_add(addr, 1);
+            env->gpr[reg] |= cpu_ldub_data(env, addr) << sh;
+            addr = addr_add(env, addr, 1);
         }
     }
 }
@@ -91,7 +90,8 @@  void helper_lsw(target_ulong addr, uint32_t nb, uint32_t reg)
  * In an other hand, IBM says this is valid, but rA won't be loaded.
  * For now, I'll follow the spec...
  */
-void helper_lswx(target_ulong addr, uint32_t reg, uint32_t ra, uint32_t rb)
+void helper_lswx(CPUPPCState *env, target_ulong addr, uint32_t reg,
+                 uint32_t ra, uint32_t rb)
 {
     if (likely(xer_bc != 0)) {
         if (unlikely((ra != 0 && reg < ra && (reg + xer_bc) > ra) ||
@@ -100,56 +100,57 @@  void helper_lswx(target_ulong addr, uint32_t
reg, uint32_t ra, uint32_t rb)
                                        POWERPC_EXCP_INVAL |
                                        POWERPC_EXCP_INVAL_LSWX);
         } else {
-            helper_lsw(addr, xer_bc, reg);
+            helper_lsw(env, addr, xer_bc, reg);
         }
     }
 }

-void helper_stsw(target_ulong addr, uint32_t nb, uint32_t reg)
+void helper_stsw(CPUPPCState *env, target_ulong addr, uint32_t nb,
+                 uint32_t reg)
 {
     int sh;

     for (; nb > 3; nb -= 4) {
-        stl(addr, env->gpr[reg]);
+        cpu_stl_data(env, addr, env->gpr[reg]);
         reg = (reg + 1) % 32;
-        addr = addr_add(addr, 4);
+        addr = addr_add(env, addr, 4);
     }
     if (unlikely(nb > 0)) {
         for (sh = 24; nb > 0; nb--, sh -= 8) {
-            stb(addr, (env->gpr[reg] >> sh) & 0xFF);
-            addr = addr_add(addr, 1);
+            cpu_stb_data(env, addr, (env->gpr[reg] >> sh) & 0xFF);
+            addr = addr_add(env, addr, 1);
         }
     }
 }

-static void do_dcbz(target_ulong addr, int dcache_line_size)
+static void do_dcbz(CPUPPCState *env, target_ulong addr, int dcache_line_size)
 {
     int i;

     addr &= ~(dcache_line_size - 1);
     for (i = 0; i < dcache_line_size; i += 4) {
-        stl(addr + i, 0);
+        cpu_stl_data(env, addr + i, 0);
     }
     if (env->reserve_addr == addr) {
         env->reserve_addr = (target_ulong)-1ULL;
     }
 }

-void helper_dcbz(target_ulong addr)
+void helper_dcbz(CPUPPCState *env, target_ulong addr)
 {
-    do_dcbz(addr, env->dcache_line_size);
+    do_dcbz(env, addr, env->dcache_line_size);
 }

-void helper_dcbz_970(target_ulong addr)
+void helper_dcbz_970(CPUPPCState *env, target_ulong addr)
 {
     if (((env->spr[SPR_970_HID5] >> 7) & 0x3) == 1) {
-        do_dcbz(addr, 32);
+        do_dcbz(env, addr, 32);
     } else {
-        do_dcbz(addr, env->dcache_line_size);
+        do_dcbz(env, addr, env->dcache_line_size);
     }
 }

-void helper_icbi(target_ulong addr)
+void helper_icbi(CPUPPCState *env, target_ulong addr)
 {
     addr &= ~(env->dcache_line_size - 1);
     /* Invalidate one cache line :
@@ -157,19 +158,19 @@  void helper_icbi(target_ulong addr)
      * (not a fetch) by the MMU. To be sure it will be so,
      * do the load "by hand".
      */
-    ldl(addr);
+    cpu_ldl_data(env, addr);
 }

 /* XXX: to be tested */
-target_ulong helper_lscbx(target_ulong addr, uint32_t reg, uint32_t ra,
-                          uint32_t rb)
+target_ulong helper_lscbx(CPUPPCState *env, target_ulong addr, uint32_t reg,
+                          uint32_t ra, uint32_t rb)
 {
     int i, c, d;

     d = 24;
     for (i = 0; i < xer_bc; i++) {
-        c = ldub(addr);
-        addr = addr_add(addr, 1);
+        c = cpu_ldub_data(env, addr);
+        addr = addr_add(env, addr, 1);
         /* ra (if not 0) and rb are never modified */
         if (likely(reg != rb && (ra == 0 || reg != ra))) {
             env->gpr[reg] = (env->gpr[reg] & ~(0xFF << d)) | (c << d);
@@ -199,7 +200,8 @@  target_ulong helper_lscbx(target_ulong addr,
uint32_t reg, uint32_t ra,
 #endif

 #define LVE(name, access, swap, element)                        \
-    void helper_##name(ppc_avr_t *r, target_ulong addr)         \
+    void helper_##name(CPUPPCState *env, ppc_avr_t *r,          \
+                       target_ulong addr)                       \
     {                                                           \
         size_t n_elems = ARRAY_SIZE(r->element);                \
         int adjust = HI_IDX*(n_elems - 1);                      \
@@ -208,21 +210,22 @@  target_ulong helper_lscbx(target_ulong addr,
uint32_t reg, uint32_t ra,
                                                                 \
         if (msr_le) {                                           \
             r->element[LO_IDX ? index : (adjust - index)] =     \
-                swap(access(addr));                             \
+                swap(access(env, addr));                        \
         } else {                                                \
             r->element[LO_IDX ? index : (adjust - index)] =     \
-                access(addr);                                   \
+                access(env, addr);                              \
         }                                                       \
     }
 #define I(x) (x)
-LVE(lvebx, ldub, I, u8)
-LVE(lvehx, lduw, bswap16, u16)
-LVE(lvewx, ldl, bswap32, u32)
+LVE(lvebx, cpu_ldub_data, I, u8)
+LVE(lvehx, cpu_lduw_data, bswap16, u16)
+LVE(lvewx, cpu_ldl_data, bswap32, u32)
 #undef I
 #undef LVE

 #define STVE(name, access, swap, element)                               \
-    void helper_##name(ppc_avr_t *r, target_ulong addr)                 \
+    void helper_##name(CPUPPCState *env, ppc_avr_t *r,                  \
+                       target_ulong addr)                               \
     {                                                                   \
         size_t n_elems = ARRAY_SIZE(r->element);                        \
         int adjust = HI_IDX * (n_elems - 1);                            \
@@ -230,15 +233,17 @@  LVE(lvewx, ldl, bswap32, u32)
         int index = (addr & 0xf) >> sh;                                 \
                                                                         \
         if (msr_le) {                                                   \
-            access(addr, swap(r->element[LO_IDX ? index : (adjust -
index)])); \
+            access(env, addr, swap(r->element[LO_IDX ? index :          \
+                                              (adjust - index)]));      \
         } else {                                                        \
-            access(addr, r->element[LO_IDX ? index : (adjust - index)]); \
+            access(env, addr, r->element[LO_IDX ? index :               \
+                                         (adjust - index)]);            \
         }                                                               \
     }
 #define I(x) (x)
-STVE(stvebx, stb, I, u8)
-STVE(stvehx, stw, bswap16, u16)
-STVE(stvewx, stl, bswap32, u32)
+STVE(stvebx, cpu_stb_data, I, u8)
+STVE(stvehx, cpu_stw_data, bswap16, u16)
+STVE(stvewx, cpu_stl_data, bswap32, u32)
 #undef I
 #undef LVE

@@ -267,15 +272,12 @@  STVE(stvewx, stl, bswap32, u32)
    NULL, it means that the function was called in C code (i.e. not
    from generated code or from helper.c) */
 /* XXX: fix it to restore all registers */
-void tlb_fill(CPUPPCState *env1, target_ulong addr, int is_write, int mmu_idx,
+void tlb_fill(CPUPPCState *env, target_ulong addr, int is_write, int mmu_idx,
               uintptr_t retaddr)
 {
     TranslationBlock *tb;
-    CPUPPCState *saved_env;
     int ret;

-    saved_env = env;
-    env = env1;
     ret = cpu_ppc_handle_mmu_fault(env, addr, is_write, mmu_idx);
     if (unlikely(ret != 0)) {
         if (likely(retaddr)) {
@@ -289,6 +291,5 @@  void tlb_fill(CPUPPCState *env1, target_ulong
addr, int is_write, int mmu_idx,
         }
         helper_raise_exception_err(env, env->exception_index, env->error_code);
     }
-    env = saved_env;
 }
 #endif /* !CONFIG_USER_ONLY */
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index bcc9933..9103fd5 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2989,7 +2989,7 @@  static void gen_lmw(DisasContext *ctx)
     t0 = tcg_temp_new();
     t1 = tcg_const_i32(rD(ctx->opcode));
     gen_addr_imm_index(ctx, t0, 0);
-    gen_helper_lmw(t0, t1);
+    gen_helper_lmw(cpu_env, t0, t1);
     tcg_temp_free(t0);
     tcg_temp_free_i32(t1);
 }
@@ -3005,7 +3005,7 @@  static void gen_stmw(DisasContext *ctx)
     t0 = tcg_temp_new();
     t1 = tcg_const_i32(rS(ctx->opcode));
     gen_addr_imm_index(ctx, t0, 0);
-    gen_helper_stmw(t0, t1);
+    gen_helper_stmw(cpu_env, t0, t1);
     tcg_temp_free(t0);
     tcg_temp_free_i32(t1);
 }
@@ -3043,7 +3043,7 @@  static void gen_lswi(DisasContext *ctx)
     gen_addr_register(ctx, t0);
     t1 = tcg_const_i32(nb);
     t2 = tcg_const_i32(start);
-    gen_helper_lsw(t0, t1, t2);
+    gen_helper_lsw(cpu_env, t0, t1, t2);
     tcg_temp_free(t0);
     tcg_temp_free_i32(t1);
     tcg_temp_free_i32(t2);
@@ -3062,7 +3062,7 @@  static void gen_lswx(DisasContext *ctx)
     t1 = tcg_const_i32(rD(ctx->opcode));
     t2 = tcg_const_i32(rA(ctx->opcode));
     t3 = tcg_const_i32(rB(ctx->opcode));
-    gen_helper_lswx(t0, t1, t2, t3);
+    gen_helper_lswx(cpu_env, t0, t1, t2, t3);
     tcg_temp_free(t0);
     tcg_temp_free_i32(t1);
     tcg_temp_free_i32(t2);
@@ -3084,7 +3084,7 @@  static void gen_stswi(DisasContext *ctx)
         nb = 32;
     t1 = tcg_const_i32(nb);
     t2 = tcg_const_i32(rS(ctx->opcode));
-    gen_helper_stsw(t0, t1, t2);
+    gen_helper_stsw(cpu_env, t0, t1, t2);
     tcg_temp_free(t0);
     tcg_temp_free_i32(t1);
     tcg_temp_free_i32(t2);
@@ -3104,7 +3104,7 @@  static void gen_stswx(DisasContext *ctx)
     tcg_gen_trunc_tl_i32(t1, cpu_xer);
     tcg_gen_andi_i32(t1, t1, 0x7F);
     t2 = tcg_const_i32(rS(ctx->opcode));
-    gen_helper_stsw(t0, t1, t2);
+    gen_helper_stsw(cpu_env, t0, t1, t2);
     tcg_temp_free(t0);
     tcg_temp_free_i32(t1);
     tcg_temp_free_i32(t2);
@@ -4116,7 +4116,7 @@  static void gen_dcbz(DisasContext *ctx)
     gen_update_nip(ctx, ctx->nip - 4);
     t0 = tcg_temp_new();
     gen_addr_reg_index(ctx, t0);
-    gen_helper_dcbz(t0);
+    gen_helper_dcbz(cpu_env, t0);
     tcg_temp_free(t0);
 }

@@ -4129,9 +4129,9 @@  static void gen_dcbz_970(DisasContext *ctx)
     t0 = tcg_temp_new();
     gen_addr_reg_index(ctx, t0);
     if (ctx->opcode & 0x00200000)
-        gen_helper_dcbz(t0);
+        gen_helper_dcbz(cpu_env, t0);
     else
-        gen_helper_dcbz_970(t0);
+        gen_helper_dcbz_970(cpu_env, t0);
     tcg_temp_free(t0);
 }

@@ -4171,7 +4171,7 @@  static void gen_icbi(DisasContext *ctx)
     gen_update_nip(ctx, ctx->nip - 4);
     t0 = tcg_temp_new();
     gen_addr_reg_index(ctx, t0);
-    gen_helper_icbi(t0);
+    gen_helper_icbi(cpu_env, t0);
     tcg_temp_free(t0);
 }

@@ -4663,7 +4663,7 @@  static void gen_lscbx(DisasContext *ctx)
     gen_addr_reg_index(ctx, t0);
     /* NIP cannot be restored if the memory exception comes from an helper */
     gen_update_nip(ctx, ctx->nip - 4);
-    gen_helper_lscbx(t0, t0, t1, t2, t3);
+    gen_helper_lscbx(t0, cpu_env, t0, t1, t2, t3);
     tcg_temp_free_i32(t1);
     tcg_temp_free_i32(t2);
     tcg_temp_free_i32(t3);
@@ -6387,7 +6387,7 @@  static void gen_lve##name(DisasContext *ctx)
                       \
         EA = tcg_temp_new();                                            \
         gen_addr_reg_index(ctx, EA);                                    \
         rs = gen_avr_ptr(rS(ctx->opcode));                              \
-        gen_helper_lve##name (rs, EA);                                  \
+        gen_helper_lve##name(cpu_env, rs, EA);                          \
         tcg_temp_free(EA);                                              \
         tcg_temp_free_ptr(rs);                                          \