Re: [Qemu-devel] [RFC qom-cpu 03/15] target-i386: Update CPU to QOM realizefn

[Eduardo Habkost]

On Thu, Jan 17, 2013 at 09:03:59AM +0100, Andreas Färber wrote:
[...]
> >> I mentioned in the cover letter that this needs to be changed once a
> >> CPUClass-level realizefn is introduced. I could introduce a no-op
> >> realizefn there and do the regular store+call.
> > 
> > That was the semantics I was expecting: base classes would safely
> > introduce realize functions without worrying if subclasses would
> > override it incorrectly and break it.
> 
> We could do that if we fix up the respective DeviceClass::init,
> SysBusDeviceClass::init etc. code. Question is (just as with some x86
> CPU code) whether it's worth cleaning up when we know that it is to be
> refactored later.

Actually I am not sure it would be nice to require every single class to
save/call the parent realize function. I am starting to like the more
relaxed requirement.  :-)


> 
> > Anyway, saving the parent function in every subclass is so cumbersome
> > that simply documenting it as "CPUClass subclasses must call
> > qemu_init_vcpu()" sounds easier than "CPUClass subclasses must save the
> > parent's realize() and call it".
> [snip]
> 
> Actually that particular piece of code is unrelated to this discussion
> since qemu_init_vcpu() still operates on CPUArchState and thus cannot be
> moved into CPUClass yet. The reason is that
> cpus.c:qemu_kvm_cpu_thread_fn sets cpu_single_env, and I do not see a
> solution for that - suggestions or patches welcome.

I used qemu_init_vcpu() as an example because it's something called by
the realize function for all targets, and one day could be called by a
common CPUClass realize function. I didn't check if it was possible to
convert it today, already.

My point is: if you need to save the pointer and call the parent realize
function only if documented and required by the parent class, the parent
could as well simply document it as "subclasses of TYPE_FOO should
manually call foo_realize() if they override the realize function"
instead of "subclasses of TYPE_FOO should save and call the parent
realize function if they override de realize function". Won't it be
easier and simpler?

> 
> However, I see that kvm-all.c:kvm_on_sigbus_vcpu() can be switched to
> CPUState now, so that cpus.c:qemu_kvm_eat_signals() can be changed to
> CPUState, used from cpus.c:qemu_kvm_wait_io_event().
> But cpus.c:cpu_thread_is_idle() still uses env->halted, which is blocked
> by the search for an acceptable solution to flush the TLB at CPUState
> level (exec.c:cpu_common_post_load()).
> 
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

-- 
Eduardo