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

["stephane eranian" <eranian@googlemail.com>]

On Wed, Nov 26, 2008 at 3:55 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Wed, 26 Nov 2008, eranian@googlemail.com 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;
}