From patchwork Wed Nov 5 08:06:43 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tiejun Chen X-Patchwork-Id: 5232671 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B3F54C11AC for ; Wed, 5 Nov 2014 08:06:53 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BF5F120122 for ; Wed, 5 Nov 2014 08:06:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8E87A20115 for ; Wed, 5 Nov 2014 08:06:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752184AbaKEIGs (ORCPT ); Wed, 5 Nov 2014 03:06:48 -0500 Received: from mga03.intel.com ([134.134.136.65]:33001 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751172AbaKEIGs (ORCPT ); Wed, 5 Nov 2014 03:06:48 -0500 Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 05 Nov 2014 00:04:45 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,318,1413270000"; d="scan'208";a="631735448" Received: from tiejunch-mobl.ccr.corp.intel.com (HELO [10.238.130.128]) ([10.238.130.128]) by orsmga002.jf.intel.com with ESMTP; 05 Nov 2014 00:06:44 -0800 Message-ID: <5459DA93.6060104@intel.com> Date: Wed, 05 Nov 2014 16:06:43 +0800 From: "Chen, Tiejun" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: "Wang, Wei W" , "kvm@vger.kernel.org" CC: "pbonzini@redhat.com" , "Zhang, Yang Z" Subject: Re: [PATCH] KVM: x86: reset RVI upon system reset References: <1415156023-1349-1-git-send-email-wei.w.wang@intel.com> <5459C020.20103@intel.com> <286AC319A985734F985F78AFA26841F77F3FCD@shsmsx102.ccr.corp.intel.com> In-Reply-To: <286AC319A985734F985F78AFA26841F77F3FCD@shsmsx102.ccr.corp.intel.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 2014/11/5 15:39, Wang, Wei W wrote: > On 05/11/2014 2:14, Tiejun Chen wrote: >>> A bug was reported as follows: when running Windows 7 32-bit guests on >>> qemu-kvm, sometimes the guests run into blue screen during reboot. The >>> problem was that a guest's RVI was not cleared when it rebooted. This >> patch has fixed the problem. >>> >>> Signed-off-by: Wei Wang >>> Signed-off-by: Yang Zhang >>> Tested-by: Rongrong Liu , Da Chun >>> >>> --- >>> arch/x86/kvm/lapic.c | 3 +++ >>> arch/x86/kvm/vmx.c | 12 ++++++------ >>> 2 files changed, 9 insertions(+), 6 deletions(-) >>> >>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index >>> 66dd173..6942742 100644 >>> --- a/arch/x86/kvm/lapic.c >>> +++ b/arch/x86/kvm/lapic.c >>> @@ -1712,6 +1712,9 @@ void kvm_apic_post_state_restore(struct >> kvm_vcpu *vcpu, >>> apic->isr_count = kvm_apic_vid_enabled(vcpu->kvm) ? >>> 1 : count_vectors(apic->regs + APIC_ISR); >>> apic->highest_isr_cache = -1; >>> + if (kvm_x86_ops->hwapic_irr_update) >>> + kvm_x86_ops->hwapic_irr_update(vcpu, >>> + apic_find_highest_irr(apic)); >> >> Could we pass 0 directly? Because looks we just need to clear RVI. >> >> kvm_x86_ops->hwapic_irr_update(vcpu, 0); >> >> I think this already makes sense based on your description. >> >> Thanks >> Tiejun > > No. This is a restore function, and we cannot assume that the callers always need to reset to the initial state. Okay. Maybe I'm confused by the following change. > > Wei >> >>> kvm_x86_ops->hwapic_isr_update(vcpu->kvm, >> apic_find_highest_isr(apic)); >>> kvm_make_request(KVM_REQ_EVENT, vcpu); >>> kvm_rtc_eoi_tracking_restore_one(vcpu); >>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index >>> fe4d2f4..d632548 100644 >>> --- a/arch/x86/kvm/vmx.c >>> +++ b/arch/x86/kvm/vmx.c >>> @@ -7292,19 +7292,19 @@ static void vmx_set_rvi(int vector) >>> static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr) >>> { >>> if (max_irr == -1) >>> + max_irr = 0; >>> + >>> + if (!is_guest_mode(vcpu)) { >>> + vmx_set_rvi(max_irr); >>> return; >>> + } >>> >>> /* >>> * If a vmexit is needed, vmx_check_nested_events handles it. >>> */ >>> - if (is_guest_mode(vcpu) && nested_exit_on_intr(vcpu)) >>> + if ((is_guest_mode(vcpu) && nested_exit_on_intr(vcpu)) || max_irr >> == >>> +0) Its really not readable to modify max_irr as 0 and check that here, and especially when you read the original comment. So what about this? * is run without virtual interrupt delivery. Thanks Tiejun >>> return; >>> >>> - if (!is_guest_mode(vcpu)) { >>> - vmx_set_rvi(max_irr); >>> - return; >>> - } >>> - >>> /* >>> * Fall back to pre-APICv interrupt injection since L2 >>> * is run without virtual interrupt delivery. >>> > > --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 0cd99d8..bc4558b 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -7280,6 +7280,9 @@ static void vmx_set_rvi(int vector) u16 status; u8 old; + if (vector == -1) + vector = 0; + status = vmcs_read16(GUEST_INTR_STATUS); old = (u8)status & 0xff; if ((u8)vector != old) { @@ -7291,9 +7294,6 @@ static void vmx_set_rvi(int vector) static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr) { - if (max_irr == -1) - return; - /* * If a vmexit is needed, vmx_check_nested_events handles it. */ @@ -7305,6 +7305,9 @@ static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr) return; } + if (max_irr == -1) + return; + /* * Fall back to pre-APICv interrupt injection since L2