| Message ID | 20201127112114.3219360-2-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: cleanup and fix userspace interrupt window | expand |
On Fri, 2020-11-27 at 06:21 -0500, Paolo Bonzini wrote:
> + * FIXME: interrupt.injected represents an interrupt that it's
You can drop the stray apostrophe from that "its" while you're moving
it...
On 11/27/20 12:21 PM, Paolo Bonzini wrote: > > 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 > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/x86/kvm/irq.c | 85 ++++++++++++++++++-------------------------- > arch/x86/kvm/lapic.c | 2 +- > 2 files changed, 35 insertions(+), 52 deletions(-) > > diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c > index 99d118ffc67d..e2d49a506e7f 100644 > --- a/arch/x86/kvm/irq.c > +++ b/arch/x86/kvm/irq.c > @@ -42,15 +42,27 @@ static int pending_userspace_extint(struct kvm_vcpu *v) > */ > static int kvm_cpu_has_extint(struct kvm_vcpu *v) > { > - u8 accept = kvm_apic_accept_pic_intr(v); > + /* > + * FIXME: interrupt.injected represents an interrupt that it's > + * side-effects have already been applied (e.g. bit from IRR > + * already moved to ISR). Therefore, it is incorrect to rely > + * on interrupt.injected 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. > + */ > + if (!lapic_in_kernel(v)) > + return v->arch.interrupt.injected; > > - 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; > } > > /* > @@ -61,20 +73,6 @@ static int kvm_cpu_has_extint(struct kvm_vcpu *v) > */ > int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) > { > - /* > - * FIXME: interrupt.injected represents an interrupt that it's > - * side-effects have already been applied (e.g. bit from IRR > - * already moved to ISR). Therefore, it is incorrect to rely > - * on interrupt.injected 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. > - */ > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.injected; > - > if (kvm_cpu_has_extint(v)) > return 1; > > @@ -91,20 +89,6 @@ EXPORT_SYMBOL_GPL(kvm_cpu_has_injectable_intr); > */ > int kvm_cpu_has_interrupt(struct kvm_vcpu *v) > { > - /* > - * FIXME: interrupt.injected represents an interrupt that it's > - * side-effects have already been applied (e.g. bit from IRR > - * already moved to ISR). Therefore, it is incorrect to rely > - * on interrupt.injected 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. > - */ > - if (!lapic_in_kernel(v)) > - return v->arch.interrupt.injected; > - > if (kvm_cpu_has_extint(v)) > return 1; > > @@ -118,16 +102,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 */ > } > > /* > @@ -135,13 +124,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 105e7859d1f2..bb5ff761d5e2 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -2465,7 +2465,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.28.0 > > Reviewed-by: Filippo Sironi <sironi@amazon.de> Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c index 99d118ffc67d..e2d49a506e7f 100644 --- a/arch/x86/kvm/irq.c +++ b/arch/x86/kvm/irq.c @@ -42,15 +42,27 @@ static int pending_userspace_extint(struct kvm_vcpu *v) */ static int kvm_cpu_has_extint(struct kvm_vcpu *v) { - u8 accept = kvm_apic_accept_pic_intr(v); + /* + * FIXME: interrupt.injected represents an interrupt that it's + * side-effects have already been applied (e.g. bit from IRR + * already moved to ISR). Therefore, it is incorrect to rely + * on interrupt.injected 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. + */ + if (!lapic_in_kernel(v)) + return v->arch.interrupt.injected; - 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; } /* @@ -61,20 +73,6 @@ static int kvm_cpu_has_extint(struct kvm_vcpu *v) */ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v) { - /* - * FIXME: interrupt.injected represents an interrupt that it's - * side-effects have already been applied (e.g. bit from IRR - * already moved to ISR). Therefore, it is incorrect to rely - * on interrupt.injected 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. - */ - if (!lapic_in_kernel(v)) - return v->arch.interrupt.injected; - if (kvm_cpu_has_extint(v)) return 1; @@ -91,20 +89,6 @@ EXPORT_SYMBOL_GPL(kvm_cpu_has_injectable_intr); */ int kvm_cpu_has_interrupt(struct kvm_vcpu *v) { - /* - * FIXME: interrupt.injected represents an interrupt that it's - * side-effects have already been applied (e.g. bit from IRR - * already moved to ISR). Therefore, it is incorrect to rely - * on interrupt.injected 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. - */ - if (!lapic_in_kernel(v)) - return v->arch.interrupt.injected; - if (kvm_cpu_has_extint(v)) return 1; @@ -118,16 +102,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 */ } /* @@ -135,13 +124,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 105e7859d1f2..bb5ff761d5e2 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -2465,7 +2465,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);
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 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/x86/kvm/irq.c | 85 ++++++++++++++++++-------------------------- arch/x86/kvm/lapic.c | 2 +- 2 files changed, 35 insertions(+), 52 deletions(-)