[xen stable-4.17] x86/irq: handle moving interrupts in _assign_irq_vector()

[patchbot@xxxxxxx]

commit 7d3fee8d8e78c3c2c25af995b9c5d1c22d98d54c
Author:     Roger Pau Monné <roger.pau@xxxxxxxxxx>
AuthorDate: Wed Jun 26 14:12:05 2024 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Jun 26 14:12:05 2024 +0200

    x86/irq: handle moving interrupts in _assign_irq_vector()
    
    Currently there's logic in fixup_irqs() that attempts to prevent
    _assign_irq_vector() from failing, as fixup_irqs() is required to evacuate 
all
    interrupts from the CPUs not present in the input mask.  The current logic 
in
    fixup_irqs() is incomplete, as it doesn't deal with interrupts that have
    move_cleanup_count > 0 and a non-empty ->arch.old_cpu_mask field.
    
    Instead of attempting to fixup the interrupt descriptor in fixup_irqs() so 
that
    _assign_irq_vector() cannot fail, introduce logic in _assign_irq_vector()
    to deal with interrupts that have either move_{in_progress,cleanup_count} 
set
    and no remaining online CPUs in ->arch.cpu_mask.
    
    If _assign_irq_vector() is requested to move an interrupt in the state
    described above, first attempt to see if ->arch.old_cpu_mask contains any 
valid
    CPUs that could be used as fallback, and if that's the case do move the
    interrupt back to the previous destination.  Note this is easier because the
    vector hasn't been released yet, so there's no need to allocate and setup a 
new
    vector on the destination.
    
    Due to the logic in fixup_irqs() that clears offline CPUs from
    ->arch.old_cpu_mask (and releases the old vector if the mask becomes empty) 
it
    shouldn't be possible to get into _assign_irq_vector() with
    ->arch.move_{in_progress,cleanup_count} set but no online CPUs in
    ->arch.old_cpu_mask.
    
    However if ->arch.move_{in_progress,cleanup_count} is set and the interrupt 
has
    also changed affinity, it's possible the members of ->arch.old_cpu_mask are 
no
    longer part of the affinity set, move the interrupt to a different CPU part 
of
    the provided mask and keep the current ->arch.old_{cpu_mask,vector} for the
    pending interrupt movement to be completed.
    
    Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    master commit: 369558924a642bbb0cb731e9a3375958867cb17b
    master date: 2024-06-18 15:15:10 +0200
---
 xen/arch/x86/irq.c | 97 ++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 68 insertions(+), 29 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index b1387cf6c9..0b4d4b44a5 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -553,7 +553,58 @@ static int _assign_irq_vector(struct irq_desc *desc, const 
cpumask_t *mask)
     }
 
     if ( desc->arch.move_in_progress || desc->arch.move_cleanup_count )
-        return -EAGAIN;
+    {
+        /*
+         * If the current destination is online refuse to shuffle.  Retry after
+         * the in-progress movement has finished.
+         */
+        if ( cpumask_intersects(desc->arch.cpu_mask, &cpu_online_map) )
+            return -EAGAIN;
+
+        /*
+         * Due to the logic in fixup_irqs() that clears offlined CPUs from
+         * ->arch.old_cpu_mask it shouldn't be possible to get here with
+         * ->arch.move_{in_progress,cleanup_count} set and no online CPUs in
+         * ->arch.old_cpu_mask.
+         */
+        ASSERT(valid_irq_vector(desc->arch.old_vector));
+        ASSERT(cpumask_intersects(desc->arch.old_cpu_mask, &cpu_online_map));
+
+        if ( cpumask_intersects(desc->arch.old_cpu_mask, mask) )
+        {
+            /*
+             * Fallback to the old destination if moving is in progress and the
+             * current destination is to be offlined.  This is only possible if
+             * the CPUs in old_cpu_mask intersect with the affinity mask passed
+             * in the 'mask' parameter.
+             */
+            desc->arch.vector = desc->arch.old_vector;
+            cpumask_and(desc->arch.cpu_mask, desc->arch.old_cpu_mask, mask);
+
+            /* Undo any possibly done cleanup. */
+            for_each_cpu(cpu, desc->arch.cpu_mask)
+                per_cpu(vector_irq, cpu)[desc->arch.vector] = irq;
+
+            /* Cancel the pending move and release the current vector. */
+            desc->arch.old_vector = IRQ_VECTOR_UNASSIGNED;
+            cpumask_clear(desc->arch.old_cpu_mask);
+            desc->arch.move_in_progress = 0;
+            desc->arch.move_cleanup_count = 0;
+            if ( desc->arch.used_vectors )
+            {
+                ASSERT(test_bit(old_vector, desc->arch.used_vectors));
+                clear_bit(old_vector, desc->arch.used_vectors);
+            }
+
+            return 0;
+        }
+
+        /*
+         * There's an interrupt movement in progress but the destination(s) in
+         * ->arch.old_cpu_mask are not suitable given the 'mask' parameter, go
+         * through the full logic to find a new vector in a suitable CPU.
+         */
+    }
 
     err = -ENOSPC;
 
@@ -609,7 +660,22 @@ next:
         current_vector = vector;
         current_offset = offset;
 
-        if ( valid_irq_vector(old_vector) )
+        if ( desc->arch.move_in_progress || desc->arch.move_cleanup_count )
+        {
+            ASSERT(!cpumask_intersects(desc->arch.cpu_mask, &cpu_online_map));
+            /*
+             * Special case when evacuating an interrupt from a CPU to be
+             * offlined and the interrupt was already in the process of being
+             * moved.  Leave ->arch.old_{vector,cpu_mask} as-is and just
+             * replace ->arch.{cpu_mask,vector} with the new destination.
+             * Cleanup will be done normally for the old fields, just release
+             * the current vector here.
+             */
+            if ( desc->arch.used_vectors &&
+                 !test_and_clear_bit(old_vector, desc->arch.used_vectors) )
+                ASSERT_UNREACHABLE();
+        }
+        else if ( valid_irq_vector(old_vector) )
         {
             cpumask_and(desc->arch.old_cpu_mask, desc->arch.cpu_mask,
                         &cpu_online_map);
@@ -2620,33 +2686,6 @@ void fixup_irqs(const cpumask_t *mask, bool verbose)
             continue;
         }
 
-        /*
-         * In order for the affinity adjustment below to be successful, we
-         * need _assign_irq_vector() to succeed. This in particular means
-         * clearing desc->arch.move_in_progress if this would otherwise
-         * prevent the function from succeeding. Since there's no way for the
-         * flag to get cleared anymore when there's no possible destination
-         * left (the only possibility then would be the IRQs enabled window
-         * after this loop), there's then also no race with us doing it here.
-         *
-         * Therefore the logic here and there need to remain in sync.
-         */
-        if ( desc->arch.move_in_progress &&
-             !cpumask_intersects(mask, desc->arch.cpu_mask) )
-        {
-            unsigned int cpu;
-
-            cpumask_and(affinity, desc->arch.old_cpu_mask, &cpu_online_map);
-
-            spin_lock(&vector_lock);
-            for_each_cpu(cpu, affinity)
-                per_cpu(vector_irq, cpu)[desc->arch.old_vector] = ~irq;
-            spin_unlock(&vector_lock);
-
-            release_old_vec(desc);
-            desc->arch.move_in_progress = 0;
-        }
-
         if ( !cpumask_intersects(mask, desc->affinity) )
         {
             break_affinity = true;
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.17