[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1 06/13] x86/PMU: Add public xenpmu.h
On 09/11/2013 04:13 AM, Jan Beulich wrote: --- /dev/null +++ b/xen/include/public/xenpmu.hThis new file is completely unacceptable as a public header. Is this the same comment as IanC made? Mark up public interfaces for doc generation? Or something else? @@ -0,0 +1,101 @@ +#ifndef __XEN_PUBLIC_XENPMU_H__ +#define __XEN_PUBLIC_XENPMU_H__ + +#include <asm/msr.h>This is a no-go.+ +#include "xen.h" + +#define XENPMU_VER_MAJ 0 +#define XENPMU_VER_MIN 0 + +/* VPMU modes */ +#define VPMU_MODE_MASK 0xffAll of these defines would need XEN_ prefixes (and types would similarly need xen_). And there ought to be some association in the comment to the field(s) that these constants would actually go into: From a cursory look I can't see this.+#define VPMU_OFF 0 +/* guests can profile themselves, (dom0 profiles itself and Xen) */Comment style.+#define VPMU_ON (1<<0) +/* + * Only dom0 has access to VPMU and it profiles everyone: itself, + * the hypervisor and the guests. + */ +#define VPMU_PRIV (1<<1) + +/* VPMU flags */ +#define VPMU_FLAGS_MASK ((uint32_t)(~VPMU_MODE_MASK)) +#define VPMU_INTEL_BTS (1<<8) /* Ignored on AMD */ + + +/* AMD PMU registers and structures */ +#define F10H_NUM_COUNTERS 4 +#define F15H_NUM_COUNTERS 6 +/* To accommodate more counters in the future (e.g. NB counters) */ +#define MAX_NUM_COUNTERS 16Perhaps better to have the number of counters in the structure?+struct amd_vpmu_context { + uint64_t counters[MAX_NUM_COUNTERS]; + uint64_t ctrls[MAX_NUM_COUNTERS]; + uint8_t msr_bitmap_set; +}; + + +/* Intel PMU registers and structures */ +static const uint32_t core2_fix_counters_msr[] = {You're kidding, aren't you? This was moved from vpmu.h. I will change it to enum (or remove altogether). -boris _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |