x86/irq: Do not touch IRQ chip_data if it does not belong to x86_vector_domain
PATCH 1/1 x86/irq: Do not touch IRQ chip_data if it does not belong to x86_vector_domain
When a CPU is about to be offlined we call fixup_irqs() that resets IRQ affinities related to the CPU in question. The same thing is also done when the system is suspended to S-states like S3 (mem).
For each IRQ we try to complete any on-going move regardless whether the IRQ is actually part of x86_vector_domain. For each IRQ descriptor we fetch its chip_data, assume it is of type struct apic_chip_data and manipulate it by clearing old_domain mask etc. For irq_chips that are not part of the x86_vector_domain, like those created by various GPIO drivers, will find their chip_data being changed unexpectly.
Below is an example where GPIO chip owned by pinctrl-sunrisepoint.c gets corrupted after resume:
# cat /sys/kernel/debug/gpio gpiochip0: GPIOs 360-511, parent: platform/INT344B:00, INT344B:00: gpio-511 ( |sysfs ) in hi
# rtcwake -s10 -mmem <10 seconds passes>
# cat /sys/kernel/debug/gpio gpiochip0: GPIOs 360-511, parent: platform/INT344B:00, INT344B:00: gpio-511 ( |sysfs ) in ?
Note '?' in the output. It means the struct gpio_chip ->get function is NULL whereas before suspend it was there.
Fix this by first checking that the IRQ belongs to x86_vector_domain before we try to use the chip_data as struct apic_chip_data.
--- arch/x86/kernel/apic/vector.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
arch/x86/kernel/apic/vector.c +20 -3
@@ -661,11 +661,28 @@ void irq_complete_move(struct irq_cfg *cfg)*/ void irq_force_complete_move (struct irq_desc *desc) { -struct irq_data *irqdata =irq_desc_get_irq_data (desc); -struct apic_chip_data *data =apic_chip_data (irqdata); -struct irq_cfg *cfg = data ? &data->cfg :NULL ; +struct irq_data *irqdata; +struct apic_chip_data *data; +struct irq_cfg *cfg;unsigned int cpu; +/* +* The function is called for all descriptors regardless of which +* irqdomain they belong to. For example if an IRQ is provided by +* an irq_chip as part of a GPIO driver, the chip data for that +* descriptor is specific to the irq_chip in question. +* +* Check first that the chip_data is what we expect +* (apic_chip_data) before touching it any further. +*/ + irqdata =irq_domain_get_irq_data (x86_vector_domain, +irq_desc_get_irq (desc)); +if (!irqdata) +return ; + + data =apic_chip_data (irqdata); + cfg = data ? &data->cfg :NULL ; +if (!cfg)return ;
Signature · 1 line
2.9.3
Hi Mika,
Quoted 25 lines · Mika Westerberg
When a CPU is about to be offlined we call
fixup_irqs()that resets IRQ affinities related to the CPU in question. The same thing is also done when the system is suspended to S-states like S3 (mem).For each IRQ we try to complete any on-going move regardless whether the IRQ is actually part of
x86_vector_domain. For each IRQ descriptor we fetch itschip_data, assume it is of type structapic_chip_dataand manipulate it by clearingold_domainmask etc. Forirq_chipsthat are not part of thex86_vector_domain, like those created by various GPIO drivers, will find theirchip_databeing changed unexpectly.Below is an example where GPIO chip owned by
pinctrl-sunrisepoint.cgets corrupted after resume:# cat /sys/kernel/debug/gpio gpiochip0: GPIOs 360-511, parent: platform/INT344B:00, INT344B:00: gpio-511 ( |sysfs ) in hi# rtcwake -s10 -mmem <10 seconds passes># cat /sys/kernel/debug/gpio gpiochip0: GPIOs 360-511, parent: platform/INT344B:00, INT344B:00: gpio-511 ( |sysfs ) in ?
Quoted 8 lines
Note '?' in the output. It means the struct
gpio_chip->get function is NULL whereas before suspend it was there.Fix this by first checking that the IRQ belongs to
x86_vector_domainbefore we try to use thechip_dataas structapic_chip_data.Reported-by Sakari Ailus <[email protected]>Signed-off-by Mika Westerberg <[email protected]>
Thanks for debugging this! I've tested it on the laptop where the SD card is no longer detected after suspend; with this patch it works fine.
Signature · 2 lines
Sakari Ailus [email protected]
tip-bot for Mika Westerberg
merge bot[tip:x86/urgent] x86/irq: Prevent force migration of irqs which are not in the vector domain
In reply to Mika Westerbergx86/irq: Prevent force migration of irqs which are not in the vector domain
Commit-ID: db91aa793ff984ac048e199ea1c54202543952fe Gitweb: http://git.kernel.org/tip/db91aa793ff984ac048e199ea1c54202543952fe Author: Mika Westerberg <[email protected]> AuthorDate: Mon, 3 Oct 2016 13:17:08 +0300 Committer: Thomas Gleixner <[email protected]> CommitDate: Tue, 4 Oct 2016 13:13:47 +0200
x86/irq: Prevent force migration of irqs which are not in the vector domain
When a CPU is about to be offlined we call fixup_irqs() that resets IRQ affinities related to the CPU in question. The same thing is also done when the system is suspended to S-states like S3 (mem).
For each IRQ we try to complete any on-going move regardless whether the IRQ is actually part of x86_vector_domain. For each IRQ descriptor we fetch its chip_data, assume it is of type struct apic_chip_data and manipulate it by clearing old_domain mask etc. For irq_chips that are not part of the x86_vector_domain, like those created by various GPIO drivers, will find their chip_data being changed unexpectly.
Below is an example where GPIO chip owned by pinctrl-sunrisepoint.c gets corrupted after resume:
# cat /sys/kernel/debug/gpio gpiochip0: GPIOs 360-511, parent: platform/INT344B:00, INT344B:00: gpio-511 ( |sysfs ) in hi
# rtcwake -s10 -mmem <10 seconds passes>
# cat /sys/kernel/debug/gpio gpiochip0: GPIOs 360-511, parent: platform/INT344B:00, INT344B:00: gpio-511 ( |sysfs ) in ?
Note '?' in the output. It means the struct gpio_chip ->get function is NULL whereas before suspend it was there.
Fix this by first checking that the IRQ belongs to x86_vector_domain before we try to use the chip_data as struct apic_chip_data.
--- arch/x86/kernel/apic/vector.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
arch/x86/kernel/apic/vector.c +20 -3
@@ -661,11 +661,28 @@ void irq_complete_move(struct irq_cfg *cfg)*/ void irq_force_complete_move (struct irq_desc *desc) { -struct irq_data *irqdata =irq_desc_get_irq_data (desc); -struct apic_chip_data *data =apic_chip_data (irqdata); -struct irq_cfg *cfg = data ? &data->cfg :NULL ; +struct irq_data *irqdata; +struct apic_chip_data *data; +struct irq_cfg *cfg;unsigned int cpu; +/* +* The function is called for all descriptors regardless of which +* irqdomain they belong to. For example if an IRQ is provided by +* an irq_chip as part of a GPIO driver, the chip data for that +* descriptor is specific to the irq_chip in question. +* +* Check first that the chip_data is what we expect +* (apic_chip_data) before touching it any further. +*/ + irqdata =irq_domain_get_irq_data (x86_vector_domain, +irq_desc_get_irq (desc)); +if (!irqdata) +return ; + + data =apic_chip_data (irqdata); + cfg = data ? &data->cfg :NULL ; +if (!cfg)return ;