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

Re: [Xen-devel] [PATCH] VPMU issue on Nehalem cpus



>>> On 22.11.10 at 08:06, Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx> wrote:
> am 19.11.2010 schrieb "Jan Beulich <JBeulich@xxxxxxxxxx>":
>> >>> On 19.11.10 at 11:29, Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx> wrote:
>> > +static int core2_get_pmc_count(void);
>> > +static void handle_nmi_quirk(u64 msr_content)
>> > +{
>> > +    int num_gen_pmc = core2_get_pmc_count();
>> > +    int num_fix_pmc  = 3;
>> > +    int i;
>> > +    u64 val;
>> > +
>> > +    if ( !is_nmi_quirk )
>> > +        return;
>> > +
>> > +    val = msr_content & ((1 << num_gen_pmc) - 1);
>> 
>> What's the point of masking if the subsequent loop looks at the
>> bottom so many bits only anyway?
> 
> Bits 0-31 flag the overflow of the general counters (currently max 4) and 
> 32-63
> flag the overflow of the fixed counter (currently max 3).
> Yes the first mask is not necessary, maybe a comment would be better?

Neither is the second mask (below) - the shift is all that's really
needed. Afaic, a comment doesn't seem necessary, but Keir
may by of different opinion here.

>> 
>> > +    for ( i = 0; i < num_gen_pmc; i++ )
>> > +    {
>> > +        if ( val & 0x1 )
>> > +        {
>> > +            u64 cnt;
>> > +            rdmsrl(MSR_P6_PERFCTR0 + i, cnt);
>> > +            if ( cnt == 0 )
>> > +                wrmsrl(MSR_P6_PERFCTR0 + i, 1);
>> > +        }
>> > +        val >>= 1;
>> > +    }
>> > +    val = (msr_content >> 32) & ((1 << num_fix_pmc) - 1);
>> 
>> Same here.
>>
>> > +    for ( i = 0; i < num_fix_pmc; i++ )
>> > +    {
>> > +        if ( val & 0x1 )
>> > +        {
>> > +            u64 cnt;
>> > +            rdmsrl(MSR_CORE_PERF_FIXED_CTR0 + i, cnt);
>> > +            if ( cnt == 0 )
>> > +                wrmsrl(MSR_CORE_PERF_FIXED_CTR0 + i, 1);
>> > +        }
>> > +        val >>= 1;
>> > +    }
>> > +}

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

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