kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] x86/kvm: fix interrupts based APF mechanism
@ 2020-09-08 13:53 Vitaly Kuznetsov
  2020-09-08 13:53 ` [PATCH 1/2] x86/kvm: properly use DEFINE_IDTENTRY_SYSVEC() macro Vitaly Kuznetsov
  2020-09-08 13:53 ` [PATCH 2/2] x86/kvm: don't forget to ACK async PF IRQ Vitaly Kuznetsov
  0 siblings, 2 replies; 6+ messages in thread
From: Vitaly Kuznetsov @ 2020-09-08 13:53 UTC (permalink / raw)
  To: kvm, x86
  Cc: Paolo Bonzini, Sean Christopherson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski

Linux-5.9 switched KVM guests to interrupt based APF mechanism for 'page
ready' events (instead of the previously used '#PF' exception) but a
collision with the newly introduced IDTENTRY magic happened and it wasn't
properly resolved. QEMU doesn't currently enable KVM_FEATURE_ASYNC_PF_INT
bit so the breakage is invisible but all KVM guests will hang as soon as
the bit will get exposed.

Vitaly Kuznetsov (2):
  x86/kvm: properly use DEFINE_IDTENTRY_SYSVEC() macro
  x86/kvm: don't forget to ACK async PF IRQ

 arch/x86/kernel/kvm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

-- 
2.25.4


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/2] x86/kvm: properly use DEFINE_IDTENTRY_SYSVEC() macro
  2020-09-08 13:53 [PATCH 0/2] x86/kvm: fix interrupts based APF mechanism Vitaly Kuznetsov
@ 2020-09-08 13:53 ` Vitaly Kuznetsov
  2020-09-08 13:53 ` [PATCH 2/2] x86/kvm: don't forget to ACK async PF IRQ Vitaly Kuznetsov
  1 sibling, 0 replies; 6+ messages in thread
From: Vitaly Kuznetsov @ 2020-09-08 13:53 UTC (permalink / raw)
  To: kvm, x86
  Cc: Paolo Bonzini, Sean Christopherson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski

DEFINE_IDTENTRY_SYSVEC() already contains irqentry_enter()/
irqentry_exit().

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kernel/kvm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 08320b0b2b27..d45f34cbe1ef 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -270,9 +270,6 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_kvm_asyncpf_interrupt)
 {
 	struct pt_regs *old_regs = set_irq_regs(regs);
 	u32 token;
-	irqentry_state_t state;
-
-	state = irqentry_enter(regs);
 
 	inc_irq_stat(irq_hv_callback_count);
 
@@ -283,7 +280,6 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_kvm_asyncpf_interrupt)
 		wrmsrl(MSR_KVM_ASYNC_PF_ACK, 1);
 	}
 
-	irqentry_exit(regs, state);
 	set_irq_regs(old_regs);
 }
 
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] x86/kvm: don't forget to ACK async PF IRQ
  2020-09-08 13:53 [PATCH 0/2] x86/kvm: fix interrupts based APF mechanism Vitaly Kuznetsov
  2020-09-08 13:53 ` [PATCH 1/2] x86/kvm: properly use DEFINE_IDTENTRY_SYSVEC() macro Vitaly Kuznetsov
@ 2020-09-08 13:53 ` Vitaly Kuznetsov
  2020-09-09  8:16   ` Ingo Molnar
  1 sibling, 1 reply; 6+ messages in thread
From: Vitaly Kuznetsov @ 2020-09-08 13:53 UTC (permalink / raw)
  To: kvm, x86
  Cc: Paolo Bonzini, Sean Christopherson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski

Merge commit 26d05b368a5c0 ("Merge branch 'kvm-async-pf-int' into HEAD")
tried to adapt the new interrupt based async PF mechanism to the newly
introduced IDTENTRY magic but unfortunately it missed the fact that
DEFINE_IDTENTRY_SYSVEC() doesn't call ack_APIC_irq() on its own and
all DEFINE_IDTENTRY_SYSVEC() users have to call it manually.

As the result all multi-CPU KVM guest hang on boot when
KVM_FEATURE_ASYNC_PF_INT is present. The breakage went unnoticed because no
KVM userspace (e.g. QEMU) currently set it (and thus async PF mechanism
is currently disabled) but we're about to change that.

Fixes: 26d05b368a5c0 ("Merge branch 'kvm-async-pf-int' into HEAD")
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kernel/kvm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index d45f34cbe1ef..9663ba31347c 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -271,6 +271,8 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_kvm_asyncpf_interrupt)
 	struct pt_regs *old_regs = set_irq_regs(regs);
 	u32 token;
 
+	ack_APIC_irq();
+
 	inc_irq_stat(irq_hv_callback_count);
 
 	if (__this_cpu_read(apf_reason.enabled)) {
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] x86/kvm: don't forget to ACK async PF IRQ
  2020-09-08 13:53 ` [PATCH 2/2] x86/kvm: don't forget to ACK async PF IRQ Vitaly Kuznetsov
@ 2020-09-09  8:16   ` Ingo Molnar
  2020-09-09  8:49     ` Vitaly Kuznetsov
  0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2020-09-09  8:16 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: kvm, x86, Paolo Bonzini, Sean Christopherson, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Andy Lutomirski


* Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> Merge commit 26d05b368a5c0 ("Merge branch 'kvm-async-pf-int' into HEAD")
> tried to adapt the new interrupt based async PF mechanism to the newly
> introduced IDTENTRY magic but unfortunately it missed the fact that
> DEFINE_IDTENTRY_SYSVEC() doesn't call ack_APIC_irq() on its own and
> all DEFINE_IDTENTRY_SYSVEC() users have to call it manually.
> 
> As the result all multi-CPU KVM guest hang on boot when
> KVM_FEATURE_ASYNC_PF_INT is present. The breakage went unnoticed because no
> KVM userspace (e.g. QEMU) currently set it (and thus async PF mechanism
> is currently disabled) but we're about to change that.
> 
> Fixes: 26d05b368a5c0 ("Merge branch 'kvm-async-pf-int' into HEAD")
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

This also fixes a kvmtool regression, but interestingly it does not set 
KVM_FEATURE_ASYNC_PF_INT either AFAICS:

  kepler:~/kvmtool.git> git grep KVM_FEATURE_ASYNC_PF_INT
  kepler:~/kvmtool.git> 

  kepler:~/kvmtool.git> grep url .git/config
	url = https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git

So either I missed the flag-setting in the kvmtools.git source, or maybe 
there's some other way to trigger this bug?

Anyway, please handle this as a v5.9 regression:

	Tested-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] x86/kvm: don't forget to ACK async PF IRQ
  2020-09-09  8:16   ` Ingo Molnar
@ 2020-09-09  8:49     ` Vitaly Kuznetsov
  2020-09-12  6:22       ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Vitaly Kuznetsov @ 2020-09-09  8:49 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: kvm, x86, Paolo Bonzini, Sean Christopherson, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Andy Lutomirski

Ingo Molnar <mingo@kernel.org> writes:

> * Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
>> Merge commit 26d05b368a5c0 ("Merge branch 'kvm-async-pf-int' into HEAD")
>> tried to adapt the new interrupt based async PF mechanism to the newly
>> introduced IDTENTRY magic but unfortunately it missed the fact that
>> DEFINE_IDTENTRY_SYSVEC() doesn't call ack_APIC_irq() on its own and
>> all DEFINE_IDTENTRY_SYSVEC() users have to call it manually.
>> 
>> As the result all multi-CPU KVM guest hang on boot when
>> KVM_FEATURE_ASYNC_PF_INT is present. The breakage went unnoticed because no
>> KVM userspace (e.g. QEMU) currently set it (and thus async PF mechanism
>> is currently disabled) but we're about to change that.
>> 
>> Fixes: 26d05b368a5c0 ("Merge branch 'kvm-async-pf-int' into HEAD")
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>
> This also fixes a kvmtool regression, but interestingly it does not set 
> KVM_FEATURE_ASYNC_PF_INT either AFAICS:
>
>   kepler:~/kvmtool.git> git grep KVM_FEATURE_ASYNC_PF_INT
>   kepler:~/kvmtool.git> 

My wild guess would be that kvmtool doesn't manually set any of the KVM
PV features:

[vitty@vitty kvmtool]$ git grep KVM_FEATURE_
[vitty@vitty kvmtool]$ 

it just blindly passes whatever it gets from KVM via
KVM_GET_SUPPORTED_CPUID to KVM_SET_CPUID2 and KVM_FEATURE_ASYNC_PF_INT
among other PV features is set there by default.

>
>   kepler:~/kvmtool.git> grep url .git/config
> 	url = https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git
>
> So either I missed the flag-setting in the kvmtools.git source, or maybe 
> there's some other way to trigger this bug?
>
> Anyway, please handle this as a v5.9 regression:
>
> 	Tested-by: Ingo Molnar <mingo@kernel.org>

Thanks!

-- 
Vitaly


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] x86/kvm: don't forget to ACK async PF IRQ
  2020-09-09  8:49     ` Vitaly Kuznetsov
@ 2020-09-12  6:22       ` Paolo Bonzini
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2020-09-12  6:22 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Ingo Molnar
  Cc: kvm, x86, Sean Christopherson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Andy Lutomirski

On 09/09/20 10:49, Vitaly Kuznetsov wrote:
> Ingo Molnar <mingo@kernel.org> writes:
> 
>> * Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>>> Merge commit 26d05b368a5c0 ("Merge branch 'kvm-async-pf-int' into HEAD")
>>> tried to adapt the new interrupt based async PF mechanism to the newly
>>> introduced IDTENTRY magic but unfortunately it missed the fact that
>>> DEFINE_IDTENTRY_SYSVEC() doesn't call ack_APIC_irq() on its own and
>>> all DEFINE_IDTENTRY_SYSVEC() users have to call it manually.
>>>
>>> As the result all multi-CPU KVM guest hang on boot when
>>> KVM_FEATURE_ASYNC_PF_INT is present. The breakage went unnoticed because no
>>> KVM userspace (e.g. QEMU) currently set it (and thus async PF mechanism
>>> is currently disabled) but we're about to change that.
>>>
>>> Fixes: 26d05b368a5c0 ("Merge branch 'kvm-async-pf-int' into HEAD")
>>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>>
>> This also fixes a kvmtool regression, but interestingly it does not set 
>> KVM_FEATURE_ASYNC_PF_INT either AFAICS:
>>
>>   kepler:~/kvmtool.git> git grep KVM_FEATURE_ASYNC_PF_INT
>>   kepler:~/kvmtool.git> 
> 
> My wild guess would be that kvmtool doesn't manually set any of the KVM
> PV features:
> 
> [vitty@vitty kvmtool]$ git grep KVM_FEATURE_
> [vitty@vitty kvmtool]$ 
> 
> it just blindly passes whatever it gets from KVM via
> KVM_GET_SUPPORTED_CPUID to KVM_SET_CPUID2 and KVM_FEATURE_ASYNC_PF_INT
> among other PV features is set there by default.
> 
>>
>>   kepler:~/kvmtool.git> grep url .git/config
>> 	url = https://git.kernel.org/pub/scm/linux/kernel/git/will/kvmtool.git
>>
>> So either I missed the flag-setting in the kvmtools.git source, or maybe 
>> there's some other way to trigger this bug?
>>
>> Anyway, please handle this as a v5.9 regression:
>>
>> 	Tested-by: Ingo Molnar <mingo@kernel.org>
> 
> Thanks!
> 

Queued, thanks.

Paolo


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-12  6:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-08 13:53 [PATCH 0/2] x86/kvm: fix interrupts based APF mechanism Vitaly Kuznetsov
2020-09-08 13:53 ` [PATCH 1/2] x86/kvm: properly use DEFINE_IDTENTRY_SYSVEC() macro Vitaly Kuznetsov
2020-09-08 13:53 ` [PATCH 2/2] x86/kvm: don't forget to ACK async PF IRQ Vitaly Kuznetsov
2020-09-09  8:16   ` Ingo Molnar
2020-09-09  8:49     ` Vitaly Kuznetsov
2020-09-12  6:22       ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).