Messages in this thread | | | Date | Fri, 5 Aug 2022 18:37:13 +0000 | From | Sean Christopherson <> | Subject | Re: [PATCH v2 1/2] KVM: x86/xen: Initialize Xen timer only once |
| |
On Fri, Jul 29, 2022, Coleman Dietsch wrote: > Add a check for existing xen timers before initializing a new one. > > Currently kvm_xen_init_timer() is called on every > KVM_XEN_VCPU_ATTR_TYPE_TIMER, which is causing the following ODEBUG > crash when vcpu->arch.xen.timer is already set. > > ODEBUG: init active (active state 0) > object type: hrtimer hint: xen_timer_callbac0 > RIP: 0010:debug_print_object+0x16e/0x250 lib/debugobjects.c:502 > Call Trace: > __debug_object_init > debug_hrtimer_init > debug_init > hrtimer_init > kvm_xen_init_timer > kvm_xen_vcpu_set_attr > kvm_arch_vcpu_ioctl > kvm_vcpu_ioctl > vfs_ioctl >
Fixes: 536395260582 ("KVM: x86/xen: handle PV timers oneshot mode") Cc: stable@vger.kernel.org
> Link: https://syzkaller.appspot.com/bug?id=8234a9dfd3aafbf092cc5a7cd9842e3ebc45fc42 > Reported-by: syzbot+e54f930ed78eb0f85281@syzkaller.appspotmail.com > Signed-off-by: Coleman Dietsch <dietschc@csp.edu> > ---
One uber nit below, otherwise:
Reviewed-by: Sean Christopherson <seanjc@google.com>
> arch/x86/kvm/xen.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c > index 610beba35907..2dd0f72a62f2 100644 > --- a/arch/x86/kvm/xen.c > +++ b/arch/x86/kvm/xen.c > @@ -713,7 +713,10 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data) > break; > } > vcpu->arch.xen.timer_virq = data->u.timer.port; > - kvm_xen_init_timer(vcpu); > + > + /* Check for existing timer */
I vote to omit the comment, "existing timer" is ambiguous, e.g. it could mean that there's an existing _running_ timer as opposed to an existing initialized timer. IMO the code is obvious enough on its own.
> + if (!vcpu->arch.xen.timer.function) > + kvm_xen_init_timer(vcpu); > > /* Restart the timer if it's set */ > if (data->u.timer.expires_ns) > -- > 2.34.1 >
| |