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

Shivansh Dhiman
2025-11-13T11:58:26Z

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@amd.com>

Other tests look good to me, and work fine.

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