irq,xen: fix event channel masking on suspend/resume

PATCH In review Juergen Gross 4 participants 9 messages Jul 17, 2017 – Jul 27, 2017

Subsystems (2)

Xen hypervisor interface IRQ

lore

Share & tools

lore

Juergen Gross

author maintainer

2017-07-17 17:47 UTC #0 lore raw

cover irq,xen: fix event channel masking on suspend/resume

Commit bf22ff45bed664aefb5c4e43029057a199b7070c ("genirq: Avoid unnecessary low level irq function calls") broke Xen suspend/resume handling as Xen fiddled with masking/unmasking of event channels (or irqs) without letting the irq subsystem know about it.

Fix this by setting the correct states in irq and remove the masking from Xen.

Juergen Gross (2):
irq: adjust state of irq in resume_irq() when IRQF_FORCE_RESUME set
xen: dont fiddle with event channel masking in suspend/resume

 drivers/xen/events/events_base.c | 13 +++----------
 kernel/irq/chip.c                | 10 ----------
 kernel/irq/internals.h           | 10 ++++++++++
 kernel/irq/pm.c                  |  2 ++
 4 files changed, 15 insertions(+), 20 deletions(-)

Signature · 1 line

2.12.3

Juergen Gross

author maintainer

2017-07-17 17:47 UTC #1 lore raw

PATCH 1/2 irq: adjust state of irq in resume_irq() when IRQF_FORCE_RESUME set

An irq pretending it got disabled should have its state set accordingly. Otherwise a potential unmask later won't succeed.

Signed-off-by Juergen Gross <[email protected]>

---
 kernel/irq/chip.c      | 10 ----------
 kernel/irq/internals.h | 10 ++++++++++
 kernel/irq/pm.c        |  2 ++
 3 files changed, 12 insertions(+), 10 deletions(-)

kernel/irq/chip.c +0 ~~-10~~

index d171bc57e1e0..a3cc37c0c85e 100644
@@ -170,21 +170,11 @@ static void irq_state_clr_disabled(struct irq_desc *desc)
 	irqd_clear(&desc->irq_data, IRQD_IRQ_DISABLED);
 }
 
-static void irq_state_set_disabled(struct irq_desc *desc)
-{
-	irqd_set(&desc->irq_data, IRQD_IRQ_DISABLED);
-}
-
 static void irq_state_clr_masked(struct irq_desc *desc)
 {
 	irqd_clear(&desc->irq_data, IRQD_IRQ_MASKED);
 }
 
-static void irq_state_set_masked(struct irq_desc *desc)
-{
-	irqd_set(&desc->irq_data, IRQD_IRQ_MASKED);
-}
-
 static void irq_state_clr_started(struct irq_desc *desc)
 {
 	irqd_clear(&desc->irq_data, IRQD_IRQ_STARTED);

kernel/irq/internals.h +10 -0

index dbfba9933ed2..a2c48058354c 100644
@@ -227,6 +227,16 @@ static inline bool irqd_has_set(struct irq_data *d, unsigned int mask)
 	return __irqd_to_state(d) & mask;
 }
 
+static inline void irq_state_set_disabled(struct irq_desc *desc)
+{
+	irqd_set(&desc->irq_data, IRQD_IRQ_DISABLED);
+}
+
+static inline void irq_state_set_masked(struct irq_desc *desc)
+{
+	irqd_set(&desc->irq_data, IRQD_IRQ_MASKED);
+}
+
 #undef __irqd_to_state
 
 static inline void kstat_incr_irqs_this_cpu(struct irq_desc *desc)

kernel/irq/pm.c +2 -0

index cea1de0161f1..6bd9b58429cc 100644
@@ -149,6 +149,8 @@ static void resume_irq(struct irq_desc *desc)
 
 	/* Pretend that it got disabled ! */
 	desc->depth++;
+	irq_state_set_disabled(desc);
+	irq_state_set_masked(desc);
 resume:
 	desc->istate &= ~IRQS_SUSPENDED;
 	__enable_irq(desc);

Signature · 7 lines

2.12.3


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Juergen Gross

author maintainer

2017-07-17 17:47 UTC #2 lore raw

PATCH 1/2 irq: adjust state of irq in resume_irq() when IRQF_FORCE_RESUME set

In reply to Juergen Gross

An irq pretending it got disabled should have its state set accordingly. Otherwise a potential unmask later won't succeed.

Signed-off-by Juergen Gross <[email protected]>

---
 kernel/irq/chip.c      | 10 ----------
 kernel/irq/internals.h | 10 ++++++++++
 kernel/irq/pm.c        |  2 ++
 3 files changed, 12 insertions(+), 10 deletions(-)

kernel/irq/chip.c +0 ~~-10~~

index d171bc57e1e0..a3cc37c0c85e 100644
@@ -170,21 +170,11 @@ static void irq_state_clr_disabled(struct irq_desc *desc)
 	irqd_clear(&desc->irq_data, IRQD_IRQ_DISABLED);
 }
 
-static void irq_state_set_disabled(struct irq_desc *desc)
-{
-	irqd_set(&desc->irq_data, IRQD_IRQ_DISABLED);
-}
-
 static void irq_state_clr_masked(struct irq_desc *desc)
 {
 	irqd_clear(&desc->irq_data, IRQD_IRQ_MASKED);
 }
 
-static void irq_state_set_masked(struct irq_desc *desc)
-{
-	irqd_set(&desc->irq_data, IRQD_IRQ_MASKED);
-}
-
 static void irq_state_clr_started(struct irq_desc *desc)
 {
 	irqd_clear(&desc->irq_data, IRQD_IRQ_STARTED);

kernel/irq/internals.h +10 -0

index dbfba9933ed2..a2c48058354c 100644
@@ -227,6 +227,16 @@ static inline bool irqd_has_set(struct irq_data *d, unsigned int mask)
 	return __irqd_to_state(d) & mask;
 }
 
+static inline void irq_state_set_disabled(struct irq_desc *desc)
+{
+	irqd_set(&desc->irq_data, IRQD_IRQ_DISABLED);
+}
+
+static inline void irq_state_set_masked(struct irq_desc *desc)
+{
+	irqd_set(&desc->irq_data, IRQD_IRQ_MASKED);
+}
+
 #undef __irqd_to_state
 
 static inline void kstat_incr_irqs_this_cpu(struct irq_desc *desc)

kernel/irq/pm.c +2 -0

index cea1de0161f1..6bd9b58429cc 100644
@@ -149,6 +149,8 @@ static void resume_irq(struct irq_desc *desc)
 
 	/* Pretend that it got disabled ! */
 	desc->depth++;
+	irq_state_set_disabled(desc);
+	irq_state_set_masked(desc);
 resume:
 	desc->istate &= ~IRQS_SUSPENDED;
 	__enable_irq(desc);

Signature · 1 line

2.12.3

tip-bot for Juergen Gross

merge bot

2017-07-17 20:38 UTC #3 lore raw

[tip:irq/urgent] genirq/PM: Properly pretend disabled state when force resuming interrupts

genirq/PM: Properly pretend disabled state when force resuming interrupts

Commit-ID:  a696712c3dd54eb58d2c5a807b4aaa27782d80d6
Gitweb:     http://git.kernel.org/tip/a696712c3dd54eb58d2c5a807b4aaa27782d80d6
Author:     Juergen Gross <[email protected]>
AuthorDate: Mon, 17 Jul 2017 19:47:02 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 17 Jul 2017 22:32:20 +0200

genirq/PM: Properly pretend disabled state when force resuming interrupts

Interrupts with the IRQF_FORCE_RESUME flag set have also the IRQF_NO_SUSPEND flag set. They are not disabled in the suspend path, but must be forcefully resumed. That's used by XEN to keep IPIs enabled beyond the suspension of device irqs. Force resume works by pretending that the interrupt was disabled and then calling __irq_enable().

Incrementing the disabled depth counter was enough to do that, but with the recent changes which use state flags to avoid unnecessary hardware access, this is not longer sufficient. If the state flags are not set, then the hardware callbacks are not invoked and the interrupt line stays disabled in "hardware".

Set the disabled and masked state when pretending that an interrupt got disabled by suspend.

Fixes bf22ff45bed6 ("genirq: Avoid unnecessary low level irq function calls")

Signed-off-by Juergen Gross <[email protected]>

Signed-off-by Thomas Gleixner <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

---
 kernel/irq/chip.c      | 10 ----------
 kernel/irq/internals.h | 10 ++++++++++
 kernel/irq/pm.c        |  2 ++
 3 files changed, 12 insertions(+), 10 deletions(-)

kernel/irq/chip.c +0 ~~-10~~

index d171bc5..a3cc37c 100644
@@ -170,21 +170,11 @@ static void irq_state_clr_disabled(struct irq_desc *desc)
 	irqd_clear(&desc->irq_data, IRQD_IRQ_DISABLED);
 }
 
-static void irq_state_set_disabled(struct irq_desc *desc)
-{
-	irqd_set(&desc->irq_data, IRQD_IRQ_DISABLED);
-}
-
 static void irq_state_clr_masked(struct irq_desc *desc)
 {
 	irqd_clear(&desc->irq_data, IRQD_IRQ_MASKED);
 }
 
-static void irq_state_set_masked(struct irq_desc *desc)
-{
-	irqd_set(&desc->irq_data, IRQD_IRQ_MASKED);
-}
-
 static void irq_state_clr_started(struct irq_desc *desc)
 {
 	irqd_clear(&desc->irq_data, IRQD_IRQ_STARTED);

kernel/irq/internals.h +10 -0

index dbfba99..a2c4805 100644
@@ -227,6 +227,16 @@ static inline bool irqd_has_set(struct irq_data *d, unsigned int mask)
 	return __irqd_to_state(d) & mask;
 }
 
+static inline void irq_state_set_disabled(struct irq_desc *desc)
+{
+	irqd_set(&desc->irq_data, IRQD_IRQ_DISABLED);
+}
+
+static inline void irq_state_set_masked(struct irq_desc *desc)
+{
+	irqd_set(&desc->irq_data, IRQD_IRQ_MASKED);
+}
+
 #undef __irqd_to_state
 
 static inline void kstat_incr_irqs_this_cpu(struct irq_desc *desc)

kernel/irq/pm.c +2 -0

index cea1de0..6bd9b58 100644
@@ -149,6 +149,8 @@ static void resume_irq(struct irq_desc *desc)
 
 	/* Pretend that it got disabled ! */
 	desc->depth++;
+	irq_state_set_disabled(desc);
+	irq_state_set_masked(desc);
 resume:
 	desc->istate &= ~IRQS_SUSPENDED;
 	__enable_irq(desc);

Juergen Gross

author maintainer

2017-07-17 17:47 UTC #4 lore raw

PATCH 2/2 xen: dont fiddle with event channel masking in suspend/resume

In reply to Juergen Gross

Instead of fiddling with masking the event channels during suspend and resume handling let do the irq subsystem do its job. It will do the mask and unmask operations as needed.

Signed-off-by Juergen Gross <[email protected]>

---
 drivers/xen/events/events_base.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

drivers/xen/events/events_base.c +3 ~~-10~~

index b241bfa529ce..bae1f5d36c26 100644
@@ -343,14 +343,6 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
 	info->cpu = cpu;
 }
 
-static void xen_evtchn_mask_all(void)
-{
-	unsigned int evtchn;
-
-	for (evtchn = 0; evtchn < xen_evtchn_nr_channels(); evtchn++)
-		mask_evtchn(evtchn);
-}
-
 /**
  * notify_remote_via_irq - send event to remote end of event channel via irq
  * @irq: irq of event channel to send event to

@@ -1573,7 +1565,6 @@ void xen_irq_resume(void)
 	struct irq_info *info;
 
 	/* New event-channel space is not 'live' yet. */
-	xen_evtchn_mask_all();
 	xen_evtchn_resume();
 
 	/* No IRQ <-> event-channel mappings. */

@@ -1681,6 +1672,7 @@ module_param(fifo_events, bool, 0);
 void __init xen_init_IRQ(void)
 {
 	int ret = -EINVAL;
+	unsigned int evtchn;
 
 	if (fifo_events)
 		ret = xen_evtchn_fifo_init();

@@ -1692,7 +1684,8 @@ void __init xen_init_IRQ(void)
 	BUG_ON(!evtchn_to_irq);
 
 	/* No event channels are 'live' right now. */
-	xen_evtchn_mask_all();
+	for (evtchn = 0; evtchn < xen_evtchn_nr_channels(); evtchn++)
+		mask_evtchn(evtchn);
 
 	pirq_needs_eoi = pirq_needs_eoi_flag;

Signature · 7 lines

2.12.3


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Juergen Gross

author maintainer

2017-07-17 17:47 UTC #5 lore raw

PATCH 2/2 xen: dont fiddle with event channel masking in suspend/resume

In reply to Juergen Gross

Instead of fiddling with masking the event channels during suspend and resume handling let do the irq subsystem do its job. It will do the mask and unmask operations as needed.

Signed-off-by Juergen Gross <[email protected]>

---
 drivers/xen/events/events_base.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

drivers/xen/events/events_base.c +3 ~~-10~~

index b241bfa529ce..bae1f5d36c26 100644
@@ -343,14 +343,6 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
 	info->cpu = cpu;
 }
 
-static void xen_evtchn_mask_all(void)
-{
-	unsigned int evtchn;
-
-	for (evtchn = 0; evtchn < xen_evtchn_nr_channels(); evtchn++)
-		mask_evtchn(evtchn);
-}
-
 /**
  * notify_remote_via_irq - send event to remote end of event channel via irq
  * @irq: irq of event channel to send event to

@@ -1573,7 +1565,6 @@ void xen_irq_resume(void)
 	struct irq_info *info;
 
 	/* New event-channel space is not 'live' yet. */
-	xen_evtchn_mask_all();
 	xen_evtchn_resume();
 
 	/* No IRQ <-> event-channel mappings. */

@@ -1681,6 +1672,7 @@ module_param(fifo_events, bool, 0);
 void __init xen_init_IRQ(void)
 {
 	int ret = -EINVAL;
+	unsigned int evtchn;
 
 	if (fifo_events)
 		ret = xen_evtchn_fifo_init();

@@ -1692,7 +1684,8 @@ void __init xen_init_IRQ(void)
 	BUG_ON(!evtchn_to_irq);
 
 	/* No event channels are 'live' right now. */
-	xen_evtchn_mask_all();
+	for (evtchn = 0; evtchn < xen_evtchn_nr_channels(); evtchn++)
+		mask_evtchn(evtchn);
 
 	pirq_needs_eoi = pirq_needs_eoi_flag;

Signature · 1 line

2.12.3

Thomas Gleixner

Acked-by on v1 2/2

2017-07-18 08:06 UTC #6 lore raw

Re: [PATCH 2/2] xen: dont fiddle with event channel masking in suspend/resume

Quoted 5 lines · Juergen Gross

Instead of fiddling with masking the event channels during suspend and resume handling let do the irq subsystem do its job. It will do the mask and unmask operations as needed.
Signed-off-by Juergen Gross <[email protected]>

Acked-by Thomas Gleixner <[email protected]>

Thomas Gleixner

Acked-by on v1 2/2

2017-07-18 08:06 UTC #7 lore raw

Re: [PATCH 2/2] xen: dont fiddle with event channel masking in suspend/resume

In reply to Juergen Gross

Quoted 5 lines · Juergen Gross

Instead of fiddling with masking the event channels during suspend and resume handling let do the irq subsystem do its job. It will do the mask and unmask operations as needed.
Signed-off-by Juergen Gross <[email protected]>

Acked-by Thomas Gleixner <[email protected]>

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Boris Ostrovsky

Reviewed-by on v1 2/2

2017-07-27 14:29 UTC #8 lore raw

Re: [PATCH 2/2] xen: dont fiddle with event channel masking in suspend/resume

In reply to Juergen Gross

Quoted 5 lines · Juergen Gross

Instead of fiddling with masking the event channels during suspend and resume handling let do the irq subsystem do its job. It will do the mask and unmask operations as needed.
Signed-off-by Juergen Gross <[email protected]>

Reviewed-by Boris Ostrovsky <[email protected]>

(I've been running dumpdata with this for the last couple of days so you can add Tested-by as well if you wish)

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel