* [PATCH] i386: Don't try to set MSR_KVM_ASYNC_PF_EN if kernel-irqchip=off
@ 2020-09-22 15:14 Eduardo Habkost
2020-09-22 15:38 ` Vitaly Kuznetsov
0 siblings, 1 reply; 7+ messages in thread
From: Eduardo Habkost @ 2020-09-22 15:14 UTC (permalink / raw)
To: qemu-devel
Cc: Dr. David Alan Gilbert, Eduardo Habkost, 1896263, kvm,
Paolo Bonzini, Richard Henderson, Marcelo Tosatti,
Vitaly Kuznetsov, Laurent Vivier, Stefan Hajnoczi
This addresses the following crash when running Linux v5.8
with kernel-irqchip=off:
qemu-system-x86_64: error: failed to set MSR 0x4b564d02 to 0x0
qemu-system-x86_64: ../target/i386/kvm.c:2714: kvm_buf_set_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
There is a kernel-side fix for the issue too (kernel commit
d831de177217 "KVM: x86: always allow writing '0' to
MSR_KVM_ASYNC_PF_EN"), but it's nice to simply not trigger
the bug if running an older kernel.
Fixes: https://bugs.launchpad.net/bugs/1896263
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target/i386/kvm.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 9efb07e7c83..1492f41349f 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2818,7 +2818,12 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
kvm_msr_entry_add(cpu, MSR_IA32_TSC, env->tsc);
kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
- if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
+ /*
+ * Some kernel versions (v5.8) won't let MSR_KVM_ASYNC_PF_EN to be set
+ * at all if kernel-irqchip=off, so don't try to set it in that case.
+ */
+ if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF) &&
+ kvm_irqchip_in_kernel()) {
kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_en_msr);
}
if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] i386: Don't try to set MSR_KVM_ASYNC_PF_EN if kernel-irqchip=off
2020-09-22 15:14 [PATCH] i386: Don't try to set MSR_KVM_ASYNC_PF_EN if kernel-irqchip=off Eduardo Habkost
@ 2020-09-22 15:38 ` Vitaly Kuznetsov
2020-09-22 16:10 ` Eduardo Habkost
0 siblings, 1 reply; 7+ messages in thread
From: Vitaly Kuznetsov @ 2020-09-22 15:38 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: Dr. David Alan Gilbert, Eduardo Habkost, 1896263, kvm,
Paolo Bonzini, Richard Henderson, Marcelo Tosatti, Laurent Vivier,
Stefan Hajnoczi
Eduardo Habkost <ehabkost@redhat.com> writes:
> This addresses the following crash when running Linux v5.8
> with kernel-irqchip=off:
>
> qemu-system-x86_64: error: failed to set MSR 0x4b564d02 to 0x0
> qemu-system-x86_64: ../target/i386/kvm.c:2714: kvm_buf_set_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
>
> There is a kernel-side fix for the issue too (kernel commit
> d831de177217 "KVM: x86: always allow writing '0' to
> MSR_KVM_ASYNC_PF_EN"), but it's nice to simply not trigger
> the bug if running an older kernel.
>
> Fixes: https://bugs.launchpad.net/bugs/1896263
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target/i386/kvm.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 9efb07e7c83..1492f41349f 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -2818,7 +2818,12 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> kvm_msr_entry_add(cpu, MSR_IA32_TSC, env->tsc);
> kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
> kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
> - if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
> + /*
> + * Some kernel versions (v5.8) won't let MSR_KVM_ASYNC_PF_EN to be set
> + * at all if kernel-irqchip=off, so don't try to set it in that case.
> + */
> + if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF) &&
> + kvm_irqchip_in_kernel()) {
> kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_en_msr);
> }
> if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
I'm not sure kvm_irqchip_in_kernel() was required before we switched to
interrupt-based APF (as we were always injecting #PF) but with
kernel-5.8+ this should work. We'll need to merge this with
https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg02963.html
(queued by Paolo) and
https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg06196.html
which fixes a bug in it.
as kvm_irqchip_in_kernel() should go around both KVM_FEATURE_ASYNC_PF
and KVM_FEATURE_ASYNC_PF_INT I believe.
--
Vitaly
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] i386: Don't try to set MSR_KVM_ASYNC_PF_EN if kernel-irqchip=off
2020-09-22 15:38 ` Vitaly Kuznetsov
@ 2020-09-22 16:10 ` Eduardo Habkost
2020-09-22 16:42 ` Vitaly Kuznetsov
0 siblings, 1 reply; 7+ messages in thread
From: Eduardo Habkost @ 2020-09-22 16:10 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: qemu-devel, Dr. David Alan Gilbert, 1896263, kvm, Paolo Bonzini,
Richard Henderson, Marcelo Tosatti, Laurent Vivier,
Stefan Hajnoczi
On Tue, Sep 22, 2020 at 05:38:12PM +0200, Vitaly Kuznetsov wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
>
> > This addresses the following crash when running Linux v5.8
> > with kernel-irqchip=off:
> >
> > qemu-system-x86_64: error: failed to set MSR 0x4b564d02 to 0x0
> > qemu-system-x86_64: ../target/i386/kvm.c:2714: kvm_buf_set_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
> >
> > There is a kernel-side fix for the issue too (kernel commit
> > d831de177217 "KVM: x86: always allow writing '0' to
> > MSR_KVM_ASYNC_PF_EN"), but it's nice to simply not trigger
> > the bug if running an older kernel.
> >
> > Fixes: https://bugs.launchpad.net/bugs/1896263
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > target/i386/kvm.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > index 9efb07e7c83..1492f41349f 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -2818,7 +2818,12 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> > kvm_msr_entry_add(cpu, MSR_IA32_TSC, env->tsc);
> > kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
> > kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
> > - if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
> > + /*
> > + * Some kernel versions (v5.8) won't let MSR_KVM_ASYNC_PF_EN to be set
> > + * at all if kernel-irqchip=off, so don't try to set it in that case.
> > + */
> > + if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF) &&
> > + kvm_irqchip_in_kernel()) {
> > kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_en_msr);
> > }
> > if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
>
> I'm not sure kvm_irqchip_in_kernel() was required before we switched to
> interrupt-based APF (as we were always injecting #PF) but with
> kernel-5.8+ this should work. [...]
Were guests able to set MSR_KVM_ASYNC_PF_EN to non-zero with
kernel-irqchip=off on hosts running Linux <= 5.7? I am assuming
kvm-asyncpf never worked with kernel-irqchip=off (and enabling it
by default with kernel-irqchip=off was a mistake).
> [...] We'll need to merge this with
>
> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg02963.html
> (queued by Paolo) and
> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg06196.html
> which fixes a bug in it.
>
> as kvm_irqchip_in_kernel() should go around both KVM_FEATURE_ASYNC_PF
> and KVM_FEATURE_ASYNC_PF_INT I believe.
Shouldn't we just disallow kvm-asyncpf-int=on if kernel-irqchip=off?
--
Eduardo
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] i386: Don't try to set MSR_KVM_ASYNC_PF_EN if kernel-irqchip=off
2020-09-22 16:10 ` Eduardo Habkost
@ 2020-09-22 16:42 ` Vitaly Kuznetsov
2020-09-22 17:22 ` Eduardo Habkost
0 siblings, 1 reply; 7+ messages in thread
From: Vitaly Kuznetsov @ 2020-09-22 16:42 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, Dr. David Alan Gilbert, 1896263, kvm, Paolo Bonzini,
Richard Henderson, Marcelo Tosatti, Laurent Vivier,
Stefan Hajnoczi
Eduardo Habkost <ehabkost@redhat.com> writes:
> On Tue, Sep 22, 2020 at 05:38:12PM +0200, Vitaly Kuznetsov wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>>
>> > This addresses the following crash when running Linux v5.8
>> > with kernel-irqchip=off:
>> >
>> > qemu-system-x86_64: error: failed to set MSR 0x4b564d02 to 0x0
>> > qemu-system-x86_64: ../target/i386/kvm.c:2714: kvm_buf_set_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
>> >
>> > There is a kernel-side fix for the issue too (kernel commit
>> > d831de177217 "KVM: x86: always allow writing '0' to
>> > MSR_KVM_ASYNC_PF_EN"), but it's nice to simply not trigger
>> > the bug if running an older kernel.
>> >
>> > Fixes: https://bugs.launchpad.net/bugs/1896263
>> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> > ---
>> > target/i386/kvm.c | 7 ++++++-
>> > 1 file changed, 6 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> > index 9efb07e7c83..1492f41349f 100644
>> > --- a/target/i386/kvm.c
>> > +++ b/target/i386/kvm.c
>> > @@ -2818,7 +2818,12 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>> > kvm_msr_entry_add(cpu, MSR_IA32_TSC, env->tsc);
>> > kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
>> > kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
>> > - if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
>> > + /*
>> > + * Some kernel versions (v5.8) won't let MSR_KVM_ASYNC_PF_EN to be set
>> > + * at all if kernel-irqchip=off, so don't try to set it in that case.
>> > + */
>> > + if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF) &&
>> > + kvm_irqchip_in_kernel()) {
>> > kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_en_msr);
>> > }
>> > if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
>>
>> I'm not sure kvm_irqchip_in_kernel() was required before we switched to
>> interrupt-based APF (as we were always injecting #PF) but with
>> kernel-5.8+ this should work. [...]
>
> Were guests able to set MSR_KVM_ASYNC_PF_EN to non-zero with
> kernel-irqchip=off on hosts running Linux <= 5.7?
lapic_in_kernel() check appeared in kernel with the following commit:
commit 9d3c447c72fb2337ca39f245c6ae89f2369de216
Author: Wanpeng Li <wanpengli@tencent.com>
Date: Mon Jun 29 18:26:31 2020 +0800
KVM: X86: Fix async pf caused null-ptr-deref
which was post-interrupt-based-APF. I *think* it was OK to enable APF
with !lapic_in_kernel() before (at least I don't see what would not
allow that).
> I am assuming
> kvm-asyncpf never worked with kernel-irqchip=off (and enabling it
> by default with kernel-irqchip=off was a mistake).
>
>
>> [...] We'll need to merge this with
>>
>> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg02963.html
>> (queued by Paolo) and
>> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg06196.html
>> which fixes a bug in it.
>>
>> as kvm_irqchip_in_kernel() should go around both KVM_FEATURE_ASYNC_PF
>> and KVM_FEATURE_ASYNC_PF_INT I believe.
>
> Shouldn't we just disallow kvm-asyncpf-int=on if kernel-irqchip=off?
(Sarcasm: if disallowing 'kernel-irqchip=off' is not an option, then)
yes, we probably can, but kvm-asyncpf-int=on is the default we have so
we can't just implicitly change it underneath or migration will break...
--
Vitaly
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] i386: Don't try to set MSR_KVM_ASYNC_PF_EN if kernel-irqchip=off
2020-09-22 16:42 ` Vitaly Kuznetsov
@ 2020-09-22 17:22 ` Eduardo Habkost
2020-09-22 17:26 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: Eduardo Habkost @ 2020-09-22 17:22 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: qemu-devel, Dr. David Alan Gilbert, 1896263, kvm, Paolo Bonzini,
Richard Henderson, Marcelo Tosatti, Laurent Vivier,
Stefan Hajnoczi
On Tue, Sep 22, 2020 at 06:42:17PM +0200, Vitaly Kuznetsov wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
>
> > On Tue, Sep 22, 2020 at 05:38:12PM +0200, Vitaly Kuznetsov wrote:
> >> Eduardo Habkost <ehabkost@redhat.com> writes:
> >>
> >> > This addresses the following crash when running Linux v5.8
> >> > with kernel-irqchip=off:
> >> >
> >> > qemu-system-x86_64: error: failed to set MSR 0x4b564d02 to 0x0
> >> > qemu-system-x86_64: ../target/i386/kvm.c:2714: kvm_buf_set_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
> >> >
> >> > There is a kernel-side fix for the issue too (kernel commit
> >> > d831de177217 "KVM: x86: always allow writing '0' to
> >> > MSR_KVM_ASYNC_PF_EN"), but it's nice to simply not trigger
> >> > the bug if running an older kernel.
> >> >
> >> > Fixes: https://bugs.launchpad.net/bugs/1896263
> >> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >> > ---
> >> > target/i386/kvm.c | 7 ++++++-
> >> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> >> > index 9efb07e7c83..1492f41349f 100644
> >> > --- a/target/i386/kvm.c
> >> > +++ b/target/i386/kvm.c
> >> > @@ -2818,7 +2818,12 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> >> > kvm_msr_entry_add(cpu, MSR_IA32_TSC, env->tsc);
> >> > kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
> >> > kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
> >> > - if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF)) {
> >> > + /*
> >> > + * Some kernel versions (v5.8) won't let MSR_KVM_ASYNC_PF_EN to be set
> >> > + * at all if kernel-irqchip=off, so don't try to set it in that case.
> >> > + */
> >> > + if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_ASYNC_PF) &&
> >> > + kvm_irqchip_in_kernel()) {
> >> > kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_en_msr);
> >> > }
> >> > if (env->features[FEAT_KVM] & (1 << KVM_FEATURE_PV_EOI)) {
> >>
> >> I'm not sure kvm_irqchip_in_kernel() was required before we switched to
> >> interrupt-based APF (as we were always injecting #PF) but with
> >> kernel-5.8+ this should work. [...]
> >
> > Were guests able to set MSR_KVM_ASYNC_PF_EN to non-zero with
> > kernel-irqchip=off on hosts running Linux <= 5.7?
>
> lapic_in_kernel() check appeared in kernel with the following commit:
>
> commit 9d3c447c72fb2337ca39f245c6ae89f2369de216
> Author: Wanpeng Li <wanpengli@tencent.com>
> Date: Mon Jun 29 18:26:31 2020 +0800
>
> KVM: X86: Fix async pf caused null-ptr-deref
>
> which was post-interrupt-based-APF. I *think* it was OK to enable APF
> with !lapic_in_kernel() before (at least I don't see what would not
> allow that).
If it was possible, did KVM break live migration of
kernel-irqchip=off guests that enabled APF? This would mean my
patch is replacing a crash with a silent migration bug. Not nice
either way.
>
> > I am assuming
> > kvm-asyncpf never worked with kernel-irqchip=off (and enabling it
> > by default with kernel-irqchip=off was a mistake).
> >
> >
> >> [...] We'll need to merge this with
> >>
> >> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg02963.html
> >> (queued by Paolo) and
> >> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg06196.html
> >> which fixes a bug in it.
> >>
> >> as kvm_irqchip_in_kernel() should go around both KVM_FEATURE_ASYNC_PF
> >> and KVM_FEATURE_ASYNC_PF_INT I believe.
> >
> > Shouldn't we just disallow kvm-asyncpf-int=on if kernel-irqchip=off?
>
> (Sarcasm: if disallowing 'kernel-irqchip=off' is not an option, then)
I'm working on it. :-)
> yes, we probably can, but kvm-asyncpf-int=on is the default we have so
> we can't just implicitly change it underneath or migration will break...
kvm-asyncpf-int wasn't merged yet, was it? This means we don't
have compatibility issues to care about.
--
Eduardo
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] i386: Don't try to set MSR_KVM_ASYNC_PF_EN if kernel-irqchip=off
2020-09-22 17:22 ` Eduardo Habkost
@ 2020-09-22 17:26 ` Paolo Bonzini
2020-09-22 19:39 ` Eduardo Habkost
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2020-09-22 17:26 UTC (permalink / raw)
To: Eduardo Habkost, Vitaly Kuznetsov
Cc: qemu-devel, Dr. David Alan Gilbert, 1896263, kvm,
Richard Henderson, Marcelo Tosatti, Laurent Vivier,
Stefan Hajnoczi
On 22/09/20 19:22, Eduardo Habkost wrote:
> If it was possible, did KVM break live migration of
> kernel-irqchip=off guests that enabled APF? This would mean my
> patch is replacing a crash with a silent migration bug. Not nice
> either way.
Let's drop kernel-irqchip=off completely so migration is not broken. :)
I'm actually serious, I don't think we need a deprecation period even.
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] i386: Don't try to set MSR_KVM_ASYNC_PF_EN if kernel-irqchip=off
2020-09-22 17:26 ` Paolo Bonzini
@ 2020-09-22 19:39 ` Eduardo Habkost
0 siblings, 0 replies; 7+ messages in thread
From: Eduardo Habkost @ 2020-09-22 19:39 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Vitaly Kuznetsov, qemu-devel, Dr. David Alan Gilbert, 1896263,
kvm, Richard Henderson, Marcelo Tosatti, Laurent Vivier,
Stefan Hajnoczi
On Tue, Sep 22, 2020 at 07:26:42PM +0200, Paolo Bonzini wrote:
> On 22/09/20 19:22, Eduardo Habkost wrote:
> > If it was possible, did KVM break live migration of
> > kernel-irqchip=off guests that enabled APF? This would mean my
> > patch is replacing a crash with a silent migration bug. Not nice
> > either way.
>
> Let's drop kernel-irqchip=off completely so migration is not broken. :)
>
> I'm actually serious, I don't think we need a deprecation period even.
I wasn't sure about this, but then I've noticed the man page says
"disabling the in-kernel irqchip completely is not recommended
except for debugging purposes."
Does this note apply to all architectures?
--
Eduardo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-09-22 19:39 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-22 15:14 [PATCH] i386: Don't try to set MSR_KVM_ASYNC_PF_EN if kernel-irqchip=off Eduardo Habkost
2020-09-22 15:38 ` Vitaly Kuznetsov
2020-09-22 16:10 ` Eduardo Habkost
2020-09-22 16:42 ` Vitaly Kuznetsov
2020-09-22 17:22 ` Eduardo Habkost
2020-09-22 17:26 ` Paolo Bonzini
2020-09-22 19:39 ` Eduardo Habkost
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox