linux-kernel - Re: [PATCH v3 12/14] x86/svm: Cleanup LBRV tests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1f39d5a3-e728-4b2b-a9c6-50cbc4fffd17@amd.com>
Date: Thu, 13 Nov 2025 17:28:11 +0530
From: Shivansh Dhiman <shivansh.dhiman@....com>
To: Yosry Ahmed <yosry.ahmed@...ux.dev>
CC: Paolo Bonzini <pbonzini@...hat.com>, Sean Christopherson
	<seanjc@...gle.com>, Kevin Cheng <chengkev@...gle.com>,
	<kvm@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 12/14] x86/svm: Cleanup LBRV tests

Hi Yosry,

I tested this on EPYC-Turin and found that some tests seem to be a bit flaky.
See below.

On 11-11-2025 04:56, Yosry Ahmed wrote:
> @@ -3058,55 +3041,64 @@ u64 dbgctl;
>  
>  static void svm_lbrv_test_guest1(void)
>  {
> +	u64 from_ip, to_ip;
> +
>  	/*
>  	 * This guest expects the LBR to be already enabled when it starts,
>  	 * it does a branch, and then disables the LBR and then checks.
>  	 */
> +	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> +	TEST_EXPECT_EQ(dbgctl, DEBUGCTLMSR_LBR);

This TEST_EXPECT_EQ is run when LBR is enabled, causing it to change last
branch. I tried to move it below wrmsr(MSR_IA32_DEBUGCTLMSR, 0) and it works
fine that way.

>  
>  	DO_BRANCH(guest_branch0);
>  
> -	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> +	/* Disable LBR before the checks to avoid changing the last branch */
>  	wrmsr(MSR_IA32_DEBUGCTLMSR, 0);> +	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> +	TEST_EXPECT_EQ(dbgctl, 0);
>  
> -	if (dbgctl != DEBUGCTLMSR_LBR)
> -		asm volatile("ud2\n");
> -	if (rdmsr(MSR_IA32_DEBUGCTLMSR) != 0)
> -		asm volatile("ud2\n");
> +	get_lbr_ips(&from_ip, &to_ip);
> +	TEST_EXPECT_EQ((u64)&guest_branch0_from, from_ip);
> +	TEST_EXPECT_EQ((u64)&guest_branch0_to, to_ip);
>  
> -	GUEST_CHECK_LBR(&guest_branch0_from, &guest_branch0_to);
>  	asm volatile ("vmmcall\n");
>  }
>  
>  static void svm_lbrv_test_guest2(void)
>  {
> +	u64 from_ip, to_ip;
> +
>  	/*
>  	 * This guest expects the LBR to be disabled when it starts,
>  	 * enables it, does a branch, disables it and then checks.
>  	 */
> -
> -	DO_BRANCH(guest_branch1);
>  	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> +	TEST_EXPECT_EQ(dbgctl, 0);
>  
> -	if (dbgctl != 0)
> -		asm volatile("ud2\n");
> +	DO_BRANCH(guest_branch1);
>  
> -	GUEST_CHECK_LBR(&host_branch2_from, &host_branch2_to);
> +	get_lbr_ips(&from_ip, &to_ip);
> +	TEST_EXPECT_EQ((u64)&host_branch2_from, from_ip);
> +	TEST_EXPECT_EQ((u64)&host_branch2_to, to_ip);
>  
>  	wrmsr(MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_LBR);
>  	dbgctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> +	TEST_EXPECT_EQ(dbgctl, DEBUGCTLMSR_LBR);

Same thing here as well.

> +
>  	DO_BRANCH(guest_branch2);
>  	wrmsr(MSR_IA32_DEBUGCTLMSR, 0);
>  
> -	if (dbgctl != DEBUGCTLMSR_LBR)
> -		asm volatile("ud2\n");
> -	GUEST_CHECK_LBR(&guest_branch2_from, &guest_branch2_to);
> +	get_lbr_ips(&from_ip, &to_ip);
> +	TEST_EXPECT_EQ((u64)&guest_branch2_from, from_ip);
> +	TEST_EXPECT_EQ((u64)&guest_branch2_to, to_ip);
>  
>  	asm volatile ("vmmcall\n");
>  }
Reviewed-by: Shivansh Dhiman <shivansh.dhiman@....com>

Other tests look good to me, and work fine.

Tested-by: Shivansh Dhiman <shivansh.dhiman@....com>