[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC qom-cpu 03/15] target-i386: Update CPU to QOM real
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [RFC qom-cpu 03/15] target-i386: Update CPU to QOM realizefn |
Date: |
Thu, 17 Jan 2013 09:03:59 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130105 Thunderbird/17.0.2 |
Am 17.01.2013 00:43, schrieb Eduardo Habkost:
> On Wed, Jan 16, 2013 at 11:52:47PM +0100, Andreas Färber wrote:
>> Am 16.01.2013 17:04, schrieb Eduardo Habkost:
>>> On Wed, Jan 16, 2013 at 06:32:48AM +0100, Andreas Färber wrote:
>>> [...]
>>>> @@ -2247,6 +2247,9 @@ static void x86_cpu_common_class_init(ObjectClass
>>>> *oc, void *data)
>>>> {
>>>> X86CPUClass *xcc = X86_CPU_CLASS(oc);
>>>> CPUClass *cc = CPU_CLASS(oc);
>>>> + DeviceClass *dc = DEVICE_CLASS(oc);
>>>> +
>>>> + dc->realize = x86_cpu_realizefn;
>>>
>>> The DeviceClass documenation says:
>>>
>>> "Any type may override the @realize and/or @unrealize callbacks but
>>> needs to call (and thus save) the parent type's implementation if so
>>> desired."
>>>
>>> Why are you not following it?
>>
>> "if so desired" - I didn't desire or need to call code that calls an
>> initfn that no longer exists after this patch. Same as the ISADevice
>> conversion series did not unnecessarily call the DeviceClass-level
>> backwards-compatibility realizefn: to save time-consuming
>> ...Class::parent_realizefn field additions and to not in the end call
>> code that doesn't NULL-check ...DeviceClass::init. That's qdev's old
>> "leaf type" concept mentioned in the same documentation.
>
> I had read "if so desired" as "if it's desired to override the realize
> callback", not as "if it's desired to call the parent realize function".
Sorry, and I thought my documentation was too verbose already. ;)
> I believed every class could assume that subclasses would never override
> realize() without calling the parent class' realize function (so we
> could add stuff to DeviceClass.realize and CPUClass.realize in the
> future and be sure that the code would be always called).
>
> But from the documentation mentioning "new leaf types should consult
> their respective parent type", it looks like this decision would be
> taken/documented in each base class. If that's the case, then OK.
I've sent out a patch improving QOM and DeviceClass documentation. :)
>> 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.
> 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.
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
[Qemu-devel] [RFC qom-cpu 02/15] target-arm: Update CPU to QOM realizefn, Andreas Färber, 2013/01/16
[Qemu-devel] [RFC qom-cpu 05/15] target-ppc: Update CPU to QOM realizefn, Andreas Färber, 2013/01/16
[Qemu-devel] [RFC qom-cpu 04/15] target-openrisc: Update CPU to QOM realizefn, Andreas Färber, 2013/01/16
[Qemu-devel] [RFC qom-cpu 07/15] target-lm32: Introduce QOM realizefn for LM32CPU, Andreas Färber, 2013/01/16
[Qemu-devel] [RFC qom-cpu 08/15] target-m68k: Introduce QOM realizefn for M68kCPU, Andreas Färber, 2013/01/16
[Qemu-devel] [RFC qom-cpu 10/15] target-mips: Introduce QOM realizefn for MIPSCPU, Andreas Färber, 2013/01/16
[Qemu-devel] [RFC qom-cpu 06/15] target-cris: Introduce QOM realizefn for CRISCPU, Andreas Färber, 2013/01/16