From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH v2 01/14] KVM: x86: change PIT discard tick policy Date: Thu, 25 Feb 2016 20:11:36 +0100 Message-ID: <56CF51E8.3040502@redhat.com> References: <1455736496-374-1-git-send-email-rkrcmar@redhat.com> <1455736496-374-2-git-send-email-rkrcmar@redhat.com> <56C5EDA9.9030204@redhat.com> <20160218165608.GC18904@potion.brq.redhat.com> <56C6004F.2050105@redhat.com> <56C60579.5040003@redhat.com> <20160219144422.GA2456@potion.brq.redhat.com> <56CF03C2.9010703@redhat.com> <20160225173615.GD18319@potion.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Yuki Shibuya , Rik van Riel , Peter Krempa To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:42577 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933114AbcBYTLm (ORCPT ); Thu, 25 Feb 2016 14:11:42 -0500 In-Reply-To: <20160225173615.GD18319@potion.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: > 2016-02-25 14:38+0100, Paolo Bonzini: >> On 19/02/2016 15:44, Radim Kr=C4=8Dm=C3=A1=C5=99 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. >=20 > We do. (Libvirt "catchup" is also QEMU "delay".) >=20 >> 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") defin= es: >=20 > delay - replay all lost ticks in a row once the guest accepts the= m > again > slew - lost ticks are gradually replayed at a higher frequency t= han > the original tick >=20 > "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 !=3D 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 be= havior. >=20 > (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 t= o me". >>> Few examples of "delay" that I find easier to accept: >>> 0, 60, 80. >> >> This is "discard". >=20 > 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 >=20 > which is fine, but > - the guest time is delayed, because there isn't a way to handle los= t > ticks >=20 > and this is incompatible with libvirt's definition of "discard" >=20 > The guest time may be delayed, unless the OS has explicit handling = of > lost ticks. >=20 > "may" doesn't fit. You can only say > - the guest time is delayed. >=20 > 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 ver= y enlightened guest. =2E.. 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 t= he >> timer device and the interrupt controller. >=20 > I disagree. IRR can catch at most one interrupt, so it is insufficie= nt > to implement libvirt's merge. (libvirt's merge also has the conditio= nal > "The guest time may be delayed".) Hmm... is your point that the i8254 _alone_ is implementing discard, an= d the tick delivery time is _actually_ 0, 20, 60, 80 (and the t=3D20 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 wit= h 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