[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <jpgvazy94wa.fsf@linux.bootlegged.copy>
Date: Thu, 21 Jul 2016 15:18:13 -0400
From: Bandan Das <bsd@...hat.com>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: kvm@...r.kernel.org, rkrcmar@...hat.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] nvmx: use warn_on for buggy cases when emulating invept/invvpid
Paolo Bonzini <pbonzini@...hat.com> 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 <bsd@...hat.com>
>> ---
>> 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
Powered by blists - more mailing lists