Re: [Xen-devel] [PATCH v2 08/13] x86/PMU: Interface for setting PMU mode and flags

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.09.13 at 11:42, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -101,6 +101,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
>  #define __HYPERVISOR_kexec_op             37
>  #define __HYPERVISOR_tmem_op              38
>  #define __HYPERVISOR_xc_reserved_op       39 /* reserved for XenClient */
> +#define __HYPERVISOR_xenpmu_op            40
>  
>  /* Architecture-specific hypercall definitions. */
>  #define __HYPERVISOR_arch_0               48

I wonder whether you wouldn't better use an arch-specific hypercall
number here - there's really very little that's generic in what I have
seen so far.

> +/* Parameters structure for HYPERVISOR_xenpmu_op call */
> +struct xenpmu_params {
> +    /* IN/OUT parameters */
> +    union {
> +        struct version {
> +            uint8_t maj;
> +            uint8_t min;
> +        } version;
> +        uint64_t pad;
> +    };
> +    union {
> +        uint64_t val;
> +        void *valp;

Without there also being a handle here I can't see how you could
make use of the pointer.

> +    };
> +
> +    /* IN parameters */
> +    uint64_t vcpu;

Do you really want a 64-bit quantity here?

> @@ -139,6 +140,9 @@ do_tmem_op(
>  extern long
>  do_xenoprof_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
>  
> +extern long
> +do_xenpmu_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);

This seems wrong - the interface above makes it so that you would
only ever pass this a struct xenpmu_params, so the handle should
be of that kind from the beginning.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel