Re: [PATCH v2 2/2] domain: expose newly introduced hypercalls as XENFEAT

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.10.2023 15:00, Roger Pau Monne wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1580,6 +1580,10 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>      {
>          struct vcpu_register_time_memory_area area;
>  
> +        rc = -ENOSYS;
> +        if ( 0 /* TODO: Dom's XENFEAT_vcpu_time_phys_area setting */ )
> +            break;
> +
>          rc = -EFAULT;
>          if ( copy_from_guest(&area.addr.p, arg, 1) )
>              break;
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1998,6 +1998,10 @@ long common_vcpu_op(int cmd, struct vcpu *v, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>      {
>          struct vcpu_register_runstate_memory_area area;
>  
> +        rc = -ENOSYS;
> +        if ( 0 /* TODO: Dom's XENFEAT_runstate_phys_area setting */ )
> +            break;
> +
>          rc = -EFAULT;
>          if ( copy_from_guest(&area.addr.p, arg, 1) )
>              break;

ENOSYS is not correct here. EPERM, EACCES, or EOPNOTSUPP would all be more
correct.

> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -607,7 +607,11 @@ long do_xen_version(int cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>          switch ( fi.submap_idx )
>          {
>          case 0:
> -            fi.submap = (1U << XENFEAT_memory_op_vnode_supported);
> +            fi.submap = (1U << XENFEAT_memory_op_vnode_supported) |
> +#ifdef CONFIG_X86
> +                        (1U << XENFEAT_vcpu_time_phys_area) |
> +#endif
> +                        (1U << XENFEAT_runstate_phys_area);

No provisions here for the "disabled for this domain" case?

Jan