All of lore.kernel.org
 help / color / mirror / Atom feed
From: kbuild test robot <lkp@intel.com>
To: kbuild-all@lists.01.org
Subject: Re: [PATCH] KVM: x86: avoid incorrect writes to host MSR_IA32_SPEC_CTRL
Date: Sat, 25 Jan 2020 09:32:10 +0800	[thread overview]
Message-ID: <202001250947.adtQpuNv%lkp@intel.com> (raw)
In-Reply-To: <1579614487-44583-3-git-send-email-pbonzini@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 9885 bytes --]

Hi Paolo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on tip/auto-latest]
[also build test WARNING on linux/master linus/master v5.5-rc7 next-20200121]
[cannot apply to kvm/linux-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Paolo-Bonzini/KVM-x86-avoid-incorrect-writes-to-host-MSR_IA32_SPEC_CTRL/20200124-083109
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 1e53251a964b1875f82a071c0b59d135dd0cc563
config: x86_64-randconfig-g003-20200125 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/export.h:43:0,
                    from include/linux/linkage.h:7,
                    from include/linux/preempt.h:10,
                    from include/linux/hardirq.h:5,
                    from include/linux/kvm_host.h:7,
                    from arch/x86//kvm/svm.c:17:
   arch/x86//kvm/svm.c: In function 'svm_set_msr':
   arch/x86//kvm/svm.c:4284:14: warning: '~' on a boolean expression [-Wbool-operation]
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
                 ^
   include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                       ^~~~
>> arch/x86//kvm/svm.c:4284:3: note: in expansion of macro 'if'
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
      ^~
   arch/x86//kvm/svm.c:4284:14: note: did you mean to use logical not?
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
                 ^
   include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                       ^~~~
>> arch/x86//kvm/svm.c:4284:3: note: in expansion of macro 'if'
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
      ^~
   arch/x86//kvm/svm.c:4284:14: warning: '~' on a boolean expression [-Wbool-operation]
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
                 ^
   include/linux/compiler.h:58:61: note: in definition of macro '__trace_if_var'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                                ^~~~
>> arch/x86//kvm/svm.c:4284:3: note: in expansion of macro 'if'
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
      ^~
   arch/x86//kvm/svm.c:4284:14: note: did you mean to use logical not?
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
                 ^
   include/linux/compiler.h:58:61: note: in definition of macro '__trace_if_var'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                                ^~~~
>> arch/x86//kvm/svm.c:4284:3: note: in expansion of macro 'if'
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
      ^~
   arch/x86//kvm/svm.c:4284:14: warning: '~' on a boolean expression [-Wbool-operation]
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
                 ^
   include/linux/compiler.h:69:3: note: in definition of macro '__trace_if_value'
     (cond) ?     \
      ^~~~
   include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
    #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                               ^~~~~~~~~~~~~~
>> arch/x86//kvm/svm.c:4284:3: note: in expansion of macro 'if'
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
      ^~
   arch/x86//kvm/svm.c:4284:14: note: did you mean to use logical not?
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
                 ^
   include/linux/compiler.h:69:3: note: in definition of macro '__trace_if_value'
     (cond) ?     \
      ^~~~
   include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
    #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                               ^~~~~~~~~~~~~~
>> arch/x86//kvm/svm.c:4284:3: note: in expansion of macro 'if'
      if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
      ^~

vim +/if +4284 arch/x86//kvm/svm.c

  4263	
  4264	static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
  4265	{
  4266		struct vcpu_svm *svm = to_svm(vcpu);
  4267	
  4268		u32 ecx = msr->index;
  4269		u64 data = msr->data;
  4270		switch (ecx) {
  4271		case MSR_IA32_CR_PAT:
  4272			if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data))
  4273				return 1;
  4274			vcpu->arch.pat = data;
  4275			svm->vmcb->save.g_pat = data;
  4276			mark_dirty(svm->vmcb, VMCB_NPT);
  4277			break;
  4278		case MSR_IA32_SPEC_CTRL:
  4279			if (!msr->host_initiated &&
  4280			    !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS) &&
  4281			    !guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))
  4282				return 1;
  4283	
> 4284			if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
  4285				return 1;
  4286	
  4287			svm->spec_ctrl = data;
  4288			if (!data)
  4289				break;
  4290	
  4291			/*
  4292			 * For non-nested:
  4293			 * When it's written (to non-zero) for the first time, pass
  4294			 * it through.
  4295			 *
  4296			 * For nested:
  4297			 * The handling of the MSR bitmap for L2 guests is done in
  4298			 * nested_svm_vmrun_msrpm.
  4299			 * We update the L1 MSR bit as well since it will end up
  4300			 * touching the MSR anyway now.
  4301			 */
  4302			set_msr_interception(svm->msrpm, MSR_IA32_SPEC_CTRL, 1, 1);
  4303			break;
  4304		case MSR_IA32_PRED_CMD:
  4305			if (!msr->host_initiated &&
  4306			    !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBPB))
  4307				return 1;
  4308	
  4309			if (data & ~PRED_CMD_IBPB)
  4310				return 1;
  4311			if (!boot_cpu_has(X86_FEATURE_AMD_IBPB))
  4312				return 1;
  4313			if (!data)
  4314				break;
  4315	
  4316			wrmsrl(MSR_IA32_PRED_CMD, PRED_CMD_IBPB);
  4317			set_msr_interception(svm->msrpm, MSR_IA32_PRED_CMD, 0, 1);
  4318			break;
  4319		case MSR_AMD64_VIRT_SPEC_CTRL:
  4320			if (!msr->host_initiated &&
  4321			    !guest_cpuid_has(vcpu, X86_FEATURE_VIRT_SSBD))
  4322				return 1;
  4323	
  4324			if (data & ~SPEC_CTRL_SSBD)
  4325				return 1;
  4326	
  4327			svm->virt_spec_ctrl = data;
  4328			break;
  4329		case MSR_STAR:
  4330			svm->vmcb->save.star = data;
  4331			break;
  4332	#ifdef CONFIG_X86_64
  4333		case MSR_LSTAR:
  4334			svm->vmcb->save.lstar = data;
  4335			break;
  4336		case MSR_CSTAR:
  4337			svm->vmcb->save.cstar = data;
  4338			break;
  4339		case MSR_KERNEL_GS_BASE:
  4340			svm->vmcb->save.kernel_gs_base = data;
  4341			break;
  4342		case MSR_SYSCALL_MASK:
  4343			svm->vmcb->save.sfmask = data;
  4344			break;
  4345	#endif
  4346		case MSR_IA32_SYSENTER_CS:
  4347			svm->vmcb->save.sysenter_cs = data;
  4348			break;
  4349		case MSR_IA32_SYSENTER_EIP:
  4350			svm->sysenter_eip = data;
  4351			svm->vmcb->save.sysenter_eip = data;
  4352			break;
  4353		case MSR_IA32_SYSENTER_ESP:
  4354			svm->sysenter_esp = data;
  4355			svm->vmcb->save.sysenter_esp = data;
  4356			break;
  4357		case MSR_TSC_AUX:
  4358			if (!boot_cpu_has(X86_FEATURE_RDTSCP))
  4359				return 1;
  4360	
  4361			/*
  4362			 * This is rare, so we update the MSR here instead of using
  4363			 * direct_access_msrs.  Doing that would require a rdmsr in
  4364			 * svm_vcpu_put.
  4365			 */
  4366			svm->tsc_aux = data;
  4367			wrmsrl(MSR_TSC_AUX, svm->tsc_aux);
  4368			break;
  4369		case MSR_IA32_DEBUGCTLMSR:
  4370			if (!boot_cpu_has(X86_FEATURE_LBRV)) {
  4371				vcpu_unimpl(vcpu, "%s: MSR_IA32_DEBUGCTL 0x%llx, nop\n",
  4372					    __func__, data);
  4373				break;
  4374			}
  4375			if (data & DEBUGCTL_RESERVED_BITS)
  4376				return 1;
  4377	
  4378			svm->vmcb->save.dbgctl = data;
  4379			mark_dirty(svm->vmcb, VMCB_LBR);
  4380			if (data & (1ULL<<0))
  4381				svm_enable_lbrv(svm);
  4382			else
  4383				svm_disable_lbrv(svm);
  4384			break;
  4385		case MSR_VM_HSAVE_PA:
  4386			svm->nested.hsave_msr = data;
  4387			break;
  4388		case MSR_VM_CR:
  4389			return svm_set_vm_cr(vcpu, data);
  4390		case MSR_VM_IGNNE:
  4391			vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", ecx, data);
  4392			break;
  4393		case MSR_F10H_DECFG: {
  4394			struct kvm_msr_entry msr_entry;
  4395	
  4396			msr_entry.index = msr->index;
  4397			if (svm_get_msr_feature(&msr_entry))
  4398				return 1;
  4399	
  4400			/* Check the supported bits */
  4401			if (data & ~msr_entry.data)
  4402				return 1;
  4403	
  4404			/* Don't allow the guest to change a bit, #GP */
  4405			if (!msr->host_initiated && (data ^ msr_entry.data))
  4406				return 1;
  4407	
  4408			svm->msr_decfg = data;
  4409			break;
  4410		}
  4411		case MSR_IA32_APICBASE:
  4412			if (kvm_vcpu_apicv_active(vcpu))
  4413				avic_update_vapic_bar(to_svm(vcpu), data);
  4414			/* Fall through */
  4415		default:
  4416			return kvm_set_msr_common(vcpu, msr);
  4417		}
  4418		return 0;
  4419	}
  4420	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32805 bytes --]

  parent reply	other threads:[~2020-01-25  1:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-21 13:48 [PATCH] KVM: x86: avoid incorrect writes to host MSR_IA32_SPEC_CTRL Paolo Bonzini
2020-01-24  8:00 ` Xiaoyao Li
2020-01-24  8:22   ` Paolo Bonzini
2020-01-25  1:32 ` kbuild test robot [this message]
2020-02-18  7:11 ` kbuild test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202001250947.adtQpuNv%lkp@intel.com \
    --to=lkp@intel.com \
    --cc=kbuild-all@lists.01.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.