From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752033AbdJESOc (ORCPT ); Thu, 5 Oct 2017 14:14:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43614 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752000AbdJESOa (ORCPT ); Thu, 5 Oct 2017 14:14:30 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BBA827EAB2 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=rkrcmar@redhat.com Date: Thu, 5 Oct 2017 20:14:27 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Wanpeng Li Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Paolo Bonzini , Wanpeng Li Subject: Re: [PATCH v5 3/3] KVM: LAPIC: Apply change to TDCR right away to the timer Message-ID: <20171005181427.GB5656@flask> References: <1507214117-2899-1-git-send-email-wanpeng.li@hotmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1507214117-2899-1-git-send-email-wanpeng.li@hotmail.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 05 Oct 2017 18:14:30 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2017-10-05 07:35-0700, Wanpeng Li: > From: Wanpeng Li > > The description in the Intel SDM of how the divide configuration > register is used: "The APIC timer frequency will be the processor's bus > clock or core crystal clock frequency divided by the value specified in > the divide configuration register." > > Observation of baremetal shown that when the TDCR is change, the TMCCT > does not change or make a big jump in value, but the rate at which it > count down change. > > The patch update the emulation to APIC timer to so that a change to the > divide configuration would be reflected in the value of the counter and > when the next interrupt is triggered. > > Cc: Paolo Bonzini > Cc: Radim Krčmář > Signed-off-by: Wanpeng Li > --- > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > @@ -1474,9 +1474,24 @@ static bool set_target_expiration(struct kvm_lapic *apic) > ktime_to_ns(ktime_add_ns(now, > apic->lapic_timer.period))); > > + delta = apic->lapic_timer.period; > + if (apic->divide_count != old_divisor) { Hm, nothing should happen if the guest writes the same value TDCR, but we'll reset the timer. (An extra argument would solve it, but maybe it would be nicer to add a new function for updating the expiration.) > + remaining = ktime_sub(apic->lapic_timer.target_expiration, now); > + if (ktime_to_ns(remaining) < 0) > + remaining = 0; > + delta = mod_64(ktime_to_ns(remaining), apic->lapic_timer.period); > + > + if (!delta) > + return false; > + > + apic->lapic_timer.period = (u64)kvm_lapic_get_reg(apic, APIC_TMICT) > + * APIC_BUS_CYCLE_NS * apic->divide_count; I'd prefer to apply the rate limiting (done earlier in this function) to the period. This version allows the guest to configure 128 times more frequent interrupts in the host. (And thinking about it, the version of [2/3] I proposed has similar problem when switching from one-shot to periodic, only there it is unpredictably limited by the speed of KVM.) Thanks. > + delta = delta * apic->divide_count / old_divisor; > + } > + > apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) + > - nsec_to_cycles(apic->vcpu, apic->lapic_timer.period); > - apic->lapic_timer.target_expiration = ktime_add_ns(now, apic->lapic_timer.period); > + nsec_to_cycles(apic->vcpu, delta); > + apic->lapic_timer.target_expiration = ktime_add_ns(now, delta); > > return true; > }