Re: [PATCH V2 5/5] perf/x86: Add Intel Nehalem-EX uncore support

[Yan, Zheng]

On 07/05/2012 10:51 PM, Andi Kleen wrote:
> On Thu, Jul 05, 2012 at 02:32:17PM +0800, Yan, Zheng wrote:
>> The uncore subsystem in Nehalem-EX consists of 7 components
>> (U-Box, C-Box, B-Box, S-Box, R-Box, M-Box and W-Box). This
>> patch is large because the way to program these boxes is
>> diverse.
> 
> Thanks for doing the driver. Lots of work.
> 
> May be worth adding a CONFIG for the uncore code now?
> Maybe even a module, so that not every distro kernel has it always
> in memory. I don't think perf has support for tracking
> module counts, but I guess it would be ok to have the module be not
> unloadable once loaded by setting the count to -1.
Yes, I think it's good to compile the uncore driver as a module. 

> 
> Also did you do some random testing by putting randomized values into
> all the exported registers and see if anything is crashable for 
> unpriv. userspace?
No, I just did functional tests for these registers. unpriv? I think perf
is only available to root by default. 

> 
>> +		 * events are functional identical, but use different
>> +		 * extra registers. If we failed to take an extra
>> +		 * register, try the alternative.
>> +		 */
>> +		if (idx % 2)
>> +			idx--;
>> +		else
>> +			idx++;
>> +		if (idx != reg1->idx % 6) {
>> +			if (idx == 2)
>> +				config1 >>= 8;
>> +			else if (idx == 3)
>> +				config1 <<= 8;
>> +			goto again;
> 
> Does this limit the retries?
Yes, the (idx != reg1->idx % 6) check does that.

Regards
Yan, Zheng


--
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/