| Message ID | 20210303213348.31319-2-gpiccoli@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Bionic kernel 4.15.0-136 causes VMM freezes due to lack of KVM patch | expand |
On 03.03.21 22:33, Guilherme G. Piccoli wrote: > From: Paolo Bonzini <pbonzini@redhat.com> > > BugLink: https://bugs.launchpad.net/bugs/1917138 > > Centralize handling of interrupts from the userspace APIC > in kvm_cpu_has_extint and kvm_cpu_get_extint, since > userspace APIC interrupts are handled more or less the > same as ExtINTs are with split irqchip. This removes > duplicated code from kvm_cpu_has_injectable_intr and > kvm_cpu_has_interrupt, and makes the code more similar > between kvm_cpu_has_{extint,interrupt} on one side > and kvm_cpu_get_{extint,interrupt} on the other. > > Cc: stable@vger.kernel.org > Reviewed-by: Filippo Sironi <sironi@amazon.de> > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > Tested-by: David Woodhouse <dwmw@amazon.co.uk> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > (backported from commit 72c3bcdcda494cbd600712a32e67702cdee60c07) > > [gpiccoli: Besides context adjustments, it's very important to notice > that Ubuntu 4.15 tree lacks the following commit, which renames the > struct member interrupt.pending to interrupt.injected: > 04140b4144cd ("KVM: x86: Rename interrupt.pending to interrupt.injected") > This may cause a very confusing comment in kvm_cpu_has_extint(), > hence I've changed both code and comment in order they do make sense.] > > Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- The patch looks consistent and there is good testing results in the bug report. I just modified the report to have a bionic nomination and marked the devel task invalid as the comments suggest that this only affects bionic. -Stefan > arch/x86/kvm/irq.c | 65 ++++++++++++++++++++++++-------------------- > arch/x86/kvm/lapic.c | 2 +- > 2 files changed, 37 insertions(+), 30 deletions(-) > > diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c > index 8978988e735d..842fcc043a8f 100644 > --- a/arch/x86/kvm/irq.c > +++ b/arch/x86/kvm/irq.c > @@ -54,15 +54,29 @@ static int pending_userspace_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_has_extint(struct kvm_vcpu *v) > { > - u8 accept = kvm_apic_accept_pic_intr(v); > + /* > + * FIXME: interrupt.pending represents an interrupt whose > + * side-effects have already been applied (e.g. bit from IRR > + * already moved to ISR). Therefore, it is incorrect to rely > + * on interrupt.pending to know if there is a pending > + * interrupt in the user-mode LAPIC. > + * This leads to nVMX/nSVM not be able to distinguish > + * if it should exit from L2 to L1 on EXTERNAL_INTERRUPT on > + * pending interrupt or should re-inject an injected > + * interrupt. > + * [backport note: interrupt.pending was renamed interrupt.injected > + * in upstream commit 04140b4144cd , not present in this tree.] > + */ > + if (!lapic_in_kernel(v)) > + return v->arch.interrupt.pending; > > - if (accept) { > - if (irqchip_split(v->kvm)) > - return pending_userspace_extint(v); > - else > - return v->kvm->arch.vpic->output; > - } else > + if (!kvm_apic_accept_pic_intr(v)) > return 0; > + > + if (irqchip_split(v->kvm)) > + return pending_userspace_extint(v); > + else > + return v->kvm->arch.vpic->output; > } > > /* > @@ -73,9 +87,6 @@ int kvm_cpu_has_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) > { > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.pending; > - > if (kvm_cpu_has_extint(v)) > return 1; > > @@ -91,9 +102,6 @@ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) > */ > int kvm_cpu_has_interrupt(struct kvm_vcpu *v) > { > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.pending; > - > if (kvm_cpu_has_extint(v)) > return 1; > > @@ -107,16 +115,21 @@ EXPORT_SYMBOL_GPL(kvm_cpu_has_interrupt); > */ > static int kvm_cpu_get_extint(struct kvm_vcpu *v) > { > - if (kvm_cpu_has_extint(v)) { > - if (irqchip_split(v->kvm)) { > - int vector = v->arch.pending_external_vector; > - > - v->arch.pending_external_vector = -1; > - return vector; > - } else > - return kvm_pic_read_irq(v->kvm); /* PIC */ > - } else > + if (!kvm_cpu_has_extint(v)) { > + WARN_ON(!lapic_in_kernel(v)); > return -1; > + } > + > + if (!lapic_in_kernel(v)) > + return v->arch.interrupt.nr; > + > + if (irqchip_split(v->kvm)) { > + int vector = v->arch.pending_external_vector; > + > + v->arch.pending_external_vector = -1; > + return vector; > + } else > + return kvm_pic_read_irq(v->kvm); /* PIC */ > } > > /* > @@ -124,13 +137,7 @@ static int kvm_cpu_get_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_get_interrupt(struct kvm_vcpu *v) > { > - int vector; > - > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.nr; > - > - vector = kvm_cpu_get_extint(v); > - > + int vector = kvm_cpu_get_extint(v); > if (vector != -1) > return vector; /* PIC */ > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index e1116584105d..a6084e5750ab 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -2217,7 +2217,7 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu) > struct kvm_lapic *apic = vcpu->arch.apic; > u32 ppr; > > - if (!kvm_apic_hw_enabled(apic)) > + if (!kvm_apic_present(vcpu)) > return -1; > > __apic_update_ppr(apic, &ppr); >
On Wed, Mar 03, 2021 at 06:33:48PM -0300, Guilherme G. Piccoli wrote: > From: Paolo Bonzini <pbonzini@redhat.com> > > BugLink: https://bugs.launchpad.net/bugs/1917138 > > Centralize handling of interrupts from the userspace APIC > in kvm_cpu_has_extint and kvm_cpu_get_extint, since > userspace APIC interrupts are handled more or less the > same as ExtINTs are with split irqchip. This removes > duplicated code from kvm_cpu_has_injectable_intr and > kvm_cpu_has_interrupt, and makes the code more similar > between kvm_cpu_has_{extint,interrupt} on one side > and kvm_cpu_get_{extint,interrupt} on the other. > > Cc: stable@vger.kernel.org > Reviewed-by: Filippo Sironi <sironi@amazon.de> > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > Tested-by: David Woodhouse <dwmw@amazon.co.uk> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > (backported from commit 72c3bcdcda494cbd600712a32e67702cdee60c07) > > [gpiccoli: Besides context adjustments, it's very important to notice > that Ubuntu 4.15 tree lacks the following commit, which renames the > struct member interrupt.pending to interrupt.injected: > 04140b4144cd ("KVM: x86: Rename interrupt.pending to interrupt.injected") > This may cause a very confusing comment in kvm_cpu_has_extint(), > hence I've changed both code and comment in order they do make sense.] > > Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com> > --- > arch/x86/kvm/irq.c | 65 ++++++++++++++++++++++++-------------------- > arch/x86/kvm/lapic.c | 2 +- > 2 files changed, 37 insertions(+), 30 deletions(-) > > diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c > index 8978988e735d..842fcc043a8f 100644 > --- a/arch/x86/kvm/irq.c > +++ b/arch/x86/kvm/irq.c > @@ -54,15 +54,29 @@ static int pending_userspace_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_has_extint(struct kvm_vcpu *v) > { > - u8 accept = kvm_apic_accept_pic_intr(v); > + /* > + * FIXME: interrupt.pending represents an interrupt whose > + * side-effects have already been applied (e.g. bit from IRR > + * already moved to ISR). Therefore, it is incorrect to rely > + * on interrupt.pending to know if there is a pending > + * interrupt in the user-mode LAPIC. > + * This leads to nVMX/nSVM not be able to distinguish > + * if it should exit from L2 to L1 on EXTERNAL_INTERRUPT on > + * pending interrupt or should re-inject an injected > + * interrupt. > + * [backport note: interrupt.pending was renamed interrupt.injected > + * in upstream commit 04140b4144cd , not present in this tree.] > + */ The comment really comes from 04140b4144cd. Which explains how the backport is not that easy to compare with the upstream version. Applying the backport and comparing to 71cc849b7093 shows really well the differences there and I am happy with the backport as it is. I appreciate all the writeup as well too. Cascardo. Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > + if (!lapic_in_kernel(v)) > + return v->arch.interrupt.pending; > > - if (accept) { > - if (irqchip_split(v->kvm)) > - return pending_userspace_extint(v); > - else > - return v->kvm->arch.vpic->output; > - } else > + if (!kvm_apic_accept_pic_intr(v)) > return 0; > + > + if (irqchip_split(v->kvm)) > + return pending_userspace_extint(v); > + else > + return v->kvm->arch.vpic->output; > } > > /* > @@ -73,9 +87,6 @@ int kvm_cpu_has_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) > { > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.pending; > - > if (kvm_cpu_has_extint(v)) > return 1; > > @@ -91,9 +102,6 @@ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) > */ > int kvm_cpu_has_interrupt(struct kvm_vcpu *v) > { > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.pending; > - > if (kvm_cpu_has_extint(v)) > return 1; > > @@ -107,16 +115,21 @@ EXPORT_SYMBOL_GPL(kvm_cpu_has_interrupt); > */ > static int kvm_cpu_get_extint(struct kvm_vcpu *v) > { > - if (kvm_cpu_has_extint(v)) { > - if (irqchip_split(v->kvm)) { > - int vector = v->arch.pending_external_vector; > - > - v->arch.pending_external_vector = -1; > - return vector; > - } else > - return kvm_pic_read_irq(v->kvm); /* PIC */ > - } else > + if (!kvm_cpu_has_extint(v)) { > + WARN_ON(!lapic_in_kernel(v)); > return -1; > + } > + > + if (!lapic_in_kernel(v)) > + return v->arch.interrupt.nr; > + > + if (irqchip_split(v->kvm)) { > + int vector = v->arch.pending_external_vector; > + > + v->arch.pending_external_vector = -1; > + return vector; > + } else > + return kvm_pic_read_irq(v->kvm); /* PIC */ > } > > /* > @@ -124,13 +137,7 @@ static int kvm_cpu_get_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_get_interrupt(struct kvm_vcpu *v) > { > - int vector; > - > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.nr; > - > - vector = kvm_cpu_get_extint(v); > - > + int vector = kvm_cpu_get_extint(v); > if (vector != -1) > return vector; /* PIC */ > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index e1116584105d..a6084e5750ab 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -2217,7 +2217,7 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu) > struct kvm_lapic *apic = vcpu->arch.apic; > u32 ppr; > > - if (!kvm_apic_hw_enabled(apic)) > + if (!kvm_apic_present(vcpu)) > return -1; > > __apic_update_ppr(apic, &ppr); > -- > 2.29.0 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 03.03.21 22:33, Guilherme G. Piccoli wrote: > From: Paolo Bonzini <pbonzini@redhat.com> > > BugLink: https://bugs.launchpad.net/bugs/1917138 > > Centralize handling of interrupts from the userspace APIC > in kvm_cpu_has_extint and kvm_cpu_get_extint, since > userspace APIC interrupts are handled more or less the > same as ExtINTs are with split irqchip. This removes > duplicated code from kvm_cpu_has_injectable_intr and > kvm_cpu_has_interrupt, and makes the code more similar > between kvm_cpu_has_{extint,interrupt} on one side > and kvm_cpu_get_{extint,interrupt} on the other. > > Cc: stable@vger.kernel.org > Reviewed-by: Filippo Sironi <sironi@amazon.de> > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > Tested-by: David Woodhouse <dwmw@amazon.co.uk> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > (backported from commit 72c3bcdcda494cbd600712a32e67702cdee60c07) > > [gpiccoli: Besides context adjustments, it's very important to notice > that Ubuntu 4.15 tree lacks the following commit, which renames the > struct member interrupt.pending to interrupt.injected: > 04140b4144cd ("KVM: x86: Rename interrupt.pending to interrupt.injected") > This may cause a very confusing comment in kvm_cpu_has_extint(), > hence I've changed both code and comment in order they do make sense.] > > Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com> Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > arch/x86/kvm/irq.c | 65 ++++++++++++++++++++++++-------------------- > arch/x86/kvm/lapic.c | 2 +- > 2 files changed, 37 insertions(+), 30 deletions(-) > > diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c > index 8978988e735d..842fcc043a8f 100644 > --- a/arch/x86/kvm/irq.c > +++ b/arch/x86/kvm/irq.c > @@ -54,15 +54,29 @@ static int pending_userspace_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_has_extint(struct kvm_vcpu *v) > { > - u8 accept = kvm_apic_accept_pic_intr(v); > + /* > + * FIXME: interrupt.pending represents an interrupt whose > + * side-effects have already been applied (e.g. bit from IRR > + * already moved to ISR). Therefore, it is incorrect to rely > + * on interrupt.pending to know if there is a pending > + * interrupt in the user-mode LAPIC. > + * This leads to nVMX/nSVM not be able to distinguish > + * if it should exit from L2 to L1 on EXTERNAL_INTERRUPT on > + * pending interrupt or should re-inject an injected > + * interrupt. > + * [backport note: interrupt.pending was renamed interrupt.injected > + * in upstream commit 04140b4144cd , not present in this tree.] > + */ > + if (!lapic_in_kernel(v)) > + return v->arch.interrupt.pending; > > - if (accept) { > - if (irqchip_split(v->kvm)) > - return pending_userspace_extint(v); > - else > - return v->kvm->arch.vpic->output; > - } else > + if (!kvm_apic_accept_pic_intr(v)) > return 0; > + > + if (irqchip_split(v->kvm)) > + return pending_userspace_extint(v); > + else > + return v->kvm->arch.vpic->output; > } > > /* > @@ -73,9 +87,6 @@ int kvm_cpu_has_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) > { > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.pending; > - > if (kvm_cpu_has_extint(v)) > return 1; > > @@ -91,9 +102,6 @@ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) > */ > int kvm_cpu_has_interrupt(struct kvm_vcpu *v) > { > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.pending; > - > if (kvm_cpu_has_extint(v)) > return 1; > > @@ -107,16 +115,21 @@ EXPORT_SYMBOL_GPL(kvm_cpu_has_interrupt); > */ > static int kvm_cpu_get_extint(struct kvm_vcpu *v) > { > - if (kvm_cpu_has_extint(v)) { > - if (irqchip_split(v->kvm)) { > - int vector = v->arch.pending_external_vector; > - > - v->arch.pending_external_vector = -1; > - return vector; > - } else > - return kvm_pic_read_irq(v->kvm); /* PIC */ > - } else > + if (!kvm_cpu_has_extint(v)) { > + WARN_ON(!lapic_in_kernel(v)); > return -1; > + } > + > + if (!lapic_in_kernel(v)) > + return v->arch.interrupt.nr; > + > + if (irqchip_split(v->kvm)) { > + int vector = v->arch.pending_external_vector; > + > + v->arch.pending_external_vector = -1; > + return vector; > + } else > + return kvm_pic_read_irq(v->kvm); /* PIC */ > } > > /* > @@ -124,13 +137,7 @@ static int kvm_cpu_get_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_get_interrupt(struct kvm_vcpu *v) > { > - int vector; > - > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.nr; > - > - vector = kvm_cpu_get_extint(v); > - > + int vector = kvm_cpu_get_extint(v); > if (vector != -1) > return vector; /* PIC */ > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index e1116584105d..a6084e5750ab 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -2217,7 +2217,7 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu) > struct kvm_lapic *apic = vcpu->arch.apic; > u32 ppr; > > - if (!kvm_apic_hw_enabled(apic)) > + if (!kvm_apic_present(vcpu)) > return -1; > > __apic_update_ppr(apic, &ppr); >
On Thu, Mar 4, 2021 at 6:45 AM Stefan Bader <stefan.bader@canonical.com> wrote: > [...] > The patch looks consistent and there is good testing results in the bug report. > I just modified the report to have a bionic nomination and marked the devel task > invalid as the comments suggest that this only affects bionic. > > -Stefan Thanks Stefan, Cascardo and Kleber for the ACKs! I agree, it's a Bionic-only issue, I forgot to add the nomination, thanks for that. I suggest we merge this patch in the tree as soon as possible, it's not urgent but we can grab it if, for some reason, a Bionic respin happens. Cheers, Guilherme
On 03.03.21 22:33, Guilherme G. Piccoli wrote: > From: Paolo Bonzini <pbonzini@redhat.com> > > BugLink: https://bugs.launchpad.net/bugs/1917138 > > Centralize handling of interrupts from the userspace APIC > in kvm_cpu_has_extint and kvm_cpu_get_extint, since > userspace APIC interrupts are handled more or less the > same as ExtINTs are with split irqchip. This removes > duplicated code from kvm_cpu_has_injectable_intr and > kvm_cpu_has_interrupt, and makes the code more similar > between kvm_cpu_has_{extint,interrupt} on one side > and kvm_cpu_get_{extint,interrupt} on the other. > > Cc: stable@vger.kernel.org > Reviewed-by: Filippo Sironi <sironi@amazon.de> > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > Tested-by: David Woodhouse <dwmw@amazon.co.uk> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > (backported from commit 72c3bcdcda494cbd600712a32e67702cdee60c07) > > [gpiccoli: Besides context adjustments, it's very important to notice > that Ubuntu 4.15 tree lacks the following commit, which renames the > struct member interrupt.pending to interrupt.injected: > 04140b4144cd ("KVM: x86: Rename interrupt.pending to interrupt.injected") > This may cause a very confusing comment in kvm_cpu_has_extint(), > hence I've changed both code and comment in order they do make sense.] > > Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com> > --- Applied to bionic:linux/master-next. Thanks. -Stefan > arch/x86/kvm/irq.c | 65 ++++++++++++++++++++++++-------------------- > arch/x86/kvm/lapic.c | 2 +- > 2 files changed, 37 insertions(+), 30 deletions(-) > > diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c > index 8978988e735d..842fcc043a8f 100644 > --- a/arch/x86/kvm/irq.c > +++ b/arch/x86/kvm/irq.c > @@ -54,15 +54,29 @@ static int pending_userspace_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_has_extint(struct kvm_vcpu *v) > { > - u8 accept = kvm_apic_accept_pic_intr(v); > + /* > + * FIXME: interrupt.pending represents an interrupt whose > + * side-effects have already been applied (e.g. bit from IRR > + * already moved to ISR). Therefore, it is incorrect to rely > + * on interrupt.pending to know if there is a pending > + * interrupt in the user-mode LAPIC. > + * This leads to nVMX/nSVM not be able to distinguish > + * if it should exit from L2 to L1 on EXTERNAL_INTERRUPT on > + * pending interrupt or should re-inject an injected > + * interrupt. > + * [backport note: interrupt.pending was renamed interrupt.injected > + * in upstream commit 04140b4144cd , not present in this tree.] > + */ > + if (!lapic_in_kernel(v)) > + return v->arch.interrupt.pending; > > - if (accept) { > - if (irqchip_split(v->kvm)) > - return pending_userspace_extint(v); > - else > - return v->kvm->arch.vpic->output; > - } else > + if (!kvm_apic_accept_pic_intr(v)) > return 0; > + > + if (irqchip_split(v->kvm)) > + return pending_userspace_extint(v); > + else > + return v->kvm->arch.vpic->output; > } > > /* > @@ -73,9 +87,6 @@ int kvm_cpu_has_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) > { > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.pending; > - > if (kvm_cpu_has_extint(v)) > return 1; > > @@ -91,9 +102,6 @@ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) > */ > int kvm_cpu_has_interrupt(struct kvm_vcpu *v) > { > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.pending; > - > if (kvm_cpu_has_extint(v)) > return 1; > > @@ -107,16 +115,21 @@ EXPORT_SYMBOL_GPL(kvm_cpu_has_interrupt); > */ > static int kvm_cpu_get_extint(struct kvm_vcpu *v) > { > - if (kvm_cpu_has_extint(v)) { > - if (irqchip_split(v->kvm)) { > - int vector = v->arch.pending_external_vector; > - > - v->arch.pending_external_vector = -1; > - return vector; > - } else > - return kvm_pic_read_irq(v->kvm); /* PIC */ > - } else > + if (!kvm_cpu_has_extint(v)) { > + WARN_ON(!lapic_in_kernel(v)); > return -1; > + } > + > + if (!lapic_in_kernel(v)) > + return v->arch.interrupt.nr; > + > + if (irqchip_split(v->kvm)) { > + int vector = v->arch.pending_external_vector; > + > + v->arch.pending_external_vector = -1; > + return vector; > + } else > + return kvm_pic_read_irq(v->kvm); /* PIC */ > } > > /* > @@ -124,13 +137,7 @@ static int kvm_cpu_get_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_get_interrupt(struct kvm_vcpu *v) > { > - int vector; > - > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.nr; > - > - vector = kvm_cpu_get_extint(v); > - > + int vector = kvm_cpu_get_extint(v); > if (vector != -1) > return vector; /* PIC */ > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index e1116584105d..a6084e5750ab 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -2217,7 +2217,7 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu) > struct kvm_lapic *apic = vcpu->arch.apic; > u32 ppr; > > - if (!kvm_apic_hw_enabled(apic)) > + if (!kvm_apic_present(vcpu)) > return -1; > > __apic_update_ppr(apic, &ppr); >
diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c index 8978988e735d..842fcc043a8f 100644 --- a/arch/x86/kvm/irq.c +++ b/arch/x86/kvm/irq.c @@ -54,15 +54,29 @@ static int pending_userspace_extint(struct kvm_vcpu *v) */ int kvm_cpu_has_extint(struct kvm_vcpu *v) { - u8 accept = kvm_apic_accept_pic_intr(v); + /* + * FIXME: interrupt.pending represents an interrupt whose + * side-effects have already been applied (e.g. bit from IRR + * already moved to ISR). Therefore, it is incorrect to rely + * on interrupt.pending to know if there is a pending + * interrupt in the user-mode LAPIC. + * This leads to nVMX/nSVM not be able to distinguish + * if it should exit from L2 to L1 on EXTERNAL_INTERRUPT on + * pending interrupt or should re-inject an injected + * interrupt. + * [backport note: interrupt.pending was renamed interrupt.injected + * in upstream commit 04140b4144cd , not present in this tree.] + */ + if (!lapic_in_kernel(v)) + return v->arch.interrupt.pending; - if (accept) { - if (irqchip_split(v->kvm)) - return pending_userspace_extint(v); - else - return v->kvm->arch.vpic->output; - } else + if (!kvm_apic_accept_pic_intr(v)) return 0; + + if (irqchip_split(v->kvm)) + return pending_userspace_extint(v); + else + return v->kvm->arch.vpic->output; } /* @@ -73,9 +87,6 @@ int kvm_cpu_has_extint(struct kvm_vcpu *v) */ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) { - if (!lapic_in_kernel(v)) - return v->arch.interrupt.pending; - if (kvm_cpu_has_extint(v)) return 1; @@ -91,9 +102,6 @@ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) */ int kvm_cpu_has_interrupt(struct kvm_vcpu *v) { - if (!lapic_in_kernel(v)) - return v->arch.interrupt.pending; - if (kvm_cpu_has_extint(v)) return 1; @@ -107,16 +115,21 @@ EXPORT_SYMBOL_GPL(kvm_cpu_has_interrupt); */ static int kvm_cpu_get_extint(struct kvm_vcpu *v) { - if (kvm_cpu_has_extint(v)) { - if (irqchip_split(v->kvm)) { - int vector = v->arch.pending_external_vector; - - v->arch.pending_external_vector = -1; - return vector; - } else - return kvm_pic_read_irq(v->kvm); /* PIC */ - } else + if (!kvm_cpu_has_extint(v)) { + WARN_ON(!lapic_in_kernel(v)); return -1; + } + + if (!lapic_in_kernel(v)) + return v->arch.interrupt.nr; + + if (irqchip_split(v->kvm)) { + int vector = v->arch.pending_external_vector; + + v->arch.pending_external_vector = -1; + return vector; + } else + return kvm_pic_read_irq(v->kvm); /* PIC */ } /* @@ -124,13 +137,7 @@ static int kvm_cpu_get_extint(struct kvm_vcpu *v) */ int kvm_cpu_get_interrupt(struct kvm_vcpu *v) { - int vector; - - if (!lapic_in_kernel(v)) - return v->arch.interrupt.nr; - - vector = kvm_cpu_get_extint(v); - + int vector = kvm_cpu_get_extint(v); if (vector != -1) return vector; /* PIC */ diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index e1116584105d..a6084e5750ab 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -2217,7 +2217,7 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu) struct kvm_lapic *apic = vcpu->arch.apic; u32 ppr; - if (!kvm_apic_hw_enabled(apic)) + if (!kvm_apic_present(vcpu)) return -1; __apic_update_ppr(apic, &ppr);