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

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

[Chris J Arges]

In the check_counters_many function measure was only being called on the last
counter, causing the pmu test to fail. This ensures that measure is called for
each counter in the array before calling verify_counter.

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);
 }
-- 
1.9.1

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

[Paolo Bonzini]

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

> 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);
>  }
> 

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

[Chris J Arges]

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);
>>  }
>>
> 

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

[Paolo Bonzini]

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


Are you using the NMI watchdog in the host?  It eats one PMU counter
and makes this test fail.

Paolo

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

[Chris J Arges]

On 08/25/2014 02:32 PM, Paolo Bonzini wrote:
>> 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.
> 
> 
> Are you using the NMI watchdog in the host?  It eats one PMU counter
> and makes this test fail.
> 
> Paolo
> 

Ah, I didn't know that. Yes disabling NMI watchdog via:
echo 0 | sudo tee /proc/sys/kernel/nmi_watchdog
Allows this test to pass.

Would it make sense to have a check if nmi_watchdog is enabled in this
test case, and skip the all counters test?

--chris j arges

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

[Paolo Bonzini]

Il 25/08/2014 21:38, Chris J Arges ha scritto:
> Ah, I didn't know that. Yes disabling NMI watchdog via:
> echo 0 | sudo tee /proc/sys/kernel/nmi_watchdog
> Allows this test to pass.
> 
> Would it make sense to have a check if nmi_watchdog is enabled in this
> test case, and skip the all counters test?

You can check for it in procfs, but that means testing in runtests.sh.
You can add a test for it and just skip the entire PMU test.  Bonus
points for somehow adding the test to unittests.cfg instead of
hardcoding the test name in the shell script.

Paolo