[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 7 Jul 2016 15:25:15 +0800
From: Haozhong Zhang <haozhong.zhang@...el.com>
To: Wanpeng Li <kernellwp@...il.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
kvm <kvm@...r.kernel.org>, Wanpeng Li <wanpeng.li@...mail.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Radim Krčmář <rkrcmar@...hat.com>,
Yunhong Jiang <yunhong.jiang@...el.com>,
Jan Kiszka <jan.kiszka@...mens.com>
Subject: Re: [PATCH v3 1/2] KVM: nVMX: Fix incorrect preemption timer vmexit
in nested guest
On 07/07/16 15:07, Wanpeng Li wrote:
> 2016-07-07 15:02 GMT+08:00 Haozhong Zhang <haozhong.zhang@...el.com>:
> > On 07/07/16 14:56, Wanpeng Li wrote:
> >> 2016-07-07 14:48 GMT+08:00 Haozhong Zhang <haozhong.zhang@...el.com>:
> >> > On 07/07/16 11:46, Wanpeng Li wrote:
> >> >> From: Wanpeng Li <wanpeng.li@...mail.com>
> >> >>
> >> >> BUG: unable to handle kernel NULL pointer dereference at (null)
> >> >> IP: [< (null)>] (null)
> >> >> PGD 0
> >> >> Oops: 0010 [#1] SMP
> >> >> Call Trace:
> >> >> ? kvm_lapic_expired_hv_timer+0x47/0x90 [kvm]
> >> >> handle_preemption_timer+0xe/0x20 [kvm_intel]
> >> >> vmx_handle_exit+0x169/0x15a0 [kvm_intel]
> >> >> ? kvm_arch_vcpu_ioctl_run+0xd5d/0x19d0 [kvm]
> >> >> kvm_arch_vcpu_ioctl_run+0xdee/0x19d0 [kvm]
> >> >> ? kvm_arch_vcpu_ioctl_run+0xd5d/0x19d0 [kvm]
> >> >> ? vcpu_load+0x1c/0x60 [kvm]
> >> >> ? kvm_arch_vcpu_load+0x57/0x260 [kvm]
> >> >> kvm_vcpu_ioctl+0x2d3/0x7c0 [kvm]
> >> >> do_vfs_ioctl+0x96/0x6a0
> >> >> ? __fget_light+0x2a/0x90
> >> >> SyS_ioctl+0x79/0x90
> >> >> do_syscall_64+0x68/0x180
> >> >> entry_SYSCALL64_slow_path+0x25/0x25
> >> >> Code: Bad RIP value.
> >> >> RIP [< (null)>] (null)
> >> >> RSP <ffff8800b5263c48>
> >> >> CR2: 0000000000000000
> >> >> ---[ end trace 9c70c48b1a2bc66e ]---
> >> >>
> >> >> This can be reproduced readily by preemption timer enabled on L0 and disabled
> >> >> on L1.
> >> >>
> >> >> Preemption timer for nested VMX is emulated by hrtimer which is started on L2
> >> >> entry, stopped on L2 exit and evaluated via the check_nested_events hook. However,
> >> >> nested_vmx_exit_handled is always return true for preemption timer vmexit, then
> >> >> the L1 preemption timer vmexit is captured and be treated as a L2 preemption
> >> >> timer vmexit, incurr a nested vmexit dereference NULL pointer.
> >> >>
> >> >> This patch fix it by depending on check_nested_events to capture L2 preemption
> >> >> timer(emulated hrtimer) expire and nested vmexit.
> >> >>
> >> >> Tested-by: Haozhong Zhang <haozhong.zhang@...el.com>
> >> >> Cc: Paolo Bonzini <pbonzini@...hat.com>
> >> >> Cc: Radim Krčmář <rkrcmar@...hat.com>
> >> >> Cc: Yunhong Jiang <yunhong.jiang@...el.com>
> >> >> Cc: Jan Kiszka <jan.kiszka@...mens.com>
> >> >> Cc: Haozhong Zhang <haozhong.zhang@...el.com>
> >> >> Signed-off-by: Wanpeng Li <wanpeng.li@...mail.com>
> >> >> ---
> >> >> v2 -> v3:
> >> >> * update patch subject
> >> >> v1 -> v2:
> >> >> * fix typo in patch description
> >> >>
> >> >> arch/x86/kvm/vmx.c | 2 ++
> >> >> 1 file changed, 2 insertions(+)
> >> >>
> >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> >> >> index 85e2f0a..29c16a8 100644
> >> >> --- a/arch/x86/kvm/vmx.c
> >> >> +++ b/arch/x86/kvm/vmx.c
> >> >> @@ -8041,6 +8041,8 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
> >> >> return nested_cpu_has2(vmcs12, SECONDARY_EXEC_XSAVES);
> >> >> case EXIT_REASON_PCOMMIT:
> >> >> return nested_cpu_has2(vmcs12, SECONDARY_EXEC_PCOMMIT);
> >> >> + case EXIT_REASON_PREEMPTION_TIMER:
> >> >> + return false;
> >> >
> >> > If patch 2 can avoid accidentally enabling preemption timer in vmcs02,
> >> > will this one still be needed?
> >>
> >> After complete "L1 TSC deadline timer to trigger while L2 is running",
> >> L0's preemption timer fire when L2 is running can result in
> >> (is_guest_mode(vcpu) && nested_vmx_exit_handled(vcpu)) be true, right?
> >>
> >
> > In prepare_vmcs02():
> >
> > exec_control &= ~PIN_BASED_VMX_PREEMPTION_TIMER;
> > ...
> > vmcs_write32(PIN_BASED_VM_EXEC_CONTROL, exec_control);
> >
> > so preemption timer will never be enabled while L2 guest is running.
>
> It is not true after Paolo's patch.
>
OK, I didn't quite follow discussions before v3 and just noticed
Paolo's patch in v2 comments is to enable L1 preemption timer while L2
is running. Then this one looks reasonable.
Thanks,
Haozhong
Powered by blists - more mailing lists