[PATCH] KVM: x86: skip host CPUID call for hypervisor leaves

[PATCH] KVM: x86: skip host CPUID call for hypervisor leaves

[Paolo Bonzini]

Hypervisor leaves are always synthesized by __do_cpuid_func.  Just return
zeroes and do not ask the host, it would return a bogus value anyway if
it were used.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/cpuid.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 3902c28fb6cb..fd949e89120a 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -692,9 +692,17 @@ static struct kvm_cpuid_entry2 *do_host_cpuid(struct kvm_cpuid_array *array,
 
 	entry = &array->entries[array->nent++];
 
+	memset(entry, 0, sizeof(*entry));
 	entry->function = function;
 	entry->index = index;
-	entry->flags = 0;
+	switch (function & 0xC0000000) {
+	case 0x40000000:
+		/* Hypervisor leaves are always synthesized by __do_cpuid_func.  */
+		return entry;
+
+	default:
+		break;
+	}
 
 	cpuid_count(entry->function, entry->index,
 		    &entry->eax, &entry->ebx, &entry->ecx, &entry->edx);
-- 
2.31.1

Re: [PATCH] KVM: x86: skip host CPUID call for hypervisor leaves

[Vitaly Kuznetsov]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Hypervisor leaves are always synthesized by __do_cpuid_func.  Just return
> zeroes and do not ask the host, it would return a bogus value anyway if
> it were used.

Why always bogus? Nested virtualization is a thing, isn't it? :-) It
is, however, true that __do_cpuid_func() will throw the result away.

>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/cpuid.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 3902c28fb6cb..fd949e89120a 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -692,9 +692,17 @@ static struct kvm_cpuid_entry2 *do_host_cpuid(struct kvm_cpuid_array *array,
>  
>  	entry = &array->entries[array->nent++];
>  
> +	memset(entry, 0, sizeof(*entry));
>  	entry->function = function;
>  	entry->index = index;
> -	entry->flags = 0;
> +	switch (function & 0xC0000000) {
> +	case 0x40000000:
> +		/* Hypervisor leaves are always synthesized by __do_cpuid_func.  */
> +		return entry;

FWIW, 0x40000XXX leaves are not the only ones where we don't use
do_host_cpuid() result at all, e.g. I can see that we also return
constant values for 0x3, 0x5, 0x6, 0xC0000002 - 0xC0000004. 

Out of pure curiosity, what's the motivation for the patch? We seem to
only use __do_cpuid_func() to serve KVM_GET_SUPPORTED_CPUID/KVM_GET_EMULATED_CPUID,
not for kvm_emulate_cpuid() so these few CPUID calls we save here should
not give us any performace gain..

> +
> +	default:
> +		break;
> +	}
>  
>  	cpuid_count(entry->function, entry->index,
>  		    &entry->eax, &entry->ebx, &entry->ecx, &entry->edx);

The patch seems to be correct, so

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

-- 
Vitaly

Re: [PATCH] KVM: x86: skip host CPUID call for hypervisor leaves

[Paolo Bonzini]

On 1/21/22 12:08, Vitaly Kuznetsov wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> Hypervisor leaves are always synthesized by __do_cpuid_func.  Just return
>> zeroes and do not ask the host, it would return a bogus value anyway if
>> it were used.
> 
> Why always bogus? Nested virtualization is a thing, isn't it? :-) It
> is, however, true that __do_cpuid_func() will throw the result away.

Well, bogus because all hypercalls and MSRs would go through us so it 
makes little if any sense (given the current hypercall and MSR code) for 
the host values to be used in KVM_GET_SUPPORTED_CPUID.

> FWIW, 0x40000XXX leaves are not the only ones where we don't use
> do_host_cpuid() result at all, e.g. I can see that we also return
> constant values for 0x3, 0x5, 0x6, 0xC0000002 - 0xC0000004.
> 
> Out of pure curiosity, what's the motivation for the patch? We seem to
> only use __do_cpuid_func() to serve KVM_GET_SUPPORTED_CPUID/KVM_GET_EMULATED_CPUID,
> not for kvm_emulate_cpuid() so these few CPUID calls we save here should
> not give us any performace gain..

I just have it in queue because of another change that I have not 
submitted yet.

Paolo

>> +
>> +	default:
>> +		break;
>> +	}
>>   
>>   	cpuid_count(entry->function, entry->index,
>>   		    &entry->eax, &entry->ebx, &entry->ecx, &entry->edx);
> 
> The patch seems to be correct, so
> 
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>