From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752181AbdJFMzn (ORCPT ); Fri, 6 Oct 2017 08:55:43 -0400 Received: from mail-oi0-f65.google.com ([209.85.218.65]:34175 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751890AbdJFMzl (ORCPT ); Fri, 6 Oct 2017 08:55:41 -0400 X-Google-Smtp-Source: AOwi7QBB3a1v13nxcAsvrzjqJLZmvvfjfQdrmfAPHouLdRxXbjmGjIZJiF4+6KpP6LiS1eDc7HK2jMNm3I3pBPpXEaU= MIME-Version: 1.0 In-Reply-To: References: <1507214117-2899-1-git-send-email-wanpeng.li@hotmail.com> <20171005181427.GB5656@flask> From: Wanpeng Li Date: Fri, 6 Oct 2017 20:55:40 +0800 Message-ID: Subject: Re: [PATCH v5 3/3] KVM: LAPIC: Apply change to TDCR right away to the timer To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Cc: "linux-kernel@vger.kernel.org" , kvm , Paolo Bonzini , Wanpeng Li Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v96CtlY0026705 2017-10-06 7:14 GMT+08:00 Wanpeng Li : > 2017-10-06 2:14 GMT+08:00 Radim Krčmář : >> 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.) > > Agreed. > >> >>> + 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.) > > We didn't stop and restart the timer, why the rate will influence us for [2/3]? Have already done in v6, please have a review. :) Regards, Wanpeng Li >> >> 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; >>> }