Re: [PATCH] kvm-unit-tests: x86: pmu: call measure for every counter in check_counters_many

[Chris J Arges <chris.j.arges@canonical.com>]

On 08/25/2014 11:45 AM, Paolo Bonzini wrote:
> Il 14/08/2014 22:58, Chris J Arges ha scritto:
>> In the check_counters_many function measure was only being called on the last
>> counter, causing the pmu test to fail.
> 
> I don't understand.  measure loops on all N counters and calls
> start_event (which in turn calls global_enable) and stop_event (which
> calls global_disable) on all counters.
> 
>> This ensures that measure is called for
>> each counter in the array before calling verify_counter.
> 
> Actually the point of this test is to run the loop while all the
> counters are active, so this patch is just masking another bug.
> 
> Paolo

Paolo,

Ok I see now where this patch doesn't make sense.
With the latest kvm tree I get:

sudo ./x86-run x86/pmu.flat -smp 1 -cpu host | grep -v PASS


qemu-system-x86_64 -enable-kvm -device pc-testdev -device
isa-debug-exit,iobase=0xf4,iosize=0x4 -display none -serial stdio
-device pci-testdev -kernel x86/pmu.flat -smp 1 -cpu host
enabling apic
paging enabled
cr0 = 80010011
cr3 = 7fff000
cr4 = 20
PMU version:         2
GP counters:         4
GP counter width:    48
Mask length:         7
Fixed counters:      3
Fixed counter width: 48
FAIL: all counters

SUMMARY: 67 tests, 1 unexpected failures
Return value from qemu: 3

I've tested this on a few Intel platforms (sandybridge/haswell), I'll
look into the code more then.

Thanks,
--chris j arges

> 
>> Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
>> ---
>>  x86/pmu.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/x86/pmu.c b/x86/pmu.c
>> index 5c85146..3402d1e 100644
>> --- a/x86/pmu.c
>> +++ b/x86/pmu.c
>> @@ -287,11 +287,11 @@ static void check_counters_many(void)
>>  		n++;
>>  	}
>>  
>> -	measure(cnt, n);
>> -
>> -	for (i = 0; i < n; i++)
>> +	for (i = 0; i < n; i++) {
>> +		measure(&cnt[i], 1);
>>  		if (!verify_counter(&cnt[i]))
>>  			break;
>> +	}
>>  
>>  	report("all counters", i == n);
>>  }
>>
>