Received-SPF: pass (google.com: domain of linux-kernel+bounces-121490-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <ea33d2fa-6e69-4904-b5fd-ecec67e43352@intel.com>
Date: Wed, 27 Mar 2024 23:20:09 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [kvm-unit-tests Patch v3 09/11] x86: pmu: Improve LLC misses
 event verification
To: Dapeng Mi <dapeng1.mi@linux.intel.com>
CC: Jim Mattson <jmattson@google.com>, Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>, "kvm@vger.kernel.org"
	<kvm@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, Zhenyu Wang <zhenyuw@linux.intel.com>,
	"Zhang, Xiong Y" <xiong.y.zhang@intel.com>, Mingwei Zhang
	<mizhang@google.com>, Like Xu <like.xu.linux@gmail.com>, Jinrong Liang
	<cloudliang@tencent.com>, "Mi, Dapeng1" <dapeng1.mi@intel.com>
References: <20240103031409.2504051-1-dapeng1.mi@linux.intel.com>
 <20240103031409.2504051-10-dapeng1.mi@linux.intel.com>
Content-Language: en-US
From: "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <20240103031409.2504051-10-dapeng1.mi@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?M1NwK3BJdlAvdXZsT0FZMlY0TGlsc0srNE9pMmYyTXZNL1lDNTlJVUVXbnht?=
 =?utf-8?B?blJDT2dSaExqRHZZcHlZZHordXVBYzNBTFVnRHIvWTJtaHlHLzBjVG0wWmRE?=
 =?utf-8?B?SzBnVXZEeGZ1Q2d0QVFPcEZJU1psdFExR2dITWFFQ1RmRjREWkpvRHh1WWcx?=
 =?utf-8?B?LzdLT1p2MjNlbjUrd2g1ekFZWEUvSlcwL0hqM0ZJbFZsRmllb3JPZ1c5b1Zh?=
 =?utf-8?B?clVyQngzQ0dWZjlYZkZCaVJoY2FxbStWazF1UHBmcmx0QzN5RDlOcGMxelE2?=
 =?utf-8?B?SmNGczVlN2FOcWhNeWoxa3VrWVBzVHBJOWhORUVXR0lhTWNLWGI2R3JLN0o4?=
 =?utf-8?B?TElYeVpZTmVVRlVCWmh1ZEpKbmQxUjlBS0ttTSttNkptM3Z1cThqaTNVRjkv?=
 =?utf-8?B?bEgwdjJTZWwzTDlwbUVlRDZJY0tMWnhoZnIxK0c4UjllV3Q0Zm5FKyt3RXgr?=
 =?utf-8?B?aUFKc1ExU3NvdktUMzAwTmpqU2V6OEN5MFc5Q1BoOFdkc2ZEV2Jzb0xuODZF?=
 =?utf-8?B?YUx5a0RKRFRQeWkwQXdSUUpXWlhVWDlOOVlwY3kvamVUR05EbWdKOUhMekc2?=
 =?utf-8?B?LytGNjdXRTV0NHY2TVNiazZ3bi9XK1VWVTFGVkhqS0J5SVJXUU4zc0J3ZUZ5?=
 =?utf-8?B?K25rUXFtSzB0bDJCMUJBbDJFY2FtT0ZwN0tCcVIyUmozdkpxNVpGZGZUcmxW?=
 =?utf-8?B?a2I4ZnU5dWFpNkMvU0pyNEJhd1lUOVlmai9EWnVFeldCWjZtQnZ4eGI3QnVF?=
 =?utf-8?B?Z3Y1NG9qLzZlUFh6NnZlU1cxU3J6QXZDWm9UdzZFNlRZK2ZyVHBRR2IzaFJN?=
 =?utf-8?B?TTVkV1BLaEJqUnhvcUVSRE1LaE05T1FBemV3WTBGWENKaFZCeU9oUEQyOEND?=
 =?utf-8?B?Nks3VXFiY2VDMUswUlc4QUZhV3FIQ05hTXB6WExLdk5LL1N0QlJsTWRPbXFT?=
 =?utf-8?B?WWVIVGN5Q3A0elp6c0NqRjVqNVVvVjRkb2UzdGo0SnUwZWNDeXVMcXBoVmVH?=
 =?utf-8?B?d0ozMHBVSDZvRlFsc0RJTyt0dTdRWFhtK2ZuNmJFd0lyL2ErUmJDSGxDcXV5?=
 =?utf-8?B?N2NYRXVwVXlOdmFJMSs3R29mV2lPNGVhVlV0VmNqWFpIZVErcHZkb2lFSnA1?=
 =?utf-8?B?WjhPR093UnZ6K0xWR1EzYXZIS2lEaXFvVmY0Uk9Wek5USTJZWUw3Q2hwTXFa?=
 =?utf-8?B?czdXRzZZaExjb01tcVFjVGFIOSt2U25IWkY3WmxVdEt3NnM5a3B2Uy9DVGRp?=
 =?utf-8?B?bTlON21uZ3d0dTdLSGwzNzA0MGdHajM3UW1WaWVFOEFKSyt5RGdWZ3Zpa3Ur?=
 =?utf-8?B?a2dRb0d5cHVIZXFkYVVMWWlwYkhRRjNxL2oreGNUWGJJcmFIWXdTV1d1RFVm?=
 =?utf-8?B?SGZPV05ZNGFLV2V4TVR0bERZbi9iQko1SVpKMFk4WXU5OE1DWWlXQjFRR2VP?=
 =?utf-8?B?WEF0NFJkNVY3ZERNWFRDeFpVclp6bi9RZ3B5QlIzWXhwb0k2YTZxWEg0T2kz?=
 =?utf-8?B?bUg0eEV5UnRFS1Z3bjdzTHFBNEY1ZXArNXMrV2V0YzBVNTZ4M1B0b2YrMkda?=
 =?utf-8?B?ZWlyc005blZZbEY1U2JjNXNFOHgyeWtyWTVwQmZzVHlUM3FiNEh3QXNrdXcy?=
 =?utf-8?B?T3A1Z0V2bFZYMm8zalFhYndYdDg0b3FEbENzQ2psRTRBV3lzcWZmS2hRWDZW?=
 =?utf-8?B?RjIwY0NOU0VLdTFPVUl6SXRTdmVFeDNrSWE3TjRoSHNwaE0vOWxhTHBtaENH?=
 =?utf-8?B?eDZjelhoMU1qd2NCbXdJQmRNUW5QQlVKUnV4QnU0ZkptWFozVHNaLy9NM2pw?=
 =?utf-8?B?QWp3ME1uTFR5anlsb08ydmZBZmtNOVJJM0lOUHh5MWxaOHlycVZDM0wxSmgy?=
 =?utf-8?B?ZkNlVEQ3WkNuaFh5d2VxU1luNFhjZHM1a0h3amh4cFhTMm83dWczTkhaZFZj?=
 =?utf-8?B?NFdKTndZa2xQak9RUTdRWmsyR245NGlTaWY5L3lNRjJIZW82Yi9jZE9xa1Z6?=
 =?utf-8?B?VEt4dk9CYWJEOGxrbXZ4Uk5XVjJTOVFYRFpCQmNEV1JkTkN6Z1V0SGxlQU9N?=
 =?utf-8?B?UzlPcjYwVDNJenhCS2x3Tkp1TmZaNTBhdXFjUlo2bXh3elNoekVFTzB4VGxY?=
 =?utf-8?B?MVkwTlk0RDM1NmpFaEQ1RlpmNWo0MzJSS3NTQVFUSHlpY2tabEZQekNuQ013?=
 =?utf-8?B?a2c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: d72c86ad-bd1c-4542-794c-08dc4e7168f0
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Mar 2024 15:20:18.1668
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: B+4iNiN7rUihNdJSRmjB8l9AsdTfcFsYEJYA5JCWClZZRN3ZDRQZiccUvYaDuoMmbiMpuoKi6s9Brb79F97pjA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB6967
X-OriginatorOrg: intel.com

On 1/3/2024 11:14 AM, 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>
> ---
>   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;

Can you move these tmp variables into macro's do...while block since they're not
needed here?

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

Ditto.

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