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

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 01/24/2014 09:54 AM, Jan Beulich wrote:
> > > > On 21.01.14 at 20:08, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> 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

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