public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: "Mi, Dapeng" <dapeng1.mi@linux.intel.com>
To: Mingwei Zhang <mizhang@google.com>
Cc: Sean Christopherson <seanjc@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Jim Mattson <jmattson@google.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Zhenyu Wang <zhenyuw@linux.intel.com>,
	Zhang Xiong <xiong.y.zhang@intel.com>,
	Like Xu <like.xu.linux@gmail.com>,
	Jinrong Liang <cloudliang@tencent.com>,
	Dapeng Mi <dapeng1.mi@intel.com>
Subject: Re: [kvm-unit-tests Patch v3 09/11] x86: pmu: Improve LLC misses event verification
Date: Wed, 27 Mar 2024 17:18:01 +0800	[thread overview]
Message-ID: <b204b992-8ad4-48d6-a0dd-d297ae23a5a2@linux.intel.com> (raw)
In-Reply-To: <ZgO7Wr0URLc_ru1S@google.com>


On 3/27/2024 2:23 PM, Mingwei Zhang wrote:
> On Wed, Jan 03, 2024, Dapeng Mi wrote:
>> When running pmu test on SPR, sometimes the following failure is
>> reported.
>>
>> 1 <= 0 <= 1000000
>> FAIL: Intel: llc misses-4
>>
>> Currently The LLC misses occurring only depends on probability. It's
>> possible that there is no LLC misses happened in the whole loop(),
>> especially along with processors have larger and larger cache size just
>> like what we observed on SPR.
>>
>> Thus, add clflush instruction into the loop() asm blob and ensure once
>> LLC miss is triggered at least.
>>
>> Suggested-by: Jim Mattson <jmattson@google.com>
>> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> I wonder if we can skip all LLC tests if CPU does not have
> clflush/clflushopt properties?

Looks reasonable, the LLC miss event should be skipped at least if 
clflush/clflushopt are not supported since the validation can't grantee 
the correctness. But for LLC reference event, I think we can keep it 
since the asm_loop() would always 1 LLC reference at least.

If no disapproval, I would change code to skip LLC miss event if 
clflush/clflushopt are not supported in next version. Thanks.


>> ---
>>   x86/pmu.c | 43 ++++++++++++++++++++++++++++++-------------
>>   1 file changed, 30 insertions(+), 13 deletions(-)
>>
>> diff --git a/x86/pmu.c b/x86/pmu.c
>> index b764827c1c3d..8fd3db0fbf81 100644
>> --- a/x86/pmu.c
>> +++ b/x86/pmu.c
>> @@ -20,19 +20,21 @@
>>   
>>   // Instrustion number of LOOP_ASM code
>>   #define LOOP_INSTRNS	10
>> -#define LOOP_ASM					\
>> +#define LOOP_ASM(_clflush)				\
>> +	_clflush "\n\t"                                 \
>> +	"mfence;\n\t"                                   \
>>   	"1: mov (%1), %2; add $64, %1;\n\t"		\
>>   	"nop; nop; nop; nop; nop; nop; nop;\n\t"	\
>>   	"loop 1b;\n\t"
>>   
>> -/*Enable GLOBAL_CTRL + disable GLOBAL_CTRL instructions */
>> -#define PRECISE_EXTRA_INSTRNS  (2 + 4)
>> +/*Enable GLOBAL_CTRL + disable GLOBAL_CTRL + clflush/mfence instructions */
>> +#define PRECISE_EXTRA_INSTRNS  (2 + 4 + 2)
>>   #define PRECISE_LOOP_INSTRNS   (N * LOOP_INSTRNS + PRECISE_EXTRA_INSTRNS)
>>   #define PRECISE_LOOP_BRANCHES  (N)
>> -#define PRECISE_LOOP_ASM						\
>> +#define PRECISE_LOOP_ASM(_clflush)					\
>>   	"wrmsr;\n\t"							\
>>   	"mov %%ecx, %%edi; mov %%ebx, %%ecx;\n\t"			\
>> -	LOOP_ASM							\
>> +	LOOP_ASM(_clflush)						\
>>   	"mov %%edi, %%ecx; xor %%eax, %%eax; xor %%edx, %%edx;\n\t"	\
>>   	"wrmsr;\n\t"
>>   
>> @@ -72,14 +74,30 @@ char *buf;
>>   static struct pmu_event *gp_events;
>>   static unsigned int gp_events_size;
>>   
>> +#define _loop_asm(_clflush)					\
>> +do {								\
>> +	asm volatile(LOOP_ASM(_clflush)				\
>> +		     : "=c"(tmp), "=r"(tmp2), "=r"(tmp3)	\
>> +		     : "0"(N), "1"(buf));			\
>> +} while (0)
>> +
>> +#define _precise_loop_asm(_clflush)				\
>> +do {								\
>> +	asm volatile(PRECISE_LOOP_ASM(_clflush)			\
>> +		     : "=b"(tmp), "=r"(tmp2), "=r"(tmp3)	\
>> +		     : "a"(eax), "d"(edx), "c"(global_ctl),	\
>> +		       "0"(N), "1"(buf)				\
>> +		     : "edi");					\
>> +} while (0)
>>   
>>   static inline void __loop(void)
>>   {
>>   	unsigned long tmp, tmp2, tmp3;
>>   
>> -	asm volatile(LOOP_ASM
>> -		     : "=c"(tmp), "=r"(tmp2), "=r"(tmp3)
>> -		     : "0"(N), "1"(buf));
>> +	if (this_cpu_has(X86_FEATURE_CLFLUSH))
>> +		_loop_asm("clflush (%1)");
>> +	else
>> +		_loop_asm("nop");
>>   }
>>   
>>   /*
>> @@ -96,11 +114,10 @@ static inline void __precise_count_loop(u64 cntrs)
>>   	u32 eax = cntrs & (BIT_ULL(32) - 1);
>>   	u32 edx = cntrs >> 32;
>>   
>> -	asm volatile(PRECISE_LOOP_ASM
>> -		     : "=b"(tmp), "=r"(tmp2), "=r"(tmp3)
>> -		     : "a"(eax), "d"(edx), "c"(global_ctl),
>> -		       "0"(N), "1"(buf)
>> -		     : "edi");
>> +	if (this_cpu_has(X86_FEATURE_CLFLUSH))
>> +		_precise_loop_asm("clflush (%1)");
>> +	else
>> +		_precise_loop_asm("nop");
>>   }
>>   
>>   static inline void loop(u64 cntrs)
>> -- 
>> 2.34.1
>>

  reply	other threads:[~2024-03-27  9:18 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-03  3:13 [kvm-unit-tests Patch v3 00/11] pmu test bugs fix and improvements Dapeng Mi
2024-01-03  3:13 ` [kvm-unit-tests Patch v3 01/11] x86: pmu: Remove duplicate code in pmu_init() Dapeng Mi
2024-03-28  1:19   ` Yang, Weijiang
2024-03-28  1:21     ` Mi, Dapeng
2024-01-03  3:14 ` [kvm-unit-tests Patch v3 02/11] x86: pmu: Enlarge cnt[] length to 64 in check_counters_many() Dapeng Mi
2024-03-25 21:41   ` Jim Mattson
2024-03-27  6:40     ` Mi, Dapeng
2024-01-03  3:14 ` [kvm-unit-tests Patch v3 03/11] x86: pmu: Add asserts to warn inconsistent fixed events and counters Dapeng Mi
2024-03-27  5:30   ` Mingwei Zhang
2024-03-27  6:43     ` Mi, Dapeng
2024-03-27 13:11   ` Jim Mattson
2024-03-28  9:29     ` Mi, Dapeng
2024-01-03  3:14 ` [kvm-unit-tests Patch v3 04/11] x86: pmu: Switch instructions and core cycles events sequence Dapeng Mi
2024-03-27  5:36   ` Mingwei Zhang
2024-03-27  8:54     ` Mi, Dapeng
2024-03-27 17:06       ` Mingwei Zhang
2024-03-28 10:09         ` Mi, Dapeng
2024-01-03  3:14 ` [kvm-unit-tests Patch v3 05/11] x86: pmu: Refine fixed_events[] names Dapeng Mi
2024-03-27  5:38   ` Mingwei Zhang
2024-01-03  3:14 ` [kvm-unit-tests Patch v3 06/11] x86: pmu: Remove blank line and redundant space Dapeng Mi
2024-03-27  5:38   ` Mingwei Zhang
2024-03-28  1:23   ` Yang, Weijiang
2024-03-28 10:12     ` Mi, Dapeng
2024-01-03  3:14 ` [kvm-unit-tests Patch v3 07/11] x86: pmu: Enable and disable PMCs in loop() asm blob Dapeng Mi
2024-03-27  6:07   ` Mingwei Zhang
2024-03-27  8:55     ` Mi, Dapeng
2024-04-08 23:17       ` Mingwei Zhang
2024-04-09  0:28         ` Mi, Dapeng
2024-01-03  3:14 ` [kvm-unit-tests Patch v3 08/11] x86: pmu: Improve instruction and branches events verification Dapeng Mi
2024-03-27  6:14   ` Mingwei Zhang
2024-03-27  8:59     ` Mi, Dapeng
2024-01-03  3:14 ` [kvm-unit-tests Patch v3 09/11] x86: pmu: Improve LLC misses event verification Dapeng Mi
2024-03-27  6:23   ` Mingwei Zhang
2024-03-27  9:18     ` Mi, Dapeng [this message]
2024-03-27 15:20   ` Yang, Weijiang
2024-01-03  3:14 ` [kvm-unit-tests Patch v3 10/11] x86: pmu: Add IBPB indirect jump asm blob Dapeng Mi
2024-01-03  3:14 ` [kvm-unit-tests Patch v3 11/11] x86: pmu: Improve branch misses event verification Dapeng Mi
2024-01-24  8:18 ` [kvm-unit-tests Patch v3 00/11] pmu test bugs fix and improvements Mi, Dapeng

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=b204b992-8ad4-48d6-a0dd-d297ae23a5a2@linux.intel.com \
    --to=dapeng1.mi@linux.intel.com \
    --cc=cloudliang@tencent.com \
    --cc=dapeng1.mi@intel.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=like.xu.linux@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mizhang@google.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=xiong.y.zhang@intel.com \
    --cc=zhenyuw@linux.intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox