Re: [patch 21/24] perfmon: Intel architectural PMU support (x86)

From: stephane eranian
Date: Mon Dec 01 2008 - 22:09:23 EST


On Wed, Nov 26, 2008 at 3:55 PM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> On Wed, 26 Nov 2008, eranian@xxxxxxxxxxxxxx wrote:
>
>> +static u64 enable_mask[PFM_MAX_PMCS];
>
> Why do we need enable_mask twice for AMD and Intel ?
>
>> +static u16 max_enable;
>> +static int pfm_intel_arch_version;
>> +
>> +DEFINE_PER_CPU(u64, saved_global_ctrl);
>
> static
>
Why you want this static instead of per-cpu?

>> +/*
>> + * layout of EAX for CPUID.0xa leaf function
>> + */
>> +struct pmu_eax {
>> + unsigned int version:8; /* architectural perfmon version */
>> + unsigned int num_cnt:8; /* number of generic counters */
>> + unsigned int cnt_width:8; /* width of generic counters */
>> + unsigned int ebx_length:8; /* number of architected events */
>> +};
>
> in arch/x86/include/asm/intel_arch_perfmon.h we have already:
>
> union cpuid10_eax {
> struct {
> unsigned int version_id:8;
> unsigned int num_counters:8;
> unsigned int bit_width:8;
> unsigned int mask_length:8;
> } split;
> unsigned int full;
> };
>
> Can we either use this or remove it ?
>

Well, I need more than eax. We could rewrite this union to include
eax, edx, So I propose we call it union cpuid10 and define it as:

union cpuid_eax {
struct {
unsigned int version_id:8;
unsigned int num_counters:8;
unsigned int bit_width:8;
unsigned int mask_length:8;
} split_eax;
struct {
unsigned int num_counters:5;
unsigned int bit_width:8;
unsigned int reserved:19;
} split_edx;
unsigned int full;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/