Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753226AbdHQM3T (ORCPT ); Thu, 17 Aug 2017 08:29:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36548 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753069AbdHQM3Q (ORCPT ); Thu, 17 Aug 2017 08:29:16 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7BC5F7CBA4 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=pbonzini@redhat.com Subject: Re: [PATCH v2 1/5] KVM: x86: Add return value to kvm_cpuid(). To: Yu Zhang , kvm@vger.kernel.org Cc: linux-kernel@vger.kernel.org, rkrcmar@redhat.com, tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, xiaoguangrong@tencent.com, joro@8bytes.org References: <1502999558-2517-1-git-send-email-yu.c.zhang@linux.intel.com> <1502999558-2517-2-git-send-email-yu.c.zhang@linux.intel.com> From: Paolo Bonzini Message-ID: <9deed5e8-52e9-8634-611f-553840a43f1a@redhat.com> Date: Thu, 17 Aug 2017 14:29:09 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1502999558-2517-2-git-send-email-yu.c.zhang@linux.intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 17 Aug 2017 12:29:16 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5974 Lines: 172 On 17/08/2017 21:52, Yu Zhang wrote: > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h > index ac15193..3e759cf 100644 > --- a/arch/x86/kvm/cpuid.h > +++ b/arch/x86/kvm/cpuid.h > @@ -21,7 +21,14 @@ int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu, > int kvm_vcpu_ioctl_get_cpuid2(struct kvm_vcpu *vcpu, > struct kvm_cpuid2 *cpuid, > struct kvm_cpuid_entry2 __user *entries); > -void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, u32 *ecx, u32 *edx); > + > +enum { > + NO_CHECK_LIMIT = 0, > + CHECK_LIMIT = 1, > +}; emulate.c should not include cpuid.h. The argument can be simply a bool, though. > +bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx, > + u32 *ecx, u32 *edx, int check_limit); > > int cpuid_query_maxphyaddr(struct kvm_vcpu *vcpu); > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index fb00559..46daa37 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -28,6 +28,7 @@ > > #include "x86.h" > #include "tss.h" > +#include "cpuid.h" > > /* > * Operand types > @@ -2333,8 +2334,10 @@ static int emulator_has_longmode(struct x86_emulate_ctxt *ctxt) > > eax = 0x80000001; > ecx = 0; > - ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx); > - return edx & bit(X86_FEATURE_LM); > + if (ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx, NO_CHECK_LIMIT)) > + return edx & bit(X86_FEATURE_LM); > + else > + return 0; > } > > #define GET_SMSTATE(type, smbase, offset) \ > @@ -2636,7 +2639,7 @@ static bool vendor_intel(struct x86_emulate_ctxt *ctxt) > u32 eax, ebx, ecx, edx; > > eax = ecx = 0; > - ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx); > + ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx, NO_CHECK_LIMIT); > return ebx == X86EMUL_CPUID_VENDOR_GenuineIntel_ebx > && ecx == X86EMUL_CPUID_VENDOR_GenuineIntel_ecx > && edx == X86EMUL_CPUID_VENDOR_GenuineIntel_edx; > @@ -2656,7 +2659,7 @@ static bool em_syscall_is_enabled(struct x86_emulate_ctxt *ctxt) > > eax = 0x00000000; > ecx = 0x00000000; > - ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx); > + ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx, NO_CHECK_LIMIT); > /* > * Intel ("GenuineIntel") > * remark: Intel CPUs only support "syscall" in 64bit > @@ -3551,7 +3554,7 @@ static int em_movbe(struct x86_emulate_ctxt *ctxt) > /* > * Check MOVBE is set in the guest-visible CPUID leaf. > */ > - ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx); > + ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx, CHECK_LIMIT); This should be NO_CHECK_LIMIT. Otherwise okay! Paolo > if (!(ecx & FFL(MOVBE))) > return emulate_ud(ctxt); > > @@ -3865,7 +3868,7 @@ static int em_cpuid(struct x86_emulate_ctxt *ctxt) > > eax = reg_read(ctxt, VCPU_REGS_RAX); > ecx = reg_read(ctxt, VCPU_REGS_RCX); > - ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx); > + ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx, CHECK_LIMIT); > *reg_write(ctxt, VCPU_REGS_RAX) = eax; > *reg_write(ctxt, VCPU_REGS_RBX) = ebx; > *reg_write(ctxt, VCPU_REGS_RCX) = ecx; > @@ -3924,7 +3927,7 @@ static int check_fxsr(struct x86_emulate_ctxt *ctxt) > { > u32 eax = 1, ebx, ecx = 0, edx; > > - ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx); > + ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx, CHECK_LIMIT); > if (!(edx & FFL(FXSR))) > return emulate_ud(ctxt); > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 1fa9ee5..9def4a8 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -1580,7 +1580,7 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > } > init_vmcb(svm); > > - kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy); > + kvm_cpuid(vcpu, &eax, &dummy, &dummy, &dummy, CHECK_LIMIT); > kvm_register_write(vcpu, VCPU_REGS_RDX, eax); > > if (kvm_vcpu_apicv_active(vcpu) && !init_event) > diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h > index 0a6cc67..8a202c4 100644 > --- a/arch/x86/kvm/trace.h > +++ b/arch/x86/kvm/trace.h > @@ -151,8 +151,8 @@ TRACE_EVENT(kvm_fast_mmio, > */ > TRACE_EVENT(kvm_cpuid, > TP_PROTO(unsigned int function, unsigned long rax, unsigned long rbx, > - unsigned long rcx, unsigned long rdx), > - TP_ARGS(function, rax, rbx, rcx, rdx), > + unsigned long rcx, unsigned long rdx, bool found), > + TP_ARGS(function, rax, rbx, rcx, rdx, found), > > TP_STRUCT__entry( > __field( unsigned int, function ) > @@ -160,6 +160,7 @@ TRACE_EVENT(kvm_cpuid, > __field( unsigned long, rbx ) > __field( unsigned long, rcx ) > __field( unsigned long, rdx ) > + __field( bool, found ) > ), > > TP_fast_assign( > @@ -168,11 +169,13 @@ TRACE_EVENT(kvm_cpuid, > __entry->rbx = rbx; > __entry->rcx = rcx; > __entry->rdx = rdx; > + __entry->found = found; > ), > > - TP_printk("func %x rax %lx rbx %lx rcx %lx rdx %lx", > + TP_printk("func %x rax %lx rbx %lx rcx %lx rdx %lx, cpuid entry %s", > __entry->function, __entry->rax, > - __entry->rbx, __entry->rcx, __entry->rdx) > + __entry->rbx, __entry->rcx, __entry->rdx, > + __entry->found ? "found" : "not found") > ); > > #define AREG(x) { APIC_##x, "APIC_" #x } > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index e40a779..ee99fc1 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5213,10 +5213,10 @@ static int emulator_intercept(struct x86_emulate_ctxt *ctxt, > return kvm_x86_ops->check_intercept(emul_to_vcpu(ctxt), info, stage); > } > > -static void emulator_get_cpuid(struct x86_emulate_ctxt *ctxt, > - u32 *eax, u32 *ebx, u32 *ecx, u32 *edx) > +static bool emulator_get_cpuid(struct x86_emulate_ctxt *ctxt, > + u32 *eax, u32 *ebx, u32 *ecx, u32 *edx, int check_limit) > { > - kvm_cpuid(emul_to_vcpu(ctxt), eax, ebx, ecx, edx); > + return kvm_cpuid(emul_to_vcpu(ctxt), eax, ebx, ecx, edx, check_limit); > } > > static ulong emulator_read_gpr(struct x86_emulate_ctxt *ctxt, unsigned reg) >