Re: [PATCH] x86/HVM: expose VM assist hypercall

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/04/2020 08:46, Jan Beulich wrote:
> In preparation for the addition of VMASST_TYPE_runstate_update_flag
> commit 72c538cca957 ("arm: add support for vm_assist hypercall") enabled
> the hypercall for Arm. I consider it not logical that it then isn't also
> exposed to x86 HVM guests (with the same single feature permitted to be
> enabled as Arm has); Linux actually tries to use it afaict.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

I'd declare this a bug in 2529c850ea4.  It was clearly intended as a
common feature, and wasn't tested for HVM guests.

However, ...

>
> --- a/xen/arch/x86/hvm/hypercall.c
> +++ b/xen/arch/x86/hvm/hypercall.c
> @@ -78,6 +78,11 @@ static long hvm_grant_table_op(
>  }
>  #endif
>  
> +static long hvm_vm_assist(unsigned int cmd, unsigned int type)
> +{
> +    return vm_assist(current->domain, cmd, type, HVM_VM_ASSIST_VALID);
> +}
> +
>  static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
>      const struct vcpu *curr = current;
> @@ -128,6 +133,7 @@ static const hypercall_table_t hvm_hyper
>  #ifdef CONFIG_GRANT_TABLE
>      HVM_CALL(grant_table_op),
>  #endif
> +    HVM_CALL(vm_assist),

... this means we've now got 3 stub functions making no-op ABI changes
for the general vm_assist() function.

Renaming it to do_vm_assist(), latch current->domain internally, and an
arch_vm_assist_valid(d) helper can cover the final parameter.

~Andrew

>      COMPAT_CALL(vcpu_op),
>      HVM_CALL(physdev_op),
>      COMPAT_CALL(xen_version),
> --- a/xen/include/asm-x86/config.h
> +++ b/xen/include/asm-x86/config.h
> @@ -319,6 +319,7 @@ extern unsigned long xen_phys_start;
>  #define VM_ASSIST_VALID          NATIVE_VM_ASSIST_VALID
>  #define COMPAT_VM_ASSIST_VALID   (NATIVE_VM_ASSIST_VALID & \
>                                    ((1UL << COMPAT_BITS_PER_LONG) - 1))
> +#define HVM_VM_ASSIST_VALID      (1UL << VMASST_TYPE_runstate_update_flag)
>  
>  #define ELFSIZE 64
>