Re: [PATCH 1/2] KVM: x86: handle !lapic_in_kernel case in kvm_cpu_*_extint

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