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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/04/2020 12:34, 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.
>
> Rather than introducing yet another thin wrapper around vm_assist(),
> make that function the main handler, requiring a per-arch
> arch_vm_assist_valid() definition instead.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: Re-work vm_assist() handling/layering at the same time. Also adjust
>     arch_set_info_guest().

Much nicer.  Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

However, ...

> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1517,20 +1517,23 @@ long do_vcpu_op(int cmd, unsigned int vc
>      return rc;
>  }
>  
> -#ifdef VM_ASSIST_VALID
> -long vm_assist(struct domain *p, unsigned int cmd, unsigned int type,
> -               unsigned long valid)
> +#ifdef arch_vm_assist_valid
> +long do_vm_assist(unsigned int cmd, unsigned int type)
>  {
> +    struct domain *currd = current->domain;
> +    const unsigned long valid = arch_vm_assist_valid(currd);
> +
>      if ( type >= BITS_PER_LONG || !test_bit(type, &valid) )
>          return -EINVAL;

As a thought, would it be better to have a guest_bits_per_long()
helper?  This type >= BITS_PER_LONG isn't terribly correct for 32bit
guests, and it would avoid needing the truncation in the arch helper,
which is asymmetric on the ARM side.

~Andrew