Re: [PATCH v12 47/60] i386/xen: handle PV timer hypercalls

[David Woodhouse]

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?