[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC PATCH 02/34] tcg+qom: QOMify core CPU defintions

From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [RFC PATCH 02/34] tcg+qom: QOMify core CPU defintions
Date: Tue, 12 May 2015 00:23:33 -0700

On Mon, May 11, 2015 at 1:18 PM, Richard Henderson <address@hidden> wrote:
> On 05/11/2015 03:24 AM, Paolo Bonzini wrote:
>> On 11/05/2015 12:18, Andreas Färber wrote:
>>>>> +    int (*cpu_mmu_index)(CPUState *cpu);
>>>>> +    void (*cpu_get_tb_cpu_state)(CPUState *cpu,
>>>>> +                                 void *pc, /* target_long * */
>>>>> +                                 void *cs_base, /* target_long */
>>>>> +                                 int *flags);
>>>>> +    void (*gen_intermediate_code)(void *env, struct TranslationBlock 
>>>>> *tb);
>>>>> +    void (*gen_intermediate_code_pc)(void *env, struct TranslationBlock 
>>>>> *tb);
>>>>> +    void (*restore_state_to_opc)(void *env, struct TranslationBlock *tb,
>>>>> +                                 int pc_pos);
>>>>> +    void (*tlb_fill)(CPUState *cs, uint64_t addr, int is_write, int 
>>>>> mmu_idx,
>>>>> +                     uintptr_t retaddr);
>>>>>  } CPUClass;
>>> [snip]
>>> Paolo had objected to this when I tried it. The counter-suggestion was
>>> something about reworking how the cputlb code is built per target -
>>> please check the archives.
>> Right.  My point was that these functions are not polymorphic.  Each
>> call to these should know exactly which function to call.
> That's some major surgery you have planned there.

Just tried this. It's ... rather hard.

> Especially the path via the qemu_ld/st helpers, where function to call is
> currently hard-coded into the tcg backend.

Yes. I actually seemed to make decent progress on the multi-compile
approach until I hit this brick wall:

tcg/i386/tcg-target.c:1131:17: error: ‘helper_ret_ldub_mmu’ undeclared
here (not in a function)
     [MO_UB]   = helper_ret_ldub_mmu,

In my multi-compile approach helper_*[ld|st]* needs to be renamed
per-arch for the multiple compiled cputlb.o. Hence I have no symbol
with the unqualified name. But even if I do solve my namespacing
problem, I still have an ambiguity of which cputlb.o provided
helper_*[ld|st]* to use from the TCG backend. This would mean all
those APIs would have to virtualised. The big question for Paolo, is
what complete set of APIs defines the common-code/non-common-code
boundary? tlb_fill does seem to do the job nicely and looking at the
architecture implementations it's not a super fast path (falling back
to a page table faulter).

Somewhere along the call path from the qemu_st_helpers uses
(tcg/i386/tcg-target.c) through to tlb_fill there has to be a
virtualised function unless I am missing something?

> I think that this is a decent step forward, modulo the conditionals along the
> use paths.  I think we ought to clean up all of the translators to the new QOM
> hooks.

So the conditional can be ditched by having the CPU base class
defaulting the hook to the globally defined function. Then arches can
be brought online one-by-one.

> I can't imagine that most of these hooks are called frequently enough that the
> indirect call really matters.  Certainly gen_intermediate_code need not use 
> the
> hook when initializing the mmu_idx in the DisasContext.

Ok so the solution to this is to opt-out of the hook via a re-#define
when we have a target-specific cpu.h handy. This will actually mean no
change to single-arch builds but multi-arch will use the hook from
core code only.

> That said, I'd approve of a goal to arrange for the correct qemu_ld/st helpers
> to be called, and a direct call to the proper tlb_fill.  But, one step at a 
> time...

I don't know what this means exactly. tlb_fill is called by functions
that are linked to common code (TCG backends) so I don't see a non
virtualized solution. Is this refactoring to move tlb_fill?


> r~

reply via email to

[Prev in Thread] Current Thread [Next in Thread]