All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1
@ 2021-04-08 15:43 ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Hi folks!

This is the spiritual successor to [1], which was over 6 years ago (!).

Background
==========

GIC mechanics
+++++++++++++

There are three IRQ operations:
o Acknowledge. This gives us the IRQ number that interrupted us, and also
  - raises the running priority of the CPU interface to that of the IRQ
  - sets the active bit of the IRQ
o Priority Drop. This "clears" the running priority.
o Deactivate. This clears the active bit of the IRQ.

o The CPU interface has a running priority value. No interrupt of lower or
  equal priority will be signaled to the CPU attached to that interface. On
  Linux, we only have two priority values: pNMIs at highest priority, and
  everything else at the other priority.
o Most GIC interrupts have an "active" bit. This bit is set on Acknowledge
  and cleared on Deactivate. A given interrupt cannot be re-signaled to a
  CPU if it has its active bit set (i.e. if it "fires" again while it's
  being handled).

EOImode fun
+++++++++++

In EOImode=0, Priority Drop and Deactivate are undissociable. The
(simplified) interrupt handling flow is as follows: 

  <~IRQ>
    Acknowledge
    Priority Drop + Deactivate
    <interrupts can once again be signaled, once interrupts are re-enabled>

With EOImode=1, we can invoke each operation individually. This gives us:

  <~IRQ>
    Acknowledge
    Priority Drop
    <*other* interrupts can be signaled from here, once interrupts are re-enabled>
    Deactivate
    <*this* interrupt can be signaled again>

What this means is that with EOImode=1, any interrupt is kept "masked" by
its active bit between Priority Drop and Deactivate.

Threaded IRQs and ONESHOT
=========================

ONESHOT threaded IRQs must remain masked between the main handler and the
threaded handler. Right now we do this using the conventional irq_mask()
operations, which looks like this: 

 <irq handler>
   Acknowledge
   Priority Drop   
   irq_mask()
   Deactivate

 <threaded handler>
   irq_unmask()

However, masking for the GICs means poking the distributor, and there's no
sysreg for that - it's an MMIO access. We've seen above that our IRQ
handling can give us masking "for free", and this is what this patch set is
all about. It turns the above handling into:

  <irq handler>
    Acknowledge
    Priority Drop

  <threaded handler>
    Deactivate

No irq_mask() => fewer MMIO accesses => happier users (or so I've been
told). This is especially relevant to PREEMPT_RT which forces threaded
IRQs.
    
Testing
=======

GICv2
+++++

I've tested this on my Juno with forced irqthreads. This makes the pl011
IRQ into a threaded ONESHOT IRQ, so I spammed my keyboard into the console
and verified via ftrace that there were no irq_mask() / irq_unmask()
involved.

GICv3
+++++

I've tested this on my Ampere eMAG, which uncovered "fun" interactions with
the MSI domains. Did the same trick as the Juno with the pl011.

pNMIs cause said eMAG to freeze, but that's true even without my patches. I
did try them out under QEMU+KVM and that looked fine, although that means I
only got to test EOImode=0. I'll try to dig into this when I get some more
cycles.

TODOs
=====

MSI hierarchy story
+++++++++++++++++++

I 'hate' patch 8/10 because it's so very easy to break. See changelog for
my thoughts on the thing.

Performance numbers
+++++++++++++++++++

The whole point of this was to shave off some time spent issuing requests
to the distributor. On my Juno the IRQ for eth0 gets forcefully threaded
and thus becomes a ONESHOT threaded IRQ - I've tried to test this with
netperf, but the delta is lost in the noise (and there is quite a lot of
noise, unfortunately).

I'm still looking for something I can benchmark on the eMAG to get some
GICv3 results.

Links
=====

[1]: https://lore.kernel.org/lkml/1414235215-10468-1-git-send-email-marc.zyngier@arm.com/

Valentin Schneider (10):
  genirq: Add chip flag to denote automatic IRQ (un)masking
  genirq: Define irq_ack() and irq_eoi() helpers
  genirq: Employ ack_irq() and eoi_irq() where relevant
  genirq: Add handle_strict_flow_irq() flow handler
  genirq: Let purely flow-masked ONESHOT irqs through
    unmask_threaded_irq()
  genirq: Don't mask IRQ within flow handler if IRQ is flow-masked
  genirq, irq-gic-v3: Make NMI flow handlers use ->irq_ack() if
    available
  irqchip/gic-v3-its: Use irq_chip_ack_parent()
  irqchip/gic: Convert to handle_strict_flow_irq()
  irqchip/gic-v3: Convert to handle_strict_flow_irq()

 drivers/irqchip/irq-gic-v3-its-pci-msi.c |   1 +
 drivers/irqchip/irq-gic-v3-its.c         |   1 +
 drivers/irqchip/irq-gic-v3.c             |  27 ++++--
 drivers/irqchip/irq-gic.c                |  14 ++-
 include/linux/irq.h                      |  15 ++-
 kernel/irq/chip.c                        | 117 +++++++++++++++++++----
 kernel/irq/debugfs.c                     |   2 +
 kernel/irq/internals.h                   |   7 ++
 kernel/irq/manage.c                      |   2 +-
 9 files changed, 154 insertions(+), 32 deletions(-)

--
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [RFC PATCH 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1
@ 2021-04-08 15:43 ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Hi folks!

This is the spiritual successor to [1], which was over 6 years ago (!).

Background
==========

GIC mechanics
+++++++++++++

There are three IRQ operations:
o Acknowledge. This gives us the IRQ number that interrupted us, and also
  - raises the running priority of the CPU interface to that of the IRQ
  - sets the active bit of the IRQ
o Priority Drop. This "clears" the running priority.
o Deactivate. This clears the active bit of the IRQ.

o The CPU interface has a running priority value. No interrupt of lower or
  equal priority will be signaled to the CPU attached to that interface. On
  Linux, we only have two priority values: pNMIs at highest priority, and
  everything else at the other priority.
o Most GIC interrupts have an "active" bit. This bit is set on Acknowledge
  and cleared on Deactivate. A given interrupt cannot be re-signaled to a
  CPU if it has its active bit set (i.e. if it "fires" again while it's
  being handled).

EOImode fun
+++++++++++

In EOImode=0, Priority Drop and Deactivate are undissociable. The
(simplified) interrupt handling flow is as follows: 

  <~IRQ>
    Acknowledge
    Priority Drop + Deactivate
    <interrupts can once again be signaled, once interrupts are re-enabled>

With EOImode=1, we can invoke each operation individually. This gives us:

  <~IRQ>
    Acknowledge
    Priority Drop
    <*other* interrupts can be signaled from here, once interrupts are re-enabled>
    Deactivate
    <*this* interrupt can be signaled again>

What this means is that with EOImode=1, any interrupt is kept "masked" by
its active bit between Priority Drop and Deactivate.

Threaded IRQs and ONESHOT
=========================

ONESHOT threaded IRQs must remain masked between the main handler and the
threaded handler. Right now we do this using the conventional irq_mask()
operations, which looks like this: 

 <irq handler>
   Acknowledge
   Priority Drop   
   irq_mask()
   Deactivate

 <threaded handler>
   irq_unmask()

However, masking for the GICs means poking the distributor, and there's no
sysreg for that - it's an MMIO access. We've seen above that our IRQ
handling can give us masking "for free", and this is what this patch set is
all about. It turns the above handling into:

  <irq handler>
    Acknowledge
    Priority Drop

  <threaded handler>
    Deactivate

No irq_mask() => fewer MMIO accesses => happier users (or so I've been
told). This is especially relevant to PREEMPT_RT which forces threaded
IRQs.
    
Testing
=======

GICv2
+++++

I've tested this on my Juno with forced irqthreads. This makes the pl011
IRQ into a threaded ONESHOT IRQ, so I spammed my keyboard into the console
and verified via ftrace that there were no irq_mask() / irq_unmask()
involved.

GICv3
+++++

I've tested this on my Ampere eMAG, which uncovered "fun" interactions with
the MSI domains. Did the same trick as the Juno with the pl011.

pNMIs cause said eMAG to freeze, but that's true even without my patches. I
did try them out under QEMU+KVM and that looked fine, although that means I
only got to test EOImode=0. I'll try to dig into this when I get some more
cycles.

TODOs
=====

MSI hierarchy story
+++++++++++++++++++

I 'hate' patch 8/10 because it's so very easy to break. See changelog for
my thoughts on the thing.

Performance numbers
+++++++++++++++++++

The whole point of this was to shave off some time spent issuing requests
to the distributor. On my Juno the IRQ for eth0 gets forcefully threaded
and thus becomes a ONESHOT threaded IRQ - I've tried to test this with
netperf, but the delta is lost in the noise (and there is quite a lot of
noise, unfortunately).

I'm still looking for something I can benchmark on the eMAG to get some
GICv3 results.

Links
=====

[1]: https://lore.kernel.org/lkml/1414235215-10468-1-git-send-email-marc.zyngier@arm.com/

Valentin Schneider (10):
  genirq: Add chip flag to denote automatic IRQ (un)masking
  genirq: Define irq_ack() and irq_eoi() helpers
  genirq: Employ ack_irq() and eoi_irq() where relevant
  genirq: Add handle_strict_flow_irq() flow handler
  genirq: Let purely flow-masked ONESHOT irqs through
    unmask_threaded_irq()
  genirq: Don't mask IRQ within flow handler if IRQ is flow-masked
  genirq, irq-gic-v3: Make NMI flow handlers use ->irq_ack() if
    available
  irqchip/gic-v3-its: Use irq_chip_ack_parent()
  irqchip/gic: Convert to handle_strict_flow_irq()
  irqchip/gic-v3: Convert to handle_strict_flow_irq()

 drivers/irqchip/irq-gic-v3-its-pci-msi.c |   1 +
 drivers/irqchip/irq-gic-v3-its.c         |   1 +
 drivers/irqchip/irq-gic-v3.c             |  27 ++++--
 drivers/irqchip/irq-gic.c                |  14 ++-
 include/linux/irq.h                      |  15 ++-
 kernel/irq/chip.c                        | 117 +++++++++++++++++++----
 kernel/irq/debugfs.c                     |   2 +
 kernel/irq/internals.h                   |   7 ++
 kernel/irq/manage.c                      |   2 +-
 9 files changed, 154 insertions(+), 32 deletions(-)

--
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [RFC PATCH 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking
  2021-04-08 15:43 ` Valentin Schneider
@ 2021-04-08 15:43   ` Valentin Schneider
  -1 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Some IRQ chips such as the Arm GICs automagically mask / unmask an
IRQ during the handling of said IRQ. This renders further mask / unmask
operations within the flow handlers redundant, which we do want to leverage
as masking by itself is not cheap (Distributor access via MMIO for GICs).

This is different from having a chip->irq_mask_ack() callback as this
masking is:
- inherent to the chip->irq_ack() and *cannot* be omitted
- a *different* masking state than chip->irq_mask() (chip->irq_mask() is
  idempotent, chip->irq_ack() really isn't)

Add a chip flag, IRQCHIP_AUTOMASKS_FLOW, to denote chips with such
behaviour. Add a new IRQ data flag, IRQD_IRQ_FLOW_MASKED, to keep this
flow-induced mask state separate from regular mask / unmask operations
(IRQD_IRQ_MASKED).

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 include/linux/irq.h    | 10 ++++++++++
 kernel/irq/chip.c      |  5 +++++
 kernel/irq/debugfs.c   |  2 ++
 kernel/irq/internals.h |  5 +++++
 4 files changed, 22 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index bee82809107c..580b1b6b1799 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -219,6 +219,8 @@ struct irq_data {
  *				  irq_chip::irq_set_affinity() when deactivated.
  * IRQD_IRQ_ENABLED_ON_SUSPEND	- Interrupt is enabled on suspend by irq pm if
  *				  irqchip have flag IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND set.
+ * IRQD_IRQ_FLOW_MASKED         - Interrupt is masked by ACK. Only EOI can
+ *                                clear this.
  */
 enum {
 	IRQD_TRIGGER_MASK		= 0xf,
@@ -245,6 +247,7 @@ enum {
 	IRQD_HANDLE_ENFORCE_IRQCTX	= (1 << 28),
 	IRQD_AFFINITY_ON_ACTIVATE	= (1 << 29),
 	IRQD_IRQ_ENABLED_ON_SUSPEND	= (1 << 30),
+	IRQD_IRQ_FLOW_MASKED            = (1 << 31),
 };
 
 #define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors)
@@ -349,6 +352,11 @@ static inline bool irqd_irq_masked(struct irq_data *d)
 	return __irqd_to_state(d) & IRQD_IRQ_MASKED;
 }
 
+static inline bool irqd_irq_flow_masked(struct irq_data *d)
+{
+	return __irqd_to_state(d) & IRQD_IRQ_FLOW_MASKED;
+}
+
 static inline bool irqd_irq_inprogress(struct irq_data *d)
 {
 	return __irqd_to_state(d) & IRQD_IRQ_INPROGRESS;
@@ -567,6 +575,7 @@ struct irq_chip {
  * IRQCHIP_SUPPORTS_NMI:              Chip can deliver NMIs, only for root irqchips
  * IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND:  Invokes __enable_irq()/__disable_irq() for wake irqs
  *                                    in the suspend path if they are in disabled state
+ * IRQCHIP_AUTOMASKS_FLOW:            chip->ack() masks and chip->eoi() unmasks
  */
 enum {
 	IRQCHIP_SET_TYPE_MASKED			= (1 <<  0),
@@ -579,6 +588,7 @@ enum {
 	IRQCHIP_SUPPORTS_LEVEL_MSI		= (1 <<  7),
 	IRQCHIP_SUPPORTS_NMI			= (1 <<  8),
 	IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND	= (1 <<  9),
+	IRQCHIP_AUTOMASKS_FLOW                  = (1 <<  10),
 };
 
 #include <linux/irqdesc.h>
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 8cc8e5713287..18c3b0e1568a 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -173,6 +173,11 @@ static void irq_state_clr_masked(struct irq_desc *desc)
 	irqd_clear(&desc->irq_data, IRQD_IRQ_MASKED);
 }
 
+static void irq_state_clr_flow_masked(struct irq_desc *desc)
+{
+	irqd_clear(&desc->irq_data, IRQD_IRQ_FLOW_MASKED);
+}
+
 static void irq_state_clr_started(struct irq_desc *desc)
 {
 	irqd_clear(&desc->irq_data, IRQD_IRQ_STARTED);
diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
index e4cff358b437..3ae83622d701 100644
--- a/kernel/irq/debugfs.c
+++ b/kernel/irq/debugfs.c
@@ -58,6 +58,7 @@ static const struct irq_bit_descr irqchip_flags[] = {
 	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_LEVEL_MSI),
 	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_NMI),
 	BIT_MASK_DESCR(IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND),
+	BIT_MASK_DESCR(IRQCHIP_AUTOMASKS_FLOW),
 };
 
 static void
@@ -103,6 +104,7 @@ static const struct irq_bit_descr irqdata_states[] = {
 	BIT_MASK_DESCR(IRQD_IRQ_STARTED),
 	BIT_MASK_DESCR(IRQD_IRQ_DISABLED),
 	BIT_MASK_DESCR(IRQD_IRQ_MASKED),
+	BIT_MASK_DESCR(IRQD_IRQ_FLOW_MASKED),
 	BIT_MASK_DESCR(IRQD_IRQ_INPROGRESS),
 
 	BIT_MASK_DESCR(IRQD_PER_CPU),
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index 54363527feea..b6c1cceddec0 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -245,6 +245,11 @@ static inline void irq_state_set_masked(struct irq_desc *desc)
 	irqd_set(&desc->irq_data, IRQD_IRQ_MASKED);
 }
 
+static inline void irq_state_set_flow_masked(struct irq_desc *desc)
+{
+	irqd_set(&desc->irq_data, IRQD_IRQ_FLOW_MASKED);
+}
+
 #undef __irqd_to_state
 
 static inline void __kstat_incr_irqs_this_cpu(struct irq_desc *desc)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking
@ 2021-04-08 15:43   ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Some IRQ chips such as the Arm GICs automagically mask / unmask an
IRQ during the handling of said IRQ. This renders further mask / unmask
operations within the flow handlers redundant, which we do want to leverage
as masking by itself is not cheap (Distributor access via MMIO for GICs).

This is different from having a chip->irq_mask_ack() callback as this
masking is:
- inherent to the chip->irq_ack() and *cannot* be omitted
- a *different* masking state than chip->irq_mask() (chip->irq_mask() is
  idempotent, chip->irq_ack() really isn't)

Add a chip flag, IRQCHIP_AUTOMASKS_FLOW, to denote chips with such
behaviour. Add a new IRQ data flag, IRQD_IRQ_FLOW_MASKED, to keep this
flow-induced mask state separate from regular mask / unmask operations
(IRQD_IRQ_MASKED).

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 include/linux/irq.h    | 10 ++++++++++
 kernel/irq/chip.c      |  5 +++++
 kernel/irq/debugfs.c   |  2 ++
 kernel/irq/internals.h |  5 +++++
 4 files changed, 22 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index bee82809107c..580b1b6b1799 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -219,6 +219,8 @@ struct irq_data {
  *				  irq_chip::irq_set_affinity() when deactivated.
  * IRQD_IRQ_ENABLED_ON_SUSPEND	- Interrupt is enabled on suspend by irq pm if
  *				  irqchip have flag IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND set.
+ * IRQD_IRQ_FLOW_MASKED         - Interrupt is masked by ACK. Only EOI can
+ *                                clear this.
  */
 enum {
 	IRQD_TRIGGER_MASK		= 0xf,
@@ -245,6 +247,7 @@ enum {
 	IRQD_HANDLE_ENFORCE_IRQCTX	= (1 << 28),
 	IRQD_AFFINITY_ON_ACTIVATE	= (1 << 29),
 	IRQD_IRQ_ENABLED_ON_SUSPEND	= (1 << 30),
+	IRQD_IRQ_FLOW_MASKED            = (1 << 31),
 };
 
 #define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors)
@@ -349,6 +352,11 @@ static inline bool irqd_irq_masked(struct irq_data *d)
 	return __irqd_to_state(d) & IRQD_IRQ_MASKED;
 }
 
+static inline bool irqd_irq_flow_masked(struct irq_data *d)
+{
+	return __irqd_to_state(d) & IRQD_IRQ_FLOW_MASKED;
+}
+
 static inline bool irqd_irq_inprogress(struct irq_data *d)
 {
 	return __irqd_to_state(d) & IRQD_IRQ_INPROGRESS;
@@ -567,6 +575,7 @@ struct irq_chip {
  * IRQCHIP_SUPPORTS_NMI:              Chip can deliver NMIs, only for root irqchips
  * IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND:  Invokes __enable_irq()/__disable_irq() for wake irqs
  *                                    in the suspend path if they are in disabled state
+ * IRQCHIP_AUTOMASKS_FLOW:            chip->ack() masks and chip->eoi() unmasks
  */
 enum {
 	IRQCHIP_SET_TYPE_MASKED			= (1 <<  0),
@@ -579,6 +588,7 @@ enum {
 	IRQCHIP_SUPPORTS_LEVEL_MSI		= (1 <<  7),
 	IRQCHIP_SUPPORTS_NMI			= (1 <<  8),
 	IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND	= (1 <<  9),
+	IRQCHIP_AUTOMASKS_FLOW                  = (1 <<  10),
 };
 
 #include <linux/irqdesc.h>
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 8cc8e5713287..18c3b0e1568a 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -173,6 +173,11 @@ static void irq_state_clr_masked(struct irq_desc *desc)
 	irqd_clear(&desc->irq_data, IRQD_IRQ_MASKED);
 }
 
+static void irq_state_clr_flow_masked(struct irq_desc *desc)
+{
+	irqd_clear(&desc->irq_data, IRQD_IRQ_FLOW_MASKED);
+}
+
 static void irq_state_clr_started(struct irq_desc *desc)
 {
 	irqd_clear(&desc->irq_data, IRQD_IRQ_STARTED);
diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
index e4cff358b437..3ae83622d701 100644
--- a/kernel/irq/debugfs.c
+++ b/kernel/irq/debugfs.c
@@ -58,6 +58,7 @@ static const struct irq_bit_descr irqchip_flags[] = {
 	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_LEVEL_MSI),
 	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_NMI),
 	BIT_MASK_DESCR(IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND),
+	BIT_MASK_DESCR(IRQCHIP_AUTOMASKS_FLOW),
 };
 
 static void
@@ -103,6 +104,7 @@ static const struct irq_bit_descr irqdata_states[] = {
 	BIT_MASK_DESCR(IRQD_IRQ_STARTED),
 	BIT_MASK_DESCR(IRQD_IRQ_DISABLED),
 	BIT_MASK_DESCR(IRQD_IRQ_MASKED),
+	BIT_MASK_DESCR(IRQD_IRQ_FLOW_MASKED),
 	BIT_MASK_DESCR(IRQD_IRQ_INPROGRESS),
 
 	BIT_MASK_DESCR(IRQD_PER_CPU),
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index 54363527feea..b6c1cceddec0 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -245,6 +245,11 @@ static inline void irq_state_set_masked(struct irq_desc *desc)
 	irqd_set(&desc->irq_data, IRQD_IRQ_MASKED);
 }
 
+static inline void irq_state_set_flow_masked(struct irq_desc *desc)
+{
+	irqd_set(&desc->irq_data, IRQD_IRQ_FLOW_MASKED);
+}
+
 #undef __irqd_to_state
 
 static inline void __kstat_incr_irqs_this_cpu(struct irq_desc *desc)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 02/10] genirq: Define irq_ack() and irq_eoi() helpers
  2021-04-08 15:43 ` Valentin Schneider
@ 2021-04-08 15:43   ` Valentin Schneider
  -1 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

The newly-added IRQCHIP_AUTOMASKS_FLOW flag requires some additional
bookkeeping around chip->{irq_ack, irq_eoi}() calls. Define wrappers around
those chip callbacks to drive the IRQD_IRQ_FLOW_MASKED state of an IRQ when
the chip has the IRQCHIP_AUTOMASKS_FLOW flag.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/irq/chip.c      | 16 ++++++++++++++++
 kernel/irq/internals.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 18c3b0e1568a..046b4486c88c 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -408,6 +408,22 @@ void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu)
 	cpumask_clear_cpu(cpu, desc->percpu_enabled);
 }
 
+void ack_irq(struct irq_desc *desc)
+{
+	desc->irq_data.chip->irq_ack(&desc->irq_data);
+
+	if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
+		irq_state_set_flow_masked(desc);
+}
+
+void eoi_irq(struct irq_desc *desc)
+{
+	desc->irq_data.chip->irq_eoi(&desc->irq_data);
+
+	if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
+		irq_state_clr_flow_masked(desc);
+}
+
 static inline void mask_ack_irq(struct irq_desc *desc)
 {
 	if (desc->irq_data.chip->irq_mask_ack) {
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index b6c1cceddec0..090bd7868845 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -87,6 +87,8 @@ extern void irq_enable(struct irq_desc *desc);
 extern void irq_disable(struct irq_desc *desc);
 extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
 extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
+extern void irq_ack(struct irq_desc *desc);
+extern void irq_eoi(struct irq_desc *desc);
 extern void mask_irq(struct irq_desc *desc);
 extern void unmask_irq(struct irq_desc *desc);
 extern void unmask_threaded_irq(struct irq_desc *desc);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 02/10] genirq: Define irq_ack() and irq_eoi() helpers
@ 2021-04-08 15:43   ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

The newly-added IRQCHIP_AUTOMASKS_FLOW flag requires some additional
bookkeeping around chip->{irq_ack, irq_eoi}() calls. Define wrappers around
those chip callbacks to drive the IRQD_IRQ_FLOW_MASKED state of an IRQ when
the chip has the IRQCHIP_AUTOMASKS_FLOW flag.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/irq/chip.c      | 16 ++++++++++++++++
 kernel/irq/internals.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 18c3b0e1568a..046b4486c88c 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -408,6 +408,22 @@ void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu)
 	cpumask_clear_cpu(cpu, desc->percpu_enabled);
 }
 
+void ack_irq(struct irq_desc *desc)
+{
+	desc->irq_data.chip->irq_ack(&desc->irq_data);
+
+	if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
+		irq_state_set_flow_masked(desc);
+}
+
+void eoi_irq(struct irq_desc *desc)
+{
+	desc->irq_data.chip->irq_eoi(&desc->irq_data);
+
+	if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
+		irq_state_clr_flow_masked(desc);
+}
+
 static inline void mask_ack_irq(struct irq_desc *desc)
 {
 	if (desc->irq_data.chip->irq_mask_ack) {
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index b6c1cceddec0..090bd7868845 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -87,6 +87,8 @@ extern void irq_enable(struct irq_desc *desc);
 extern void irq_disable(struct irq_desc *desc);
 extern void irq_percpu_enable(struct irq_desc *desc, unsigned int cpu);
 extern void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu);
+extern void irq_ack(struct irq_desc *desc);
+extern void irq_eoi(struct irq_desc *desc);
 extern void mask_irq(struct irq_desc *desc);
 extern void unmask_irq(struct irq_desc *desc);
 extern void unmask_threaded_irq(struct irq_desc *desc);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 03/10] genirq: Employ ack_irq() and eoi_irq() where relevant
  2021-04-08 15:43 ` Valentin Schneider
@ 2021-04-08 15:43   ` Valentin Schneider
  -1 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

This can easily be coccinelle'd to replace all existing chip->irq_{ack,
eoi} callsites, however not all callsites benefit from this
replacement: fasteoi flow handlers for instance only deal with an
->irq_eoi() callback. Instead, only patch callsites that can benefit from
the added functionality.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/irq/chip.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 046b4486c88c..936ef247b13d 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -429,10 +429,12 @@ static inline void mask_ack_irq(struct irq_desc *desc)
 	if (desc->irq_data.chip->irq_mask_ack) {
 		desc->irq_data.chip->irq_mask_ack(&desc->irq_data);
 		irq_state_set_masked(desc);
+		if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
+			irq_state_set_flow_masked(desc);
 	} else {
 		mask_irq(desc);
 		if (desc->irq_data.chip->irq_ack)
-			desc->irq_data.chip->irq_ack(&desc->irq_data);
+			ack_irq(desc);
 	}
 }
 
@@ -463,7 +465,7 @@ void unmask_threaded_irq(struct irq_desc *desc)
 	struct irq_chip *chip = desc->irq_data.chip;
 
 	if (chip->flags & IRQCHIP_EOI_THREADED)
-		chip->irq_eoi(&desc->irq_data);
+		eoi_irq(desc);
 
 	unmask_irq(desc);
 }
@@ -680,7 +682,7 @@ EXPORT_SYMBOL_GPL(handle_level_irq);
 static void cond_unmask_eoi_irq(struct irq_desc *desc, struct irq_chip *chip)
 {
 	if (!(desc->istate & IRQS_ONESHOT)) {
-		chip->irq_eoi(&desc->irq_data);
+		eoi_irq(desc);
 		return;
 	}
 	/*
@@ -691,10 +693,10 @@ static void cond_unmask_eoi_irq(struct irq_desc *desc, struct irq_chip *chip)
 	 */
 	if (!irqd_irq_disabled(&desc->irq_data) &&
 	    irqd_irq_masked(&desc->irq_data) && !desc->threads_oneshot) {
-		chip->irq_eoi(&desc->irq_data);
+		eoi_irq(desc);
 		unmask_irq(desc);
 	} else if (!(chip->flags & IRQCHIP_EOI_THREADED)) {
-		chip->irq_eoi(&desc->irq_data);
+		eoi_irq(desc);
 	}
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 03/10] genirq: Employ ack_irq() and eoi_irq() where relevant
@ 2021-04-08 15:43   ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

This can easily be coccinelle'd to replace all existing chip->irq_{ack,
eoi} callsites, however not all callsites benefit from this
replacement: fasteoi flow handlers for instance only deal with an
->irq_eoi() callback. Instead, only patch callsites that can benefit from
the added functionality.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/irq/chip.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 046b4486c88c..936ef247b13d 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -429,10 +429,12 @@ static inline void mask_ack_irq(struct irq_desc *desc)
 	if (desc->irq_data.chip->irq_mask_ack) {
 		desc->irq_data.chip->irq_mask_ack(&desc->irq_data);
 		irq_state_set_masked(desc);
+		if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
+			irq_state_set_flow_masked(desc);
 	} else {
 		mask_irq(desc);
 		if (desc->irq_data.chip->irq_ack)
-			desc->irq_data.chip->irq_ack(&desc->irq_data);
+			ack_irq(desc);
 	}
 }
 
@@ -463,7 +465,7 @@ void unmask_threaded_irq(struct irq_desc *desc)
 	struct irq_chip *chip = desc->irq_data.chip;
 
 	if (chip->flags & IRQCHIP_EOI_THREADED)
-		chip->irq_eoi(&desc->irq_data);
+		eoi_irq(desc);
 
 	unmask_irq(desc);
 }
@@ -680,7 +682,7 @@ EXPORT_SYMBOL_GPL(handle_level_irq);
 static void cond_unmask_eoi_irq(struct irq_desc *desc, struct irq_chip *chip)
 {
 	if (!(desc->istate & IRQS_ONESHOT)) {
-		chip->irq_eoi(&desc->irq_data);
+		eoi_irq(desc);
 		return;
 	}
 	/*
@@ -691,10 +693,10 @@ static void cond_unmask_eoi_irq(struct irq_desc *desc, struct irq_chip *chip)
 	 */
 	if (!irqd_irq_disabled(&desc->irq_data) &&
 	    irqd_irq_masked(&desc->irq_data) && !desc->threads_oneshot) {
-		chip->irq_eoi(&desc->irq_data);
+		eoi_irq(desc);
 		unmask_irq(desc);
 	} else if (!(chip->flags & IRQCHIP_EOI_THREADED)) {
-		chip->irq_eoi(&desc->irq_data);
+		eoi_irq(desc);
 	}
 }
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 04/10] genirq: Add handle_strict_flow_irq() flow handler
  2021-04-08 15:43 ` Valentin Schneider
@ 2021-04-08 15:43   ` Valentin Schneider
  -1 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

The GIC family of irqchips have been so far treated as "fasteoi"
chips. As handle_fasteoi_irq() states, this implies:

 *	Only a single callback will be issued to the chip: an ->eoi()
 *	call when the interrupt has been serviced.

However, the GICs have an operating mode (EOImode=1) which requires an
additional chip interaction during the IRQ handling. Said operating mode
already has some uses with virtualization, but could also be leveraged to
slightly optimize ONESHOT IRQs.

This extra interaction is currently hidden away in the drivers, but further
exploiting its effects (see IRQD_IRQ_FLOW_MASKED) requires lifting it from
the driver code into core code. It so happens that it fits the role of
->irq_ack(); unfortunately, the GICs require both callbacks to be strictly
paired with one another: for a given IRQ activation, there must be a single
->irq_ack() followed by a single ->irq_eoi(). No more, no less, and in that
order.

Introduce a new flow handler which guarantees said ack / eoi pairing. Note
that it is strikingly similar to handle_fasteoi_mask_irq() for now, but
will be further modified in later patches

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 include/linux/irq.h |  1 +
 kernel/irq/chip.c   | 48 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 580b1b6b1799..b605f0929d97 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -661,6 +661,7 @@ extern void handle_edge_irq(struct irq_desc *desc);
 extern void handle_edge_eoi_irq(struct irq_desc *desc);
 extern void handle_simple_irq(struct irq_desc *desc);
 extern void handle_untracked_irq(struct irq_desc *desc);
+extern void handle_strict_flow_irq(struct irq_desc *desc);
 extern void handle_percpu_irq(struct irq_desc *desc);
 extern void handle_percpu_devid_irq(struct irq_desc *desc);
 extern void handle_bad_irq(struct irq_desc *desc);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 936ef247b13d..6c033b0044cb 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -896,6 +896,54 @@ void handle_edge_eoi_irq(struct irq_desc *desc)
 }
 #endif
 
+/**
+ *	handle_strict_flow_irq - irq handler for strict controllers
+ *	@desc:	the interrupt description structure for this irq
+ *
+ *      Ensures strict pairing of ->ack() and ->eoi() for any IRQ passing
+ *      through here. The ->eoi() may be deferred to the tail of the IRQ thread
+ *      for ONESHOT IRQs.
+ */
+void handle_strict_flow_irq(struct irq_desc *desc)
+{
+	struct irq_chip *chip = desc->irq_data.chip;
+
+	raw_spin_lock(&desc->lock);
+	mask_ack_irq(desc);
+
+	if (!irq_may_run(desc))
+		goto out;
+
+	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
+
+	/*
+	 * If it's disabled or no action available then keep it masked and
+	 * get out of here:
+	 */
+	if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data))) {
+		desc->istate |= IRQS_PENDING;
+		goto out;
+	}
+
+	kstat_incr_irqs_this_cpu(desc);
+
+	handle_irq_event(desc);
+
+	cond_unmask_eoi_irq(desc, chip);
+
+	raw_spin_unlock(&desc->lock);
+	return;
+out:
+	/*
+	 * XXX: this is where IRQCHIP_EOI_IF_HANDLED would be checked, but
+	 * it's conceptually incompatible with this handler (it breaks the
+	 * strict pairing)
+	 */
+	eoi_irq(desc);
+	raw_spin_unlock(&desc->lock);
+}
+EXPORT_SYMBOL_GPL(handle_strict_flow_irq);
+
 /**
  *	handle_percpu_irq - Per CPU local irq handler
  *	@desc:	the interrupt description structure for this irq
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 04/10] genirq: Add handle_strict_flow_irq() flow handler
@ 2021-04-08 15:43   ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

The GIC family of irqchips have been so far treated as "fasteoi"
chips. As handle_fasteoi_irq() states, this implies:

 *	Only a single callback will be issued to the chip: an ->eoi()
 *	call when the interrupt has been serviced.

However, the GICs have an operating mode (EOImode=1) which requires an
additional chip interaction during the IRQ handling. Said operating mode
already has some uses with virtualization, but could also be leveraged to
slightly optimize ONESHOT IRQs.

This extra interaction is currently hidden away in the drivers, but further
exploiting its effects (see IRQD_IRQ_FLOW_MASKED) requires lifting it from
the driver code into core code. It so happens that it fits the role of
->irq_ack(); unfortunately, the GICs require both callbacks to be strictly
paired with one another: for a given IRQ activation, there must be a single
->irq_ack() followed by a single ->irq_eoi(). No more, no less, and in that
order.

Introduce a new flow handler which guarantees said ack / eoi pairing. Note
that it is strikingly similar to handle_fasteoi_mask_irq() for now, but
will be further modified in later patches

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 include/linux/irq.h |  1 +
 kernel/irq/chip.c   | 48 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 580b1b6b1799..b605f0929d97 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -661,6 +661,7 @@ extern void handle_edge_irq(struct irq_desc *desc);
 extern void handle_edge_eoi_irq(struct irq_desc *desc);
 extern void handle_simple_irq(struct irq_desc *desc);
 extern void handle_untracked_irq(struct irq_desc *desc);
+extern void handle_strict_flow_irq(struct irq_desc *desc);
 extern void handle_percpu_irq(struct irq_desc *desc);
 extern void handle_percpu_devid_irq(struct irq_desc *desc);
 extern void handle_bad_irq(struct irq_desc *desc);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 936ef247b13d..6c033b0044cb 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -896,6 +896,54 @@ void handle_edge_eoi_irq(struct irq_desc *desc)
 }
 #endif
 
+/**
+ *	handle_strict_flow_irq - irq handler for strict controllers
+ *	@desc:	the interrupt description structure for this irq
+ *
+ *      Ensures strict pairing of ->ack() and ->eoi() for any IRQ passing
+ *      through here. The ->eoi() may be deferred to the tail of the IRQ thread
+ *      for ONESHOT IRQs.
+ */
+void handle_strict_flow_irq(struct irq_desc *desc)
+{
+	struct irq_chip *chip = desc->irq_data.chip;
+
+	raw_spin_lock(&desc->lock);
+	mask_ack_irq(desc);
+
+	if (!irq_may_run(desc))
+		goto out;
+
+	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
+
+	/*
+	 * If it's disabled or no action available then keep it masked and
+	 * get out of here:
+	 */
+	if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data))) {
+		desc->istate |= IRQS_PENDING;
+		goto out;
+	}
+
+	kstat_incr_irqs_this_cpu(desc);
+
+	handle_irq_event(desc);
+
+	cond_unmask_eoi_irq(desc, chip);
+
+	raw_spin_unlock(&desc->lock);
+	return;
+out:
+	/*
+	 * XXX: this is where IRQCHIP_EOI_IF_HANDLED would be checked, but
+	 * it's conceptually incompatible with this handler (it breaks the
+	 * strict pairing)
+	 */
+	eoi_irq(desc);
+	raw_spin_unlock(&desc->lock);
+}
+EXPORT_SYMBOL_GPL(handle_strict_flow_irq);
+
 /**
  *	handle_percpu_irq - Per CPU local irq handler
  *	@desc:	the interrupt description structure for this irq
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 05/10] genirq: Let purely flow-masked ONESHOT irqs through unmask_threaded_irq()
  2021-04-08 15:43 ` Valentin Schneider
@ 2021-04-08 15:43   ` Valentin Schneider
  -1 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

A subsequent patch will let IRQs end up in irq_finalize_oneshot() without
IRQD_IRQ_MASKED, but with IRQD_IRQ_FLOW_MASKED set instead. Let such IRQs
receive their final ->irq_eoi().

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/irq/manage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index e976c4927b25..59c8056d6714 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1076,7 +1076,7 @@ static void irq_finalize_oneshot(struct irq_desc *desc,
 	desc->threads_oneshot &= ~action->thread_mask;
 
 	if (!desc->threads_oneshot && !irqd_irq_disabled(&desc->irq_data) &&
-	    irqd_irq_masked(&desc->irq_data))
+	    (irqd_irq_masked(&desc->irq_data) || irqd_irq_flow_masked(&desc->irq_data)))
 		unmask_threaded_irq(desc);
 
 out_unlock:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 05/10] genirq: Let purely flow-masked ONESHOT irqs through unmask_threaded_irq()
@ 2021-04-08 15:43   ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

A subsequent patch will let IRQs end up in irq_finalize_oneshot() without
IRQD_IRQ_MASKED, but with IRQD_IRQ_FLOW_MASKED set instead. Let such IRQs
receive their final ->irq_eoi().

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/irq/manage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index e976c4927b25..59c8056d6714 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1076,7 +1076,7 @@ static void irq_finalize_oneshot(struct irq_desc *desc,
 	desc->threads_oneshot &= ~action->thread_mask;
 
 	if (!desc->threads_oneshot && !irqd_irq_disabled(&desc->irq_data) &&
-	    irqd_irq_masked(&desc->irq_data))
+	    (irqd_irq_masked(&desc->irq_data) || irqd_irq_flow_masked(&desc->irq_data)))
 		unmask_threaded_irq(desc);
 
 out_unlock:
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 06/10] genirq: Don't mask IRQ within flow handler if IRQ is flow-masked
  2021-04-08 15:43 ` Valentin Schneider
@ 2021-04-08 15:43   ` Valentin Schneider
  -1 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

mask_irq() lets an IRQ with IRQD_IRQ_FLOW_MASKED set be further masked via
chip->irq_mask(). This is necessary for unhandled IRQs as we want to keep
them masked beyond eoi_irq() (which clears IRQD_IRQ_FLOW_MASKED).

This is however not necessary in paths that do end up handling the IRQ and
are bounded by a final eoi_irq() - this is the case for chips with
IRQCHIP_AUTOMASKS_FLOW and IRQCHIP_EOI_THREADED.

Make handle_strict_flow_irq() leverage IRQCHIP_AUTOMASKS_FLOW and issue an
ack_irq() rather than a mask_ack_irq() when possible.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/irq/chip.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 6c033b0044cb..1add0b4f0662 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -909,10 +909,9 @@ void handle_strict_flow_irq(struct irq_desc *desc)
 	struct irq_chip *chip = desc->irq_data.chip;
 
 	raw_spin_lock(&desc->lock);
-	mask_ack_irq(desc);
 
 	if (!irq_may_run(desc))
-		goto out;
+		goto out_mask;
 
 	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
 
@@ -922,10 +921,21 @@ void handle_strict_flow_irq(struct irq_desc *desc)
 	 */
 	if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data))) {
 		desc->istate |= IRQS_PENDING;
-		goto out;
+		goto out_mask;
 	}
 
 	kstat_incr_irqs_this_cpu(desc);
+	/*
+	 * Masking is required if IRQ is ONESHOT and we can't rely on the
+	 * flow-masking persisting down to irq_finalize_oneshot()
+	 * (in the IRQ thread).
+	 */
+	if ((desc->istate & IRQS_ONESHOT) &&
+	    (!(chip->flags & IRQCHIP_AUTOMASKS_FLOW) ||
+	     !(chip->flags & IRQCHIP_EOI_THREADED)))
+		mask_ack_irq(desc);
+	else
+		ack_irq(desc);
 
 	handle_irq_event(desc);
 
@@ -933,7 +943,8 @@ void handle_strict_flow_irq(struct irq_desc *desc)
 
 	raw_spin_unlock(&desc->lock);
 	return;
-out:
+out_mask:
+	mask_ack_irq(desc);
 	/*
 	 * XXX: this is where IRQCHIP_EOI_IF_HANDLED would be checked, but
 	 * it's conceptually incompatible with this handler (it breaks the
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 06/10] genirq: Don't mask IRQ within flow handler if IRQ is flow-masked
@ 2021-04-08 15:43   ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

mask_irq() lets an IRQ with IRQD_IRQ_FLOW_MASKED set be further masked via
chip->irq_mask(). This is necessary for unhandled IRQs as we want to keep
them masked beyond eoi_irq() (which clears IRQD_IRQ_FLOW_MASKED).

This is however not necessary in paths that do end up handling the IRQ and
are bounded by a final eoi_irq() - this is the case for chips with
IRQCHIP_AUTOMASKS_FLOW and IRQCHIP_EOI_THREADED.

Make handle_strict_flow_irq() leverage IRQCHIP_AUTOMASKS_FLOW and issue an
ack_irq() rather than a mask_ack_irq() when possible.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 kernel/irq/chip.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 6c033b0044cb..1add0b4f0662 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -909,10 +909,9 @@ void handle_strict_flow_irq(struct irq_desc *desc)
 	struct irq_chip *chip = desc->irq_data.chip;
 
 	raw_spin_lock(&desc->lock);
-	mask_ack_irq(desc);
 
 	if (!irq_may_run(desc))
-		goto out;
+		goto out_mask;
 
 	desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
 
@@ -922,10 +921,21 @@ void handle_strict_flow_irq(struct irq_desc *desc)
 	 */
 	if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data))) {
 		desc->istate |= IRQS_PENDING;
-		goto out;
+		goto out_mask;
 	}
 
 	kstat_incr_irqs_this_cpu(desc);
+	/*
+	 * Masking is required if IRQ is ONESHOT and we can't rely on the
+	 * flow-masking persisting down to irq_finalize_oneshot()
+	 * (in the IRQ thread).
+	 */
+	if ((desc->istate & IRQS_ONESHOT) &&
+	    (!(chip->flags & IRQCHIP_AUTOMASKS_FLOW) ||
+	     !(chip->flags & IRQCHIP_EOI_THREADED)))
+		mask_ack_irq(desc);
+	else
+		ack_irq(desc);
 
 	handle_irq_event(desc);
 
@@ -933,7 +943,8 @@ void handle_strict_flow_irq(struct irq_desc *desc)
 
 	raw_spin_unlock(&desc->lock);
 	return;
-out:
+out_mask:
+	mask_ack_irq(desc);
 	/*
 	 * XXX: this is where IRQCHIP_EOI_IF_HANDLED would be checked, but
 	 * it's conceptually incompatible with this handler (it breaks the
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 07/10] genirq, irq-gic-v3: Make NMI flow handlers use ->irq_ack() if available
  2021-04-08 15:43 ` Valentin Schneider
@ 2021-04-08 15:43   ` Valentin Schneider
  -1 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Subsequent patches will make the gic-v3 irqchip use an ->irq_ack()
callback. As a preparation, make the NMI flow handlers call said callback
if it is available.

Since this departs from the fasteoi scheme of only issuing a suffix
->eoi(), rename the NMI flow handlers.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 drivers/irqchip/irq-gic-v3.c |  4 ++--
 include/linux/irq.h          |  4 ++--
 kernel/irq/chip.c            | 25 ++++++++++++++-----------
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 94b89258d045..242a8fc5ec86 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -484,10 +484,10 @@ static int gic_irq_nmi_setup(struct irq_data *d)
 		/* Setting up PPI as NMI, only switch handler for first NMI */
 		if (!refcount_inc_not_zero(&ppi_nmi_refs[idx])) {
 			refcount_set(&ppi_nmi_refs[idx], 1);
-			desc->handle_irq = handle_percpu_devid_fasteoi_nmi;
+			desc->handle_irq = handle_percpu_devid_nmi;
 		}
 	} else {
-		desc->handle_irq = handle_fasteoi_nmi;
+		desc->handle_irq = handle_nmi;
 	}
 
 	gic_irq_set_prio(d, GICD_INT_NMI_PRI);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index b605f0929d97..c39f46e37487 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -667,8 +667,8 @@ extern void handle_percpu_devid_irq(struct irq_desc *desc);
 extern void handle_bad_irq(struct irq_desc *desc);
 extern void handle_nested_irq(unsigned int irq);
 
-extern void handle_fasteoi_nmi(struct irq_desc *desc);
-extern void handle_percpu_devid_fasteoi_nmi(struct irq_desc *desc);
+extern void handle_nmi(struct irq_desc *desc);
+extern void handle_percpu_devid_nmi(struct irq_desc *desc);
 
 extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
 extern int irq_chip_pm_get(struct irq_data *data);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 1add0b4f0662..ea077333d676 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -748,18 +748,16 @@ void handle_fasteoi_irq(struct irq_desc *desc)
 EXPORT_SYMBOL_GPL(handle_fasteoi_irq);
 
 /**
- *	handle_fasteoi_nmi - irq handler for NMI interrupt lines
+ *	handle_nmi - irq handler for NMI interrupt lines
  *	@desc:	the interrupt description structure for this irq
  *
  *	A simple NMI-safe handler, considering the restrictions
  *	from request_nmi.
  *
- *	Only a single callback will be issued to the chip: an ->eoi()
- *	call when the interrupt has been serviced. This enables support
- *	for modern forms of interrupt handlers, which handle the flow
- *	details in hardware, transparently.
+ *      An ->ack() callback will be issued before servicing the interrupt,
+ *      followed by an ->eoi() call.
  */
-void handle_fasteoi_nmi(struct irq_desc *desc)
+void handle_nmi(struct irq_desc *desc)
 {
 	struct irq_chip *chip = irq_desc_get_chip(desc);
 	struct irqaction *action = desc->action;
@@ -768,6 +766,9 @@ void handle_fasteoi_nmi(struct irq_desc *desc)
 
 	__kstat_incr_irqs_this_cpu(desc);
 
+	if (chip->irq_ack)
+		chip->irq_ack(&desc->irq_data);
+
 	trace_irq_handler_entry(irq, action);
 	/*
 	 * NMIs cannot be shared, there is only one action.
@@ -778,7 +779,7 @@ void handle_fasteoi_nmi(struct irq_desc *desc)
 	if (chip->irq_eoi)
 		chip->irq_eoi(&desc->irq_data);
 }
-EXPORT_SYMBOL_GPL(handle_fasteoi_nmi);
+EXPORT_SYMBOL_GPL(handle_nmi);
 
 /**
  *	handle_edge_irq - edge type IRQ handler
@@ -1027,14 +1028,13 @@ void handle_percpu_devid_irq(struct irq_desc *desc)
 }
 
 /**
- * handle_percpu_devid_fasteoi_nmi - Per CPU local NMI handler with per cpu
+ * handle_percpu_devid_nmi - Per CPU local NMI handler with per cpu
  *				     dev ids
  * @desc:	the interrupt description structure for this irq
  *
- * Similar to handle_fasteoi_nmi, but handling the dev_id cookie
- * as a percpu pointer.
+ * Similar to handle_nmi, but handling the dev_id cookie as a percpu pointer.
  */
-void handle_percpu_devid_fasteoi_nmi(struct irq_desc *desc)
+void handle_percpu_devid_nmi(struct irq_desc *desc)
 {
 	struct irq_chip *chip = irq_desc_get_chip(desc);
 	struct irqaction *action = desc->action;
@@ -1043,6 +1043,9 @@ void handle_percpu_devid_fasteoi_nmi(struct irq_desc *desc)
 
 	__kstat_incr_irqs_this_cpu(desc);
 
+	if (chip->irq_ack)
+		chip->irq_ack(&desc->irq_data);
+
 	trace_irq_handler_entry(irq, action);
 	res = action->handler(irq, raw_cpu_ptr(action->percpu_dev_id));
 	trace_irq_handler_exit(irq, action, res);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 07/10] genirq, irq-gic-v3: Make NMI flow handlers use ->irq_ack() if available
@ 2021-04-08 15:43   ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Subsequent patches will make the gic-v3 irqchip use an ->irq_ack()
callback. As a preparation, make the NMI flow handlers call said callback
if it is available.

Since this departs from the fasteoi scheme of only issuing a suffix
->eoi(), rename the NMI flow handlers.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 drivers/irqchip/irq-gic-v3.c |  4 ++--
 include/linux/irq.h          |  4 ++--
 kernel/irq/chip.c            | 25 ++++++++++++++-----------
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 94b89258d045..242a8fc5ec86 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -484,10 +484,10 @@ static int gic_irq_nmi_setup(struct irq_data *d)
 		/* Setting up PPI as NMI, only switch handler for first NMI */
 		if (!refcount_inc_not_zero(&ppi_nmi_refs[idx])) {
 			refcount_set(&ppi_nmi_refs[idx], 1);
-			desc->handle_irq = handle_percpu_devid_fasteoi_nmi;
+			desc->handle_irq = handle_percpu_devid_nmi;
 		}
 	} else {
-		desc->handle_irq = handle_fasteoi_nmi;
+		desc->handle_irq = handle_nmi;
 	}
 
 	gic_irq_set_prio(d, GICD_INT_NMI_PRI);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index b605f0929d97..c39f46e37487 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -667,8 +667,8 @@ extern void handle_percpu_devid_irq(struct irq_desc *desc);
 extern void handle_bad_irq(struct irq_desc *desc);
 extern void handle_nested_irq(unsigned int irq);
 
-extern void handle_fasteoi_nmi(struct irq_desc *desc);
-extern void handle_percpu_devid_fasteoi_nmi(struct irq_desc *desc);
+extern void handle_nmi(struct irq_desc *desc);
+extern void handle_percpu_devid_nmi(struct irq_desc *desc);
 
 extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
 extern int irq_chip_pm_get(struct irq_data *data);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 1add0b4f0662..ea077333d676 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -748,18 +748,16 @@ void handle_fasteoi_irq(struct irq_desc *desc)
 EXPORT_SYMBOL_GPL(handle_fasteoi_irq);
 
 /**
- *	handle_fasteoi_nmi - irq handler for NMI interrupt lines
+ *	handle_nmi - irq handler for NMI interrupt lines
  *	@desc:	the interrupt description structure for this irq
  *
  *	A simple NMI-safe handler, considering the restrictions
  *	from request_nmi.
  *
- *	Only a single callback will be issued to the chip: an ->eoi()
- *	call when the interrupt has been serviced. This enables support
- *	for modern forms of interrupt handlers, which handle the flow
- *	details in hardware, transparently.
+ *      An ->ack() callback will be issued before servicing the interrupt,
+ *      followed by an ->eoi() call.
  */
-void handle_fasteoi_nmi(struct irq_desc *desc)
+void handle_nmi(struct irq_desc *desc)
 {
 	struct irq_chip *chip = irq_desc_get_chip(desc);
 	struct irqaction *action = desc->action;
@@ -768,6 +766,9 @@ void handle_fasteoi_nmi(struct irq_desc *desc)
 
 	__kstat_incr_irqs_this_cpu(desc);
 
+	if (chip->irq_ack)
+		chip->irq_ack(&desc->irq_data);
+
 	trace_irq_handler_entry(irq, action);
 	/*
 	 * NMIs cannot be shared, there is only one action.
@@ -778,7 +779,7 @@ void handle_fasteoi_nmi(struct irq_desc *desc)
 	if (chip->irq_eoi)
 		chip->irq_eoi(&desc->irq_data);
 }
-EXPORT_SYMBOL_GPL(handle_fasteoi_nmi);
+EXPORT_SYMBOL_GPL(handle_nmi);
 
 /**
  *	handle_edge_irq - edge type IRQ handler
@@ -1027,14 +1028,13 @@ void handle_percpu_devid_irq(struct irq_desc *desc)
 }
 
 /**
- * handle_percpu_devid_fasteoi_nmi - Per CPU local NMI handler with per cpu
+ * handle_percpu_devid_nmi - Per CPU local NMI handler with per cpu
  *				     dev ids
  * @desc:	the interrupt description structure for this irq
  *
- * Similar to handle_fasteoi_nmi, but handling the dev_id cookie
- * as a percpu pointer.
+ * Similar to handle_nmi, but handling the dev_id cookie as a percpu pointer.
  */
-void handle_percpu_devid_fasteoi_nmi(struct irq_desc *desc)
+void handle_percpu_devid_nmi(struct irq_desc *desc)
 {
 	struct irq_chip *chip = irq_desc_get_chip(desc);
 	struct irqaction *action = desc->action;
@@ -1043,6 +1043,9 @@ void handle_percpu_devid_fasteoi_nmi(struct irq_desc *desc)
 
 	__kstat_incr_irqs_this_cpu(desc);
 
+	if (chip->irq_ack)
+		chip->irq_ack(&desc->irq_data);
+
 	trace_irq_handler_entry(irq, action);
 	res = action->handler(irq, raw_cpu_ptr(action->percpu_dev_id));
 	trace_irq_handler_exit(irq, action, res);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 08/10] irqchip/gic-v3-its: Use irq_chip_ack_parent()
  2021-04-08 15:43 ` Valentin Schneider
@ 2021-04-08 15:43   ` Valentin Schneider
  -1 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Subsequent patches will make the GIC irqchips use a flow handler that
issues an ->irq_ack(). irqchips of child domains need to handle this.

Note: I'm very much not fond of this; this is treacherous and explodes if
any parent chip doesn't have an ->ack() callback. It turns out okay with
EOImode=0 because handle_fasteoi_irq() doesn't issue any ->ack(), but that
is very fragile at best.

An alternative would be to
o make irq_chip_ack_parent() check the callback against NULL
o make irq_chip_ack_parent() the default chip->irq_ack() via
  MSI_FLAG_USE_DEF_CHIP_OPS.

XXX: what about pMSI and fMSI ?

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 drivers/irqchip/irq-gic-v3-its-pci-msi.c | 1 +
 drivers/irqchip/irq-gic-v3-its.c         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index ad2810c017ed..5bc2787ee86a 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -27,6 +27,7 @@ static struct irq_chip its_msi_irq_chip = {
 	.name			= "ITS-MSI",
 	.irq_unmask		= its_unmask_msi_irq,
 	.irq_mask		= its_mask_msi_irq,
+	.irq_ack		= irq_chip_ack_parent,
 	.irq_eoi		= irq_chip_eoi_parent,
 	.irq_write_msi_msg	= pci_msi_domain_write_msg,
 };
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c3485b230d70..d6856750c084 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1971,6 +1971,7 @@ static struct irq_chip its_irq_chip = {
 	.name			= "ITS",
 	.irq_mask		= its_mask_irq,
 	.irq_unmask		= its_unmask_irq,
+	.irq_ack		= irq_chip_ack_parent,
 	.irq_eoi		= irq_chip_eoi_parent,
 	.irq_set_affinity	= its_set_affinity,
 	.irq_compose_msi_msg	= its_irq_compose_msi_msg,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 08/10] irqchip/gic-v3-its: Use irq_chip_ack_parent()
@ 2021-04-08 15:43   ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Subsequent patches will make the GIC irqchips use a flow handler that
issues an ->irq_ack(). irqchips of child domains need to handle this.

Note: I'm very much not fond of this; this is treacherous and explodes if
any parent chip doesn't have an ->ack() callback. It turns out okay with
EOImode=0 because handle_fasteoi_irq() doesn't issue any ->ack(), but that
is very fragile at best.

An alternative would be to
o make irq_chip_ack_parent() check the callback against NULL
o make irq_chip_ack_parent() the default chip->irq_ack() via
  MSI_FLAG_USE_DEF_CHIP_OPS.

XXX: what about pMSI and fMSI ?

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 drivers/irqchip/irq-gic-v3-its-pci-msi.c | 1 +
 drivers/irqchip/irq-gic-v3-its.c         | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index ad2810c017ed..5bc2787ee86a 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -27,6 +27,7 @@ static struct irq_chip its_msi_irq_chip = {
 	.name			= "ITS-MSI",
 	.irq_unmask		= its_unmask_msi_irq,
 	.irq_mask		= its_mask_msi_irq,
+	.irq_ack		= irq_chip_ack_parent,
 	.irq_eoi		= irq_chip_eoi_parent,
 	.irq_write_msi_msg	= pci_msi_domain_write_msg,
 };
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c3485b230d70..d6856750c084 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1971,6 +1971,7 @@ static struct irq_chip its_irq_chip = {
 	.name			= "ITS",
 	.irq_mask		= its_mask_irq,
 	.irq_unmask		= its_unmask_irq,
+	.irq_ack		= irq_chip_ack_parent,
 	.irq_eoi		= irq_chip_eoi_parent,
 	.irq_set_affinity	= its_set_affinity,
 	.irq_compose_msi_msg	= its_irq_compose_msi_msg,
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 09/10] irqchip/gic: Convert to handle_strict_flow_irq()
  2021-04-08 15:43 ` Valentin Schneider
@ 2021-04-08 15:43   ` Valentin Schneider
  -1 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Now that the proper infrastructure is in place, convert the irq-gic chip to
use handle_strict_flow_irq() along with IRQCHIP_AUTOMASKS_FLOW.

For EOImode=1, the Priority Drop is moved from gic_handle_irq() into
chip->irq_ack(). This effectively pushes the EOI write down into
->handle_irq(), but doesn't change its ordering wrt the irqaction
handling.

The EOImode=1 irqchip also gains IRQCHIP_EOI_THREADED, which allows the
->irq_eoi() call to be deferred to the tail of ONESHOT IRQ threads. This
means a threaded ONESHOT IRQ can now be handled entirely without a single
chip->irq_mask() call.

EOImode=0 handling remains unchanged.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 drivers/irqchip/irq-gic.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index b1d9c22caf2e..4919478c3e41 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -344,8 +344,6 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
 		if (unlikely(irqnr >= 1020))
 			break;
 
-		if (static_branch_likely(&supports_deactivate_key))
-			writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
 		isb();
 
 		/*
@@ -1012,7 +1010,9 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
 		break;
 	default:
 		irq_domain_set_info(d, irq, hw, &gic->chip, d->host_data,
-				    handle_fasteoi_irq, NULL, NULL);
+				    static_branch_likely(&supports_deactivate_key) ?
+				    handle_strict_flow_irq : handle_fasteoi_irq,
+				    NULL, NULL);
 		irq_set_probe(irq);
 		irqd_set_single_target(irqd);
 		break;
@@ -1116,8 +1116,16 @@ static void gic_init_chip(struct gic_chip_data *gic, struct device *dev,
 
 	if (use_eoimode1) {
 		gic->chip.irq_mask = gic_eoimode1_mask_irq;
+		gic->chip.irq_ack = gic_eoi_irq;
 		gic->chip.irq_eoi = gic_eoimode1_eoi_irq;
 		gic->chip.irq_set_vcpu_affinity = gic_irq_set_vcpu_affinity;
+
+		/*
+		 * eoimode0 shouldn't expose FLOW_MASK because the priority
+		 * drop is undissociable from the deactivation, and we do need
+		 * the priority drop to happen within the flow handler.
+		 */
+		gic->chip.flags |= IRQCHIP_AUTOMASKS_FLOW | IRQCHIP_EOI_THREADED;
 	}
 
 	if (gic == &gic_data[0]) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 09/10] irqchip/gic: Convert to handle_strict_flow_irq()
@ 2021-04-08 15:43   ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Now that the proper infrastructure is in place, convert the irq-gic chip to
use handle_strict_flow_irq() along with IRQCHIP_AUTOMASKS_FLOW.

For EOImode=1, the Priority Drop is moved from gic_handle_irq() into
chip->irq_ack(). This effectively pushes the EOI write down into
->handle_irq(), but doesn't change its ordering wrt the irqaction
handling.

The EOImode=1 irqchip also gains IRQCHIP_EOI_THREADED, which allows the
->irq_eoi() call to be deferred to the tail of ONESHOT IRQ threads. This
means a threaded ONESHOT IRQ can now be handled entirely without a single
chip->irq_mask() call.

EOImode=0 handling remains unchanged.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 drivers/irqchip/irq-gic.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index b1d9c22caf2e..4919478c3e41 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -344,8 +344,6 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
 		if (unlikely(irqnr >= 1020))
 			break;
 
-		if (static_branch_likely(&supports_deactivate_key))
-			writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
 		isb();
 
 		/*
@@ -1012,7 +1010,9 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
 		break;
 	default:
 		irq_domain_set_info(d, irq, hw, &gic->chip, d->host_data,
-				    handle_fasteoi_irq, NULL, NULL);
+				    static_branch_likely(&supports_deactivate_key) ?
+				    handle_strict_flow_irq : handle_fasteoi_irq,
+				    NULL, NULL);
 		irq_set_probe(irq);
 		irqd_set_single_target(irqd);
 		break;
@@ -1116,8 +1116,16 @@ static void gic_init_chip(struct gic_chip_data *gic, struct device *dev,
 
 	if (use_eoimode1) {
 		gic->chip.irq_mask = gic_eoimode1_mask_irq;
+		gic->chip.irq_ack = gic_eoi_irq;
 		gic->chip.irq_eoi = gic_eoimode1_eoi_irq;
 		gic->chip.irq_set_vcpu_affinity = gic_irq_set_vcpu_affinity;
+
+		/*
+		 * eoimode0 shouldn't expose FLOW_MASK because the priority
+		 * drop is undissociable from the deactivation, and we do need
+		 * the priority drop to happen within the flow handler.
+		 */
+		gic->chip.flags |= IRQCHIP_AUTOMASKS_FLOW | IRQCHIP_EOI_THREADED;
 	}
 
 	if (gic == &gic_data[0]) {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 10/10] irqchip/gic-v3: Convert to handle_strict_flow_irq()
  2021-04-08 15:43 ` Valentin Schneider
@ 2021-04-08 15:43   ` Valentin Schneider
  -1 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Now that the proper infrastructure is in place, convert the irq-gic-v3 chip
to use handle_strict_flow_irq() along with IRQCHIP_AUTOMASKS_FLOW.

For EOImode=1, the Priority Drop is moved from gic_handle_irq() into
chip->irq_ack(). This effectively pushes the EOIR write down into
->handle_irq(), but doesn't change its ordering wrt the irqaction
handling.

The EOImode=1 irqchip also gains IRQCHIP_EOI_THREADED, which allows the
->irq_eoi() call to be deferred to the tail of ONESHOT IRQ threads. This
means a threaded ONESHOT IRQ can now be handled entirely without a single
chip->irq_mask() call.

Despite not having an Active state, LPIs are made to use
handle_strict_flow_irq() as well. This lets them re-use
gic_eoimode1_chip.irq_ack() as Priority Drop, rather than special-case them
in gic_handle_irq().

EOImode=0 handling remains unchanged.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 drivers/irqchip/irq-gic-v3.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 242a8fc5ec86..da73398117a7 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -626,8 +626,6 @@ static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs)
 	if (irqs_enabled)
 		nmi_enter();
 
-	if (static_branch_likely(&supports_deactivate_key))
-		gic_write_eoir(irqnr);
 	/*
 	 * Leave the PSR.I bit set to prevent other NMIs to be
 	 * received while handling this one.
@@ -663,9 +661,11 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs
 	if ((irqnr >= 1020 && irqnr <= 1023))
 		return;
 
-	if (static_branch_likely(&supports_deactivate_key))
-		gic_write_eoir(irqnr);
-	else
+	/*
+	 * eoimode1 will give us an isb in handle_domain_irq(), before
+	 * handle_irq_event().
+	 */
+	if (!static_branch_likely(&supports_deactivate_key))
 		isb();
 
 	if (handle_domain_irq(gic_data.domain, irqnr, regs)) {
@@ -1276,6 +1276,7 @@ static struct irq_chip gic_eoimode1_chip = {
 	.name			= "GICv3",
 	.irq_mask		= gic_eoimode1_mask_irq,
 	.irq_unmask		= gic_unmask_irq,
+	.irq_ack                = gic_eoi_irq,
 	.irq_eoi		= gic_eoimode1_eoi_irq,
 	.irq_set_type		= gic_set_type,
 	.irq_set_affinity	= gic_set_affinity,
@@ -1288,7 +1289,9 @@ static struct irq_chip gic_eoimode1_chip = {
 	.ipi_send_mask		= gic_ipi_send_mask,
 	.flags			= IRQCHIP_SET_TYPE_MASKED |
 				  IRQCHIP_SKIP_SET_WAKE |
-				  IRQCHIP_MASK_ON_SUSPEND,
+				  IRQCHIP_MASK_ON_SUSPEND |
+				  IRQCHIP_AUTOMASKS_FLOW |
+				  IRQCHIP_EOI_THREADED,
 };
 
 static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
@@ -1312,7 +1315,9 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
 	case SPI_RANGE:
 	case ESPI_RANGE:
 		irq_domain_set_info(d, irq, hw, chip, d->host_data,
-				    handle_fasteoi_irq, NULL, NULL);
+				    static_branch_likely(&supports_deactivate_key) ?
+				    handle_strict_flow_irq : handle_fasteoi_irq,
+				    NULL, NULL);
 		irq_set_probe(irq);
 		irqd_set_single_target(irqd);
 		break;
@@ -1321,7 +1326,9 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
 		if (!gic_dist_supports_lpis())
 			return -EPERM;
 		irq_domain_set_info(d, irq, hw, chip, d->host_data,
-				    handle_fasteoi_irq, NULL, NULL);
+				    static_branch_likely(&supports_deactivate_key) ?
+				    handle_strict_flow_irq : handle_fasteoi_irq,
+				    NULL, NULL);
 		break;
 
 	default:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 10/10] irqchip/gic-v3: Convert to handle_strict_flow_irq()
@ 2021-04-08 15:43   ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-08 15:43 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

Now that the proper infrastructure is in place, convert the irq-gic-v3 chip
to use handle_strict_flow_irq() along with IRQCHIP_AUTOMASKS_FLOW.

For EOImode=1, the Priority Drop is moved from gic_handle_irq() into
chip->irq_ack(). This effectively pushes the EOIR write down into
->handle_irq(), but doesn't change its ordering wrt the irqaction
handling.

The EOImode=1 irqchip also gains IRQCHIP_EOI_THREADED, which allows the
->irq_eoi() call to be deferred to the tail of ONESHOT IRQ threads. This
means a threaded ONESHOT IRQ can now be handled entirely without a single
chip->irq_mask() call.

Despite not having an Active state, LPIs are made to use
handle_strict_flow_irq() as well. This lets them re-use
gic_eoimode1_chip.irq_ack() as Priority Drop, rather than special-case them
in gic_handle_irq().

EOImode=0 handling remains unchanged.

Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
---
 drivers/irqchip/irq-gic-v3.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 242a8fc5ec86..da73398117a7 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -626,8 +626,6 @@ static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs)
 	if (irqs_enabled)
 		nmi_enter();
 
-	if (static_branch_likely(&supports_deactivate_key))
-		gic_write_eoir(irqnr);
 	/*
 	 * Leave the PSR.I bit set to prevent other NMIs to be
 	 * received while handling this one.
@@ -663,9 +661,11 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs
 	if ((irqnr >= 1020 && irqnr <= 1023))
 		return;
 
-	if (static_branch_likely(&supports_deactivate_key))
-		gic_write_eoir(irqnr);
-	else
+	/*
+	 * eoimode1 will give us an isb in handle_domain_irq(), before
+	 * handle_irq_event().
+	 */
+	if (!static_branch_likely(&supports_deactivate_key))
 		isb();
 
 	if (handle_domain_irq(gic_data.domain, irqnr, regs)) {
@@ -1276,6 +1276,7 @@ static struct irq_chip gic_eoimode1_chip = {
 	.name			= "GICv3",
 	.irq_mask		= gic_eoimode1_mask_irq,
 	.irq_unmask		= gic_unmask_irq,
+	.irq_ack                = gic_eoi_irq,
 	.irq_eoi		= gic_eoimode1_eoi_irq,
 	.irq_set_type		= gic_set_type,
 	.irq_set_affinity	= gic_set_affinity,
@@ -1288,7 +1289,9 @@ static struct irq_chip gic_eoimode1_chip = {
 	.ipi_send_mask		= gic_ipi_send_mask,
 	.flags			= IRQCHIP_SET_TYPE_MASKED |
 				  IRQCHIP_SKIP_SET_WAKE |
-				  IRQCHIP_MASK_ON_SUSPEND,
+				  IRQCHIP_MASK_ON_SUSPEND |
+				  IRQCHIP_AUTOMASKS_FLOW |
+				  IRQCHIP_EOI_THREADED,
 };
 
 static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
@@ -1312,7 +1315,9 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
 	case SPI_RANGE:
 	case ESPI_RANGE:
 		irq_domain_set_info(d, irq, hw, chip, d->host_data,
-				    handle_fasteoi_irq, NULL, NULL);
+				    static_branch_likely(&supports_deactivate_key) ?
+				    handle_strict_flow_irq : handle_fasteoi_irq,
+				    NULL, NULL);
 		irq_set_probe(irq);
 		irqd_set_single_target(irqd);
 		break;
@@ -1321,7 +1326,9 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
 		if (!gic_dist_supports_lpis())
 			return -EPERM;
 		irq_domain_set_info(d, irq, hw, chip, d->host_data,
-				    handle_fasteoi_irq, NULL, NULL);
+				    static_branch_likely(&supports_deactivate_key) ?
+				    handle_strict_flow_irq : handle_fasteoi_irq,
+				    NULL, NULL);
 		break;
 
 	default:
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 02/10] genirq: Define irq_ack() and irq_eoi() helpers
  2021-04-08 15:43   ` Valentin Schneider
  (?)
@ 2021-04-08 17:33   ` kernel test robot
  -1 siblings, 0 replies; 32+ messages in thread
From: kernel test robot @ 2021-04-08 17:33 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2382 bytes --]

Hi Valentin,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on tip/irq/core]
[also build test WARNING on linux/master soc/for-next linus/master v5.12-rc6 next-20210408]
[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]

url:    https://github.com/0day-ci/linux/commits/Valentin-Schneider/irqchip-irq-gic-Optimize-masking-by-leveraging-EOImode-1/20210408-234521
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 883ccef355b910398b99dfaf96d40557479a7e9b
config: i386-tinyconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/3d3685bb1cb34be4f8cbb99f050a94ac0d1f98a9
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Valentin-Schneider/irqchip-irq-gic-Optimize-masking-by-leveraging-EOImode-1/20210408-234521
        git checkout 3d3685bb1cb34be4f8cbb99f050a94ac0d1f98a9
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> kernel/irq/chip.c:411:6: warning: no previous prototype for 'ack_irq' [-Wmissing-prototypes]
     411 | void ack_irq(struct irq_desc *desc)
         |      ^~~~~~~
>> kernel/irq/chip.c:419:6: warning: no previous prototype for 'eoi_irq' [-Wmissing-prototypes]
     419 | void eoi_irq(struct irq_desc *desc)
         |      ^~~~~~~


vim +/ack_irq +411 kernel/irq/chip.c

   410	
 > 411	void ack_irq(struct irq_desc *desc)
   412	{
   413		desc->irq_data.chip->irq_ack(&desc->irq_data);
   414	
   415		if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
   416			irq_state_set_flow_masked(desc);
   417	}
   418	
 > 419	void eoi_irq(struct irq_desc *desc)
   420	{
   421		desc->irq_data.chip->irq_eoi(&desc->irq_data);
   422	
   423		if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
   424			irq_state_clr_flow_masked(desc);
   425	}
   426	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 7346 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 02/10] genirq: Define irq_ack() and irq_eoi() helpers
  2021-04-08 15:43   ` Valentin Schneider
  (?)
  (?)
@ 2021-04-08 19:33   ` kernel test robot
  -1 siblings, 0 replies; 32+ messages in thread
From: kernel test robot @ 2021-04-08 19:33 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3151 bytes --]

Hi Valentin,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on tip/irq/core]
[also build test WARNING on linux/master soc/for-next linus/master v5.12-rc6 next-20210408]
[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]

url:    https://github.com/0day-ci/linux/commits/Valentin-Schneider/irqchip-irq-gic-Optimize-masking-by-leveraging-EOImode-1/20210408-234521
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 883ccef355b910398b99dfaf96d40557479a7e9b
config: x86_64-randconfig-r013-20210408 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 56ea2e2fdd691136d5e6631fa0e447173694b82c)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/3d3685bb1cb34be4f8cbb99f050a94ac0d1f98a9
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Valentin-Schneider/irqchip-irq-gic-Optimize-masking-by-leveraging-EOImode-1/20210408-234521
        git checkout 3d3685bb1cb34be4f8cbb99f050a94ac0d1f98a9
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> kernel/irq/chip.c:411:6: warning: no previous prototype for function 'ack_irq' [-Wmissing-prototypes]
   void ack_irq(struct irq_desc *desc)
        ^
   kernel/irq/chip.c:411:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void ack_irq(struct irq_desc *desc)
   ^
   static 
>> kernel/irq/chip.c:419:6: warning: no previous prototype for function 'eoi_irq' [-Wmissing-prototypes]
   void eoi_irq(struct irq_desc *desc)
        ^
   kernel/irq/chip.c:419:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void eoi_irq(struct irq_desc *desc)
   ^
   static 
   2 warnings generated.


vim +/ack_irq +411 kernel/irq/chip.c

   410	
 > 411	void ack_irq(struct irq_desc *desc)
   412	{
   413		desc->irq_data.chip->irq_ack(&desc->irq_data);
   414	
   415		if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
   416			irq_state_set_flow_masked(desc);
   417	}
   418	
 > 419	void eoi_irq(struct irq_desc *desc)
   420	{
   421		desc->irq_data.chip->irq_eoi(&desc->irq_data);
   422	
   423		if (desc->irq_data.chip->flags & IRQCHIP_AUTOMASKS_FLOW)
   424			irq_state_clr_flow_masked(desc);
   425	}
   426	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35979 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking
  2021-04-08 15:43   ` Valentin Schneider
@ 2021-04-09  9:17     ` Jonathan Cameron
  -1 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2021-04-09  9:17 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, Marc Zyngier, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

On Thu, 8 Apr 2021 16:43:17 +0100
Valentin Schneider <valentin.schneider@arm.com> wrote:

> Some IRQ chips such as the Arm GICs automagically mask / unmask an
> IRQ during the handling of said IRQ. This renders further mask / unmask
> operations within the flow handlers redundant, which we do want to leverage
> as masking by itself is not cheap (Distributor access via MMIO for GICs).
> 
> This is different from having a chip->irq_mask_ack() callback as this
> masking is:
> - inherent to the chip->irq_ack() and *cannot* be omitted
> - a *different* masking state than chip->irq_mask() (chip->irq_mask() is
>   idempotent, chip->irq_ack() really isn't)
> 
> Add a chip flag, IRQCHIP_AUTOMASKS_FLOW, to denote chips with such
> behaviour. Add a new IRQ data flag, IRQD_IRQ_FLOW_MASKED, to keep this
> flow-induced mask state separate from regular mask / unmask operations
> (IRQD_IRQ_MASKED).
> 
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Hi Valentin,

One totally trivial thing noticed whilst reading.

> ---
>  include/linux/irq.h    | 10 ++++++++++
>  kernel/irq/chip.c      |  5 +++++
>  kernel/irq/debugfs.c   |  2 ++
>  kernel/irq/internals.h |  5 +++++
>  4 files changed, 22 insertions(+)
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index bee82809107c..580b1b6b1799 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -219,6 +219,8 @@ struct irq_data {
>   *				  irq_chip::irq_set_affinity() when deactivated.
>   * IRQD_IRQ_ENABLED_ON_SUSPEND	- Interrupt is enabled on suspend by irq pm if
>   *				  irqchip have flag IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND set.
> + * IRQD_IRQ_FLOW_MASKED         - Interrupt is masked by ACK. Only EOI can
> + *                                clear this.

Nitpick of the day : Seems text above is using tabs for white space blocks
whereas you have used spaces. Make it consistent.
It's not consistent in the file so I guess you could clean that up, or
just go with making it consistent in this block.

>   */
>  enum {
>  	IRQD_TRIGGER_MASK		= 0xf,
> @@ -245,6 +247,7 @@ enum {
>  	IRQD_HANDLE_ENFORCE_IRQCTX	= (1 << 28),
>  	IRQD_AFFINITY_ON_ACTIVATE	= (1 << 29),
>  	IRQD_IRQ_ENABLED_ON_SUSPEND	= (1 << 30),
> +	IRQD_IRQ_FLOW_MASKED            = (1 << 31),
>  };
>  
>  #define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors)
> @@ -349,6 +352,11 @@ static inline bool irqd_irq_masked(struct irq_data *d)
>  	return __irqd_to_state(d) & IRQD_IRQ_MASKED;
>  }
>  
> +static inline bool irqd_irq_flow_masked(struct irq_data *d)
> +{
> +	return __irqd_to_state(d) & IRQD_IRQ_FLOW_MASKED;
> +}
> +
>  static inline bool irqd_irq_inprogress(struct irq_data *d)
>  {
>  	return __irqd_to_state(d) & IRQD_IRQ_INPROGRESS;
> @@ -567,6 +575,7 @@ struct irq_chip {
>   * IRQCHIP_SUPPORTS_NMI:              Chip can deliver NMIs, only for root irqchips
>   * IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND:  Invokes __enable_irq()/__disable_irq() for wake irqs
>   *                                    in the suspend path if they are in disabled state
> + * IRQCHIP_AUTOMASKS_FLOW:            chip->ack() masks and chip->eoi() unmasks
>   */
>  enum {
>  	IRQCHIP_SET_TYPE_MASKED			= (1 <<  0),
> @@ -579,6 +588,7 @@ enum {
>  	IRQCHIP_SUPPORTS_LEVEL_MSI		= (1 <<  7),
>  	IRQCHIP_SUPPORTS_NMI			= (1 <<  8),
>  	IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND	= (1 <<  9),
> +	IRQCHIP_AUTOMASKS_FLOW                  = (1 <<  10),
>  };
>  
>  #include <linux/irqdesc.h>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 8cc8e5713287..18c3b0e1568a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -173,6 +173,11 @@ static void irq_state_clr_masked(struct irq_desc *desc)
>  	irqd_clear(&desc->irq_data, IRQD_IRQ_MASKED);
>  }
>  
> +static void irq_state_clr_flow_masked(struct irq_desc *desc)
> +{
> +	irqd_clear(&desc->irq_data, IRQD_IRQ_FLOW_MASKED);
> +}
> +
>  static void irq_state_clr_started(struct irq_desc *desc)
>  {
>  	irqd_clear(&desc->irq_data, IRQD_IRQ_STARTED);
> diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
> index e4cff358b437..3ae83622d701 100644
> --- a/kernel/irq/debugfs.c
> +++ b/kernel/irq/debugfs.c
> @@ -58,6 +58,7 @@ static const struct irq_bit_descr irqchip_flags[] = {
>  	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_LEVEL_MSI),
>  	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_NMI),
>  	BIT_MASK_DESCR(IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND),
> +	BIT_MASK_DESCR(IRQCHIP_AUTOMASKS_FLOW),
>  };
>  
>  static void
> @@ -103,6 +104,7 @@ static const struct irq_bit_descr irqdata_states[] = {
>  	BIT_MASK_DESCR(IRQD_IRQ_STARTED),
>  	BIT_MASK_DESCR(IRQD_IRQ_DISABLED),
>  	BIT_MASK_DESCR(IRQD_IRQ_MASKED),
> +	BIT_MASK_DESCR(IRQD_IRQ_FLOW_MASKED),
>  	BIT_MASK_DESCR(IRQD_IRQ_INPROGRESS),
>  
>  	BIT_MASK_DESCR(IRQD_PER_CPU),
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index 54363527feea..b6c1cceddec0 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -245,6 +245,11 @@ static inline void irq_state_set_masked(struct irq_desc *desc)
>  	irqd_set(&desc->irq_data, IRQD_IRQ_MASKED);
>  }
>  
> +static inline void irq_state_set_flow_masked(struct irq_desc *desc)
> +{
> +	irqd_set(&desc->irq_data, IRQD_IRQ_FLOW_MASKED);
> +}
> +
>  #undef __irqd_to_state
>  
>  static inline void __kstat_incr_irqs_this_cpu(struct irq_desc *desc)


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking
@ 2021-04-09  9:17     ` Jonathan Cameron
  0 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2021-04-09  9:17 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, Marc Zyngier, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

On Thu, 8 Apr 2021 16:43:17 +0100
Valentin Schneider <valentin.schneider@arm.com> wrote:

> Some IRQ chips such as the Arm GICs automagically mask / unmask an
> IRQ during the handling of said IRQ. This renders further mask / unmask
> operations within the flow handlers redundant, which we do want to leverage
> as masking by itself is not cheap (Distributor access via MMIO for GICs).
> 
> This is different from having a chip->irq_mask_ack() callback as this
> masking is:
> - inherent to the chip->irq_ack() and *cannot* be omitted
> - a *different* masking state than chip->irq_mask() (chip->irq_mask() is
>   idempotent, chip->irq_ack() really isn't)
> 
> Add a chip flag, IRQCHIP_AUTOMASKS_FLOW, to denote chips with such
> behaviour. Add a new IRQ data flag, IRQD_IRQ_FLOW_MASKED, to keep this
> flow-induced mask state separate from regular mask / unmask operations
> (IRQD_IRQ_MASKED).
> 
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
Hi Valentin,

One totally trivial thing noticed whilst reading.

> ---
>  include/linux/irq.h    | 10 ++++++++++
>  kernel/irq/chip.c      |  5 +++++
>  kernel/irq/debugfs.c   |  2 ++
>  kernel/irq/internals.h |  5 +++++
>  4 files changed, 22 insertions(+)
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index bee82809107c..580b1b6b1799 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -219,6 +219,8 @@ struct irq_data {
>   *				  irq_chip::irq_set_affinity() when deactivated.
>   * IRQD_IRQ_ENABLED_ON_SUSPEND	- Interrupt is enabled on suspend by irq pm if
>   *				  irqchip have flag IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND set.
> + * IRQD_IRQ_FLOW_MASKED         - Interrupt is masked by ACK. Only EOI can
> + *                                clear this.

Nitpick of the day : Seems text above is using tabs for white space blocks
whereas you have used spaces. Make it consistent.
It's not consistent in the file so I guess you could clean that up, or
just go with making it consistent in this block.

>   */
>  enum {
>  	IRQD_TRIGGER_MASK		= 0xf,
> @@ -245,6 +247,7 @@ enum {
>  	IRQD_HANDLE_ENFORCE_IRQCTX	= (1 << 28),
>  	IRQD_AFFINITY_ON_ACTIVATE	= (1 << 29),
>  	IRQD_IRQ_ENABLED_ON_SUSPEND	= (1 << 30),
> +	IRQD_IRQ_FLOW_MASKED            = (1 << 31),
>  };
>  
>  #define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors)
> @@ -349,6 +352,11 @@ static inline bool irqd_irq_masked(struct irq_data *d)
>  	return __irqd_to_state(d) & IRQD_IRQ_MASKED;
>  }
>  
> +static inline bool irqd_irq_flow_masked(struct irq_data *d)
> +{
> +	return __irqd_to_state(d) & IRQD_IRQ_FLOW_MASKED;
> +}
> +
>  static inline bool irqd_irq_inprogress(struct irq_data *d)
>  {
>  	return __irqd_to_state(d) & IRQD_IRQ_INPROGRESS;
> @@ -567,6 +575,7 @@ struct irq_chip {
>   * IRQCHIP_SUPPORTS_NMI:              Chip can deliver NMIs, only for root irqchips
>   * IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND:  Invokes __enable_irq()/__disable_irq() for wake irqs
>   *                                    in the suspend path if they are in disabled state
> + * IRQCHIP_AUTOMASKS_FLOW:            chip->ack() masks and chip->eoi() unmasks
>   */
>  enum {
>  	IRQCHIP_SET_TYPE_MASKED			= (1 <<  0),
> @@ -579,6 +588,7 @@ enum {
>  	IRQCHIP_SUPPORTS_LEVEL_MSI		= (1 <<  7),
>  	IRQCHIP_SUPPORTS_NMI			= (1 <<  8),
>  	IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND	= (1 <<  9),
> +	IRQCHIP_AUTOMASKS_FLOW                  = (1 <<  10),
>  };
>  
>  #include <linux/irqdesc.h>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 8cc8e5713287..18c3b0e1568a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -173,6 +173,11 @@ static void irq_state_clr_masked(struct irq_desc *desc)
>  	irqd_clear(&desc->irq_data, IRQD_IRQ_MASKED);
>  }
>  
> +static void irq_state_clr_flow_masked(struct irq_desc *desc)
> +{
> +	irqd_clear(&desc->irq_data, IRQD_IRQ_FLOW_MASKED);
> +}
> +
>  static void irq_state_clr_started(struct irq_desc *desc)
>  {
>  	irqd_clear(&desc->irq_data, IRQD_IRQ_STARTED);
> diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
> index e4cff358b437..3ae83622d701 100644
> --- a/kernel/irq/debugfs.c
> +++ b/kernel/irq/debugfs.c
> @@ -58,6 +58,7 @@ static const struct irq_bit_descr irqchip_flags[] = {
>  	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_LEVEL_MSI),
>  	BIT_MASK_DESCR(IRQCHIP_SUPPORTS_NMI),
>  	BIT_MASK_DESCR(IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND),
> +	BIT_MASK_DESCR(IRQCHIP_AUTOMASKS_FLOW),
>  };
>  
>  static void
> @@ -103,6 +104,7 @@ static const struct irq_bit_descr irqdata_states[] = {
>  	BIT_MASK_DESCR(IRQD_IRQ_STARTED),
>  	BIT_MASK_DESCR(IRQD_IRQ_DISABLED),
>  	BIT_MASK_DESCR(IRQD_IRQ_MASKED),
> +	BIT_MASK_DESCR(IRQD_IRQ_FLOW_MASKED),
>  	BIT_MASK_DESCR(IRQD_IRQ_INPROGRESS),
>  
>  	BIT_MASK_DESCR(IRQD_PER_CPU),
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index 54363527feea..b6c1cceddec0 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -245,6 +245,11 @@ static inline void irq_state_set_masked(struct irq_desc *desc)
>  	irqd_set(&desc->irq_data, IRQD_IRQ_MASKED);
>  }
>  
> +static inline void irq_state_set_flow_masked(struct irq_desc *desc)
> +{
> +	irqd_set(&desc->irq_data, IRQD_IRQ_FLOW_MASKED);
> +}
> +
>  #undef __irqd_to_state
>  
>  static inline void __kstat_incr_irqs_this_cpu(struct irq_desc *desc)


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking
  2021-04-09  9:17     ` Jonathan Cameron
@ 2021-04-09 10:34       ` Valentin Schneider
  -1 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-09 10:34 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-kernel, linux-arm-kernel, Marc Zyngier, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

Hi Jonathan,

Thanks for taking a peek :)

On 09/04/21 10:17, Jonathan Cameron wrote:
> On Thu, 8 Apr 2021 16:43:17 +0100
> Valentin Schneider <valentin.schneider@arm.com> wrote:
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index bee82809107c..580b1b6b1799 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -219,6 +219,8 @@ struct irq_data {
>>   *				  irq_chip::irq_set_affinity() when deactivated.
>>   * IRQD_IRQ_ENABLED_ON_SUSPEND	- Interrupt is enabled on suspend by irq pm if
>>   *				  irqchip have flag IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND set.
>> + * IRQD_IRQ_FLOW_MASKED         - Interrupt is masked by ACK. Only EOI can
>> + *                                clear this.
>
> Nitpick of the day : Seems text above is using tabs for white space blocks
> whereas you have used spaces. Make it consistent.
> It's not consistent in the file so I guess you could clean that up, or
> just go with making it consistent in this block.
>

I usually let my editor take the wheel when it comes to whitespace vs tabs,
but it does look like it's not aligned with the rest here.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking
@ 2021-04-09 10:34       ` Valentin Schneider
  0 siblings, 0 replies; 32+ messages in thread
From: Valentin Schneider @ 2021-04-09 10:34 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-kernel, linux-arm-kernel, Marc Zyngier, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

Hi Jonathan,

Thanks for taking a peek :)

On 09/04/21 10:17, Jonathan Cameron wrote:
> On Thu, 8 Apr 2021 16:43:17 +0100
> Valentin Schneider <valentin.schneider@arm.com> wrote:
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index bee82809107c..580b1b6b1799 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -219,6 +219,8 @@ struct irq_data {
>>   *				  irq_chip::irq_set_affinity() when deactivated.
>>   * IRQD_IRQ_ENABLED_ON_SUSPEND	- Interrupt is enabled on suspend by irq pm if
>>   *				  irqchip have flag IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND set.
>> + * IRQD_IRQ_FLOW_MASKED         - Interrupt is masked by ACK. Only EOI can
>> + *                                clear this.
>
> Nitpick of the day : Seems text above is using tabs for white space blocks
> whereas you have used spaces. Make it consistent.
> It's not consistent in the file so I guess you could clean that up, or
> just go with making it consistent in this block.
>

I usually let my editor take the wheel when it comes to whitespace vs tabs,
but it does look like it's not aligned with the rest here.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 05/10] genirq: Let purely flow-masked ONESHOT irqs through unmask_threaded_irq()
  2021-04-08 15:43   ` Valentin Schneider
@ 2021-04-09 13:52     ` Thomas Gleixner
  -1 siblings, 0 replies; 32+ messages in thread
From: Thomas Gleixner @ 2021-04-09 13:52 UTC (permalink / raw)
  To: Valentin Schneider, linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Lorenzo Pieralisi, Vincenzo Frascino

On Thu, Apr 08 2021 at 16:43, Valentin Schneider wrote:
> A subsequent patch will let IRQs end up in irq_finalize_oneshot() without
> IRQD_IRQ_MASKED, but with IRQD_IRQ_FLOW_MASKED set instead. Let such IRQs
> receive their final ->irq_eoi().
>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> ---
>  kernel/irq/manage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index e976c4927b25..59c8056d6714 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1076,7 +1076,7 @@ static void irq_finalize_oneshot(struct irq_desc *desc,
>  	desc->threads_oneshot &= ~action->thread_mask;
>  
>  	if (!desc->threads_oneshot && !irqd_irq_disabled(&desc->irq_data) &&
> -	    irqd_irq_masked(&desc->irq_data))
> +	    (irqd_irq_masked(&desc->irq_data) || irqd_irq_flow_masked(&desc->irq_data)))
>  		unmask_threaded_irq(desc);

This creates better code when you have '|' instead of '||' because the
compiler can spare a conditional and combine the bit check into one.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 05/10] genirq: Let purely flow-masked ONESHOT irqs through unmask_threaded_irq()
@ 2021-04-09 13:52     ` Thomas Gleixner
  0 siblings, 0 replies; 32+ messages in thread
From: Thomas Gleixner @ 2021-04-09 13:52 UTC (permalink / raw)
  To: Valentin Schneider, linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Lorenzo Pieralisi, Vincenzo Frascino

On Thu, Apr 08 2021 at 16:43, Valentin Schneider wrote:
> A subsequent patch will let IRQs end up in irq_finalize_oneshot() without
> IRQD_IRQ_MASKED, but with IRQD_IRQ_FLOW_MASKED set instead. Let such IRQs
> receive their final ->irq_eoi().
>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> ---
>  kernel/irq/manage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index e976c4927b25..59c8056d6714 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1076,7 +1076,7 @@ static void irq_finalize_oneshot(struct irq_desc *desc,
>  	desc->threads_oneshot &= ~action->thread_mask;
>  
>  	if (!desc->threads_oneshot && !irqd_irq_disabled(&desc->irq_data) &&
> -	    irqd_irq_masked(&desc->irq_data))
> +	    (irqd_irq_masked(&desc->irq_data) || irqd_irq_flow_masked(&desc->irq_data)))
>  		unmask_threaded_irq(desc);

This creates better code when you have '|' instead of '||' because the
compiler can spare a conditional and combine the bit check into one.

Thanks,

        tglx

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 06/10] genirq: Don't mask IRQ within flow handler if IRQ is flow-masked
  2021-04-08 15:43   ` Valentin Schneider
@ 2021-04-09 13:57     ` Thomas Gleixner
  -1 siblings, 0 replies; 32+ messages in thread
From: Thomas Gleixner @ 2021-04-09 13:57 UTC (permalink / raw)
  To: Valentin Schneider, linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Lorenzo Pieralisi, Vincenzo Frascino

On Thu, Apr 08 2021 at 16:43, Valentin Schneider wrote:
> +	/*
> +	 * Masking is required if IRQ is ONESHOT and we can't rely on the
> +	 * flow-masking persisting down to irq_finalize_oneshot()
> +	 * (in the IRQ thread).
> +	 */
> +	if ((desc->istate & IRQS_ONESHOT) &&
> +	    (!(chip->flags & IRQCHIP_AUTOMASKS_FLOW) ||
> +	     !(chip->flags & IRQCHIP_EOI_THREADED)))

#define XXXX (IRQCHIP_AUTOMASKS_FLOW | IRQCHIP_EOI_THREADED)

        ((chip->flags & XXXX) != XXXX)

Hmm?

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 06/10] genirq: Don't mask IRQ within flow handler if IRQ is flow-masked
@ 2021-04-09 13:57     ` Thomas Gleixner
  0 siblings, 0 replies; 32+ messages in thread
From: Thomas Gleixner @ 2021-04-09 13:57 UTC (permalink / raw)
  To: Valentin Schneider, linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Lorenzo Pieralisi, Vincenzo Frascino

On Thu, Apr 08 2021 at 16:43, Valentin Schneider wrote:
> +	/*
> +	 * Masking is required if IRQ is ONESHOT and we can't rely on the
> +	 * flow-masking persisting down to irq_finalize_oneshot()
> +	 * (in the IRQ thread).
> +	 */
> +	if ((desc->istate & IRQS_ONESHOT) &&
> +	    (!(chip->flags & IRQCHIP_AUTOMASKS_FLOW) ||
> +	     !(chip->flags & IRQCHIP_EOI_THREADED)))

#define XXXX (IRQCHIP_AUTOMASKS_FLOW | IRQCHIP_EOI_THREADED)

        ((chip->flags & XXXX) != XXXX)

Hmm?

Thanks,

        tglx

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2021-04-09 13:59 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-08 15:43 [RFC PATCH 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
2021-04-08 15:43 ` Valentin Schneider
2021-04-08 15:43 ` [RFC PATCH 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking Valentin Schneider
2021-04-08 15:43   ` Valentin Schneider
2021-04-09  9:17   ` Jonathan Cameron
2021-04-09  9:17     ` Jonathan Cameron
2021-04-09 10:34     ` Valentin Schneider
2021-04-09 10:34       ` Valentin Schneider
2021-04-08 15:43 ` [RFC PATCH 02/10] genirq: Define irq_ack() and irq_eoi() helpers Valentin Schneider
2021-04-08 15:43   ` Valentin Schneider
2021-04-08 17:33   ` kernel test robot
2021-04-08 19:33   ` kernel test robot
2021-04-08 15:43 ` [RFC PATCH 03/10] genirq: Employ ack_irq() and eoi_irq() where relevant Valentin Schneider
2021-04-08 15:43   ` Valentin Schneider
2021-04-08 15:43 ` [RFC PATCH 04/10] genirq: Add handle_strict_flow_irq() flow handler Valentin Schneider
2021-04-08 15:43   ` Valentin Schneider
2021-04-08 15:43 ` [RFC PATCH 05/10] genirq: Let purely flow-masked ONESHOT irqs through unmask_threaded_irq() Valentin Schneider
2021-04-08 15:43   ` Valentin Schneider
2021-04-09 13:52   ` Thomas Gleixner
2021-04-09 13:52     ` Thomas Gleixner
2021-04-08 15:43 ` [RFC PATCH 06/10] genirq: Don't mask IRQ within flow handler if IRQ is flow-masked Valentin Schneider
2021-04-08 15:43   ` Valentin Schneider
2021-04-09 13:57   ` Thomas Gleixner
2021-04-09 13:57     ` Thomas Gleixner
2021-04-08 15:43 ` [RFC PATCH 07/10] genirq, irq-gic-v3: Make NMI flow handlers use ->irq_ack() if available Valentin Schneider
2021-04-08 15:43   ` Valentin Schneider
2021-04-08 15:43 ` [RFC PATCH 08/10] irqchip/gic-v3-its: Use irq_chip_ack_parent() Valentin Schneider
2021-04-08 15:43   ` Valentin Schneider
2021-04-08 15:43 ` [RFC PATCH 09/10] irqchip/gic: Convert to handle_strict_flow_irq() Valentin Schneider
2021-04-08 15:43   ` Valentin Schneider
2021-04-08 15:43 ` [RFC PATCH 10/10] irqchip/gic-v3: " Valentin Schneider
2021-04-08 15:43   ` Valentin Schneider

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.