linux-kernel - Re: [PATCH v4 03/26] KVM: selftests: Add a test for LBR save/restore (ft. nested)

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: <aYU99UWEIKwSzlJI@google.com>
Date: Thu, 5 Feb 2026 17:03:49 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Yosry Ahmed <yosry.ahmed@...ux.dev>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 03/26] KVM: selftests: Add a test for LBR save/restore
 (ft. nested)

On Thu, Jan 15, 2026, Yosry Ahmed wrote:
> @@ -1359,6 +1359,11 @@ static inline bool kvm_is_ignore_msrs(void)
>  	return get_kvm_param_bool("ignore_msrs");
>  }
>  
> +static inline bool kvm_is_lbrv_enabled(void)
> +{
> +	return !!get_kvm_amd_param_integer("lbrv");

Argh, KVM and its stupid "bools are ints" params.

> +}
> +
>  uint64_t *vm_get_pte(struct kvm_vm *vm, uint64_t vaddr);
>  
>  uint64_t kvm_hypercall(uint64_t nr, uint64_t a0, uint64_t a1, uint64_t a2,
> diff --git a/tools/testing/selftests/kvm/x86/svm_lbr_nested_state.c b/tools/testing/selftests/kvm/x86/svm_lbr_nested_state.c
> new file mode 100644
> index 000000000000..a343279546fd
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/svm_lbr_nested_state.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * svm_lbr_nested_state
> + *
> + * Test that LBRs are maintained correctly in both L1 and L2 during
> + * save/restore.

Drop the file comments.  The name of the test is beyond useless, and if the
reader can't quickly figure out what the test is doing, then the code itself
needs more comments.

> + *
> + * Copyright (C) 2025, Google, Inc.
> + */
> +
> +#include "test_util.h"
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "svm_util.h"
> +
> +
> +#define L2_GUEST_STACK_SIZE 64
> +
> +#define DO_BRANCH() asm volatile("jmp 1f\n 1: nop")
> +
> +struct lbr_branch {
> +	u64 from, to;
> +};
> +
> +volatile struct lbr_branch l2_branch;
> +
> +#define RECORD_BRANCH(b, s)						\

RECORD_AND_CHECK_BRANCH?

> +({									\

Use do-while (0) unless the macro _needs_ to return a value.  do-while provides
compile-time safety against some goofs.

> +	wrmsr(MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_LBR);			\
> +	DO_BRANCH();							\
> +	(b)->from = rdmsr(MSR_IA32_LASTBRANCHFROMIP);			\
> +	(b)->to = rdmsr(MSR_IA32_LASTBRANCHTOIP);			\
> +	/* Disabe LBR right after to avoid overriding the IPs */	\
> +	wrmsr(MSR_IA32_DEBUGCTLMSR, 0);					\
> +									\
> +	GUEST_ASSERT_NE((b)->from, 0);					\
> +	GUEST_ASSERT_NE((b)->to, 0);					\
> +	GUEST_PRINTF("%s: (0x%lx, 0x%lx)\n", (s), (b)->from, (b)->to);	\

Why print here?  Won't the asserts below print useful information if they fail?

> +})									\