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

[Alexander Graf]

On 01.05.2012, at 11:15, Blue Swirl <address@hidden> wrote:

> On Mon, Apr 30, 2012 at 11:51, Alexander Graf <address@hidden> 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