From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1947404AbcBRRdJ (ORCPT ); Thu, 18 Feb 2016 12:33:09 -0500 Received: from mail-wm0-f68.google.com ([74.125.82.68]:33723 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1947388AbcBRRdF (ORCPT ); Thu, 18 Feb 2016 12:33:05 -0500 Subject: Re: [PATCH v2 01/14] KVM: x86: change PIT discard tick policy To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= 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> Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Yuki Shibuya , Rik van Riel From: Paolo Bonzini X-Enigmail-Draft-Status: N1110 Message-ID: <56C6004F.2050105@redhat.com> Date: Thu, 18 Feb 2016 18:33:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <20160218165608.GC18904@potion.brq.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 18/02/2016 17:56, Radim Krčmář wrote: > 2016-02-18 17:13+0100, Paolo Bonzini: > > On 17/02/2016 20:14, Radim Krčmář wrote: > > > Discard policy uses ack_notifiers to prevent injection of PIT interrupts > > > before EOI from the last one. > > > > > > This patch changes the policy to always try to deliver the interrupt, > > > which makes a difference when its vector is in ISR. > > > Old implementation would drop the interrupt, but proposed one injects to > > > IRR, like real hardware would. > > > > This seems like what libvirt calls the "merge" policy: > > Oops, I never looked beyond QEMU after seeing that the naming in libvirt > doesn't even match ... > > I think the policy that KVM implements (which I call discard) is "delay" > in libvirt. (https://libvirt.org/formatdomain.html#elementsTime) Suppose the scheduled ticks are at times 0, 20, 40, 60, 80. The EOI for time 0 is only delivered at time 42, other EOIs are timely. The resulting injections are: - for discard: 0, 60, 80. - for catchup, which QEMU calls slew: 0, 42, 51, 60, 80. - for merge: 0, 20 (in IRR, delivered at 42), 60, 80. For delay I *think* it would be 0, 42, 62, 82, 102. You know the i8254 code better than I do. Does this make sense to you? (Or in other words, does the code *really* do the above?...) > The "may be delayed" there makes me feel like the timer has to support a > guest visible counter of missed ticks. Yes, it depends whether the guest uses PIT to count time, or just to do periodic stuff (and then it reads the time from e.g. the PMTimer). In either case, only catchup ensures that time is not delayed, and it's used for Windows which uses the RTC periodic clock to count time. >> > where the merged tick is the one placed into IRR. Unlike discard, >> > "merge" can starve the guest through an interrupt storm. > Yeah, starving a VCPU with an interrupt storm is more likely with the > changed policy. It's a pretty sad situation if all the time that VCPU > gets isn't even enough to run a PIT handler, so I didn't care. True. On one hand the hardware policy is clearly merge, not discard. The i8259 has an IRR! On the other hand I'm a bit wary of changing the policy without seeing exactly what the old OSes were doing in the PIT handler. > The NMI watchdog bug can also be solved without changing the policy. > (It's a hack in any case.) Can you send a patch for that? Paolo