linux-kernel - Re: [PATCH] KVM: x86: Do not mask LVTPC when handling a PMI on AMD platforms

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: <ZeoJCrx7K_FLGLNA@google.com>
Date: Thu, 7 Mar 2024 10:35:54 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Sandipan Das <sandipan.das@....com>
Cc: Jim Mattson <jmattson@...gle.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	pbonzini@...hat.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, 
	mlevitsk@...hat.com, vkuznets@...hat.com, mizhang@...gle.com, 
	tao1.su@...ux.intel.com, andriy.shevchenko@...ux.intel.com, 
	ravi.bangoria@....com, ananth.narayan@....com, nikunj.dadhania@....com, 
	santosh.shukla@....com, manali.shukla@....com
Subject: Re: [PATCH] KVM: x86: Do not mask LVTPC when handling a PMI on AMD platforms

On Thu, Mar 07, 2024, Sandipan Das wrote:
> On 3/5/2024 2:19 AM, Sean Christopherson wrote:
> The following are excerpts from some changes that I have been working on. Instead
> of a boolean flag, this saves the compatible vendor in kvm_vcpu_arch and tries
> to match it against X86_VENDOR_* values. The goal is to replace
> guest_cpuid_is_{intel,amd_or_hygon}() with
> guest_vendor_is_compatible(vcpu, X86_VENDOR_{INTEL,AMD}). Is this viable?
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index d271ba20a0b2..c4ada5b12fc3 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1042,6 +1042,7 @@ struct kvm_vcpu_arch {
>  #if IS_ENABLED(CONFIG_HYPERV)
>         hpa_t hv_root_tdp;
>  #endif
> +       u8 compat_vendor;

Ooh, clever, much better than my idea of using multiple booleans.

One potential flaw though: the vCPU structure is zero-allocated, and so this will
get a false positive on X86_VENDOR_INTEL if userspace never sets guest CPUID.

That might actually be desirable though?  E.g. it's closer to KVM's current
behavior.  Maybe.  I dunno :-)

Anyways, KVM *does* need to be consistent, i.e. the default needs to yield the
same behavior as guest CPUID without entry 0x0 so that setting *other* CPUID
entries doesn't change from INTEL=>UNKNOWN.  More below.

>  };
> 
>  struct kvm_lpage_info {
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index adba49afb5fe..00170762e72a 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -376,6 +376,13 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
>         kvm_hv_set_cpuid(vcpu, kvm_cpuid_has_hyperv(vcpu->arch.cpuid_entries,
>                                                     vcpu->arch.cpuid_nent));
> 
> +       if (guest_cpuid_is_intel_compatible(vcpu))
> +               vcpu->arch.compat_vendor = X86_VENDOR_INTEL;
> +       else if (guest_cpuid_is_amd_or_hygon(vcpu))
> +               vcpu->arch.compat_vendor = X86_VENDOR_AMD;
> +       else
> +               vcpu->arch.compat_vendor = X86_VENDOR_UNKNOWN;

I would prefer to provide a helper for just getting the compat vendor.  E.g.

static u8 kvm_vcpu_get_compat_vendor(struct kvm_vcpu *vcpu)
{
	struct kvm_cpuid_entry2 *best;

	best = kvm_find_cpuid_entry(vcpu, 0);
	if (!best)
		return ???;

	if (is_guest_vendor_intel(best->ebx, best->ecx, best->edx) ||     
	    is_guest_vendor_centaur(best->ebx, best->ecx, best->edx) ||   
	    is_guest_vendor_zhaoxin(best->ebx, best->ecx, best->edx))
		return X86_VENDOR_INTEL;

	if (is_guest_vendor_amd(best->ebx, best->ecx, best->edx) ||
	    is_guest_vendor_hygon(best->ebx, best->ecx, best->edx))
		return X86_VENDOR_AMD;

	return X86_VENDOR_UNKNOWN;
}

and then a follow-up patch can remove guest_cpuid_is_amd_or_hygon() once all
users are converted to guest_vendor_is_compatible().