* Re: [Qemu-devel] [Qemu-ppc] [PATCH 20/22] ppc: move load and store helpers, switch to AREG0 free mode [not found] ` <5B21E504-3B25-45B3-8951-5AB66D7833B1@suse.de> @ 2012-05-01 9:15 ` Blue Swirl 2012-05-01 14:25 ` Alexander Graf [not found] ` <alpine.LNX.2.00.1204301933220.2648@linmac> 1 sibling, 1 reply; 4+ messages in thread From: Blue Swirl @ 2012-05-01 9:15 UTC (permalink / raw) To: Alexander Graf; +Cc: qemu-ppc, qemu-devel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 20/22] ppc: move load and store helpers, switch to AREG0 free mode 2012-05-01 9:15 ` [Qemu-devel] [Qemu-ppc] [PATCH 20/22] ppc: move load and store helpers, switch to AREG0 free mode Blue Swirl @ 2012-05-01 14:25 ` Alexander Graf 2012-05-01 16:58 ` Blue Swirl 0 siblings, 1 reply; 4+ messages in thread From: Alexander Graf @ 2012-05-01 14:25 UTC (permalink / raw) To: Blue Swirl; +Cc: qemu-ppc@nongnu.org, qemu-devel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 20/22] ppc: move load and store helpers, switch to AREG0 free mode 2012-05-01 14:25 ` Alexander Graf @ 2012-05-01 16:58 ` Blue Swirl 0 siblings, 0 replies; 4+ messages in thread From: Blue Swirl @ 2012-05-01 16:58 UTC (permalink / raw) To: Alexander Graf; +Cc: qemu-ppc@nongnu.org, qemu-devel 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 > ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <alpine.LNX.2.00.1204301933220.2648@linmac>]
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 20/22] ppc: move load and store helpers, switch to AREG0 free mode [not found] ` <alpine.LNX.2.00.1204301933220.2648@linmac> @ 2012-05-02 13:00 ` Alexander Graf 0 siblings, 0 replies; 4+ messages in thread From: Alexander Graf @ 2012-05-02 13:00 UTC (permalink / raw) To: malc; +Cc: Blue Swirl, qemu-ppc, qemu-devel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-05-02 13:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CAAu8pHsxreCwtRftLOaw8LYwjoUquODcQqDigkytMME6Jz7+7Q@mail.gmail.com>
[not found] ` <EF93EC8A-17FA-416D-9BCF-D7AE1C7FC1C9@suse.de>
[not found] ` <5B21E504-3B25-45B3-8951-5AB66D7833B1@suse.de>
2012-05-01 9:15 ` [Qemu-devel] [Qemu-ppc] [PATCH 20/22] ppc: move load and store helpers, switch to AREG0 free mode Blue Swirl
2012-05-01 14:25 ` Alexander Graf
2012-05-01 16:58 ` Blue Swirl
[not found] ` <alpine.LNX.2.00.1204301933220.2648@linmac>
2012-05-02 13:00 ` Alexander Graf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).