Re: [Qemu-devel] [PATCH qom-next 57/59] cpu: Introduce mandatory tlb_flush callback

[Andreas Färber]

Am 30.05.2012 11:28, schrieb Peter Maydell:
> On 23 May 2012 04:08, Andreas Färber <address@hidden> wrote:
>> +void cpu_tlb_flush(CPUState *cpu, bool flush_global)
>> +{
>> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>> +
>> +    g_assert(cc->tlb_flush != NULL);
>> +
>> +    cc->tlb_flush(cpu, flush_global);
>> +}
> 
> This needs to be able to call tlb_flush() itself
> rather than having to have every single subclass of CPUState
> implement an identical tlb_flush method. You could do this
> if there was a CPU_GET_ENV()...

Which is exactly the point: CPUState does not know about the
target-specific "env". And CPU_GET_ENV() is just plain wrong
conceptually because it adds yet another cpu.h dependency.

What we could do is have a void *envp field in CPUState that points to
env but then we'd still need the new function as a wrapper.

For the TLB the plan was to split the TLB off from CPUState mid-term,
see the sun4v-softmmu discussion and the one on part 3. The problem
there is that it contains both targer_phys_addr_t and target_ulong, so
some Makefile fiddling will be required to make it work, so I'd like to
defer that. We're already seeing two new softmmu targets on the list
plus my RL78 that I'm still rebasing onto QOM, so I'd like to move
forward with a subset of all possible improvements before I have to
touch and get acks for even more targets irrelevant to SUSE's business.
We can trim patches from the back if need be.

>> 16 files changed, 173 insertions(+), 0 deletions(-)
> 
> ...which is a lot of extra code to be inserting to do nothing
> that's specific to a particular target.

You're forgetting, env is in fact specific to the particular target.

There's a separation between old code using env and new, clean code:
Just like Anthony doesn't want old concepts rewritten with the new type
(cf. object_realize() discussion) I don't want the old cpu.h #define
mess leaking into code that I'm redesigning specifically to get rid of
that target-*/cpu.h dependency in favor of a single qemu/cpu.h.
qom/cpu.c is by definition not compiled per target so it cannot contain
any target-specific code.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg