Re: [PATCH v2 01/14] KVM: x86: change PIT discard tick policy

From: Paolo Bonzini
Date: Thu Feb 25 2016 - 14:11:46 EST



> 2016-02-25 14:38+0100, Paolo Bonzini:
>> On 19/02/2016 15:44, Radim KrÄmÃÅ wrote:
>>>> The resulting injections are:
>>>>> - for catchup, which QEMU calls slew: 0, 42, 51, 60, 80.
>>
>> I think we agree that 0, 42, 43, 60, 80 is also a "catchup"/"slew"
>> policy.
>
> We do. (Libvirt "catchup" is also QEMU "delay".)
>
>> So we can change QEMU's kvm-i8254 to accept "slew" and warn if
>> "delay" is given.
> **
> QEMU 4e4fa398db69 ("qdev: Introduce lost tick policy property") defines:
>
> delay - replay all lost ticks in a row once the guest accepts them
> again
> slew - lost ticks are gradually replayed at a higher frequency than
> the original tick
>
> "delay" is exactly how kvm-i8254 behaves (in its "reinject" mode), so I
> think we shouldn't change it.

Ooh, I missed this commit message indeed. Then libvirt delay != QEMU
delay, isn't it?

>> In fact "slew" means "a large number or quantity of something" and
>> indeed that's a good word to characterize kvm-i8254's reinjection behavior.
>
> (Isn't it a verb, with a similar meaning as "drift"? ;])

It's a noun too, like "I've just gotten a whole slew of bugs assigned to
me".

>>> Few examples of "delay" that I find easier to accept:
>>> 0, 60, 80.
>>
>> This is "discard".
>
> At 80, the guest thinks that the time is 40, so every action it does
> will still be delayed. I don't see why it isn't libvirt "delay":
> - ticks are delivered at the normal rate
> - guest time is delayed

I can buy this. :)

> I don't think it is libvirt "discard":
> - missed ticks were thrown away
> - future injection continues normally
>
> which is fine, but
> - the guest time is delayed, because there isn't a way to handle lost
> ticks
>
> and this is incompatible with libvirt's definition of "discard"
>
> The guest time may be delayed, unless the OS has explicit handling of
> lost ticks.
>
> "may" doesn't fit. You can only say
> - the guest time is delayed.
>
> which is best described by "delay".

I think we can safely ignore the "may be" -- you cannot say for sure
that the guest time "will" be delayed since you could always have a very
enlightened guest.

... but then, by removing the handwavy "may be" would you say that
libvirt delay and libvirt discard are the same? Would 0, 42, 62, 82 be
a valid implementation of the libvirt "delay" policy?

>> Therefore, it _also_ happens that thanks to IRR and NMI latching you can
>> implement "merge" without having that kind of relationship between the
>> timer device and the interrupt controller.
>
> I disagree. IRR can catch at most one interrupt, so it is insufficient
> to implement libvirt's merge. (libvirt's merge also has the conditional
> "The guest time may be delayed".)

Hmm... is your point that the i8254 _alone_ is implementing discard, and
the tick delivery time is _actually_ 0, 20, 60, 80 (and the t=20 tick is
delivered late but not lost due to the i8259 buffer)? And hence the
QEMU device model should see it as discard. I can definitely agree with
that.

There is still the matter of:

- improving the documentation

- clarifying the meaning of libvirt delay

- deciding whether it's worth changing the meaning of QEMU delay to
match libvirt's (and the default kvm-pit policy from delay to slew)

But if we can agree on this, I can apply patch 1 as is, even for 4.5.

Paolo