[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.