| Message ID | 20190926000418.115956-2-jmattson@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] kvm: x86: Improve emulation of CPUID leaves 0BH and 1FH | expand |
On 9/26/2019 8:04 AM, Jim Mattson wrote: > When the guest CPUID information represents an AMD vCPU, return all > zeroes for queries of undefined CPUID leaves, whether or not they are > in range. > > Signed-off-by: Jim Mattson <jmattson@google.com> > Fixes: bd22f5cfcfe8f6 ("KVM: move and fix substitue search for missing CPUID entries") > Reviewed-by: Marc Orr <marcorr@google.com> > Reviewed-by: Peter Shier <pshier@google.com> > Reviewed-by: Jacob Xu <jacobhxu@google.com> > Cc: Sean Christopherson <sean.j.christopherson@intel.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/x86/kvm/cpuid.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 35e2f930a4b79..0377d2820a7aa 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -988,9 +988,11 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, > /* > * Intel CPUID semantics treats any query for an out-of-range > * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were > - * requested. > + * requested. AMD CPUID semantics returns all zeroes for any > + * undefined leaf, whether or not the leaf is in range. > */ > - if (!entry && check_limit && !cpuid_function_in_range(vcpu, function)) { > + if (!entry && check_limit && !guest_cpuid_is_amd(vcpu) && > + !cpuid_function_in_range(vcpu, function)) { IIUC, the parameter check_limit is to indicate whether return highest basic leaf when out-of-range. Here you just makes check_limit meaningless. Maybe we can do like this to use check_limit reasonably： diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 0377d2820a7a..e6a61f3f6c0c 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -1035,7 +1035,8 @@ int kvm_emulate_cpuid(struct kvm_vcpu *vcpu) eax = kvm_rax_read(vcpu); ecx = kvm_rcx_read(vcpu); - kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx, true); + kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx, + guest_cpuid_is_amd(vcpu) ? false: true); kvm_rax_write(vcpu, eax); kvm_rbx_write(vcpu, ebx); kvm_rcx_write(vcpu, ecx); > max = kvm_find_cpuid_entry(vcpu, 0, 0); > if (max) { > function = max->eax; >
On 26/09/19 02:04, Jim Mattson wrote: > When the guest CPUID information represents an AMD vCPU, return all > zeroes for queries of undefined CPUID leaves, whether or not they are > in range. > > Signed-off-by: Jim Mattson <jmattson@google.com> > Fixes: bd22f5cfcfe8f6 ("KVM: move and fix substitue search for missing CPUID entries") > Reviewed-by: Marc Orr <marcorr@google.com> > Reviewed-by: Peter Shier <pshier@google.com> > Reviewed-by: Jacob Xu <jacobhxu@google.com> > Cc: Sean Christopherson <sean.j.christopherson@intel.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/x86/kvm/cpuid.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 35e2f930a4b79..0377d2820a7aa 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -988,9 +988,11 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, > /* > * Intel CPUID semantics treats any query for an out-of-range > * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were > - * requested. > + * requested. AMD CPUID semantics returns all zeroes for any > + * undefined leaf, whether or not the leaf is in range. > */ > - if (!entry && check_limit && !cpuid_function_in_range(vcpu, function)) { > + if (!entry && check_limit && !guest_cpuid_is_amd(vcpu) && > + !cpuid_function_in_range(vcpu, function)) { > max = kvm_find_cpuid_entry(vcpu, 0, 0); > if (max) { > function = max->eax; > Queued both, thanks. Paolo
On Wed, Sep 25, 2019 at 7:30 PM Xiaoyao Li <xiaoyao.li@intel.com> wrote: > > On 9/26/2019 8:04 AM, Jim Mattson wrote: > > When the guest CPUID information represents an AMD vCPU, return all > > zeroes for queries of undefined CPUID leaves, whether or not they are > > in range. > > > > Signed-off-by: Jim Mattson <jmattson@google.com> > > Fixes: bd22f5cfcfe8f6 ("KVM: move and fix substitue search for missing CPUID entries") > > Reviewed-by: Marc Orr <marcorr@google.com> > > Reviewed-by: Peter Shier <pshier@google.com> > > Reviewed-by: Jacob Xu <jacobhxu@google.com> > > Cc: Sean Christopherson <sean.j.christopherson@intel.com> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > --- > > arch/x86/kvm/cpuid.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > > index 35e2f930a4b79..0377d2820a7aa 100644 > > --- a/arch/x86/kvm/cpuid.c > > +++ b/arch/x86/kvm/cpuid.c > > @@ -988,9 +988,11 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, > > /* > > * Intel CPUID semantics treats any query for an out-of-range > > * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were > > - * requested. > > + * requested. AMD CPUID semantics returns all zeroes for any > > + * undefined leaf, whether or not the leaf is in range. > > */ > > - if (!entry && check_limit && !cpuid_function_in_range(vcpu, function)) { > > + if (!entry && check_limit && !guest_cpuid_is_amd(vcpu) && > > + !cpuid_function_in_range(vcpu, function)) { > > IIUC, the parameter check_limit is to indicate whether return highest > basic leaf when out-of-range. Here you just makes check_limit meaningless. That's right. For AMD CPUID semantics, there is no need for check_limit. > Maybe we can do like this to use check_limit reasonably： > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 0377d2820a7a..e6a61f3f6c0c 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -1035,7 +1035,8 @@ int kvm_emulate_cpuid(struct kvm_vcpu *vcpu) > > eax = kvm_rax_read(vcpu); > ecx = kvm_rcx_read(vcpu); > - kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx, true); > + kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx, > + guest_cpuid_is_amd(vcpu) ? false: true); > kvm_rax_write(vcpu, eax); > kvm_rbx_write(vcpu, ebx); > kvm_rcx_write(vcpu, ecx); > > > max = kvm_find_cpuid_entry(vcpu, 0, 0); > > if (max) { > > function = max->eax; Since over-limit CPUID queries should be rare, it seems unfortunate to pay the cost of guest_cpuid_is_amd() for every emulated CPUID instruction.
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 35e2f930a4b79..0377d2820a7aa 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -988,9 +988,11 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, /* * Intel CPUID semantics treats any query for an out-of-range * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were - * requested. + * requested. AMD CPUID semantics returns all zeroes for any + * undefined leaf, whether or not the leaf is in range. */ - if (!entry && check_limit && !cpuid_function_in_range(vcpu, function)) { + if (!entry && check_limit && !guest_cpuid_is_amd(vcpu) && + !cpuid_function_in_range(vcpu, function)) { max = kvm_find_cpuid_entry(vcpu, 0, 0); if (max) { function = max->eax;