Re: [Xen-devel] [PATCH v2 06/13] x86/PMU: Add public xenpmu.h

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.09.13 at 11:42, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
> +struct amd_vpmu_context {
> +    uint64_t counters[XENPMU_AMD_MAX_COUNTERS];
> +    uint64_t ctrls[XENPMU_AMD_MAX_COUNTERS];
> +    uint8_t msr_bitmap_set;               /* Used by HVM only */
> +};

sizeof() this will not be the same for this in a 64-bit an a 32-bit guest.
Are you intentionally creating a need for translation here?

> +
> +/* Intel PMU registers and structures */
> +#define XENPMU_CORE2_MAX_ARCH_PMCS     16
> +#define XENPMU_CORE2_MAX_FIXED_PMCS    4
> +struct core2_vpmu_context {
> +    uint64_t global_ctrl;
> +    uint64_t global_ovf_ctrl;
> +    uint64_t global_status;
> +    uint64_t global_ovf_status;
> +    uint64_t fixed_ctrl;
> +    uint64_t ds_area;
> +    uint64_t pebs_enable;
> +    uint64_t debugctl;
> +    uint64_t fix_counters[XENPMU_CORE2_MAX_FIXED_PMCS];
> +    struct {
> +        uint64_t counter;
> +        uint64_t control;
> +    } arch_msr_pair[XENPMU_CORE2_MAX_ARCH_PMCS];
> +};

I realize that using embedded arrays in both AMD and Intel
structures makes things easier to implement, but it reduces
forward compatibility. I'd therefore prefer those to be made
handles.

> +#define MAX(x, y)                 ((x) > (y) ? (x) : (y))

I think we already agreed in the context of someone else's reply
that this has to go away.

> +struct arch_xenpmu {
> +    union {
> +        struct cpu_user_regs regs;

Oh, so you need to do translation for 32-bit guests anyway...

> --- /dev/null
> +++ b/xen/include/public/xenpmu.h
> @@ -0,0 +1,38 @@
> +#ifndef __XEN_PUBLIC_XENPMU_H__
> +#define __XEN_PUBLIC_XENPMU_H__
> +
> +#include "xen.h"
> +#if defined(__i386__) || defined(__x86_64__)
> +#include "arch-x86/xenpmu-x86.h"
> +#elif defined (__arm__) || defined (__aarch64__)
> +#include "arch-arm.h"

???

Jan


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