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

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: <40d65efe-69dc-f1d6-b26c-a5cd243002a6@redhat.com>
Date:   Wed, 26 Jan 2022 18:23:28 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH] KVM: x86: skip host CPUID call for hypervisor leaves

On 1/21/22 12:08, Vitaly Kuznetsov wrote:
> Paolo Bonzini <pbonzini@...hat.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@...hat.com>
>