linux-kernel - Re: [PATCH v2 44/49] KVM: x86: Update guest cpu_caps at runtime for dynamic CPUID-based features

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: <ZoyDTJ3nb_MQ38nW@google.com>
Date: Mon, 8 Jul 2024 17:24:44 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Maxim Levitsky <mlevitsk@...hat.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, Vitaly Kuznetsov <vkuznets@...hat.com>, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Hou Wenlong <houwenlong.hwl@...group.com>, 
	Kechen Lu <kechenl@...dia.com>, Oliver Upton <oliver.upton@...ux.dev>, 
	Binbin Wu <binbin.wu@...ux.intel.com>, Yang Weijiang <weijiang.yang@...el.com>, 
	Robert Hoo <robert.hoo.linux@...il.com>
Subject: Re: [PATCH v2 44/49] KVM: x86: Update guest cpu_caps at runtime for
 dynamic CPUID-based features

On Thu, Jul 04, 2024, Maxim Levitsky wrote:
> On Fri, 2024-05-17 at 10:39 -0700, Sean Christopherson wrote:
> > -		cpuid_entry_change(best, X86_FEATURE_OSPKE,
> > -				   kvm_is_cr4_bit_set(vcpu, X86_CR4_PKE));
> > +		kvm_update_feature_runtime(vcpu, best, X86_FEATURE_OSPKE,
> > +					   kvm_is_cr4_bit_set(vcpu, X86_CR4_PKE));
> > +
> >  
> >  	best = kvm_find_cpuid_entry_index(vcpu, 0xD, 0);
> >  	if (best)
> 
> 
> I am not 100% sure that we need to do this.
> 
> Runtime cpuid changes are a hack that Intel did back then, due to various
> reasons, These changes don't really change the feature set that CPU supports,
> but merly as you like to say 'massage' the output of the CPUID instruction to
> make the unmodified OS happy usually.
> 
> Thus it feels to me that CPU caps should not include the dynamic features,
> and neither KVM should use the value of these as a source for truth, but
> rather the underlying source of the truth (e.g CR4).
> 
> But if you insist, I don't really have a very strong reason to object this.

FWIW, I think I agree that CR4 should be the source of truth, but it's largely a
moot point because KVM doesn't actually check OSXSAVE or OSPKE, as KVM never
emulates the relevant instructions.  So for those, it's indeed not strictly
necessary.

Unfortunately, KVM has established ABI for checking X86_FEATURE_MWAIT when
"emulating" MONITOR and MWAIT, i.e. KVM can't use vcpu->arch.ia32_misc_enable_msr
as the source of truth.  So for MWAIT, KVM does need to update CPU caps (or carry
even more awful MWAIT code), at which point extending the behavior to the CR4
features (and to X86_FEATURE_APIC) is practically free.