Re: [Xen-devel] [PATCH v2 13/13] x86/domctl: Implement XEN_DOMCTL_get_cpu_policy

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.07.18 at 22:03, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1523,6 +1523,40 @@ long arch_do_domctl(
>          recalculate_cpuid_policy(d);
>          break;
>  
> +    case XEN_DOMCTL_get_cpu_policy:
> +        if ( !guest_handle_is_null(domctl->u.cpu_policy.cpuid_policy) )
> +        {
> +            if ( (ret = x86_cpuid_copy_to_buffer(
> +                      d->arch.cpuid,
> +                      domctl->u.cpu_policy.cpuid_policy,
> +                      &domctl->u.cpu_policy.nr_leaves)) )
> +                break;
> +
> +            if ( __copy_field_to_guest(u_domctl, domctl,
> +                                       u.cpu_policy.nr_leaves) )
> +            {
> +                ret = -EFAULT;
> +                break;
> +            }
> +        }
> +
> +        if ( !guest_handle_is_null(domctl->u.cpu_policy.msr_policy) )
> +        {
> +            if ( (ret = x86_msr_copy_to_buffer(
> +                      d->arch.msr,
> +                      domctl->u.cpu_policy.msr_policy,
> +                      &domctl->u.cpu_policy.nr_msrs)) )
> +                break;
> +
> +            if ( __copy_field_to_guest(u_domctl, domctl,
> +                                       u.cpu_policy.nr_msrs) )
> +            {
> +                ret = -EFAULT;
> +                break;
> +            }
> +        }
> +        break;

Am I getting it right that the array sizing here is supposed to be done
based on the sysctl output? That's probably okay because we expect
callers to use libxc, but it doesn't make for a very consistent interface
(set).

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -635,6 +635,22 @@ struct xen_domctl_cpuid {
>    uint32_t ecx;
>    uint32_t edx;
>  };
> +
> +/*
> + * XEN_SYSCTL_{get,set}_cpu_policy (x86 specific)
> + *
> + * Query or set the CPUID and MSR policies for a specific domain.
> + */
> +struct xen_domctl_cpu_policy {
> +    uint32_t nr_leaves; /* IN/OUT: Number of leaves in/written to
> +                         * 'cpuid_policy'. */
> +    uint32_t nr_msrs;   /* IN/OUT: Number of MSRs in/written to
> +                         * 'msr_domain_policy' */
> +    XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* IN/OUT: */
> +    XEN_GUEST_HANDLE_64(xen_msr_entry_t) msr_policy;    /* IN/OUT: */

Stray colons again in the comments? With them dropped, and
despite the remark above
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel