All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: nVMX: do not warn when MSR bitmap address is not backed
@ 2017-03-07 16:51 Radim Krčmář
  2017-03-07 17:12 ` Jim Mattson
  0 siblings, 1 reply; 3+ messages in thread
From: Radim Krčmář @ 2017-03-07 16:51 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Jim Mattson, Dmitry Vyukov, Paolo Bonzini

Before trying to do nested_get_page() in nested_vmx_merge_msr_bitmap(),
we have already checked that the MSR bitmap address is valid (4k aligned
and within physical limits).  SDM doesn't specify what happens if the
there is no memory mapped at the valid address, but Intel CPUs treat the
situation as if the bitmap was configured to trap all MSRs.

KVM already does that by returning false and a correct handling doesn't
need the guest-trigerrable warning that was reported by syzkaller:
(The warning was originally there to catch some possible bugs in nVMX.)

  ------------[ cut here ]------------
  WARNING: CPU: 0 PID: 7832 at arch/x86/kvm/vmx.c:9709
  nested_vmx_merge_msr_bitmap arch/x86/kvm/vmx.c:9709 [inline]
  WARNING: CPU: 0 PID: 7832 at arch/x86/kvm/vmx.c:9709
  nested_get_vmcs12_pages+0xfb6/0x15c0 arch/x86/kvm/vmx.c:9640
  Kernel panic - not syncing: panic_on_warn set ...
  CPU: 0 PID: 7832 Comm: syz-executor1 Not tainted 4.10.0+ #229
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
  Call Trace:
   __dump_stack lib/dump_stack.c:15 [inline]
   dump_stack+0x2ee/0x3ef lib/dump_stack.c:51
   panic+0x1fb/0x412 kernel/panic.c:179
   __warn+0x1c4/0x1e0 kernel/panic.c:540
   warn_slowpath_null+0x2c/0x40 kernel/panic.c:583
   nested_vmx_merge_msr_bitmap arch/x86/kvm/vmx.c:9709 [inline]
   nested_get_vmcs12_pages+0xfb6/0x15c0 arch/x86/kvm/vmx.c:9640
   enter_vmx_non_root_mode arch/x86/kvm/vmx.c:10471 [inline]
   nested_vmx_run+0x6186/0xaab0 arch/x86/kvm/vmx.c:10561
   handle_vmlaunch+0x1a/0x20 arch/x86/kvm/vmx.c:7312
   vmx_handle_exit+0xfc0/0x3f00 arch/x86/kvm/vmx.c:8526
   vcpu_enter_guest arch/x86/kvm/x86.c:6982 [inline]
   vcpu_run arch/x86/kvm/x86.c:7044 [inline]
   kvm_arch_vcpu_ioctl_run+0x1418/0x4840 arch/x86/kvm/x86.c:7205
   kvm_vcpu_ioctl+0x673/0x1120 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2570

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 arch/x86/kvm/vmx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ab338581b3ec..98e82ee1e699 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -9680,10 +9680,8 @@ static inline bool nested_vmx_merge_msr_bitmap(struct kvm_vcpu *vcpu,
 		return false;
 
 	page = nested_get_page(vcpu, vmcs12->msr_bitmap);
-	if (!page) {
-		WARN_ON(1);
+	if (!page)
 		return false;
-	}
 	msr_bitmap_l1 = (unsigned long *)kmap(page);
 
 	memset(msr_bitmap_l0, 0xff, PAGE_SIZE);
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: nVMX: do not warn when MSR bitmap address is not backed
  2017-03-07 16:51 [PATCH] KVM: nVMX: do not warn when MSR bitmap address is not backed Radim Krčmář
@ 2017-03-07 17:12 ` Jim Mattson
  2017-03-07 19:22   ` Radim Krčmář
  0 siblings, 1 reply; 3+ messages in thread
From: Jim Mattson @ 2017-03-07 17:12 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: LKML, kvm list, Dmitry Vyukov, Paolo Bonzini

I believe this behavior would be documented in the chipset data sheet
rather than the SDM, since the chipset returns all 1s for an unclaimed
read.

Reviewed-by: Jim Mattson <jmattson@google.com>

On Tue, Mar 7, 2017 at 8:51 AM, Radim Krčmář <rkrcmar@redhat.com> wrote:
> Before trying to do nested_get_page() in nested_vmx_merge_msr_bitmap(),
> we have already checked that the MSR bitmap address is valid (4k aligned
> and within physical limits).  SDM doesn't specify what happens if the
> there is no memory mapped at the valid address, but Intel CPUs treat the
> situation as if the bitmap was configured to trap all MSRs.
>
> KVM already does that by returning false and a correct handling doesn't
> need the guest-trigerrable warning that was reported by syzkaller:
> (The warning was originally there to catch some possible bugs in nVMX.)
>
>   ------------[ cut here ]------------
>   WARNING: CPU: 0 PID: 7832 at arch/x86/kvm/vmx.c:9709
>   nested_vmx_merge_msr_bitmap arch/x86/kvm/vmx.c:9709 [inline]
>   WARNING: CPU: 0 PID: 7832 at arch/x86/kvm/vmx.c:9709
>   nested_get_vmcs12_pages+0xfb6/0x15c0 arch/x86/kvm/vmx.c:9640
>   Kernel panic - not syncing: panic_on_warn set ...
>   CPU: 0 PID: 7832 Comm: syz-executor1 Not tainted 4.10.0+ #229
>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>   Call Trace:
>    __dump_stack lib/dump_stack.c:15 [inline]
>    dump_stack+0x2ee/0x3ef lib/dump_stack.c:51
>    panic+0x1fb/0x412 kernel/panic.c:179
>    __warn+0x1c4/0x1e0 kernel/panic.c:540
>    warn_slowpath_null+0x2c/0x40 kernel/panic.c:583
>    nested_vmx_merge_msr_bitmap arch/x86/kvm/vmx.c:9709 [inline]
>    nested_get_vmcs12_pages+0xfb6/0x15c0 arch/x86/kvm/vmx.c:9640
>    enter_vmx_non_root_mode arch/x86/kvm/vmx.c:10471 [inline]
>    nested_vmx_run+0x6186/0xaab0 arch/x86/kvm/vmx.c:10561
>    handle_vmlaunch+0x1a/0x20 arch/x86/kvm/vmx.c:7312
>    vmx_handle_exit+0xfc0/0x3f00 arch/x86/kvm/vmx.c:8526
>    vcpu_enter_guest arch/x86/kvm/x86.c:6982 [inline]
>    vcpu_run arch/x86/kvm/x86.c:7044 [inline]
>    kvm_arch_vcpu_ioctl_run+0x1418/0x4840 arch/x86/kvm/x86.c:7205
>    kvm_vcpu_ioctl+0x673/0x1120 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2570
>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  arch/x86/kvm/vmx.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ab338581b3ec..98e82ee1e699 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -9680,10 +9680,8 @@ static inline bool nested_vmx_merge_msr_bitmap(struct kvm_vcpu *vcpu,
>                 return false;
>
>         page = nested_get_page(vcpu, vmcs12->msr_bitmap);
> -       if (!page) {
> -               WARN_ON(1);
> +       if (!page)
>                 return false;
> -       }
>         msr_bitmap_l1 = (unsigned long *)kmap(page);
>
>         memset(msr_bitmap_l0, 0xff, PAGE_SIZE);
> --
> 2.12.0
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: nVMX: do not warn when MSR bitmap address is not backed
  2017-03-07 17:12 ` Jim Mattson
@ 2017-03-07 19:22   ` Radim Krčmář
  0 siblings, 0 replies; 3+ messages in thread
From: Radim Krčmář @ 2017-03-07 19:22 UTC (permalink / raw)
  To: Jim Mattson; +Cc: LKML, kvm list, Dmitry Vyukov, Paolo Bonzini

2017-03-07 09:12-0800, Jim Mattson:
> I believe this behavior would be documented in the chipset data sheet
> rather than the SDM, since the chipset returns all 1s for an unclaimed
> read.

Good point, thanks.  I'll add it as commit comment when applying.

I looked at some data sheets, but couldn't find where they say that :(
Emulating a (preferable userspace-configurable) fallback would need
deeper changes, though.

> Reviewed-by: Jim Mattson <jmattson@google.com>
> 
> On Tue, Mar 7, 2017 at 8:51 AM, Radim Krčmář <rkrcmar@redhat.com> wrote:
>> Before trying to do nested_get_page() in nested_vmx_merge_msr_bitmap(),
>> we have already checked that the MSR bitmap address is valid (4k aligned
>> and within physical limits).  SDM doesn't specify what happens if the
>> there is no memory mapped at the valid address, but Intel CPUs treat the
>> situation as if the bitmap was configured to trap all MSRs.
>>
>> KVM already does that by returning false and a correct handling doesn't
>> need the guest-trigerrable warning that was reported by syzkaller:
>> (The warning was originally there to catch some possible bugs in nVMX.)
>>
>>   ------------[ cut here ]------------
>>   WARNING: CPU: 0 PID: 7832 at arch/x86/kvm/vmx.c:9709
>>   nested_vmx_merge_msr_bitmap arch/x86/kvm/vmx.c:9709 [inline]
>>   WARNING: CPU: 0 PID: 7832 at arch/x86/kvm/vmx.c:9709
>>   nested_get_vmcs12_pages+0xfb6/0x15c0 arch/x86/kvm/vmx.c:9640
>>   Kernel panic - not syncing: panic_on_warn set ...
>>   CPU: 0 PID: 7832 Comm: syz-executor1 Not tainted 4.10.0+ #229
>>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>   Call Trace:
>>    __dump_stack lib/dump_stack.c:15 [inline]
>>    dump_stack+0x2ee/0x3ef lib/dump_stack.c:51
>>    panic+0x1fb/0x412 kernel/panic.c:179
>>    __warn+0x1c4/0x1e0 kernel/panic.c:540
>>    warn_slowpath_null+0x2c/0x40 kernel/panic.c:583
>>    nested_vmx_merge_msr_bitmap arch/x86/kvm/vmx.c:9709 [inline]
>>    nested_get_vmcs12_pages+0xfb6/0x15c0 arch/x86/kvm/vmx.c:9640
>>    enter_vmx_non_root_mode arch/x86/kvm/vmx.c:10471 [inline]
>>    nested_vmx_run+0x6186/0xaab0 arch/x86/kvm/vmx.c:10561
>>    handle_vmlaunch+0x1a/0x20 arch/x86/kvm/vmx.c:7312
>>    vmx_handle_exit+0xfc0/0x3f00 arch/x86/kvm/vmx.c:8526
>>    vcpu_enter_guest arch/x86/kvm/x86.c:6982 [inline]
>>    vcpu_run arch/x86/kvm/x86.c:7044 [inline]
>>    kvm_arch_vcpu_ioctl_run+0x1418/0x4840 arch/x86/kvm/x86.c:7205
>>    kvm_vcpu_ioctl+0x673/0x1120 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2570
>>
>> Reported-by: Dmitry Vyukov <dvyukov@google.com>
>> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
>> ---
>>  arch/x86/kvm/vmx.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index ab338581b3ec..98e82ee1e699 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -9680,10 +9680,8 @@ static inline bool nested_vmx_merge_msr_bitmap(struct kvm_vcpu *vcpu,
>>                 return false;
>>
>>         page = nested_get_page(vcpu, vmcs12->msr_bitmap);
>> -       if (!page) {
>> -               WARN_ON(1);
>> +       if (!page)
>>                 return false;
>> -       }
>>         msr_bitmap_l1 = (unsigned long *)kmap(page);
>>
>>         memset(msr_bitmap_l0, 0xff, PAGE_SIZE);
>> --
>> 2.12.0
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-07 20:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-07 16:51 [PATCH] KVM: nVMX: do not warn when MSR bitmap address is not backed Radim Krčmář
2017-03-07 17:12 ` Jim Mattson
2017-03-07 19:22   ` Radim Krčmář

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.