All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christopher Covington <cov@codeaurora.org>
To: Andrew Jones <drjones@redhat.com>
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org,
	kvmarm@lists.cs.columbia.edu, wei@redhat.com,
	alindsay@codeaurora.org, croberts@codeaurora.org,
	shannon.zhao@linaro.org, alistair.francis@xilinx.com
Subject: Re: [Qemu-devel] [kvm-unit-tests PATCHv4 3/3] arm: pmu: Add CPI checking
Date: Mon, 19 Oct 2015 11:44:30 -0400	[thread overview]
Message-ID: <56250FDE.8000008@codeaurora.org> (raw)
In-Reply-To: <20151018182821.GC12158@hawk.localdomain>

Hi Drew,

I appreciate your feedback on these patches.

On 10/18/2015 02:28 PM, Andrew Jones wrote:

>> --- a/arm/pmu.c
>> +++ b/arm/pmu.c
>> @@ -37,6 +37,18 @@ static inline unsigned long get_pmccntr(void)
>>  	asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
>>  	return cycles;
>>  }
>> +
>> +static inline void loop(int i, uint32_t pmcr)
>> +{
>> +	uint32_t z = 0;
>> +
>> +	asm volatile(
>> +		"	mcr p15, 0, %[pmcr], c9, c12, 0\n"
>> +		"	1: subs %[i], %[i], #1\n"
>> +		"	bgt 1b\n"
>> +		"	mcr p15, 0, %[z], c9, c12, 0\n"
>> +	: [i] "+r" (i) : [pmcr] "r" (pmcr), [z] "r" (z) : "cc");
> 
> Assembly is always ugly, but we can do a bit better formatting with tabs
> 
> 	asm volatile(
> 	"	mcr	p15, 0, %[pmcr], c9, c12, 0\n"
> 	"1:	subs	%[i], %[i], #1\n"
> 	"	bgt	1b\n"
> 	"	mcr	p15, 0, %[z], c9, c12, 0\n"
> 	: [i] "+r" (i)
> 	: [pmcr] "r" (pmcr), [z] "r" (z)
> 	: "cc");
> 
> Actually it can be even cleaner because you already created set_pmcr()
> 
> 	set_pmcr(pmcr);
> 
> 	asm volatile(
> 	"1:	subs	%0, %0, #1\n"
> 	"	bgt     1b\n"
> 	: "+r" (i) : : "cc");
> 
> 	set_pmcr(0);

Is there any way to ensure that the compiler won't for example put a `mov rd,
#0` between the `bgt 1b` and the `mcr <pmcr>, rn`?

>> @@ -125,12 +147,79 @@ static bool check_cycles_increase(void)
>>  	return true;
>>  }
>>  
>> -int main(void)
>> +/*
>> + * Execute a known number of guest instructions. Only odd instruction counts
>> + * greater than or equal to 3 are supported by the in-line assembly code. The
> 
> Not all odd counts, right? But rather all multiples of 3? IIUC this is because
> the loop is two instructions (sub + branch), and then the clearing of the pmcr
> register counts as the 3rd?

Clearing the PMCR doesn't happen as part of the loop, but as part of the loop
exit or epilogue.

total_instrs = iteration_count * loop_instrs + eipilogue_instrs
total_instrs = iteration_count * 2 + 1

Thanks,
Christopher Covington

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

WARNING: multiple messages have this Message-ID (diff)
From: Christopher Covington <cov@codeaurora.org>
To: Andrew Jones <drjones@redhat.com>
Cc: wei@redhat.com, alindsay@codeaurora.org, kvm@vger.kernel.org,
	croberts@codeaurora.org, qemu-devel@nongnu.org,
	alistair.francis@xilinx.com, shannon.zhao@linaro.org,
	kvmarm@lists.cs.columbia.edu
Subject: Re: [Qemu-devel] [kvm-unit-tests PATCHv4 3/3] arm: pmu: Add CPI checking
Date: Mon, 19 Oct 2015 11:44:30 -0400	[thread overview]
Message-ID: <56250FDE.8000008@codeaurora.org> (raw)
In-Reply-To: <20151018182821.GC12158@hawk.localdomain>

Hi Drew,

I appreciate your feedback on these patches.

On 10/18/2015 02:28 PM, Andrew Jones wrote:

>> --- a/arm/pmu.c
>> +++ b/arm/pmu.c
>> @@ -37,6 +37,18 @@ static inline unsigned long get_pmccntr(void)
>>  	asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
>>  	return cycles;
>>  }
>> +
>> +static inline void loop(int i, uint32_t pmcr)
>> +{
>> +	uint32_t z = 0;
>> +
>> +	asm volatile(
>> +		"	mcr p15, 0, %[pmcr], c9, c12, 0\n"
>> +		"	1: subs %[i], %[i], #1\n"
>> +		"	bgt 1b\n"
>> +		"	mcr p15, 0, %[z], c9, c12, 0\n"
>> +	: [i] "+r" (i) : [pmcr] "r" (pmcr), [z] "r" (z) : "cc");
> 
> Assembly is always ugly, but we can do a bit better formatting with tabs
> 
> 	asm volatile(
> 	"	mcr	p15, 0, %[pmcr], c9, c12, 0\n"
> 	"1:	subs	%[i], %[i], #1\n"
> 	"	bgt	1b\n"
> 	"	mcr	p15, 0, %[z], c9, c12, 0\n"
> 	: [i] "+r" (i)
> 	: [pmcr] "r" (pmcr), [z] "r" (z)
> 	: "cc");
> 
> Actually it can be even cleaner because you already created set_pmcr()
> 
> 	set_pmcr(pmcr);
> 
> 	asm volatile(
> 	"1:	subs	%0, %0, #1\n"
> 	"	bgt     1b\n"
> 	: "+r" (i) : : "cc");
> 
> 	set_pmcr(0);

Is there any way to ensure that the compiler won't for example put a `mov rd,
#0` between the `bgt 1b` and the `mcr <pmcr>, rn`?

>> @@ -125,12 +147,79 @@ static bool check_cycles_increase(void)
>>  	return true;
>>  }
>>  
>> -int main(void)
>> +/*
>> + * Execute a known number of guest instructions. Only odd instruction counts
>> + * greater than or equal to 3 are supported by the in-line assembly code. The
> 
> Not all odd counts, right? But rather all multiples of 3? IIUC this is because
> the loop is two instructions (sub + branch), and then the clearing of the pmcr
> register counts as the 3rd?

Clearing the PMCR doesn't happen as part of the loop, but as part of the loop
exit or epilogue.

total_instrs = iteration_count * loop_instrs + eipilogue_instrs
total_instrs = iteration_count * 2 + 1

Thanks,
Christopher Covington

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

  reply	other threads:[~2015-10-19 15:44 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-01 19:47 [PATCH] arm: Add PMU test Christopher Covington
2015-10-01 19:47 ` [Qemu-devel] " Christopher Covington
2015-10-02  9:58 ` Andrew Jones
2015-10-02  9:58   ` Andrew Jones
2015-10-02 15:48 ` [kvm-unit-tests PATCHv2] " Christopher Covington
2015-10-02 15:48   ` [Qemu-devel] " Christopher Covington
2015-10-05 21:37   ` Wei Huang
2015-10-05 21:37     ` [Qemu-devel] " Wei Huang
2015-10-06 17:49     ` [kvm-unit-tests PATCHv3] ARM PMU tests Christopher Covington
2015-10-06 17:49       ` [Qemu-devel] " Christopher Covington
2015-10-06 17:49       ` [kvm-unit-tests PATCHv3 1/3] arm: Add PMU test Christopher Covington
2015-10-06 17:49         ` [Qemu-devel] " Christopher Covington
2015-10-06 19:38         ` Andrew Jones
2015-10-06 19:38           ` [Qemu-devel] " Andrew Jones
2015-10-06 17:49       ` [kvm-unit-tests PATCHv3 2/3] arm: pmu: Check cycle count increases Christopher Covington
2015-10-06 17:49         ` [Qemu-devel] " Christopher Covington
2015-10-06 19:49         ` Andrew Jones
2015-10-06 19:49           ` Andrew Jones
2015-10-06 17:49       ` [kvm-unit-tests PATCHv3 3/3] arm: pmu: Add CPI checking Christopher Covington
2015-10-06 17:49         ` [Qemu-devel] " Christopher Covington
2015-10-06 20:14         ` Andrew Jones
2015-10-06 20:14           ` [Qemu-devel] " Andrew Jones
2015-10-12 15:07     ` [kvm-unit-tests PATCHv4] ARM PMU tests Christopher Covington
2015-10-12 15:07       ` [Qemu-devel] " Christopher Covington
2015-10-12 15:07       ` [kvm-unit-tests PATCHv4 1/3] arm: Add PMU test Christopher Covington
2015-10-12 15:07         ` [Qemu-devel] " Christopher Covington
2015-10-18 17:54         ` Andrew Jones
2015-10-18 17:54           ` Andrew Jones
2015-10-12 15:07       ` [kvm-unit-tests PATCHv4 2/3] arm: pmu: Check cycle count increases Christopher Covington
2015-10-12 15:07         ` [Qemu-devel] " Christopher Covington
2015-10-18 18:10         ` Andrew Jones
2015-10-18 18:10           ` Andrew Jones
2015-10-12 15:07       ` [kvm-unit-tests PATCHv4 3/3] arm: pmu: Add CPI checking Christopher Covington
2015-10-12 15:07         ` [Qemu-devel] " Christopher Covington
2015-10-18 18:28         ` Andrew Jones
2015-10-18 18:28           ` Andrew Jones
2015-10-19 15:44           ` Christopher Covington [this message]
2015-10-19 15:44             ` Christopher Covington
2015-10-26 12:25             ` Andrew Jones
2015-10-26 12:25               ` Andrew Jones
2015-10-18 18:29       ` [Qemu-devel] [kvm-unit-tests PATCHv4] ARM PMU tests Andrew Jones
2015-10-18 18:29         ` Andrew Jones
2015-10-26 15:38       ` [kvm-unit-tests PATCHv5] " Christopher Covington
2015-10-26 15:38         ` [Qemu-devel] " Christopher Covington
2015-10-26 15:38         ` [kvm-unit-tests PATCHv5 1/3] arm: Add PMU test Christopher Covington
2015-10-26 15:38           ` [Qemu-devel] " Christopher Covington
2015-10-26 15:38         ` [kvm-unit-tests PATCHv5 2/3] arm: pmu: Check cycle count increases Christopher Covington
2015-10-26 15:38           ` [Qemu-devel] " Christopher Covington
2015-10-26 15:58           ` Andrew Jones
2015-10-26 15:58             ` Andrew Jones
2015-10-26 16:04             ` Christopher Covington
2015-10-26 16:04               ` Christopher Covington
2015-10-26 16:04             ` Andrew Jones
2015-10-26 16:04               ` Andrew Jones
2015-10-26 15:38         ` [kvm-unit-tests PATCHv5 3/3] arm: pmu: Add CPI checking Christopher Covington
2015-10-26 15:38           ` [Qemu-devel] " Christopher Covington
2015-10-26 16:28           ` Andrew Jones
2015-10-26 16:28             ` Andrew Jones
2015-10-28 19:12       ` [kvm-unit-tests PATCHv6] ARM PMU tests Christopher Covington
2015-10-28 19:12         ` [Qemu-devel] " Christopher Covington
2015-10-28 19:12         ` [kvm-unit-tests PATCHv5 1/3] arm: Add PMU test Christopher Covington
2015-10-28 19:12           ` [Qemu-devel] " Christopher Covington
2015-10-28 19:12         ` [kvm-unit-tests PATCHv5 2/3] arm: pmu: Check cycle count increases Christopher Covington
2015-10-28 19:12           ` [Qemu-devel] " Christopher Covington
2015-10-28 19:12         ` [kvm-unit-tests PATCHv5 3/3] arm: pmu: Add CPI checking Christopher Covington
2015-10-28 19:12           ` [Qemu-devel] " Christopher Covington
2015-10-30 13:00           ` Andrew Jones
2015-10-30 13:00             ` Andrew Jones
2015-10-30 19:32             ` Christopher Covington
2015-10-30 19:32               ` Christopher Covington
2015-11-02 15:58               ` Andrew Jones
2015-11-11  2:05                 ` Andrew Jones
2015-11-11 12:50                   ` Christopher Covington

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56250FDE.8000008@codeaurora.org \
    --to=cov@codeaurora.org \
    --cc=alindsay@codeaurora.org \
    --cc=alistair.francis@xilinx.com \
    --cc=croberts@codeaurora.org \
    --cc=drjones@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=qemu-devel@nongnu.org \
    --cc=shannon.zhao@linaro.org \
    --cc=wei@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.