Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1447511 > unrolled thread
| Started by | Bandan Das <bsd@redhat.com> |
|---|---|
| First post | 2016-07-21 00:30 +0200 |
| Last post | 2016-07-21 21:20 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 1/4] nvmx: use warn_on for buggy cases when emulating invept/invvpid Bandan Das <bsd@redhat.com> - 2016-07-21 00:30 +0200
Re: [PATCH 1/4] nvmx: use warn_on for buggy cases when emulating invept/invvpid Paolo Bonzini <pbonzini@redhat.com> - 2016-07-21 11:20 +0200
Re: [PATCH 1/4] nvmx: use warn_on for buggy cases when emulating invept/invvpid Bandan Das <bsd@redhat.com> - 2016-07-21 21:20 +0200
| From | Bandan Das <bsd@redhat.com> |
|---|---|
| Date | 2016-07-21 00:30 +0200 |
| Subject | [PATCH 1/4] nvmx: use warn_on for buggy cases when emulating invept/invvpid |
| Message-ID | <rX8a6-7mz-7@gated-at.bofh.it> |
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@redhat.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 */ *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; } -- 2.5.5
[toc] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2016-07-21 11:20 +0200 |
| Subject | Re: [PATCH 1/4] nvmx: use warn_on for buggy cases when emulating invept/invvpid |
| Message-ID | <rXij8-5uu-33@gated-at.bofh.it> |
| In reply to | #1447511 |
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@redhat.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;
}
Guest-triggerable WARNs are only just a little better than
guest-triggerable BUGs. Guest-triggerable messages should be
rate-limited printk.
I don't object to the change, but the commit message should be
modified (or the change dropped).
Paolo
[toc] | [prev] | [next] | [standalone]
| From | Bandan Das <bsd@redhat.com> |
|---|---|
| Date | 2016-07-21 21:20 +0200 |
| Message-ID | <rXrFL-3jF-7@gated-at.bofh.it> |
| In reply to | #1447733 |
Paolo Bonzini <pbonzini@redhat.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@redhat.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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web