On Mon, May 11, 2015 at 1:18 PM, Richard Henderson <r...@twiddle.net> 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; >>>>> >>>>> #ifdef HOST_WORDS_BIGENDIAN >>> [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? Regards, Peter > > r~ >