Re: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

[Avi Kivity <avi@redhat.com>]

On 01/11/2011 04:28 PM, Anthony Liguori wrote:
> On 01/11/2011 08:18 AM, Avi Kivity wrote:
>> On 01/11/2011 04:00 PM, Anthony Liguori wrote:
>>> On 01/11/2011 03:01 AM, Avi Kivity wrote:
>>>> On 01/10/2011 10:23 PM, Anthony Liguori wrote:
>>>>>>> I don't see how ioapic, pit, or pic have a system scope.
>>>>>> They are not bound to any CPU like the APIC which you may have in 
>>>>>> mind.
>>>>>
>>>>> And none of the above interact with KVM.
>>>>
>>>> They're implemented by kvm.  What deeper interaction do you have in 
>>>> mind?
>>>
>>> The emulated ioapic/pit/pic do not interact with KVM at all.
>>
>> How can they "not interact" with kvm if they're implemented by kvm?
>>
>> I really don't follow here.
>
> "emulated ioapic/pit/pic" == versions implemented in QEMU.  That's 
> what I'm trying to say.  When not using the KVM versions of the 
> devices, there are no interactions with KVM.

Okay.  Isn't that the same for the cpu?  Yet we use the same CPUState 
and are live-migration compatible (as long as cpuids match).

>
>>>
>>> The KVM versions should be completely separate devices.
>>>
>>
>> Why?
>
> Because the KVM versions are replacements.

Only the implementation.  The guest doesn't see the replacement.  They 
have exactly the same state.

>
>>>> I don't see why.  Those are just two different implementations for 
>>>> the same guest visible device.
>>>
>>> Right, they should appear the same to the guest but the fact that 
>>> they're two different implementations should be reflected in the 
>>> device tree.
>>
>> Why?
>>
>> To move beyond single-word questions, what is the purpose of the 
>> device tree?  In my mind, it reflects the virtual hardware.  What's 
>> important is that we have a PIC, virtio network adapter, and IDE 
>> disk.  Not that they're backed by kvm, vhost-net, and qcow2.
>
> Let me give a very concrete example to illustrate my point.
>
> One thing I have on my TODO is to implement catch-up support for the 
> emulated devices.  I want to implement three modes of catch-up 
> support: drop, fast, and gradual.  Gradual is the best policy IMHO but 
> fast is necessary on older kernels without highres timers.  Drop is 
> necessary to maintain compatibility with what we have today.
>
> The kernel PIT only implements one mode and even if the other two were 
> added, even the newest version of QEMU needs to deal with the fact 
> that there's old kernels out there with PIT's that only do fast.
>
> So how does this get exposed to management tools?  Do you check for 
> drift-mode=fast and transparently enable the KVM pit?  Do you fail if 
> anything but drift-mode=fast is specified?
>
> We need to have the following mechanisms:
>
> 1) the ability to select an in-kernel PIT vs. a userspace PIT
>
> 2) an independent mechanism to configure the userspace PIT
>
> 3) an independent mechanism to configure the in-kernel PIT.
>
> The best way to do this is to make the in-kernel PIT a separate 
> device.  Then we get all of this for free.

And it buys us live migration and ABI issues for the same price.

Really, can't we do

     class i8254 {
         ...
         virtual void set_catchup_policy(std::string policy) = 0;
         ...
     }

to deal with the differences?

>
>>>
>>> 2) a user can explicitly create either the emulated version of the 
>>> device or the in-kernel version of the device (no need for 
>>> -no-kvm-irqchip)
>>
>> -device ioapic,model=kernel vs. -device kvm-ioapic?
>>
>> Is it really important to do that? 110% of the time we want the 
>> kernel irqchips.  The remaining -10% are only used for testing.
>
> If model=kernel makes the support options different, then you end up 
> introduce another layer of option validation.  By using the later 
> form, you get to leverage the option validation of qdev plus it makes 
> it much clearer to users what options are supported in what model 
> because now the documentation is explicit about it.

Option validation = internals.  ABI = ABI.  We can deal with the former 
in any number of ways, but exposing it to the ABI is forever.

>
>>>
>>> 3) a user can pass parameters directly to the in-kernel version of 
>>> the device that are different from the userspace version (like 
>>> selecting different interrupt catch-up methods)
>>
>> -device pit,model=qemu,catchup=slew
>>
>> error: catchup=slew not supported in this model
>>
>> I'm not overly concerned about the implementation part.  Though I 
>> think it's better to have a single implementation with kvm acting as 
>> an accelerator, having it the other way is no big deal.  What I am 
>> worried about is exposing it as a monitor and migration ABI.  IMO the 
>> only important thing is the spec that the device implements, not what 
>> piece of code implements it.
>
> Just as we do in the PIT, there's nothing wrong with making the 
> device's migration compatible. 

Then the two devices have the same migration section id?  That's my 
biggest worry.  Not really worried about PIT and PIC (no one uses the 
user PIT now), but more about future devices moving into the kernel, if 
we have to do that.

> I'm not entirely sure what your concerns about the monitor are but 
> there's simply no way to hide the fact that a device is implemented in 
> KVM at the monitor level. 

Why is that?  a PIT is a PIT.  Why does the monitor care where the state 
is managed?

> But really, is this something that management tools want?  I doubt 
> it.  I think they want to have ultimate control over what gets created 
> with us providing a recommended set of defaults.

They also want a forward migration path.  Splitting into two separate 
devices (at the ABI level, ignoring the source level for now) denies 
them that.

-- 
error compiling committee.c: too many arguments to function