linux-kernel - Re: [PATCH 25/26] KVM: x86: Handle main Intel PT CPUID leaf in vendor code

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: <20200130003840.GA24606@linux.intel.com>
Date:   Wed, 29 Jan 2020 16:38:40 -0800
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 25/26] KVM: x86: Handle main Intel PT CPUID leaf in
 vendor code

On Wed, Jan 29, 2020 at 03:46:39PM -0800, Sean Christopherson wrote:
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index d06fb54c9c0d..ca766c460318 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -409,7 +409,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
>  	unsigned f_gbpages = 0;
>  	unsigned f_lm = 0;
>  #endif
> -	unsigned f_intel_pt = kvm_x86_ops->pt_supported() ? F(INTEL_PT) : 0;
>  
>  	/* cpuid 1.edx */
>  	const u32 kvm_cpuid_1_edx_x86_features =
> @@ -648,22 +647,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
>  		break;
>  	}
>  	/* Intel PT */
> -	case 0x14: {
> -		int t, times = entry->eax;
> -
> -		if (!f_intel_pt) {
> -			entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
> -			break;
> -		}
> -
> -		for (t = 1; t <= times; ++t) {
> -			if (*nent >= maxnent)
> -				goto out;
> -			do_host_cpuid(&entry[t], function, t);
> -			++*nent;
> -		}
> +	case 0x14:
>  		break;
> -	}
>  	case KVM_CPUID_SIGNATURE: {
>  		static const char signature[12] = "KVMKVMKVM\0\0";
>  		const u32 *sigptr = (const u32 *)signature;
> @@ -778,6 +763,21 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
>  
>  	kvm_x86_ops->set_supported_cpuid(entry);
>  
> +	/*
> +	 * Add feature-dependent sub-leafs after ->set_supported_cpuid() to
> +	 * properly handle the feature being disabled by SVM/VMX.
> +	 */
> +	if (function == 0x14) {
> +		int t, times = entry->eax;
> +
> +		for (t = 1; t <= times; ++t) {
> +			if (*nent >= maxnent)
> +				goto out;
> +			do_host_cpuid(&entry[t], function, t);
> +			++*nent;
> +		}
> +	}
> +
>  	r = 0;

I belatedly thought of an alternative that I think I like better.  Instead
of adding the sub-leafs in common code, introduce a new kvm_x86_ops hook to
add vendor specific sub-leafs, e.g.:

        kvm_x86_ops->set_supported_cpuid(entry);

        r = kvm_x86_ops->add_cpuid_sub_leafs(entry, nent, maxent,
					     do_host_cpuid);

That gets Intel PT (and SGX if/when it gets merged) sub-leafs out of the
common x86 code without polluting ->set_supported_cpuid with the extra
params and return value.  The other hiccup is that SGX will want access to
cpuid_mask(), but I don't see an issue with moving that to cpuid.h.