| Message ID | 20230220204736.2639601-48-dwmw2@infradead.org |
|---|---|
| State | New |
| Headers | show |
| Series | Xen HVM support under KVM | expand |
On Mon, 2023-02-20 at 20:47 +0000, David Woodhouse wrote: > @@ -1246,6 +1470,16 @@ static bool do_kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) > } > > switch (code) { > + case __HYPERVISOR_set_timer_op: > + if (exit->u.hcall.longmode) { > + return kvm_xen_hcall_set_timer_op(exit, cpu, > + exit->u.hcall.params[0]); > + } else { > + /* In 32-bit mode, the 64-bit timer value is in two args. */ > + uint64_t val = ((uint64_t)exit->u.hcall.params[1]) << 32 | > + (uint32_t)exit->u.hcall.params[0]; > + return kvm_xen_hcall_set_timer_op(exit, cpu, val); > + } Argh, there I'm returning -errno from a function that ought to set it in exit->u.hcall.result and return 'true' for a handled syscall. Again. Still *slightly* regretting my life choices there and wishing the compiler caught that for me, but not enough to change it because we really *do* want to track which unhandled calls guests are trying to make. I'll fix it and then (if I make load_multiboot() tolerate 64-bit binaries as previously discussed) the XTF tests work: $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none -kernel$XTFDIR/tests/set_timer_op/test-hvm64-set_timer_op --- Xen Test Framework --- Environment: HVM 64bit (Long mode 4 levels) Test hypercall set_timer_op Test result: SUCCESS ****************************** PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned ****************************** QEMU: Terminated $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none -kernel $XTFDIR/tests/set_timer_op/test-hvm32-set_timer_op --- Xen Test Framework --- Environment: HVM 32bit (No paging) Test hypercall set_timer_op Test result: SUCCESS ****************************** PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned ****************************** QEMU: Terminated (Dunno why it whines about poweroff; it isn't even calling the hypercall. And the test to explicitly test that hypercall does work.) --- a/target/i386/kvm/xen-emu.c +++ b/target/i386/kvm/xen-emu.c @@ -1118,14 +1118,18 @@ static int vcpuop_stop_singleshot_timer(CPUState *cs) return 0; } -static int kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, - uint64_t timeout) +static bool kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, + uint64_t timeout) { + int err; + if (unlikely(timeout == 0)) { - return vcpuop_stop_singleshot_timer(CPU(cpu)); + err = vcpuop_stop_singleshot_timer(CPU(cpu)); } else { - return do_set_singleshot_timer(CPU(cpu), timeout, false, true); + err = do_set_singleshot_timer(CPU(cpu), timeout, false, true); } + exit->u.hcall.result = err; + return true; } static bool kvm_xen_hcall_vcpu_op(struct kvm_xen_exit *exit, X86CPU *cpu,
On 22/02/2023 09:21, David Woodhouse wrote: > On Mon, 2023-02-20 at 20:47 +0000, David Woodhouse wrote: >> @@ -1246,6 +1470,16 @@ static bool do_kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) >> } >> >> switch (code) { >> + case __HYPERVISOR_set_timer_op: >> + if (exit->u.hcall.longmode) { >> + return kvm_xen_hcall_set_timer_op(exit, cpu, >> + exit->u.hcall.params[0]); >> + } else { >> + /* In 32-bit mode, the 64-bit timer value is in two args. */ >> + uint64_t val = ((uint64_t)exit->u.hcall.params[1]) << 32 | >> + (uint32_t)exit->u.hcall.params[0]; >> + return kvm_xen_hcall_set_timer_op(exit, cpu, val); >> + } > > Argh, there I'm returning -errno from a function that ought to set it > in exit->u.hcall.result and return 'true' for a handled syscall. Again. > > Still *slightly* regretting my life choices there and wishing the > compiler caught that for me, but not enough to change it because we > really *do* want to track which unhandled calls guests are trying to > make. I'll fix it and then (if I make load_multiboot() tolerate 64-bit > binaries as previously discussed) the XTF tests work: > > $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none -kernel$XTFDIR/tests/set_timer_op/test-hvm64-set_timer_op > --- Xen Test Framework --- > Environment: HVM 64bit (Long mode 4 levels) > Test hypercall set_timer_op > Test result: SUCCESS > ****************************** > PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned > ****************************** > QEMU: Terminated > $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none -kernel $XTFDIR/tests/set_timer_op/test-hvm32-set_timer_op > --- Xen Test Framework --- > Environment: HVM 32bit (No paging) > Test hypercall set_timer_op > Test result: SUCCESS > ****************************** > PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned > ****************************** > QEMU: Terminated > > > (Dunno why it whines about poweroff; it isn't even calling the > hypercall. And the test to explicitly test that hypercall does work.) > > > --- a/target/i386/kvm/xen-emu.c > +++ b/target/i386/kvm/xen-emu.c > @@ -1118,14 +1118,18 @@ static int vcpuop_stop_singleshot_timer(CPUState *cs) > return 0; > } > > -static int kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, > - uint64_t timeout) > +static bool kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, > + uint64_t timeout) > { > + int err; > + > if (unlikely(timeout == 0)) { > - return vcpuop_stop_singleshot_timer(CPU(cpu)); > + err = vcpuop_stop_singleshot_timer(CPU(cpu)); > } else { > - return do_set_singleshot_timer(CPU(cpu), timeout, false, true); > + err = do_set_singleshot_timer(CPU(cpu), timeout, false, true); > } > + exit->u.hcall.result = err; > + return true; > } > > static bool kvm_xen_hcall_vcpu_op(struct kvm_xen_exit *exit, X86CPU *cpu, > That looks better :-) Paul
On 22/02/2023 12:03, Paul Durrant wrote: > On 22/02/2023 09:21, David Woodhouse wrote: >> On Mon, 2023-02-20 at 20:47 +0000, David Woodhouse wrote: >>> @@ -1246,6 +1470,16 @@ static bool do_kvm_xen_handle_exit(X86CPU >>> *cpu, struct kvm_xen_exit *exit) >>> } >>> switch (code) { >>> + case __HYPERVISOR_set_timer_op: >>> + if (exit->u.hcall.longmode) { >>> + return kvm_xen_hcall_set_timer_op(exit, cpu, >>> + exit->u.hcall.params[0]); >>> + } else { >>> + /* In 32-bit mode, the 64-bit timer value is in two >>> args. */ >>> + uint64_t val = ((uint64_t)exit->u.hcall.params[1]) << 32 | >>> + (uint32_t)exit->u.hcall.params[0]; >>> + return kvm_xen_hcall_set_timer_op(exit, cpu, val); >>> + } >> >> Argh, there I'm returning -errno from a function that ought to set it >> in exit->u.hcall.result and return 'true' for a handled syscall. Again. >> >> Still *slightly* regretting my life choices there and wishing the >> compiler caught that for me, but not enough to change it because we >> really *do* want to track which unhandled calls guests are trying to >> make. I'll fix it and then (if I make load_multiboot() tolerate 64-bit >> binaries as previously discussed) the XTF tests work: >> >> $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel >> kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none >> -kernel$XTFDIR/tests/set_timer_op/test-hvm64-set_timer_op >> --- Xen Test Framework --- >> Environment: HVM 64bit (Long mode 4 levels) >> Test hypercall set_timer_op >> Test result: SUCCESS >> ****************************** >> PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned >> ****************************** >> QEMU: Terminated >> $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel >> kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none >> -kernel $XTFDIR/tests/set_timer_op/test-hvm32-set_timer_op >> --- Xen Test Framework --- >> Environment: HVM 32bit (No paging) >> Test hypercall set_timer_op >> Test result: SUCCESS >> ****************************** >> PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned >> ****************************** >> QEMU: Terminated >> >> >> (Dunno why it whines about poweroff; it isn't even calling the >> hypercall. And the test to explicitly test that hypercall does work.) >> >> >> --- a/target/i386/kvm/xen-emu.c >> +++ b/target/i386/kvm/xen-emu.c >> @@ -1118,14 +1118,18 @@ static int >> vcpuop_stop_singleshot_timer(CPUState *cs) >> return 0; >> } >> -static int kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, >> X86CPU *cpu, >> - uint64_t timeout) >> +static bool kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, >> X86CPU *cpu, >> + uint64_t timeout) >> { >> + int err; >> + >> if (unlikely(timeout == 0)) { >> - return vcpuop_stop_singleshot_timer(CPU(cpu)); >> + err = vcpuop_stop_singleshot_timer(CPU(cpu)); >> } else { >> - return do_set_singleshot_timer(CPU(cpu), timeout, false, true); >> + err = do_set_singleshot_timer(CPU(cpu), timeout, false, true); >> } >> + exit->u.hcall.result = err; >> + return true; >> } >> static bool kvm_xen_hcall_vcpu_op(struct kvm_xen_exit *exit, X86CPU >> *cpu, >> > > That looks better :-) > NB I think you still need to fix kvm_xen_hcall_vcpu_op() to not return the -ENOENT too.
On 22 February 2023 12:14:00 GMT, Paul Durrant <xadimgnik@gmail.com> wrote: >On 22/02/2023 12:03, Paul Durrant wrote: >> On 22/02/2023 09:21, David Woodhouse wrote: >>> On Mon, 2023-02-20 at 20:47 +0000, David Woodhouse wrote: >>>> @@ -1246,6 +1470,16 @@ static bool do_kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) >>>> } >>>> switch (code) { >>>> + case __HYPERVISOR_set_timer_op: >>>> + if (exit->u.hcall.longmode) { >>>> + return kvm_xen_hcall_set_timer_op(exit, cpu, >>>> + exit->u.hcall.params[0]); >>>> + } else { >>>> + /* In 32-bit mode, the 64-bit timer value is in two args. */ >>>> + uint64_t val = ((uint64_t)exit->u.hcall.params[1]) << 32 | >>>> + (uint32_t)exit->u.hcall.params[0]; >>>> + return kvm_xen_hcall_set_timer_op(exit, cpu, val); >>>> + } >>> >>> Argh, there I'm returning -errno from a function that ought to set it >>> in exit->u.hcall.result and return 'true' for a handled syscall. Again. >>> >>> Still *slightly* regretting my life choices there and wishing the >>> compiler caught that for me, but not enough to change it because we >>> really *do* want to track which unhandled calls guests are trying to >>> make. I'll fix it and then (if I make load_multiboot() tolerate 64-bit >>> binaries as previously discussed) the XTF tests work: >>> >>> $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none -kernel$XTFDIR/tests/set_timer_op/test-hvm64-set_timer_op >>> --- Xen Test Framework --- >>> Environment: HVM 64bit (Long mode 4 levels) >>> Test hypercall set_timer_op >>> Test result: SUCCESS >>> ****************************** >>> PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned >>> ****************************** >>> QEMU: Terminated >>> $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none -kernel $XTFDIR/tests/set_timer_op/test-hvm32-set_timer_op >>> --- Xen Test Framework --- >>> Environment: HVM 32bit (No paging) >>> Test hypercall set_timer_op >>> Test result: SUCCESS >>> ****************************** >>> PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned >>> ****************************** >>> QEMU: Terminated >>> >>> >>> (Dunno why it whines about poweroff; it isn't even calling the >>> hypercall. And the test to explicitly test that hypercall does work.) >>> >>> >>> --- a/target/i386/kvm/xen-emu.c >>> +++ b/target/i386/kvm/xen-emu.c >>> @@ -1118,14 +1118,18 @@ static int vcpuop_stop_singleshot_timer(CPUState *cs) >>> return 0; >>> } >>> -static int kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, >>> - uint64_t timeout) >>> +static bool kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, >>> + uint64_t timeout) >>> { >>> + int err; >>> + >>> if (unlikely(timeout == 0)) { >>> - return vcpuop_stop_singleshot_timer(CPU(cpu)); >>> + err = vcpuop_stop_singleshot_timer(CPU(cpu)); >>> } else { >>> - return do_set_singleshot_timer(CPU(cpu), timeout, false, true); >>> + err = do_set_singleshot_timer(CPU(cpu), timeout, false, true); >>> } >>> + exit->u.hcall.result = err; >>> + return true; >>> } >>> static bool kvm_xen_hcall_vcpu_op(struct kvm_xen_exit *exit, X86CPU *cpu, >>> >> >> That looks better :-) >> > >NB I think you still need to fix kvm_xen_hcall_vcpu_op() to not return the -ENOENT too. Didn't I already do that?
On 22/02/2023 12:51, David Woodhouse wrote: > > On 22 February 2023 12:14:00 GMT, Paul Durrant <xadimgnik@gmail.com> wrote: >> On 22/02/2023 12:03, Paul Durrant wrote: >>> On 22/02/2023 09:21, David Woodhouse wrote: >>>> On Mon, 2023-02-20 at 20:47 +0000, David Woodhouse wrote: >>>>> @@ -1246,6 +1470,16 @@ static bool do_kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) >>>>> } >>>>> switch (code) { >>>>> + case __HYPERVISOR_set_timer_op: >>>>> + if (exit->u.hcall.longmode) { >>>>> + return kvm_xen_hcall_set_timer_op(exit, cpu, >>>>> + exit->u.hcall.params[0]); >>>>> + } else { >>>>> + /* In 32-bit mode, the 64-bit timer value is in two args. */ >>>>> + uint64_t val = ((uint64_t)exit->u.hcall.params[1]) << 32 | >>>>> + (uint32_t)exit->u.hcall.params[0]; >>>>> + return kvm_xen_hcall_set_timer_op(exit, cpu, val); >>>>> + } >>>> >>>> Argh, there I'm returning -errno from a function that ought to set it >>>> in exit->u.hcall.result and return 'true' for a handled syscall. Again. >>>> >>>> Still *slightly* regretting my life choices there and wishing the >>>> compiler caught that for me, but not enough to change it because we >>>> really *do* want to track which unhandled calls guests are trying to >>>> make. I'll fix it and then (if I make load_multiboot() tolerate 64-bit >>>> binaries as previously discussed) the XTF tests work: >>>> >>>> $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none -kernel$XTFDIR/tests/set_timer_op/test-hvm64-set_timer_op >>>> --- Xen Test Framework --- >>>> Environment: HVM 64bit (Long mode 4 levels) >>>> Test hypercall set_timer_op >>>> Test result: SUCCESS >>>> ****************************** >>>> PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned >>>> ****************************** >>>> QEMU: Terminated >>>> $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none -kernel $XTFDIR/tests/set_timer_op/test-hvm32-set_timer_op >>>> --- Xen Test Framework --- >>>> Environment: HVM 32bit (No paging) >>>> Test hypercall set_timer_op >>>> Test result: SUCCESS >>>> ****************************** >>>> PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned >>>> ****************************** >>>> QEMU: Terminated >>>> >>>> >>>> (Dunno why it whines about poweroff; it isn't even calling the >>>> hypercall. And the test to explicitly test that hypercall does work.) >>>> >>>> >>>> --- a/target/i386/kvm/xen-emu.c >>>> +++ b/target/i386/kvm/xen-emu.c >>>> @@ -1118,14 +1118,18 @@ static int vcpuop_stop_singleshot_timer(CPUState *cs) >>>> return 0; >>>> } >>>> -static int kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, >>>> - uint64_t timeout) >>>> +static bool kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, >>>> + uint64_t timeout) >>>> { >>>> + int err; >>>> + >>>> if (unlikely(timeout == 0)) { >>>> - return vcpuop_stop_singleshot_timer(CPU(cpu)); >>>> + err = vcpuop_stop_singleshot_timer(CPU(cpu)); >>>> } else { >>>> - return do_set_singleshot_timer(CPU(cpu), timeout, false, true); >>>> + err = do_set_singleshot_timer(CPU(cpu), timeout, false, true); >>>> } >>>> + exit->u.hcall.result = err; >>>> + return true; >>>> } >>>> static bool kvm_xen_hcall_vcpu_op(struct kvm_xen_exit *exit, X86CPU *cpu, >>>> >>> >>> That looks better :-) >>> >> >> NB I think you still need to fix kvm_xen_hcall_vcpu_op() to not return the -ENOENT too. > > > Didn't I already do that? Ah, so you did. So many versions... but with the above change incorporated... Reviewed-by: Paul Durrant <paul@xen.org>
On 22/02/2023 12:51, David Woodhouse wrote: > > On 22 February 2023 12:14:00 GMT, Paul Durrant <xadimgnik@gmail.com> wrote: >> On 22/02/2023 12:03, Paul Durrant wrote: >>> On 22/02/2023 09:21, David Woodhouse wrote: >>>> On Mon, 2023-02-20 at 20:47 +0000, David Woodhouse wrote: >>>>> @@ -1246,6 +1470,16 @@ static bool do_kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) >>>>> } >>>>> switch (code) { >>>>> + case __HYPERVISOR_set_timer_op: >>>>> + if (exit->u.hcall.longmode) { >>>>> + return kvm_xen_hcall_set_timer_op(exit, cpu, >>>>> + exit->u.hcall.params[0]); >>>>> + } else { >>>>> + /* In 32-bit mode, the 64-bit timer value is in two args. */ >>>>> + uint64_t val = ((uint64_t)exit->u.hcall.params[1]) << 32 | >>>>> + (uint32_t)exit->u.hcall.params[0]; >>>>> + return kvm_xen_hcall_set_timer_op(exit, cpu, val); >>>>> + } >>>> >>>> Argh, there I'm returning -errno from a function that ought to set it >>>> in exit->u.hcall.result and return 'true' for a handled syscall. Again. >>>> >>>> Still *slightly* regretting my life choices there and wishing the >>>> compiler caught that for me, but not enough to change it because we >>>> really *do* want to track which unhandled calls guests are trying to >>>> make. I'll fix it and then (if I make load_multiboot() tolerate 64-bit >>>> binaries as previously discussed) the XTF tests work: >>>> >>>> $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none -kernel$XTFDIR/tests/set_timer_op/test-hvm64-set_timer_op >>>> --- Xen Test Framework --- >>>> Environment: HVM 64bit (Long mode 4 levels) >>>> Test hypercall set_timer_op >>>> Test result: SUCCESS >>>> ****************************** >>>> PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned >>>> ****************************** >>>> QEMU: Terminated >>>> $ ./bkvm/qemu-system-x86_64 -serial mon:stdio -accel kvm,xen-version=0x4000a,kernel-irqchip=split -cpu host -display none -kernel $XTFDIR/tests/set_timer_op/test-hvm32-set_timer_op >>>> --- Xen Test Framework --- >>>> Environment: HVM 32bit (No paging) >>>> Test hypercall set_timer_op >>>> Test result: SUCCESS >>>> ****************************** >>>> PANIC: xtf_exit(): hypercall_shutdown(SHUTDOWN_poweroff) returned >>>> ****************************** >>>> QEMU: Terminated >>>> >>>> >>>> (Dunno why it whines about poweroff; it isn't even calling the >>>> hypercall. And the test to explicitly test that hypercall does work.) >>>> >>>> >>>> --- a/target/i386/kvm/xen-emu.c >>>> +++ b/target/i386/kvm/xen-emu.c >>>> @@ -1118,14 +1118,18 @@ static int vcpuop_stop_singleshot_timer(CPUState *cs) >>>> return 0; >>>> } >>>> -static int kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, >>>> - uint64_t timeout) >>>> +static bool kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, >>>> + uint64_t timeout) >>>> { >>>> + int err; >>>> + >>>> if (unlikely(timeout == 0)) { >>>> - return vcpuop_stop_singleshot_timer(CPU(cpu)); >>>> + err = vcpuop_stop_singleshot_timer(CPU(cpu)); >>>> } else { >>>> - return do_set_singleshot_timer(CPU(cpu), timeout, false, true); >>>> + err = do_set_singleshot_timer(CPU(cpu), timeout, false, true); >>>> } >>>> + exit->u.hcall.result = err; >>>> + return true; >>>> } >>>> static bool kvm_xen_hcall_vcpu_op(struct kvm_xen_exit *exit, X86CPU *cpu, >>>> >>> >>> That looks better :-) >>> >> >> NB I think you still need to fix kvm_xen_hcall_vcpu_op() to not return the -ENOENT too. > > > Didn't I already do that? Ah, so you did. So many versions... but with the above change incorporated... Reviewed-by: Paul Durrant <paul@xen.org>
On Wed, 2023-02-22 at 13:32 +0000, Paul Durrant wrote: > > > > > > NB I think you still need to fix kvm_xen_hcall_vcpu_op() to not > > > return the -ENOENT too. > > > > > > Didn't I already do that? > > Ah, so you did. So many versions... but with the above change > incorporated... Indeed, but once I post version 13 I think we might actually be ready to merge this series of "only" 60 patches, and we can work on refining the next batch to add the XenStore and the PV back end support :) > Reviewed-by: Paul Durrant <paul@xen.org> I think that's the last one I needed to complete the set. Thanks!
diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c index 5d5996641d..06572b3e10 100644 --- a/hw/i386/kvm/xen_evtchn.c +++ b/hw/i386/kvm/xen_evtchn.c @@ -1220,6 +1220,37 @@ int xen_evtchn_send_op(struct evtchn_send *send) return ret; } +int xen_evtchn_set_port(uint16_t port) +{ + XenEvtchnState *s = xen_evtchn_singleton; + XenEvtchnPort *p; + int ret = -EINVAL; + + if (!s) { + return -ENOTSUP; + } + + if (!valid_port(port)) { + return -EINVAL; + } + + qemu_mutex_lock(&s->port_lock); + + p = &s->port_table[port]; + + /* QEMU has no business sending to anything but these */ + if (p->type == EVTCHNSTAT_virq || + (p->type == EVTCHNSTAT_interdomain && + (p->type_val & PORT_INFO_TYPEVAL_REMOTE_QEMU))) { + set_port_pending(s, port); + ret = 0; + } + + qemu_mutex_unlock(&s->port_lock); + + return ret; +} + EvtchnInfoList *qmp_xen_event_list(Error **errp) { XenEvtchnState *s = xen_evtchn_singleton; diff --git a/hw/i386/kvm/xen_evtchn.h b/hw/i386/kvm/xen_evtchn.h index b03c3108bc..24611478b8 100644 --- a/hw/i386/kvm/xen_evtchn.h +++ b/hw/i386/kvm/xen_evtchn.h @@ -20,6 +20,8 @@ int xen_evtchn_set_callback_param(uint64_t param); void xen_evtchn_connect_gsis(qemu_irq *system_gsis); void xen_evtchn_set_callback_level(int level); +int xen_evtchn_set_port(uint16_t port); + struct evtchn_status; struct evtchn_close; struct evtchn_unmask; diff --git a/target/i386/cpu.h b/target/i386/cpu.h index e8718c31e5..b579f0f0f8 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -26,6 +26,7 @@ #include "exec/cpu-defs.h" #include "qapi/qapi-types-common.h" #include "qemu/cpu-float.h" +#include "qemu/timer.h" #define XEN_NR_VIRQS 24 @@ -1800,6 +1801,10 @@ typedef struct CPUArchState { bool xen_callback_asserted; uint16_t xen_virq[XEN_NR_VIRQS]; uint64_t xen_singleshot_timer_ns; + QEMUTimer *xen_singleshot_timer; + uint64_t xen_periodic_timer_period; + QEMUTimer *xen_periodic_timer; + QemuMutex xen_timers_lock; #endif #if defined(CONFIG_HVF) HVFX86LazyFlags hvf_lflags; diff --git a/target/i386/kvm/xen-emu.c b/target/i386/kvm/xen-emu.c index c0a59b5fc9..a05dc2b928 100644 --- a/target/i386/kvm/xen-emu.c +++ b/target/i386/kvm/xen-emu.c @@ -38,6 +38,9 @@ #include "xen-compat.h" +static void xen_vcpu_singleshot_timer_event(void *opaque); +static void xen_vcpu_periodic_timer_event(void *opaque); + #ifdef TARGET_X86_64 #define hypercall_compat32(longmode) (!(longmode)) #else @@ -201,6 +204,23 @@ int kvm_xen_init_vcpu(CPUState *cs) env->xen_vcpu_time_info_gpa = INVALID_GPA; env->xen_vcpu_runstate_gpa = INVALID_GPA; + qemu_mutex_init(&env->xen_timers_lock); + env->xen_singleshot_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, + xen_vcpu_singleshot_timer_event, + cpu); + if (!env->xen_singleshot_timer) { + return -ENOMEM; + } + env->xen_singleshot_timer->opaque = cs; + + env->xen_periodic_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, + xen_vcpu_periodic_timer_event, + cpu); + if (!env->xen_periodic_timer) { + return -ENOMEM; + } + env->xen_periodic_timer->opaque = cs; + return 0; } @@ -232,7 +252,8 @@ static bool kvm_xen_hcall_xen_version(struct kvm_xen_exit *exit, X86CPU *cpu, 1 << XENFEAT_writable_descriptor_tables | 1 << XENFEAT_auto_translated_physmap | 1 << XENFEAT_supervisor_mode_kernel | - 1 << XENFEAT_hvm_callback_vector; + 1 << XENFEAT_hvm_callback_vector | + 1 << XENFEAT_hvm_safe_pvclock; } err = kvm_copy_to_gva(CPU(cpu), arg, &fi, sizeof(fi)); @@ -875,13 +896,193 @@ static int vcpuop_register_runstate_info(CPUState *cs, CPUState *target, return 0; } +static uint64_t kvm_get_current_ns(void) +{ + struct kvm_clock_data data; + int ret; + + ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data); + if (ret < 0) { + fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", strerror(ret)); + abort(); + } + + return data.clock; +} + +static void xen_vcpu_singleshot_timer_event(void *opaque) +{ + CPUState *cpu = opaque; + CPUX86State *env = &X86_CPU(cpu)->env; + uint16_t port = env->xen_virq[VIRQ_TIMER]; + + if (likely(port)) { + xen_evtchn_set_port(port); + } + + qemu_mutex_lock(&env->xen_timers_lock); + env->xen_singleshot_timer_ns = 0; + qemu_mutex_unlock(&env->xen_timers_lock); +} + +static void xen_vcpu_periodic_timer_event(void *opaque) +{ + CPUState *cpu = opaque; + CPUX86State *env = &X86_CPU(cpu)->env; + uint16_t port = env->xen_virq[VIRQ_TIMER]; + int64_t qemu_now; + + if (likely(port)) { + xen_evtchn_set_port(port); + } + + qemu_mutex_lock(&env->xen_timers_lock); + + qemu_now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + timer_mod_ns(env->xen_periodic_timer, + qemu_now + env->xen_periodic_timer_period); + + qemu_mutex_unlock(&env->xen_timers_lock); +} + +static int do_set_periodic_timer(CPUState *target, uint64_t period_ns) +{ + CPUX86State *tenv = &X86_CPU(target)->env; + int64_t qemu_now; + + timer_del(tenv->xen_periodic_timer); + + qemu_mutex_lock(&tenv->xen_timers_lock); + + qemu_now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + timer_mod_ns(tenv->xen_periodic_timer, qemu_now + period_ns); + tenv->xen_periodic_timer_period = period_ns; + + qemu_mutex_unlock(&tenv->xen_timers_lock); + return 0; +} + +#define MILLISECS(_ms) ((int64_t)((_ms) * 1000000ULL)) +#define MICROSECS(_us) ((int64_t)((_us) * 1000ULL)) +#define STIME_MAX ((time_t)((int64_t)~0ull >> 1)) +/* Chosen so (NOW() + delta) wont overflow without an uptime of 200 years */ +#define STIME_DELTA_MAX ((int64_t)((uint64_t)~0ull >> 2)) + +static int vcpuop_set_periodic_timer(CPUState *cs, CPUState *target, + uint64_t arg) +{ + struct vcpu_set_periodic_timer spt; + + qemu_build_assert(sizeof(spt) == 8); + if (kvm_copy_from_gva(cs, arg, &spt, sizeof(spt))) { + return -EFAULT; + } + + if (spt.period_ns < MILLISECS(1) || spt.period_ns > STIME_DELTA_MAX) { + return -EINVAL; + } + + return do_set_periodic_timer(target, spt.period_ns); +} + +static int vcpuop_stop_periodic_timer(CPUState *target) +{ + CPUX86State *tenv = &X86_CPU(target)->env; + + qemu_mutex_lock(&tenv->xen_timers_lock); + + timer_del(tenv->xen_periodic_timer); + tenv->xen_periodic_timer_period = 0; + + qemu_mutex_unlock(&tenv->xen_timers_lock); + return 0; +} + +static int do_set_singleshot_timer(CPUState *cs, uint64_t timeout_abs, + bool future, bool linux_wa) +{ + CPUX86State *env = &X86_CPU(cs)->env; + int64_t now = kvm_get_current_ns(); + int64_t qemu_now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + int64_t delta = timeout_abs - now; + + if (future && timeout_abs < now) { + return -ETIME; + } + + if (linux_wa && unlikely((int64_t)timeout_abs < 0 || + (delta > 0 && (uint32_t)(delta >> 50) != 0))) { + /* + * Xen has a 'Linux workaround' in do_set_timer_op() which checks + * for negative absolute timeout values (caused by integer + * overflow), and for values about 13 days in the future (2^50ns) + * which would be caused by jiffies overflow. For those cases, it + * sets the timeout 100ms in the future (not *too* soon, since if + * a guest really did set a long timeout on purpose we don't want + * to keep churning CPU time by waking it up). + */ + delta = (100 * SCALE_MS); + timeout_abs = now + delta; + } + + qemu_mutex_lock(&env->xen_timers_lock); + + timer_mod_ns(env->xen_singleshot_timer, qemu_now + delta); + env->xen_singleshot_timer_ns = now + delta; + + qemu_mutex_unlock(&env->xen_timers_lock); + return 0; +} + +static int vcpuop_set_singleshot_timer(CPUState *cs, uint64_t arg) +{ + struct vcpu_set_singleshot_timer sst; + + qemu_build_assert(sizeof(sst) == 16); + if (kvm_copy_from_gva(cs, arg, &sst, sizeof(sst))) { + return -EFAULT; + } + + return do_set_singleshot_timer(cs, sst.timeout_abs_ns, + !!(sst.flags & VCPU_SSHOTTMR_future), + false); +} + +static int vcpuop_stop_singleshot_timer(CPUState *cs) +{ + CPUX86State *env = &X86_CPU(cs)->env; + + qemu_mutex_lock(&env->xen_timers_lock); + + timer_del(env->xen_singleshot_timer); + env->xen_singleshot_timer_ns = 0; + + qemu_mutex_unlock(&env->xen_timers_lock); + return 0; +} + +static int kvm_xen_hcall_set_timer_op(struct kvm_xen_exit *exit, X86CPU *cpu, + uint64_t timeout) +{ + if (unlikely(timeout == 0)) { + return vcpuop_stop_singleshot_timer(CPU(cpu)); + } else { + return do_set_singleshot_timer(CPU(cpu), timeout, false, true); + } +} + static bool kvm_xen_hcall_vcpu_op(struct kvm_xen_exit *exit, X86CPU *cpu, int cmd, int vcpu_id, uint64_t arg) { - CPUState *dest = qemu_get_cpu(vcpu_id); CPUState *cs = CPU(cpu); + CPUState *dest = cs->cpu_index == vcpu_id ? cs : qemu_get_cpu(vcpu_id); int err; + if (!dest) { + err = -ENOENT; + goto out; + } + switch (cmd) { case VCPUOP_register_runstate_memory_area: err = vcpuop_register_runstate_info(cs, dest, arg); @@ -892,11 +1093,34 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_xen_exit *exit, X86CPU *cpu, case VCPUOP_register_vcpu_info: err = vcpuop_register_vcpu_info(cs, dest, arg); break; + case VCPUOP_set_singleshot_timer: { + if (cs->cpu_index == vcpu_id) { + err = vcpuop_set_singleshot_timer(dest, arg); + } else { + err = -EINVAL; + } + break; + } + case VCPUOP_stop_singleshot_timer: + if (cs->cpu_index == vcpu_id) { + err = vcpuop_stop_singleshot_timer(dest); + } else { + err = -EINVAL; + } + break; + case VCPUOP_set_periodic_timer: { + err = vcpuop_set_periodic_timer(cs, dest, arg); + break; + } + case VCPUOP_stop_periodic_timer: + err = vcpuop_stop_periodic_timer(dest); + break; default: return false; } + out: exit->u.hcall.result = err; return true; } @@ -1246,6 +1470,16 @@ static bool do_kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) } switch (code) { + case __HYPERVISOR_set_timer_op: + if (exit->u.hcall.longmode) { + return kvm_xen_hcall_set_timer_op(exit, cpu, + exit->u.hcall.params[0]); + } else { + /* In 32-bit mode, the 64-bit timer value is in two args. */ + uint64_t val = ((uint64_t)exit->u.hcall.params[1]) << 32 | + (uint32_t)exit->u.hcall.params[0]; + return kvm_xen_hcall_set_timer_op(exit, cpu, val); + } case __HYPERVISOR_grant_table_op: return kvm_xen_hcall_gnttab_op(exit, cpu, exit->u.hcall.params[0], exit->u.hcall.params[1], @@ -1355,7 +1589,25 @@ int kvm_put_xen_state(CPUState *cs) } } + if (env->xen_periodic_timer_period) { + ret = do_set_periodic_timer(cs, env->xen_periodic_timer_period); + if (ret < 0) { + return ret; + } + } + if (!kvm_xen_has_cap(EVTCHN_SEND)) { + /* + * If the kernel has EVTCHN_SEND support then it handles timers too, + * so the timer will be restored by kvm_xen_set_vcpu_timer() below. + */ + if (env->xen_singleshot_timer_ns) { + ret = do_set_singleshot_timer(cs, env->xen_singleshot_timer_ns, + false, false); + if (ret < 0) { + return ret; + } + } return 0; } diff --git a/target/i386/machine.c b/target/i386/machine.c index 603a1077e3..c7ac8084b2 100644 --- a/target/i386/machine.c +++ b/target/i386/machine.c @@ -1277,6 +1277,7 @@ static const VMStateDescription vmstate_xen_vcpu = { VMSTATE_UINT8(env.xen_vcpu_callback_vector, X86CPU), VMSTATE_UINT16_ARRAY(env.xen_virq, X86CPU, XEN_NR_VIRQS), VMSTATE_UINT64(env.xen_singleshot_timer_ns, X86CPU), + VMSTATE_UINT64(env.xen_periodic_timer_period, X86CPU), VMSTATE_END_OF_LIST() } };