Re: [RFC PATCH v2 16/22] i386/xen: handle VCPUOP_register_vcpu_info

[Paul Durrant]

On 09/12/2022 09:56, David Woodhouse wrote:
> From: Joao Martins <joao.m.martins@oracle.com>
>
> Handle the hypercall to set a per vcpu info, and also wire up the default
> vcpu_info in the shared_info page for the first 32 vCPUs.
>
> To avoid deadlock within KVM a vCPU thread must set its *own* vcpu_info
> rather than it being set from the context in which the hypercall is
> invoked.
>
> Add the vcpu_info (and default) GPA to the vmstate_x86_cpu for migration,
> and restore it in kvm_arch_put_registers() appropriately.
>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   target/i386/cpu.h        |  2 ++
>   target/i386/kvm/kvm.c    | 19 +++++++++++
>   target/i386/machine.c    | 21 ++++++++++++
>   target/i386/trace-events |  1 +
>   target/i386/xen.c        | 74 +++++++++++++++++++++++++++++++++++++---
>   target/i386/xen.h        |  1 +
>   6 files changed, 113 insertions(+), 5 deletions(-)
>
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index c6c57baed5..109b2e5669 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1788,6 +1788,8 @@ typedef struct CPUArchState {
>   #endif
>   #if defined(CONFIG_KVM)
>       struct kvm_nested_state *nested_state;
> +    uint64_t xen_vcpu_info_gpa;
> +    uint64_t xen_vcpu_info_default_gpa;
>   #endif
>   #if defined(CONFIG_HVF)
>       HVFX86LazyFlags hvf_lflags;
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index ebde6bc204..fa45e2f99a 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -1811,6 +1811,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
>           has_msr_hv_hypercall = true;
>       }
>   
> +    env->xen_vcpu_info_gpa = UINT64_MAX;
> +    env->xen_vcpu_info_default_gpa = UINT64_MAX;


There was an INVALID_GPA definition for shared info. Looks like we could 
use it here too.

> +
>       xen_version = kvm_arch_xen_version(MACHINE(qdev_get_machine()));
>       if (xen_version) {
>   #ifdef CONFIG_XEN_EMU
> @@ -4728,6 +4731,22 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
>           kvm_arch_set_tsc_khz(cpu);
>       }
>   
> +#ifdef CONFIG_XEN_EMU
> +    if (level == KVM_PUT_FULL_STATE) {
> +        uint64_t gpa = x86_cpu->env.xen_vcpu_info_gpa;
> +        if (gpa == UINT64_MAX) {
> +            gpa = x86_cpu->env.xen_vcpu_info_default_gpa;
> +        }
> +
> +        if (gpa != UINT64_MAX) {
> +            ret = kvm_xen_set_vcpu_attr(cpu, KVM_XEN_VCPU_ATTR_TYPE_VCPU_INFO, 
> gpa);
> +            if (ret < 0) {
> +                return ret;
> +            }
> +        }
> +    }
> +#endif
> +
>       ret = kvm_getput_regs(x86_cpu, 1);
>       if (ret < 0) {
>           return ret;
[snip]
> @@ -195,19 +240,38 @@ static bool kvm_xen_hcall_hvm_op(struct kvm_xen_exit 
> *exit,
>       }
>   }
>   
> +static int vcpuop_register_vcpu_info(CPUState *cs, CPUState *target,
> +                                     uint64_t arg)
> +{
> +    struct vcpu_register_vcpu_info rvi;
> +    uint64_t gpa;
> +
> +    if (!target)
> +            return -ENOENT;
> +
> +    if (kvm_copy_from_gva(cs, arg, &rvi, sizeof(rvi))) {
> +        return -EFAULT;
> +    }
> +
> +    gpa = ((rvi.mfn << TARGET_PAGE_BITS) + rvi.offset);

Some sanity checks wouldn't go a miss here...

rvi.offset should:
a) be < TARGET_PAGE_SIZE, and
b) ba aligned to vcpu_info_t size

  Paul

> +    async_run_on_cpu(target, do_set_vcpu_info_gpa, RUN_ON_CPU_HOST_ULONG(gpa));
> +    return 0;
> +}
> +