From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932349AbcGHKSq (ORCPT ); Fri, 8 Jul 2016 06:18:46 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:35399 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754112AbcGHKSh (ORCPT ); Fri, 8 Jul 2016 06:18:37 -0400 Subject: Re: [PATCH v4 2/2] KVM: nVMX: Fix preemption timer bit set in vmcs02 even if L1 doesn't enable it To: Wanpeng Li References: <1467893939-3335-1-git-send-email-wanpeng.li@hotmail.com> <1467893939-3335-2-git-send-email-wanpeng.li@hotmail.com> <7e971b90-164b-9c2c-49f3-acfde4d0be47@redhat.com> Cc: "linux-kernel@vger.kernel.org" , kvm , Wanpeng Li , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Yunhong Jiang , Jan Kiszka , Haozhong Zhang From: Paolo Bonzini Message-ID: <2c360513-3ac7-3139-6614-45dfdff9646b@redhat.com> Date: Fri, 8 Jul 2016 12:18:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: 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 08/07/2016 02:38, Wanpeng Li wrote: > 2016-07-07 22:11 GMT+08:00 Paolo Bonzini : >> >> >> On 07/07/2016 15:23, Wanpeng Li wrote: >>>>>>> >>>>>>> if (kvm_lapic_hv_timer_in_use(vcpu) && >>>>>>> + (is_guest_mode(vcpu) || >>>>>>> kvm_x86_ops->set_hv_timer(vcpu, >>>>>>> - kvm_get_lapic_tscdeadline_msr(vcpu))) >>>>>>> + kvm_get_lapic_tscdeadline_msr(vcpu)))) >>>>>>> kvm_lapic_switch_to_sw_timer(vcpu); >>>>>>> if (check_tsc_unstable()) { >>>>>>> u64 offset = kvm_compute_tsc_offset(vcpu, >>>>>>> >>>>> >>>>> Thanks, this is good as a fallback. I'll try to fix it by getting the >>>>> pin-based execution controls right but if I fail this patch is okay. >>> I believe we still need this patch even if you implement "L1 TSC >>> deadline timer to trigger while L2 is running" eventually, the codes >>> you posted before: >>> >>> exec_control = vmcs12->pin_based_vm_exec_control; >>> +exec_control &= ~PIN_BASED_VMX_PREEMPTION_TIMER; >>> exec_control |= vmcs_config.pin_based_exec_ctrl; >>> - exec_control &= ~PIN_BASED_VMX_PREEMPTION_TIMER; >>> + if (vmx->hv_deadline_tsc == -1) >>> + exec_control &= ~PIN_BASED_VMX_PREEMPTION_TIMER; >>> >>> So there is still case the preemption timer bit of vmcs02 is not set, >>> however, the scenario I mentioned above in kvm_arch_vcpu_load() will >>> set it unnecessary. >> >> kvm_x86_ops->set_hv_timer _will_ set the preemption timer bit of vmcs02 >> if vmcs02 is the loaded one. >> >> This can happen if L2 has access to L1's local APIC registers (i.e. L1 >> passes the local APIC instead of emulating it, as is the case in a >> partitioning hypervisor). While L2 runs, it writes to the TSC deadline >> MSR of L1. This causes a call to kvm_x86_ops->set_hv_timer while the >> active VMCS is a vmcs02. > > Yes, in the scenario you pointed out the call to > kvm_x86_ops->set_hv_timer while the active VMCS is vmcs02 is correct, > however, in the scenario I mentioned in the patch description is not > correct even if enable "L1 TSC deadline timer to trigger while L2 is > running". It doesn't help that you have not explained how to reproduce the bug---this is what the cover letter and commit messages are for, too. Your patch 1 is enough for me to boot L2 Windows 2008 inside L1 KVM 4.1. So I have an updated patch to handle the TSC deadline timer while L2 is running, but I have no idea how to test its correctness. I'll send the patch shortly. Paolo