linux-kernel - Re: [PATCH v4 2/2] KVM: nVMX: Fix preemption timer bit set in vmcs02 even if L1 doesn't enable it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 8 Jul 2016 08:38:27 +0800
From:	Wanpeng Li <kernellwp@...il.com>
To:	Paolo Bonzini <pbonzini@...hat.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	kvm <kvm@...r.kernel.org>, Wanpeng Li <wanpeng.li@...mail.com>,
	Radim Krčmář <rkrcmar@...hat.com>,
	Yunhong Jiang <yunhong.jiang@...el.com>,
	Jan Kiszka <jan.kiszka@...mens.com>,
	Haozhong Zhang <haozhong.zhang@...el.com>
Subject: Re: [PATCH v4 2/2] KVM: nVMX: Fix preemption timer bit set in vmcs02
 even if L1 doesn't enable it

2016-07-07 22:11 GMT+08:00 Paolo Bonzini <pbonzini@...hat.com>:
>
>
> 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".

Regards,
Wanpeng Li