[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH qom-next 57/59] cpu: Introduce mandatory tlb_flu
From: |
Paul Brook |
Subject: |
Re: [Qemu-devel] [PATCH qom-next 57/59] cpu: Introduce mandatory tlb_flush callback |
Date: |
Thu, 31 May 2012 19:53:30 +0100 |
User-agent: |
KMail/1.13.7 (Linux/3.2.0-2-amd64; KDE/4.7.4; x86_64; ; ) |
> >> +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.
Maybe so, but having every single taget implement its own copy of the exact
same target independent wrapper seems even more wrong.
> 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.
At minimum it should be clearly documented[1] that this is a transitional
hack, and how it should be removed. There have already been two posts in this
thread suggesting this is a feature, implying that this operation is somehow
target specific. I think the opposite is true: This is a target agnostic
detail of the TCG implementation, and implementing architecturally defined
MMU/TLB behavior here is activley wrong.
Paul
[1] In the code, not the commit message. Commit logs are not documentation.
Commit logs are transient information valid only when the patch is applied.
After that point they become archeological evidence, and you should not expect
subsequent developers to be aware of them.
- [Qemu-devel] [PATCH qom-next 43/59] cpus: Pass CPUState to [qemu_]cpu_has_work(), (continued)
- [Qemu-devel] [PATCH qom-next 43/59] cpus: Pass CPUState to [qemu_]cpu_has_work(), Andreas Färber, 2012/05/22
- [Qemu-devel] [PATCH qom-next 54/59] target-mips: Pass MIPSCPU to mips_tc_sleep(), Andreas Färber, 2012/05/22
- [Qemu-devel] [PATCH qom-next 44/59] target-i386: Pass X86CPU to kvm_mce_inject(), Andreas Färber, 2012/05/22
- [Qemu-devel] [PATCH qom-next 57/59] cpu: Introduce mandatory tlb_flush callback, Andreas Färber, 2012/05/22
- Re: [Qemu-devel] [PATCH qom-next 57/59] cpu: Introduce mandatory tlb_flush callback, Peter Maydell, 2012/05/30
[Qemu-devel] [PATCH qom-next 55/59] target-mips: Pass MIPSCPU to mips_vpe_sleep(), Andreas Färber, 2012/05/22
[Qemu-devel] [PATCH qom-next 45/59] target-i386: Pass X86CPU to cpu_x86_inject_mce(), Andreas Färber, 2012/05/23
[Qemu-devel] [PATCH qom-next 40/59] spapr: Pass PowerPCCPU to spapr_hypercall(), Andreas Färber, 2012/05/23
[Qemu-devel] [PATCH qom-next 41/59] spapr: Pass PowerPCCPU to hypercalls, Andreas Färber, 2012/05/23
[Qemu-devel] [PATCH qom-next 49/59] target-i386: Drop version 5 CPU VMState support, Andreas Färber, 2012/05/23
[Qemu-devel] [PATCH qom-next 47/59] cpu: Move thread_id to CPUState, Andreas Färber, 2012/05/23
[Qemu-devel] [PATCH qom-next 53/59] target-mips: Pass MIPSCPU to mips_vpe_is_wfi(), Andreas Färber, 2012/05/23
[Qemu-devel] [PATCH qom-next 48/59] target-i386: Pass X86CPU to cpu_x86_load_seg_cache_sipi(), Andreas Färber, 2012/05/23
[Qemu-devel] [PATCH qom-next 46/59] cpus: Pass CPUState to run_on_cpu(), Andreas Färber, 2012/05/23