[PATCH 0/2] gpiolib: GPIO line irq_set_affinity helper

[PATCH 0/2] gpiolib: GPIO line irq_set_affinity helper

[Liang Hao]

Hi,

Several GPIO irqchips omit irq_chip->irq_set_affinity on line IRQs, so
user writes to /proc/irq/<gpio-line>/smp_affinity* fail. The helper
covers hierarchical domains with parent_data and chained gpio_irq_chips
that use parent_handler and parent virqs without parent_data on the line.

This series adds a shared helper so drivers can expose affinity on GPIO
line IRQs: the mask is applied on the parent interrupt, and the line
IRQ reflects the result for proc and validation. Product init or resume
scripts that restore per-line affinity depend on a line-level callback;
without it those writes never reach the parent.

Patch 1 adds gpiochip_irq_set_affinity() and GPIOCHIP_IRQ_AFFINITY_HELPERS:
hierarchical lines use irq_chip_set_affinity_parent(); chained lines
resolve the parent virq from gpio_irq_chip and call the parent
irq_chip->irq_set_affinity() directly. Unknown parent: -EINVAL.

Patch 2 enables the macro on gpio-dwapb (snps,dw-apb-gpio, DT chained).

Tested on arm64: sysfs GPIO export + edge, write child gpiolib virq
smp_affinity_list/smp_affinity — fails without the series, succeeds
and reads back with it.

Thanks,
Liang Hao

Liang Hao (2):
  gpiolib: Add irq_set_affinity helper for GPIO line IRQs
  gpio: dwapb: Use gpiolib irq_set_affinity helper

 drivers/gpio/gpio-dwapb.c   |  1 +
 drivers/gpio/gpiolib.c      | 79 +++++++++++++++++++++++++++++++++++++
 include/linux/gpio/driver.h |  5 +++
 3 files changed, 85 insertions(+)

-- 
2.54.0 (Apple Git-157)

[PATCH 1/2] gpiolib: Add irq_set_affinity helper for GPIO line IRQs

[Liang Hao]

GPIO line IRQs need irq_chip->irq_set_affinity so user space can steer
affinity to the upstream controller. Hierarchical domains use
parent_data; chained controllers (parent_handler, no parent_data on the
line) store parent virqs in gpio_irq_chip instead.

Add gpiochip_irq_set_affinity() and GPIOCHIP_IRQ_AFFINITY_HELPERS.
Hierarchical lines delegate via irq_chip_set_affinity_parent().
Chained lines resolve the parent virq and call the parent
irq_chip->irq_set_affinity() directly. Return -EINVAL if the parent
virq cannot be resolved.

Signed-off-by: Liang Hao <haohlliang@gmail.com>
---
 drivers/gpio/gpiolib.c      | 79 +++++++++++++++++++++++++++++++++++++
 include/linux/gpio/driver.h |  5 +++
 2 files changed, 84 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 2987051d9904..0a4626579963 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2104,6 +2104,85 @@ void gpiochip_irq_relres(struct irq_data *d)
 }
 EXPORT_SYMBOL(gpiochip_irq_relres);
 
+static unsigned int gpiochip_chained_parent_virq(struct gpio_chip *gc,
+						 struct irq_data *d)
+{
+	struct gpio_irq_chip *girq = &gc->irq;
+	unsigned int hwirq = irqd_to_hwirq(d);
+	struct irq_desc *desc;
+
+	if (!girq->parent_handler)
+		return 0;
+
+	if (!girq->initialized || !girq->num_parents || !girq->parents)
+		return 0;
+
+	if (girq->num_parents == 1)
+		return girq->parents[0];
+
+	if (girq->map) {
+		if (!gpiochip_irqchip_irq_valid(gc, hwirq))
+			return 0;
+		return girq->map[hwirq];
+	}
+
+	desc = irq_to_desc(d->irq);
+	if (desc && desc->parent_irq)
+		return desc->parent_irq;
+
+	return 0;
+}
+
+/**
+ * gpiochip_irq_set_affinity() - irq_chip set_affinity helper for GPIO lines
+ * @d: Pointer to the GPIO line irq_data
+ * @dest: Affinity mask to set
+ * @force: True to enforce the mask without online CPU checks
+ *
+ * Hierarchical irqdomains with parent_data delegate through
+ * irq_chip_set_affinity_parent(). GPIO irqchips that use a chained parent
+ * handler resolve the parent virq and apply the mask through the parent
+ * irq_chip as described in gpiochip_chained_parent_virq().
+ *
+ * Affinity is programmed on the parent irq, not independently per line. GPIO
+ * lines that share one parent irq also share its affinity; updating one line
+ * replaces the mask for all lines on that parent.
+ *
+ * Return: IRQ_SET_MASK_* on success, or a negative errno on error.
+ */
+int gpiochip_irq_set_affinity(struct irq_data *d,
+			      const struct cpumask *dest, bool force)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct irq_data *pdata;
+	unsigned int parent;
+	int ret;
+
+	if (d->parent_data)
+		return irq_chip_set_affinity_parent(d, dest, force);
+
+	parent = gpiochip_chained_parent_virq(gc, d);
+	if (!parent)
+		return -EINVAL;
+
+	pdata = irq_get_irq_data(parent);
+	if (!pdata || !pdata->chip || !pdata->chip->irq_set_affinity)
+		return -EINVAL;
+
+	ret = pdata->chip->irq_set_affinity(pdata, dest, force);
+	switch (ret) {
+	case IRQ_SET_MASK_OK:
+	case IRQ_SET_MASK_OK_NOCOPY:
+	case IRQ_SET_MASK_OK_DONE:
+		irq_data_update_effective_affinity(d,
+			irq_data_get_effective_affinity_mask(pdata));
+		return IRQ_SET_MASK_OK;
+	default:
+		return ret;
+	}
+}
+EXPORT_SYMBOL_GPL(gpiochip_irq_set_affinity);
+
 static void gpiochip_irq_mask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 4077dc678cae..a772bf1707aa 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -658,12 +658,17 @@ void gpiochip_enable_irq(struct gpio_chip *gc, unsigned int offset);
 /* irq_data versions of the above */
 int gpiochip_irq_reqres(struct irq_data *data);
 void gpiochip_irq_relres(struct irq_data *data);
+int gpiochip_irq_set_affinity(struct irq_data *data,
+			      const struct cpumask *dest, bool force);
 
 /* Paste this in your irq_chip structure  */
 #define	GPIOCHIP_IRQ_RESOURCE_HELPERS					\
 		.irq_request_resources  = gpiochip_irq_reqres,		\
 		.irq_release_resources  = gpiochip_irq_relres
 
+#define GPIOCHIP_IRQ_AFFINITY_HELPERS					\
+		.irq_set_affinity = gpiochip_irq_set_affinity
+
 static inline void gpio_irq_chip_set_chip(struct gpio_irq_chip *girq,
 					  const struct irq_chip *chip)
 {
-- 
2.54.0 (Apple Git-157)

Re: [PATCH 1/2] gpiolib: Add irq_set_affinity helper for GPIO line IRQs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] gpiolib: unlocked parent affinity update in gpiochip_irq_set_affinity()
- [Low] gpiolib: missing hierarchy Kconfig guard in gpiochip_irq_set_affinity()
--

commit 6be16d8d1664dad3beb504901b90bb4ff317a568
Author: Liang Hao <haohlliang@gmail.com>

gpiolib: Add irq_set_affinity helper for GPIO line IRQs

This commit adds gpiochip_irq_set_affinity() and a helper macro so GPIO line
interrupts can forward affinity updates to upstream interrupt controllers in
both hierarchical and chained irqchip setups.

> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 2987051d99043..0a46265799631 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
[ ... ]
> +int gpiochip_irq_set_affinity(struct irq_data *d,
> +			      const struct cpumask *dest, bool force)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +	struct irq_data *pdata;
> +	unsigned int parent;
> +	int ret;
> +
> +	if (d->parent_data)
> +		return irq_chip_set_affinity_parent(d, dest, force);

[Severity: Low]
Will this fail to build when hierarchical irqdomains are not enabled in the
kernel configuration?

In drivers/gpio/Kconfig, the gpiolib irqchip option only selects the base
irqdomain support:

drivers/gpio/Kconfig:
	config GPIOLIB_IRQCHIP
		select IRQ_DOMAIN
		bool

However, both the parent_data member in struct irq_data and the declaration of
irq_chip_set_affinity_parent() in include/linux/irq.h are guarded by the
hierarchy configuration option:

include/linux/irq.h:
	struct irq_data {
		...
	#ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
		struct irq_data		*parent_data;
	#endif
		void			*chip_data;
	};
	...
	#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
	...
	extern int irq_chip_set_affinity_parent(struct irq_data *data,
						const struct cpumask *dest,
						bool force);

Should the d->parent_data check and irq_chip_set_affinity_parent() call in
gpiochip_irq_set_affinity() be guarded by an #ifdef just like the other
hierarchical helpers in drivers/gpio/gpiolib.c?

> +
> +	parent = gpiochip_chained_parent_virq(gc, d);
> +	if (!parent)
> +		return -EINVAL;
> +
> +	pdata = irq_get_irq_data(parent);
> +	if (!pdata || !pdata->chip || !pdata->chip->irq_set_affinity)
> +		return -EINVAL;
> +
> +	ret = pdata->chip->irq_set_affinity(pdata, dest, force);
> +	switch (ret) {
> +	case IRQ_SET_MASK_OK:
> +	case IRQ_SET_MASK_OK_NOCOPY:
> +	case IRQ_SET_MASK_OK_DONE:
> +		irq_data_update_effective_affinity(d,
> +			irq_data_get_effective_affinity_mask(pdata));
> +		return IRQ_SET_MASK_OK;
> +	default:
> +		return ret;
> +	}
> +}

[Severity: High]
Is it safe to invoke pdata->chip->irq_set_affinity() directly on a chained
parent interrupt without holding the parent irq_desc lock or updating the
parent descriptor's affinity state?

Unlike hierarchical domains where d and d->parent_data belong to the same
struct irq_desc, a chained parent virq has a separate struct irq_desc with its
own lock and its own struct irq_common_data. When the genirq core invokes
gpiochip_irq_set_affinity() through irq_do_set_affinity(), only the child
GPIO line's desc->lock is held.

Calling pdata->chip->irq_set_affinity(pdata, dest, force) unlocked from
gpiochip_irq_set_affinity() appears to introduce several issues:

1. Concurrent affinity updates on sibling GPIO lines sharing the same parent
virq (or concurrent operations on the parent virq itself, such as CPU hotplug
migration in migrate_one_irq()) will race inside the parent irq_chip callback
and on pdata's effective_affinity mask. Upstream irqchip drivers assume their
own irq_desc->lock serializes irq_set_affinity(), for example in
drivers/irqchip/irq-gic-v3.c:gic_set_affinity():

	enabled = gic_peek_irq(d, GICD_ISENABLER);
	if (enabled)
		gic_mask_irq(d);
	...
	gic_write_irouter(val, reg);

	if (enabled)
		gic_unmask_irq(d);

	irq_data_update_effective_affinity(d, cpumask_of(cpu));

and in drivers/irqchip/irq-sifive-plic.c:plic_set_affinity():

	plic_irq_toggle(irq_data_get_effective_affinity_mask(d), d, 0);

	irq_data_update_effective_affinity(d, cpumask_of(cpu));

	if (!irqd_irq_disabled(d))
		plic_irq_toggle(irq_data_get_effective_affinity_mask(d), d, 1);

Without holding the parent descriptor's lock, concurrent calls can corrupt the
parent routing state or observe the parent temporarily masked and leave it
permanently disabled.

2. Because irq_do_set_affinity() only updates desc->irq_common_data.affinity
on the child descriptor:

kernel/irq/manage.c:irq_do_set_affinity()
	switch (ret) {
	case IRQ_SET_MASK_OK:
	case IRQ_SET_MASK_OK_DONE:
		cpumask_copy(desc->irq_common_data.affinity, mask);
		fallthrough;
	case IRQ_SET_MASK_OK_NOCOPY:
		irq_validate_effective_affinity(data);
		irq_set_thread_affinity(desc);
		ret = 0;
	}

bypassing irq_do_set_affinity() on the parent descriptor leaves the parent's
irq_common_data.affinity unmodified (and skips deferred affinity movement
checks). As a result, CPU hot-unplug in
kernel/irq/cpuhotplug.c:migrate_one_irq() will migrate the parent interrupt
using its stale affinity mask, and on configurations without effective
affinity masks, irq_data_get_effective_affinity_mask(pdata) will also return
that stale parent affinity mask:

include/linux/irq.h:irq_data_get_effective_affinity_mask()
	static inline
	const struct cpumask *irq_data_get_effective_affinity_mask(struct irq_data *d)
	{
		return irq_data_get_affinity_mask(d);
	}

3. When multiple GPIO lines share a chained parent virq, requesting and
starting up any sibling GPIO line later via
request_threaded_irq() -> __setup_irq() -> irq_startup() -> irq_setup_affinity()
will see a fresh child descriptor without a user-configured affinity mask and
call irq_do_set_affinity() with irq_default_affinity:

kernel/irq/manage.c:irq_setup_affinity()
	if (irqd_affinity_is_managed(&desc->irq_data) ||
	    irqd_has_set(&desc->irq_data, IRQD_AFFINITY_SET)) {
		if (cpumask_intersects(desc->irq_common_data.affinity,
				       cpu_online_mask))
			set = desc->irq_common_data.affinity;
		else
			irqd_clear(&desc->irq_data, IRQD_AFFINITY_SET);
	}
	...
	return irq_do_set_affinity(&desc->irq_data, &mask, false);

Won't starting a second GPIO line on the same parent virq silently reset the
shared parent interrupt's hardware affinity back to the default mask, leaving
any previously configured sibling GPIO line out of sync with the hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023151209.58772-1-haohlliang@gmail.com?part=1

Re: [PATCH 1/2] gpiolib: Add irq_set_affinity helper for GPIO line IRQs

[kernel test robot]

Hi Liang,

kernel test robot noticed the following build errors:

[auto build test ERROR on brgl/gpio/for-next]
[also build test ERROR on linus/master v7.3 next-20261022]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Liang-Hao/gpiolib-Add-irq_set_affinity-helper-for-GPIO-line-IRQs/20261023-231104
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20261023151209.58772-2-haohlliang%40gmail.com
patch subject: [PATCH 1/2] gpiolib: Add irq_set_affinity helper for GPIO line IRQs
config: mips-ci20_defconfig (https://download.01.org/0day-ci/archive/20261024/202610240908.4J3PwdPn-lkp@intel.com/config)
compiler: clang version 24.0.0git (https://github.com/llvm/llvm-project 186e70f4e6a1521991d2340d3d49b23b1beff11f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261024/202610240908.4J3PwdPn-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610240908.4J3PwdPn-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpio/gpiolib.c:2154:9: error: no member named 'parent_data' in 'struct irq_data'
    2154 |         if (d->parent_data)
         |             ~  ^
>> drivers/gpio/gpiolib.c:2155:10: error: call to undeclared function 'irq_chip_set_affinity_parent'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    2155 |                 return irq_chip_set_affinity_parent(d, dest, force);
         |                        ^
   drivers/gpio/gpiolib.c:2155:10: note: did you mean 'irq_set_affinity_hint'?
   include/linux/interrupt.h:407:19: note: 'irq_set_affinity_hint' declared here
     407 | static inline int irq_set_affinity_hint(unsigned int irq,
         |                   ^
   2 errors generated.


vim +2154 drivers/gpio/gpiolib.c

  2128	
  2129	/**
  2130	 * gpiochip_irq_set_affinity() - irq_chip set_affinity helper for GPIO lines
  2131	 * @d: Pointer to the GPIO line irq_data
  2132	 * @dest: Affinity mask to set
  2133	 * @force: True to enforce the mask without online CPU checks
  2134	 *
  2135	 * Hierarchical irqdomains with parent_data delegate through
  2136	 * irq_chip_set_affinity_parent(). GPIO irqchips that use a chained parent
  2137	 * handler resolve the parent virq and apply the mask through the parent
  2138	 * irq_chip as described in gpiochip_chained_parent_virq().
  2139	 *
  2140	 * Affinity is programmed on the parent irq, not independently per line. GPIO
  2141	 * lines that share one parent irq also share its affinity; updating one line
  2142	 * replaces the mask for all lines on that parent.
  2143	 *
  2144	 * Return: IRQ_SET_MASK_* on success, or a negative errno on error.
  2145	 */
  2146	int gpiochip_irq_set_affinity(struct irq_data *d,
  2147				      const struct cpumask *dest, bool force)
  2148	{
  2149		struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
  2150		struct irq_data *pdata;
  2151		unsigned int parent;
  2152		int ret;
  2153	
> 2154		if (d->parent_data)
> 2155			return irq_chip_set_affinity_parent(d, dest, force);
  2156	
  2157		parent = gpiochip_chained_parent_virq(gc, d);
  2158		if (!parent)
  2159			return -EINVAL;
  2160	
  2161		pdata = irq_get_irq_data(parent);
  2162		if (!pdata || !pdata->chip || !pdata->chip->irq_set_affinity)
  2163			return -EINVAL;
  2164	
  2165		ret = pdata->chip->irq_set_affinity(pdata, dest, force);
  2166		switch (ret) {
  2167		case IRQ_SET_MASK_OK:
  2168		case IRQ_SET_MASK_OK_NOCOPY:
  2169		case IRQ_SET_MASK_OK_DONE:
  2170			irq_data_update_effective_affinity(d,
  2171				irq_data_get_effective_affinity_mask(pdata));
  2172			return IRQ_SET_MASK_OK;
  2173		default:
  2174			return ret;
  2175		}
  2176	}
  2177	EXPORT_SYMBOL_GPL(gpiochip_irq_set_affinity);
  2178	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/2] gpiolib: Add irq_set_affinity helper for GPIO line IRQs

[kernel test robot]

Hi Liang,

kernel test robot noticed the following build errors:

[auto build test ERROR on brgl/gpio/for-next]
[also build test ERROR on linus/master v7.3 next-20261022]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Liang-Hao/gpiolib-Add-irq_set_affinity-helper-for-GPIO-line-IRQs/20261023-231104
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20261023151209.58772-2-haohlliang%40gmail.com
patch subject: [PATCH 1/2] gpiolib: Add irq_set_affinity helper for GPIO line IRQs
config: microblaze-defconfig (https://download.01.org/0day-ci/archive/20261024/202610241003.gRosyohv-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 16.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261024/202610241003.gRosyohv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610241003.gRosyohv-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpio/gpiolib.c: In function 'gpiochip_irq_set_affinity':
>> drivers/gpio/gpiolib.c:2154:14: error: 'struct irq_data' has no member named 'parent_data'
    2154 |         if (d->parent_data)
         |              ^~
>> drivers/gpio/gpiolib.c:2155:24: error: implicit declaration of function 'irq_chip_set_affinity_parent'; did you mean 'irq_set_affinity_hint'? [-Wimplicit-function-declaration]
    2155 |                 return irq_chip_set_affinity_parent(d, dest, force);
         |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                        irq_set_affinity_hint


vim +2154 drivers/gpio/gpiolib.c

  2128	
  2129	/**
  2130	 * gpiochip_irq_set_affinity() - irq_chip set_affinity helper for GPIO lines
  2131	 * @d: Pointer to the GPIO line irq_data
  2132	 * @dest: Affinity mask to set
  2133	 * @force: True to enforce the mask without online CPU checks
  2134	 *
  2135	 * Hierarchical irqdomains with parent_data delegate through
  2136	 * irq_chip_set_affinity_parent(). GPIO irqchips that use a chained parent
  2137	 * handler resolve the parent virq and apply the mask through the parent
  2138	 * irq_chip as described in gpiochip_chained_parent_virq().
  2139	 *
  2140	 * Affinity is programmed on the parent irq, not independently per line. GPIO
  2141	 * lines that share one parent irq also share its affinity; updating one line
  2142	 * replaces the mask for all lines on that parent.
  2143	 *
  2144	 * Return: IRQ_SET_MASK_* on success, or a negative errno on error.
  2145	 */
  2146	int gpiochip_irq_set_affinity(struct irq_data *d,
  2147				      const struct cpumask *dest, bool force)
  2148	{
  2149		struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
  2150		struct irq_data *pdata;
  2151		unsigned int parent;
  2152		int ret;
  2153	
> 2154		if (d->parent_data)
> 2155			return irq_chip_set_affinity_parent(d, dest, force);
  2156	
  2157		parent = gpiochip_chained_parent_virq(gc, d);
  2158		if (!parent)
  2159			return -EINVAL;
  2160	
  2161		pdata = irq_get_irq_data(parent);
  2162		if (!pdata || !pdata->chip || !pdata->chip->irq_set_affinity)
  2163			return -EINVAL;
  2164	
  2165		ret = pdata->chip->irq_set_affinity(pdata, dest, force);
  2166		switch (ret) {
  2167		case IRQ_SET_MASK_OK:
  2168		case IRQ_SET_MASK_OK_NOCOPY:
  2169		case IRQ_SET_MASK_OK_DONE:
  2170			irq_data_update_effective_affinity(d,
  2171				irq_data_get_effective_affinity_mask(pdata));
  2172			return IRQ_SET_MASK_OK;
  2173		default:
  2174			return ret;
  2175		}
  2176	}
  2177	EXPORT_SYMBOL_GPL(gpiochip_irq_set_affinity);
  2178	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 2/2] gpio: dwapb: Use gpiolib irq_set_affinity helper

[Liang Hao]

Wire GPIOCHIP_IRQ_AFFINITY_HELPERS into dwapb_irq_chip.

Signed-off-by: Liang Hao <haohlliang@gmail.com>
---
 drivers/gpio/gpio-dwapb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index aa7c08e60707..9e63c3ecb5b4 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -412,6 +412,7 @@ static const struct irq_chip dwapb_irq_chip = {
 	.irq_enable	= dwapb_irq_enable,
 	.irq_disable	= dwapb_irq_disable,
 	.irq_set_wake	= pm_sleep_ptr(dwapb_irq_set_wake),
+	GPIOCHIP_IRQ_AFFINITY_HELPERS,
 	.flags		= IRQCHIP_IMMUTABLE,
 	GPIOCHIP_IRQ_RESOURCE_HELPERS,
 };
-- 
2.54.0 (Apple Git-157)

Re: [PATCH 2/2] gpio: dwapb: Use gpiolib irq_set_affinity helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] gpio: dwapb: broken irq_set_affinity on multi-IRQ DT and ACPI platforms
- [High] gpio: unlocked parent call and state desync in gpiochip_irq_set_affinity()
- [Low] gpio: unguarded parent_data access in gpiochip_irq_set_affinity()
--

commit 72a8fcde432031570e3b16d2b7058ce326e44df1
Author: Liang Hao <haohlliang@gmail.com>

gpio: dwapb: Use gpiolib irq_set_affinity helper

This commit wires GPIOCHIP_IRQ_AFFINITY_HELPERS into dwapb_irq_chip so that
dwapb GPIO interrupts use the gpiolib irq_set_affinity helper.

> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index aa7c08e607074..9e63c3ecb5b45 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -412,6 +412,7 @@ static const struct irq_chip dwapb_irq_chip = {
>  	.irq_enable	= dwapb_irq_enable,
>  	.irq_disable	= dwapb_irq_disable,
>  	.irq_set_wake	= pm_sleep_ptr(dwapb_irq_set_wake),
> +	GPIOCHIP_IRQ_AFFINITY_HELPERS,

[Severity: High]
Does wiring GPIOCHIP_IRQ_AFFINITY_HELPERS unconditionally into dwapb_irq_chip
cause irq_set_affinity to fail with -EINVAL on multi-IRQ Device Tree
platforms and ACPI platforms?

In drivers/gpio/gpio-dwapb.c, dwapb_convert_irqs() compacts non-zero per-line
parent IRQs into pirq->irq[]:

drivers/gpio/gpio-dwapb.c:dwapb_convert_irqs() {
	for (i = 0; i < pp->ngpio; ++i) {
		if (!pp->irq[i])
			continue;

		pirq->irq[pirq->nr_irqs++] = pp->irq[i];
	}
}

And dwapb_configure_irqs() sets up girq differently depending on whether
there is an ACPI companion or Device Tree interrupts:

drivers/gpio/gpio-dwapb.c:dwapb_configure_irqs() {
	if (has_acpi_companion(gpio->dev)) {
		girq->num_parents = 0;
		girq->parents = NULL;
		girq->parent_handler = NULL;

		err = devm_request_irq(gpio->dev, pp->irq[0],
				       dwapb_irq_handler_mfd,
				       IRQF_SHARED, DWAPB_DRIVER_NAME, gpio);
		...
	} else {
		girq->num_parents = pirq->nr_irqs;
		girq->parents = pirq->irq;
		girq->parent_handler_data = gpio;
		girq->parent_handler = dwapb_irq_handler;
	}
}

On Device Tree platforms with multiple parent interrupts (where
pirq->nr_irqs > 1, such as platforms wiring dedicated per-GPIO interrupts on
Port A), girq->map is left NULL. In drivers/gpio/gpiolib.c,
gpiochip_irq_map() only calls irq_set_parent() when gc->irq.num_parents == 1
or gc->irq.map is non-NULL, leaving desc->parent_irq == 0:

drivers/gpio/gpiolib.c:gpiochip_irq_map() {
	if (gc->irq.num_parents == 1)
		ret = irq_set_parent(irq, gc->irq.parents[0]);
	else if (gc->irq.map)
		ret = irq_set_parent(irq, gc->irq.map[hwirq]);
}

When gpiochip_irq_set_affinity() -> gpiochip_chained_parent_virq() runs,
girq->num_parents != 1, girq->map is NULL, and desc->parent_irq is 0, so
gpiochip_chained_parent_virq() returns 0 and gpiochip_irq_set_affinity()
returns -EINVAL. Furthermore, if per-line parent IRQs are sparsely populated
such that only a single line j > 0 has a valid parent IRQ,
dwapb_convert_irqs() compacts it into pirq->irq[0] with pirq->nr_irqs == 1,
causing gpiochip_chained_parent_virq() to return line j's parent virq for any
GPIO line on Port A.

On ACPI platforms (has_acpi_companion(gpio->dev)), girq->parent_handler is
NULL, so gpiochip_chained_parent_virq() also immediately returns 0:

drivers/gpio/gpiolib.c:gpiochip_chained_parent_virq() {
	if (!girq->parent_handler)
		return 0;
	...
	if (girq->num_parents == 1)
		return girq->parents[0];

	if (girq->map) {
		if (!gpiochip_irqchip_irq_valid(gc, hwirq))
			return 0;
		return girq->map[hwirq];
	}

	desc = irq_to_desc(d->irq);
	if (desc && desc->parent_irq)
		return desc->parent_irq;

	return 0;
}

Because dwapb_irq_chip.irq_set_affinity is now non-NULL,
__irq_can_set_affinity() and irq_can_set_affinity_usr() evaluate to true.
Won't this cause irq_startup() -> irq_setup_affinity() to fail with -EINVAL
when requesting any GPIO IRQ, migrate_one_irq() in kernel/irq/cpuhotplug.c to
log ratelimited warnings during CPU hot-unplug, and writes to
/proc/irq/<N>/smp_affinity to fail on these platforms?

[Severity: High]
Can calling pdata->chip->irq_set_affinity() through
gpiochip_irq_set_affinity() on a chained dwapb irq_chip race on the parent
irq_chip without parent_desc->lock held and desynchronize parent and sibling
IRQ affinity state?

For chained dwapb GPIO interrupts, d->parent_data is NULL and the parent is a
separate virq with its own struct irq_desc. When the genirq core invokes
dwapb_irq_chip.irq_set_affinity, only the child GPIO descriptor lock is held,
yet gpiochip_irq_set_affinity() directly calls into the parent chip:

drivers/gpio/gpiolib.c:gpiochip_irq_set_affinity() {
	pdata = irq_get_irq_data(parent);
	if (!pdata || !pdata->chip || !pdata->chip->irq_set_affinity)
		return -EINVAL;

	ret = pdata->chip->irq_set_affinity(pdata, dest, force);
	switch (ret) {
	case IRQ_SET_MASK_OK:
	case IRQ_SET_MASK_OK_NOCOPY:
	case IRQ_SET_MASK_OK_DONE:
		irq_data_update_effective_affinity(d,
			irq_data_get_effective_affinity_mask(pdata));
		return IRQ_SET_MASK_OK;
}

Parent irq_chip callbacks such as gic_set_affinity() in
drivers/irqchip/irq-gic-v3.c or plic_set_affinity() in
drivers/irqchip/irq-sifive-plic.c expect their own parent_desc->lock to be
held while masking, updating routing registers, unmasking, and updating
pdata->common->effective_affinity. If two GPIO lines sharing a parent IRQ
update affinity concurrently (or race with operations on the parent virq),
can this corrupt hardware routing state and effective_affinity masks?

In addition, because calling pdata->chip->irq_set_affinity() directly
bypasses irq_do_set_affinity() on parent_desc,
parent_desc->irq_common_data.affinity and IRQD_AFFINITY_SET are never updated
on the parent descriptor. And when any sibling GPIO line sharing the same
parent IRQ is later started via request_threaded_irq() -> irq_startup() ->
irq_setup_affinity() in kernel/irq/manage.c:

kernel/irq/manage.c:irq_setup_affinity() {
	struct cpumask *set = irq_default_affinity;
	...
	if (!__irq_can_set_affinity(desc))
		return 0;
	...
	return irq_do_set_affinity(&desc->irq_data, &mask, false);
}

won't starting that second GPIO line overwrite the shared parent IRQ's
hardware routing back to irq_default_affinity while leaving the first GPIO
line's descriptor reporting stale affinity masks?

[Severity: Low]
Will gpiochip_irq_set_affinity() in drivers/gpio/gpiolib.c fail to build when
CONFIG_GPIOLIB_IRQCHIP is enabled without CONFIG_IRQ_DOMAIN_HIERARCHY?

In include/linux/irq.h, parent_data in struct irq_data and
irq_chip_set_affinity_parent() are only declared when
CONFIG_IRQ_DOMAIN_HIERARCHY is enabled:

include/linux/irq.h:struct irq_data {
#ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
	struct irq_data		*parent_data;
#endif
}

However, gpiochip_irq_set_affinity() accesses d->parent_data and calls
irq_chip_set_affinity_parent() without a CONFIG_IRQ_DOMAIN_HIERARCHY guard:

drivers/gpio/gpiolib.c:gpiochip_irq_set_affinity() {
	if (d->parent_data)
		return irq_chip_set_affinity_parent(d, dest, force);
}

Since CONFIG_GPIO_DWAPB and CONFIG_GPIOLIB_IRQCHIP do not select
CONFIG_IRQ_DOMAIN_HIERARCHY, should this block in gpiochip_irq_set_affinity()
be guarded with CONFIG_IRQ_DOMAIN_HIERARCHY like the other parent_data users
in drivers/gpio/gpiolib.c and drivers/gpio/gpio-dwapb.c?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023151209.58772-1-haohlliang@gmail.com?part=2