From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bandan Das Subject: Re: [PATCH 1/4] nvmx: use warn_on for buggy cases when emulating invept/invvpid Date: Thu, 21 Jul 2016 15:18:13 -0400 Message-ID: References: <1469053536-11130-1-git-send-email-bsd@redhat.com> <1469053536-11130-2-git-send-email-bsd@redhat.com> Mime-Version: 1.0 Content-Type: text/plain Cc: kvm@vger.kernel.org, rkrcmar@redhat.com, linux-kernel@vger.kernel.org To: Paolo Bonzini Return-path: In-Reply-To: (Paolo Bonzini's message of "Thu, 21 Jul 2016 11:13:37 +0200") Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org Paolo Bonzini writes: > On 21/07/2016 00:25, Bandan Das wrote: >> If L1 hypervisor decides to try out something weird, alert the >> host but only less aggressively. Also, remove the comment >> regarding nested vpid support since it is no longer valid. >> >> Signed-off-by: Bandan Das >> --- >> arch/x86/kvm/vmx.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 64a79f2..9fd0681 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -2854,7 +2854,6 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) >> vmx->nested.nested_vmx_secondary_ctls_high); >> break; >> case MSR_IA32_VMX_EPT_VPID_CAP: >> - /* Currently, no nested vpid support */ > > This is okay. > >> *pdata = vmx->nested.nested_vmx_ept_caps | >> ((u64)vmx->nested.nested_vmx_vpid_caps << 32); >> break; >> @@ -7462,7 +7461,7 @@ static int handle_invept(struct kvm_vcpu *vcpu) >> break; >> default: >> /* Trap single context invalidation invept calls */ >> - BUG_ON(1); >> + WARN_ON(1); >> break; >> } >> >> @@ -7525,7 +7524,7 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) >> break; >> default: >> /* Trap individual address invalidation invvpid calls */ >> - BUG_ON(1); >> + WARN_ON(1); >> break; >> } >> >> > > These are BUGs because they are checked above: > > if (!(types & (1UL << type))) { > nested_vmx_failValid(vcpu, > VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID); > skip_emulated_instruction(vcpu); > return 1; > } Ah ok, this should be sufficient I think. > Guest-triggerable WARNs are only just a little better than > guest-triggerable BUGs. Guest-triggerable messages should be Yeah, a trace isn't really necessary. We know where it's from. BUG() can also leave the module in an unclean state and prevent it from getting unloaded which I why I think it shouldn't be on any path that can be guest triggered. > rate-limited printk. > > I don't object to the change, but the commit message should be > modified (or the change dropped). I will drop it and modify the commit message accordingly. > Paolo