Re: [PATCH v4 07/17] x86/VPMU: Add public xenpmu.h

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

On 01/24/2014 09:54 AM, Jan Beulich wrote:
>>>> On 21.01.14 at 20:08, Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote:
>> Add xenpmu.h header file,
> To me, naming a public Xen header (other than the core one) xen*.h
> is redundant. There's no information lost if you just called it pmu.h.

I was trying to keep filename and top-level data structures the same 
(although now that I changed xenpmu_ prefix to xen_pmu_ they no longer 
are).

>
> Also I think you ought to use plural here.

I'd prefer to keep the arch-independent and -dependent file names the same.

...

>
>> +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;
>> +    uint64_t fixed_counters;  /* Offset to fixed counter MSRs */
>> +    uint64_t arch_counters;   /* Offset to architectural counter MSRs */
> I think these last two could easily be uint32_t.
>
>> +/* Shared between hypervisor and PV domain */
>> +struct xen_pmu_data {
>> +    uint32_t domain_id;
>> +    uint32_t vcpu_id;
>> +    uint32_t pcpu_id;
>> +    uint32_t pmu_flags;
>> +
>> +    xen_arch_pmu_t pmu;
>> +};
> So if this got included by an architecture independent source file
> on ARM, how would this build? You at least need a stub definition
> there for xen_arch_pmu_t afaict (if already give the impression -
> further up - that you're supporting ARM compilation of this header).

I was supposed to have an entry in arch-arm.h but dropped it somewhere 
along the way. I'll put it back.

-boris