Re: [PATCH v2 3/6] xen/PMU: Initialization code for Xen PMU

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 06/06/2014 02:57 PM, Andrew Cooper wrote:
> On 06/06/14 18:44, Boris Ostrovsky wrote:
>> Map shared data structure that will hold CPU registers, VPMU context, V/PCPU IDs
>> of the CPU interrupted by PMU interrupt. Hypervisor fills this information in
>> its handler and passes it to the guest for further processing.
>>
>> Set up PMU VIRQ.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> ---
>>   arch/x86/include/asm/xen/interface.h |  41 ++++++++++
>>   arch/x86/xen/Makefile                |   2 +-
>>   arch/x86/xen/pmu.c                   | 146 +++++++++++++++++++++++++++++++++++
>>   arch/x86/xen/pmu.h                   |  11 +++
>>   arch/x86/xen/smp.c                   |  31 +++++++-
>>   include/xen/interface/xen.h          |   1 +
>>   include/xen/interface/xenpmu.h       |  19 +++++
>>   7 files changed, 249 insertions(+), 2 deletions(-)
>>   create mode 100644 arch/x86/xen/pmu.c
>>   create mode 100644 arch/x86/xen/pmu.h
>>
>> diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
>> index fd9cb76..c4b92d3 100644
>> --- a/arch/x86/include/asm/xen/interface.h
>> +++ b/arch/x86/include/asm/xen/interface.h
>> @@ -169,6 +169,47 @@ struct vcpu_guest_context {
>>   #endif
>>   };
>>   DEFINE_GUEST_HANDLE_STRUCT(vcpu_guest_context);
>> +
>> +/* AMD PMU registers and structures */
>> +struct xen_pmu_amd_ctxt {
>> +	uint32_t counters;       /* Offset to counter MSRs */
>> +	uint32_t ctrls;          /* Offset to control MSRs */
>> +};
>> +
>> +/* Intel PMU registers and structures */
>> +struct xen_pmu_cntr_pair {
>> +	uint64_t counter;
>> +	uint64_t control;
>> +};
>> +
>> +struct xen_pmu_intel_ctxt {
>> +	uint64_t global_ctrl;
>> +	uint64_t global_ovf_ctrl;
>> +	uint64_t global_status;
>> +	uint64_t fixed_ctrl;
>> +	uint64_t ds_area;
>> +	uint64_t pebs_enable;
>> +	uint64_t debugctl;
>> +	uint32_t fixed_counters;  /* Offset to fixed counter MSRs */
>> +	uint32_t arch_counters;   /* Offset to architectural counter MSRs */
>> +};
>> +
>> +struct xen_arch_pmu {
>> +	union {
>> +		struct cpu_user_regs regs;
>> +		uint8_t pad1[256];
>> +	};
>> +	union {
>> +		uint32_t lapic_lvtpc;
>> +		uint64_t pad2;
>> +	};
>> +	union {
>> +		struct xen_pmu_amd_ctxt amd;
>> +		struct xen_pmu_intel_ctxt intel;
>> +#define XENPMU_CTXT_PAD_SZ  128
>> +		uint8_t pad3[XENPMU_CTXT_PAD_SZ];
>> +	};
>> +};
>>   #endif	/* !__ASSEMBLY__ */
> You appear to have a define for XENPMU_CTXT_PAD_SZ but not for any other
> bits of padding.

The only reason for the define is because of the BUILD_BUG_ON test (in 
hypervisor). I suppose I could add another define for registers and do a 
similar test.

>
> Also, I presume there is no sensible way to coalesce the Intel and AMD
> variations?

Oh no. They are *completely* different implementations, with absolutely 
nothing in common.


-boris