[3/6] x86/IRQ: simplify pending EOI stack logic for internally used IRQs
diff mbox series

Message ID b916a5d3-2ce0-4232-fa42-33ea02bf655f@suse.com
State New
Headers show
Series
  • x86: IRQ handling adjustments
Related show

Commit Message

Jan Beulich Dec. 20, 2019, 1:29 p.m. UTC
In 5655ce8b1ec2 ("x86/IRQ: make internally used IRQs also honor the
pending EOI stack") it was mentioned that both the check_eoi_deferral
per-CPU variable and the cpu_has_pending_apic_eoi() were added just to
have as little impact on existing behavior as possible, to reduce the
risk of a last minute regression in 4.13.

Upon closer inspection, dropping the variable is an option only if all
callers of ->end() would assume the responsibility of also calling
flush_ready_eoi(). Therefore only drop the cpu_has_pending_apic_eoi()
guard now.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
TBD: In the end I'm not sure this is really worth it then.

Comments

Andrew Cooper Dec. 20, 2019, 1:51 p.m. UTC | #1
On 20/12/2019 13:29, Jan Beulich wrote:
> In 5655ce8b1ec2 ("x86/IRQ: make internally used IRQs also honor the
> pending EOI stack") it was mentioned that both the check_eoi_deferral
> per-CPU variable and the cpu_has_pending_apic_eoi() were added just to
> have as little impact on existing behavior as possible, to reduce the
> risk of a last minute regression in 4.13.
>
> Upon closer inspection, dropping the variable is an option only if all
> callers of ->end() would assume the responsibility of also calling
> flush_ready_eoi(). Therefore only drop the cpu_has_pending_apic_eoi()
> guard now.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> TBD: In the end I'm not sure this is really worth it then.

The resulting logic is more simple, which is justification alone.

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Patch
diff mbox series

--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1991,18 +1991,13 @@  void do_IRQ(struct cpu_user_regs *regs)
          * If higher priority vectors still have their EOIs pending, we may
          * not issue an EOI here, as this would EOI the highest priority one.
          */
-        if ( cpu_has_pending_apic_eoi() )
-        {
-            this_cpu(check_eoi_deferral) = true;
-            desc->handler->end(desc, vector);
-            this_cpu(check_eoi_deferral) = false;
-
-            spin_unlock(&desc->lock);
-            flush_ready_eoi();
-            goto out_no_unlock;
-        }
-
+        this_cpu(check_eoi_deferral) = true;
         desc->handler->end(desc, vector);
+        this_cpu(check_eoi_deferral) = false;
+
+        spin_unlock(&desc->lock);
+        flush_ready_eoi();
+        goto out_no_unlock;
     }
 
  out_no_end: