genirq/affinity: Handle affinity setting on inactive interrupts correctly

Setting interrupt affinity on inactive interrupts is inconsistent when
hierarchical irq domains are enabled. The core code should just store the
affinity and not call into the irq chip driver for inactive interrupts
because the chip drivers may not be in a state to handle such requests.

X86 has a hacky workaround for that but all other irq chips have not which
causes problems e.g. on GIC V3 ITS.

Instead of adding more ugly hacks all over the place, solve the problem in
the core code. If the affinity is set on an inactive interrupt then:

    - Store it in the irq descriptors affinity mask
    - Update the effective affinity to reflect that so user space has
      a consistent view
    - Don't call into the irq chip driver

This is the core equivalent of the X86 workaround and works correctly
because the affinity setting is established in the irq chip when the
interrupt is activated later on.

Note, that this is only effective when hierarchical irq domains are enabled
by the architecture. Doing it unconditionally would break legacy irq chip
implementations.

For hierarchial irq domains this works correctly as none of the drivers can
have a dependency on affinity setting in inactive state by design.

Remove the X86 workaround as it is not longer required.

Fixes: 02edee152d6e ("x86/apic/vector: Ignore set_affinity call for inactive interrupts")
Reported-by: Ali Saidi <alisaidi@amazon.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Ali Saidi <alisaidi@amazon.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20200529015501.15771-1-alisaidi@amazon.com
Link: https://lkml.kernel.org/r/877dv2rv25.fsf@nanos.tec.linutronix.de

diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index cc8b16f89dd40b721788c8af0e253c490f8e9b3d..7649da2478d8af00ff347c9169add6cab4691eb0 100644 (file)
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -446,12 +446,10 @@ static int x86_vector_activate(struct irq_domain *dom, struct irq_data *irqd,
        trace_vector_activate(irqd->irq, apicd->is_managed,
                              apicd->can_reserve, reserve);
 
-       /* Nothing to do for fixed assigned vectors */
-       if (!apicd->can_reserve && !apicd->is_managed)
-               return 0;
-
        raw_spin_lock_irqsave(&vector_lock, flags);
-       if (reserve || irqd_is_managed_and_shutdown(irqd))
+       if (!apicd->can_reserve && !apicd->is_managed)
+               assign_irq_vector_any_locked(irqd);
+       else if (reserve || irqd_is_managed_and_shutdown(irqd))
                vector_assign_managed_shutdown(irqd);
        else if (apicd->is_managed)
                ret = activate_managed(irqd);
@@ -774,20 +772,10 @@ void lapic_offline(void)
 static int apic_set_affinity(struct irq_data *irqd,
                             const struct cpumask *dest, bool force)
 {
-       struct apic_chip_data *apicd = apic_chip_data(irqd);
        int err;
 
-       /*
-        * Core code can call here for inactive interrupts. For inactive
-        * interrupts which use managed or reservation mode there is no
-        * point in going through the vector assignment right now as the
-        * activation will assign a vector which fits the destination
-        * cpumask. Let the core code store the destination mask and be
-        * done with it.
-        */
-       if (!irqd_is_activated(irqd) &&
-           (apicd->is_managed || apicd->can_reserve))
-               return IRQ_SET_MASK_OK;
+       if (WARN_ON_ONCE(!irqd_is_activated(irqd)))
+               return -EIO;
 
        raw_spin_lock(&vector_lock);
        cpumask_and(vector_searchmask, dest, cpu_online_mask);

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 76191116843879405269061b9b437a3d0762fea4..2a9fec53e15910124dc8190c13ad4bfde3baca63 100644 (file)
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -195,9 +195,9 @@ void irq_set_thread_affinity(struct irq_desc *desc)
                        set_bit(IRQTF_AFFINITY, &action->thread_flags);
 }
 
+#ifdef CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK
 static void irq_validate_effective_affinity(struct irq_data *data)
 {
-#ifdef CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK
        const struct cpumask *m = irq_data_get_effective_affinity_mask(data);
        struct irq_chip *chip = irq_data_get_irq_chip(data);
 
@@ -205,9 +205,19 @@ static void irq_validate_effective_affinity(struct irq_data *data)
                return;
        pr_warn_once("irq_chip %s did not update eff. affinity mask of irq %u\n",
                     chip->name, data->irq);
-#endif
 }
 
+static inline void irq_init_effective_affinity(struct irq_data *data,
+                                              const struct cpumask *mask)
+{
+       cpumask_copy(irq_data_get_effective_affinity_mask(data), mask);
+}
+#else
+static inline void irq_validate_effective_affinity(struct irq_data *data) { }
+static inline void irq_init_effective_affinity(struct irq_data *data,
+                                              const struct cpumask *mask) { }
+#endif
+
 int irq_do_set_affinity(struct irq_data *data, const struct cpumask *mask,
                        bool force)
 {
@@ -304,6 +314,26 @@ static int irq_try_set_affinity(struct irq_data *data,
        return ret;
 }
 
+static bool irq_set_affinity_deactivated(struct irq_data *data,
+                                        const struct cpumask *mask, bool force)
+{
+       struct irq_desc *desc = irq_data_to_desc(data);
+
+       /*
+        * If the interrupt is not yet activated, just store the affinity
+        * mask and do not call the chip driver at all. On activation the
+        * driver has to make sure anyway that the interrupt is in a
+        * useable state so startup works.
+        */
+       if (!IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) || irqd_is_activated(data))
+               return false;
+
+       cpumask_copy(desc->irq_common_data.affinity, mask);
+       irq_init_effective_affinity(data, mask);
+       irqd_set(data, IRQD_AFFINITY_SET);
+       return true;
+}
+
 int irq_set_affinity_locked(struct irq_data *data, const struct cpumask *mask,
                            bool force)
 {
@@ -314,6 +344,9 @@ int irq_set_affinity_locked(struct irq_data *data, const struct cpumask *mask,
        if (!chip || !chip->irq_set_affinity)
                return -EINVAL;
 
+       if (irq_set_affinity_deactivated(data, mask, force))
+               return 0;
+
        if (irq_can_move_pcntxt(data) && !irqd_is_setaffinity_pending(data)) {
                ret = irq_try_set_affinity(data, mask, force);
        } else {