[an error occurred while processing this directive]
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] [day] [month] [year] [list]
Message-ID: <aIkQvhGhRKisonmh@google.com>
Date: Tue, 29 Jul 2025 11:19:42 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Chao Gao <chao.gao@...el.com>
Cc: Xin Li <xin@...or.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	x86@...nel.org, pbonzini@...hat.com, dave.hansen@...el.com, 
	rick.p.edgecombe@...el.com, mlevitsk@...hat.com, john.allen@....com, 
	weijiang.yang@...el.com, minipli@...ecurity.net, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH v11 01/23] KVM: x86: Rename kvm_{g,s}et_msr()* to show
 that they emulate guest accesses

On Tue, Jul 29, 2025, Chao Gao wrote:
> On Mon, Jul 28, 2025 at 03:31:41PM -0700, Xin Li wrote:
> >>   	/* Set L1 segment info according to Intel SDM
> >>   	    27.5.2 Loading Host Segment and Descriptor-Table Registers */
> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> index 7543dac7ae70..11d84075cd14 100644
> >> --- a/arch/x86/kvm/x86.c
> >> +++ b/arch/x86/kvm/x86.c
> >> @@ -1929,33 +1929,35 @@ static int kvm_get_msr_ignored_check(struct kvm_vcpu *vcpu,
> >>   				 __kvm_get_msr);
> >>   }
> >> -int kvm_get_msr_with_filter(struct kvm_vcpu *vcpu, u32 index, u64 *data)
> >> +int kvm_emulate_msr_read_with_filter(struct kvm_vcpu *vcpu, u32 index,
> >> +				     u64 *data)
> >
> >I think the extra new line doesn't improve readability, but it's the
> >maintainer's call.
> >
> 
> Sure. Seems "let it poke out" is Sean's preference. I saw he made similar
> requests several times. e.g.,

Depends on the situation.  I'd probably mentally flip a coin in this case.

But what I'd actually do here is choose names that are (a) less verbose and (b)
capture the relationship between the APIs.  Instead of:

  int kvm_emulate_msr_read_with_filter(struct kvm_vcpu *vcpu, u32 index, u64 *data);
  int kvm_emulate_msr_write_with_filter(struct kvm_vcpu *vcpu, u32 index, u64 data);
  int kvm_emulate_msr_read(struct kvm_vcpu *vcpu, u32 index, u64 *data);
  int kvm_emulate_msr_write(struct kvm_vcpu *vcpu, u32 index, u64 data);

rename to:

  int kvm_emulate_msr_read(struct kvm_vcpu *vcpu, u32 index, u64 *data);
  int kvm_emulate_msr_write(struct kvm_vcpu *vcpu, u32 index, u64 data);
  int __kvm_emulate_msr_read(struct kvm_vcpu *vcpu, u32 index, u64 *data);
  int __kvm_emulate_msr_write(struct kvm_vcpu *vcpu, u32 index, u64 data);

And then we can do a follow-up patch to solidify the relationship:

--
From: Sean Christopherson <seanjc@...gle.com>
Date: Tue, 29 Jul 2025 11:13:48 -0700
Subject: [PATCH] KVM: x86: Use double-underscore read/write MSR helpers as
 appropriate

Use the double-underscore helpers for emulating MSR reads and writes in
he no-underscore versions to better capture the relationship between the
two sets of APIs (the double-underscore versions don't honor userspace MSR
filters).

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@...gle.com>
---
 arch/x86/kvm/x86.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 09b106a5afdf..65c787bcfe8b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1932,11 +1932,24 @@ static int kvm_get_msr_ignored_check(struct kvm_vcpu *vcpu,
 				 __kvm_get_msr);
 }
 
+int __kvm_emulate_msr_read(struct kvm_vcpu *vcpu, u32 index, u64 *data)
+{
+	return kvm_get_msr_ignored_check(vcpu, index, data, false);
+}
+EXPORT_SYMBOL_GPL(__kvm_emulate_msr_read);
+
+int __kvm_emulate_msr_write(struct kvm_vcpu *vcpu, u32 index, u64 data)
+{
+	return kvm_set_msr_ignored_check(vcpu, index, data, false);
+}
+EXPORT_SYMBOL_GPL(__kvm_emulate_msr_write);
+
 int kvm_emulate_msr_read(struct kvm_vcpu *vcpu, u32 index, u64 *data)
 {
 	if (!kvm_msr_allowed(vcpu, index, KVM_MSR_FILTER_READ))
 		return KVM_MSR_RET_FILTERED;
-	return kvm_get_msr_ignored_check(vcpu, index, data, false);
+
+	return __kvm_emulate_msr_read(vcpu, index, data);
 }
 EXPORT_SYMBOL_GPL(kvm_emulate_msr_read);
 
@@ -1944,21 +1957,11 @@ int kvm_emulate_msr_write(struct kvm_vcpu *vcpu, u32 index, u64 data)
 {
 	if (!kvm_msr_allowed(vcpu, index, KVM_MSR_FILTER_WRITE))
 		return KVM_MSR_RET_FILTERED;
-	return kvm_set_msr_ignored_check(vcpu, index, data, false);
+
+	return __kvm_emulate_msr_write(vcpu, index, data);
 }
 EXPORT_SYMBOL_GPL(kvm_emulate_msr_write);
 
-int __kvm_emulate_msr_read(struct kvm_vcpu *vcpu, u32 index, u64 *data)
-{
-	return kvm_get_msr_ignored_check(vcpu, index, data, false);
-}
-EXPORT_SYMBOL_GPL(__kvm_emulate_msr_read);
-
-int __kvm_emulate_msr_write(struct kvm_vcpu *vcpu, u32 index, u64 data)
-{
-	return kvm_set_msr_ignored_check(vcpu, index, data, false);
-}
-EXPORT_SYMBOL_GPL(__kvm_emulate_msr_write);
 
 static void complete_userspace_rdmsr(struct kvm_vcpu *vcpu)
 {

base-commit: 1877e7b0749cbaa2d2ba4056eeda93adb373f7d4
--