linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1
@ 2021-05-25 17:32 Valentin Schneider
  2021-05-25 17:32 ` [RFC PATCH v2 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking Valentin Schneider
                   ` (11 more replies)
  0 siblings, 12 replies; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 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 (!).

Revisions
=========

RFCv1 -> RFCv2
++++++++++++++

o Rebased against latest tip/irq/core
o Applied cleanups suggested by Thomas

o Collected some performance results

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.
    
Functional 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.

Performance impact
==================

Benchmark
+++++++++

Finding a benchmark that leverages a force-threaded IRQ has proved to be
somewhat of a pain, so I crafted my own. It's a bit daft, but so are most
benchmarks (though this one might win a prize).

Long story short, I'm picking an unused IRQ and have it be
force-threaded. The benchmark then is:

  <bench thread>
    loop:
      irq_set_irqchip_state(irq, IRQCHIP_STATE_PENDING, true);
      wait_for_completion(&done);

  <threaded handler>
    complete(&done);

A more complete picture would be:

  <bench thread>   <whatever is on CPU0>   <IRQ thread>
    raise IRQ
    wait
		    run flow handler
		      wake IRQ thread
					    finish handling
					    wake bench thread
    
Letting this run for a fixed amount of time lets me measure an entire IRQ
handling cycle, which is what I'm after since there's one less mask() in
the flow handler and one less unmask() in the threaded handler.

You'll note there's some potential "noise" in there due to scheduling both
the benchmark thread and the IRQ thread. However, the IRQ thread is pinned
to the IRQ's affinity, and I also pinned the benchmark thread in my tests,
which should keep this noise to a minimum.

Results
+++++++

On a Juno r0, 20 iterations of 5 seconds of that benchmark yields
(measuring irqs/sec): 

  | mean | median | 90th percentile | 99th percentile |
  |------+--------+-----------------+-----------------|
  | +11% |   +11% |            +12% |            +14% |

On an Ampere eMAG, 20 iterations of 5 seconds of that benchmark yields
(measuring irqs/sec):

  | mean | median | 90th percentile | 99th percentile |
  |------+--------+-----------------+-----------------|
  | +20% |   +20% |            +20% |            +20% |

This is still quite "artificial", but it reassures me in that skipping those
(un)mask operations can yield some measurable improvement.

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                        | 122 ++++++++++++++++++++---
 kernel/irq/debugfs.c                     |   2 +
 kernel/irq/internals.h                   |   7 ++
 kernel/irq/manage.c                      |   2 +-
 9 files changed, 159 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] 25+ messages in thread

* [RFC PATCH v2 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
@ 2021-05-25 17:32 ` Valentin Schneider
  2021-05-25 17:32 ` [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers Valentin Schneider
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 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 8e9a9ae471a6..ef179245a642 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -221,6 +221,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,
@@ -247,6 +249,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)
@@ -351,6 +354,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;
@@ -569,6 +577,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),
@@ -581,6 +590,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 7f04c7d8296e..21a21baa1366 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] 25+ messages in thread

* [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
  2021-05-25 17:32 ` [RFC PATCH v2 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking Valentin Schneider
@ 2021-05-25 17:32 ` Valentin Schneider
  2021-05-27 10:55   ` Marc Zyngier
  2021-05-25 17:32 ` [RFC PATCH v2 03/10] genirq: Employ ack_irq() and eoi_irq() where relevant Valentin Schneider
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 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 21a21baa1366..793dbd8307b9 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] 25+ messages in thread

* [RFC PATCH v2 03/10] genirq: Employ ack_irq() and eoi_irq() where relevant
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
  2021-05-25 17:32 ` [RFC PATCH v2 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking Valentin Schneider
  2021-05-25 17:32 ` [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers Valentin Schneider
@ 2021-05-25 17:32 ` Valentin Schneider
  2021-05-25 17:32 ` [RFC PATCH v2 04/10] genirq: Add handle_strict_flow_irq() flow handler Valentin Schneider
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 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 793dbd8307b9..4d3bde55c5d9 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] 25+ messages in thread

* [RFC PATCH v2 04/10] genirq: Add handle_strict_flow_irq() flow handler
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
                   ` (2 preceding siblings ...)
  2021-05-25 17:32 ` [RFC PATCH v2 03/10] genirq: Employ ack_irq() and eoi_irq() where relevant Valentin Schneider
@ 2021-05-25 17:32 ` Valentin Schneider
  2021-05-25 17:32 ` [RFC PATCH v2 05/10] genirq: Let purely flow-masked ONESHOT irqs through unmask_threaded_irq() Valentin Schneider
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 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 ef179245a642..37075929e329 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -663,6 +663,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 4d3bde55c5d9..699e70b51aae 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] 25+ messages in thread

* [RFC PATCH v2 05/10] genirq: Let purely flow-masked ONESHOT irqs through unmask_threaded_irq()
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
                   ` (3 preceding siblings ...)
  2021-05-25 17:32 ` [RFC PATCH v2 04/10] genirq: Add handle_strict_flow_irq() flow handler Valentin Schneider
@ 2021-05-25 17:32 ` Valentin Schneider
  2021-05-25 17:32 ` [RFC PATCH v2 06/10] genirq: Don't mask IRQ within flow handler if IRQ is flow-masked Valentin Schneider
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 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 ef30b4762947..e6d6d32ddcbc 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1107,7 +1107,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] 25+ messages in thread

* [RFC PATCH v2 06/10] genirq: Don't mask IRQ within flow handler if IRQ is flow-masked
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
                   ` (4 preceding siblings ...)
  2021-05-25 17:32 ` [RFC PATCH v2 05/10] genirq: Let purely flow-masked ONESHOT irqs through unmask_threaded_irq() Valentin Schneider
@ 2021-05-25 17:32 ` Valentin Schneider
  2021-05-25 17:32 ` [RFC PATCH v2 07/10] genirq, irq-gic-v3: Make NMI flow handlers use ->irq_ack() if available Valentin Schneider
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 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 | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 699e70b51aae..c2ca6b748987 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -896,6 +896,12 @@ void handle_edge_eoi_irq(struct irq_desc *desc)
 }
 #endif
 
+/*
+ * AUTOMASKS_FLOW tells us ack/eoi handle the masking, EOI_THREADED tells us
+ * that masking will persist until irq_finalize_oneshot()
+ */
+#define ONESHOT_AUTOMASK_FLAGS (IRQCHIP_AUTOMASKS_FLOW | IRQCHIP_EOI_THREADED)
+
 /**
  *	handle_strict_flow_irq - irq handler for strict controllers
  *	@desc:	the interrupt description structure for this irq
@@ -909,10 +915,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 +927,20 @@ 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 & ONESHOT_AUTOMASK_FLAGS) != ONESHOT_AUTOMASK_FLAGS))
+		mask_ack_irq(desc);
+	else
+		ack_irq(desc);
 
 	handle_irq_event(desc);
 
@@ -933,7 +948,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] 25+ messages in thread

* [RFC PATCH v2 07/10] genirq, irq-gic-v3: Make NMI flow handlers use ->irq_ack() if available
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
                   ` (5 preceding siblings ...)
  2021-05-25 17:32 ` [RFC PATCH v2 06/10] genirq: Don't mask IRQ within flow handler if IRQ is flow-masked Valentin Schneider
@ 2021-05-25 17:32 ` Valentin Schneider
  2021-05-25 17:32 ` [RFC PATCH v2 08/10] irqchip/gic-v3-its: Use irq_chip_ack_parent() Valentin Schneider
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 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 37a23aa6de37..af11396996e3 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 37075929e329..0b45e42812d6 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -669,8 +669,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 c2ca6b748987..099bc7e13d1b 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
@@ -1032,14 +1033,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;
@@ -1048,6 +1048,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] 25+ messages in thread

* [RFC PATCH v2 08/10] irqchip/gic-v3-its: Use irq_chip_ack_parent()
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
                   ` (6 preceding siblings ...)
  2021-05-25 17:32 ` [RFC PATCH v2 07/10] genirq, irq-gic-v3: Make NMI flow handlers use ->irq_ack() if available Valentin Schneider
@ 2021-05-25 17:32 ` Valentin Schneider
  2021-05-27 12:17   ` Marc Zyngier
  2021-05-25 17:32 ` [RFC PATCH v2 09/10] irqchip/gic: Convert to handle_strict_flow_irq() Valentin Schneider
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 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 2e6923c2c8a8..ce39d52409e9 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1976,6 +1976,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] 25+ messages in thread

* [RFC PATCH v2 09/10] irqchip/gic: Convert to handle_strict_flow_irq()
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
                   ` (7 preceding siblings ...)
  2021-05-25 17:32 ` [RFC PATCH v2 08/10] irqchip/gic-v3-its: Use irq_chip_ack_parent() Valentin Schneider
@ 2021-05-25 17:32 ` Valentin Schneider
  2021-05-27 12:21   ` Marc Zyngier
  2021-05-25 17:32 ` [RFC PATCH v2 10/10] irqchip/gic-v3: " Valentin Schneider
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 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] 25+ messages in thread

* [RFC PATCH v2 10/10] irqchip/gic-v3: Convert to handle_strict_flow_irq()
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
                   ` (8 preceding siblings ...)
  2021-05-25 17:32 ` [RFC PATCH v2 09/10] irqchip/gic: Convert to handle_strict_flow_irq() Valentin Schneider
@ 2021-05-25 17:32 ` Valentin Schneider
  2021-05-25 17:34 ` [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
  2021-05-27 11:17 ` Marc Zyngier
  11 siblings, 0 replies; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:32 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 af11396996e3..c2677c5353a4 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
 		gic_arch_enable_irqs();
 	}
 
-	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] 25+ messages in thread

* Re: [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
                   ` (9 preceding siblings ...)
  2021-05-25 17:32 ` [RFC PATCH v2 10/10] irqchip/gic-v3: " Valentin Schneider
@ 2021-05-25 17:34 ` Valentin Schneider
  2021-05-27 11:17 ` Marc Zyngier
  11 siblings, 0 replies; 25+ messages in thread
From: Valentin Schneider @ 2021-05-25 17:34 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi, Vincenzo Frascino

On 25/05/21 18:32, Valentin Schneider wrote:
> Hi folks!
>
> This is the spiritual successor to [1], which was over 6 years ago (!).
>

This went missing from the wall of text:

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

_______________________________________________
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] 25+ messages in thread

* Re: [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
  2021-05-25 17:32 ` [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers Valentin Schneider
@ 2021-05-27 10:55   ` Marc Zyngier
  2021-05-27 10:58     ` Marc Zyngier
  0 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-05-27 10:55 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

On Tue, 25 May 2021 18:32:47 +0100,
Valentin Schneider <valentin.schneider@arm.com> wrote:
> 
> 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 21a21baa1366..793dbd8307b9 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);

Nit: we have {un,}mask_irq, but you add irq_{ack,eoi}. It'd be good to
have some naming consistency (yes, this may/will clash with existing
code, but we can fix that as well).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
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] 25+ messages in thread

* Re: [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
  2021-05-27 10:55   ` Marc Zyngier
@ 2021-05-27 10:58     ` Marc Zyngier
  2021-06-01 10:25       ` Valentin Schneider
  0 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-05-27 10:58 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

On Thu, 27 May 2021 11:55:50 +0100,
Marc Zyngier <maz@kernel.org> wrote:
> 
> On Tue, 25 May 2021 18:32:47 +0100,
> Valentin Schneider <valentin.schneider@arm.com> wrote:
> > 
> > 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 21a21baa1366..793dbd8307b9 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);
> 
> Nit: we have {un,}mask_irq, but you add irq_{ack,eoi}. It'd be good to
> have some naming consistency (yes, this may/will clash with existing
> code, but we can fix that as well).

Actually, the helpers do have the right naming, but the internal
declarations are the ones that are wrong...

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
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] 25+ messages in thread

* Re: [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1
  2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
                   ` (10 preceding siblings ...)
  2021-05-25 17:34 ` [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
@ 2021-05-27 11:17 ` Marc Zyngier
  2021-06-01 10:25   ` Valentin Schneider
  11 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-05-27 11:17 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

On Tue, 25 May 2021 18:32:45 +0100,
Valentin Schneider <valentin.schneider@arm.com> wrote:
> 
> Hi folks!
> 
> This is the spiritual successor to [1], which was over 6 years ago (!).
> 
> Revisions
> =========
> 
> RFCv1 -> RFCv2
> ++++++++++++++
> 
> o Rebased against latest tip/irq/core
> o Applied cleanups suggested by Thomas
> 
> o Collected some performance results
> 
> 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.
>     
> Functional 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.

That's interesting/worrying. As far as I remember, this machine uses
GIC500, which is a well known quantity. If pNMIs are causing issues,
that'd probably be a CPU interface problem. Can you elaborate on how
you tried to test that part? Just using the below benchmark?

> 
> Performance impact
> ==================
> 
> Benchmark
> +++++++++
> 
> Finding a benchmark that leverages a force-threaded IRQ has proved to be
> somewhat of a pain, so I crafted my own. It's a bit daft, but so are most
> benchmarks (though this one might win a prize).

I love it (and wrote similar hacks in my time)! :D Can you put that up
somewhere so that I can run the same test on my own zoo and find out
how it fares?

> 
> Long story short, I'm picking an unused IRQ and have it be
> force-threaded. The benchmark then is:
> 
>   <bench thread>
>     loop:
>       irq_set_irqchip_state(irq, IRQCHIP_STATE_PENDING, true);
>       wait_for_completion(&done);
> 
>   <threaded handler>
>     complete(&done);
> 
> A more complete picture would be:
> 
>   <bench thread>   <whatever is on CPU0>   <IRQ thread>
>     raise IRQ
>     wait
> 		    run flow handler
> 		      wake IRQ thread
> 					    finish handling
> 					    wake bench thread
>     
> Letting this run for a fixed amount of time lets me measure an entire IRQ
> handling cycle, which is what I'm after since there's one less mask() in
> the flow handler and one less unmask() in the threaded handler.
> 
> You'll note there's some potential "noise" in there due to scheduling both
> the benchmark thread and the IRQ thread. However, the IRQ thread is pinned
> to the IRQ's affinity, and I also pinned the benchmark thread in my tests,
> which should keep this noise to a minimum.
> 
> Results
> +++++++
> 
> On a Juno r0, 20 iterations of 5 seconds of that benchmark yields
> (measuring irqs/sec): 
> 
>   | mean | median | 90th percentile | 99th percentile |
>   |------+--------+-----------------+-----------------|
>   | +11% |   +11% |            +12% |            +14% |
> 
> On an Ampere eMAG, 20 iterations of 5 seconds of that benchmark yields
> (measuring irqs/sec):
> 
>   | mean | median | 90th percentile | 99th percentile |
>   |------+--------+-----------------+-----------------|
>   | +20% |   +20% |            +20% |            +20% |
> 
> This is still quite "artificial", but it reassures me in that skipping those
> (un)mask operations can yield some measurable improvement.

20% improvement is even higher than I suspected!

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
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] 25+ messages in thread

* Re: [RFC PATCH v2 08/10] irqchip/gic-v3-its: Use irq_chip_ack_parent()
  2021-05-25 17:32 ` [RFC PATCH v2 08/10] irqchip/gic-v3-its: Use irq_chip_ack_parent() Valentin Schneider
@ 2021-05-27 12:17   ` Marc Zyngier
  2021-06-01 10:25     ` Valentin Schneider
  0 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-05-27 12:17 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

On Tue, 25 May 2021 18:32:53 +0100,
Valentin Schneider <valentin.schneider@arm.com> wrote:
> 
> 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

That's an overhead I'd like to avoid in the general case, given that
we already have a bunch of users.

> o make irq_chip_ack_parent() the default chip->irq_ack() via
>   MSI_FLAG_USE_DEF_CHIP_OPS.

Seem like a reasonable approach: how about a custom irq_ack() callback
that iterates over the hierarchy until it finds an a non-NULL entry?
Flows that don't use ack won't be impacted, users that need ack will
provide one if they want, and the default will do something slightly
slower, but at least unsurprising.

> XXX: what about pMSI and fMSI ?

Same thing. They are just bus-specific domains on top of the ITS
domain, and must follow the same convention.

However, this patch is perfectly acceptable to me (as long as you take
care of platform and fsl -MSI).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
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] 25+ messages in thread

* Re: [RFC PATCH v2 09/10] irqchip/gic: Convert to handle_strict_flow_irq()
  2021-05-25 17:32 ` [RFC PATCH v2 09/10] irqchip/gic: Convert to handle_strict_flow_irq() Valentin Schneider
@ 2021-05-27 12:21   ` Marc Zyngier
  2021-06-01 10:25     ` Valentin Schneider
  0 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-05-27 12:21 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

On Tue, 25 May 2021 18:32:54 +0100,
Valentin Schneider <valentin.schneider@arm.com> wrote:
> 
> 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]) {

How about GICv2M, GICv3-MBI, and the collection of widget that build a
domain on top of a GIC domain? I'm worried that they now all need
updating one way or another...

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
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] 25+ messages in thread

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

On 27/05/21 12:17, Marc Zyngier wrote:
> On Tue, 25 May 2021 18:32:45 +0100,
> Valentin Schneider <valentin.schneider@arm.com> wrote:
>> 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.
>
> That's interesting/worrying. As far as I remember, this machine uses
> GIC500, which is a well known quantity. If pNMIs are causing issues,
> that'd probably be a CPU interface problem. Can you elaborate on how
> you tried to test that part? Just using the below benchmark?
>

Not even that, it would hang somewhere at boot. Julien suggested offline
that it might be a problem with the secondaries' PMR initial value, but I
really never got to do dig into it.

>>
>> Performance impact
>> ==================
>>
>> Benchmark
>> +++++++++
>>
>> Finding a benchmark that leverages a force-threaded IRQ has proved to be
>> somewhat of a pain, so I crafted my own. It's a bit daft, but so are most
>> benchmarks (though this one might win a prize).
>
> I love it (and wrote similar hacks in my time)! :D

Yay!

> Can you put that up
> somewhere so that I can run the same test on my own zoo and find out
> how it fares?
>

The setup part is really fugly and I was too ashamed of it to link it in
the cover letter; for ACPI I could simply use acpi_register_gsi() since
that uses the right domain by default, but for DT I ended up adding a DT
entry and a match table.

I'll see about unifying this and I'll send it out your way.

_______________________________________________
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] 25+ messages in thread

* Re: [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers
  2021-05-27 10:58     ` Marc Zyngier
@ 2021-06-01 10:25       ` Valentin Schneider
  0 siblings, 0 replies; 25+ messages in thread
From: Valentin Schneider @ 2021-06-01 10:25 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

On 27/05/21 11:58, Marc Zyngier wrote:
> On Thu, 27 May 2021 11:55:50 +0100,
> Marc Zyngier <maz@kernel.org> wrote:
>>
>> On Tue, 25 May 2021 18:32:47 +0100,
>> Valentin Schneider <valentin.schneider@arm.com> wrote:
>> > 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);
>>
>> Nit: we have {un,}mask_irq, but you add irq_{ack,eoi}. It'd be good to
>> have some naming consistency (yes, this may/will clash with existing
>> code, but we can fix that as well).
>
> Actually, the helpers do have the right naming, but the internal
> declarations are the ones that are wrong...
>

Doh!

>       M.
>
> --
> Without deviation from the norm, progress is not possible.

_______________________________________________
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] 25+ messages in thread

* Re: [RFC PATCH v2 08/10] irqchip/gic-v3-its: Use irq_chip_ack_parent()
  2021-05-27 12:17   ` Marc Zyngier
@ 2021-06-01 10:25     ` Valentin Schneider
  0 siblings, 0 replies; 25+ messages in thread
From: Valentin Schneider @ 2021-06-01 10:25 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

On 27/05/21 13:17, Marc Zyngier wrote:
> On Tue, 25 May 2021 18:32:53 +0100,
> Valentin Schneider <valentin.schneider@arm.com> wrote:
>> o make irq_chip_ack_parent() the default chip->irq_ack() via
>>   MSI_FLAG_USE_DEF_CHIP_OPS.
>
> Seem like a reasonable approach: how about a custom irq_ack() callback
> that iterates over the hierarchy until it finds an a non-NULL entry?
> Flows that don't use ack won't be impacted, users that need ack will
> provide one if they want, and the default will do something slightly
> slower, but at least unsurprising.
>

Sounds about right!

>> XXX: what about pMSI and fMSI ?
>
> Same thing. They are just bus-specific domains on top of the ITS
> domain, and must follow the same convention.
>
> However, this patch is perfectly acceptable to me (as long as you take
> care of platform and fsl -MSI).
>

Noted.

> Thanks,
>
>       M.
>
> --
> Without deviation from the norm, progress is not possible.

_______________________________________________
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] 25+ messages in thread

* Re: [RFC PATCH v2 09/10] irqchip/gic: Convert to handle_strict_flow_irq()
  2021-05-27 12:21   ` Marc Zyngier
@ 2021-06-01 10:25     ` Valentin Schneider
  2021-06-15 15:20       ` Valentin Schneider
  0 siblings, 1 reply; 25+ messages in thread
From: Valentin Schneider @ 2021-06-01 10:25 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino

On 27/05/21 13:21, Marc Zyngier wrote:
> On Tue, 25 May 2021 18:32:54 +0100,
> Valentin Schneider <valentin.schneider@arm.com> wrote:
>> @@ -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]) {
>
> How about GICv2M, GICv3-MBI, and the collection of widget that build a
> domain on top of a GIC domain? I'm worried that they now all need
> updating one way or another...
>

Hmph, that's a good point. It's been a while since I've last stared at the
v2m, I'll go try to page that back in.

>       M.
>
> --
> Without deviation from the norm, progress is not possible.

_______________________________________________
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] 25+ messages in thread

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

On 01/06/21 11:25, Valentin Schneider wrote:
> On 27/05/21 12:17, Marc Zyngier wrote:
>> On Tue, 25 May 2021 18:32:45 +0100,
>> Valentin Schneider <valentin.schneider@arm.com> wrote:
>>> Benchmark
>>> +++++++++
>>>
>>> Finding a benchmark that leverages a force-threaded IRQ has proved to be
>>> somewhat of a pain, so I crafted my own. It's a bit daft, but so are most
>>> benchmarks (though this one might win a prize).
>>
>> I love it (and wrote similar hacks in my time)! :D
>
> Yay!
>
>> Can you put that up
>> somewhere so that I can run the same test on my own zoo and find out
>> how it fares?
>>
>
> The setup part is really fugly and I was too ashamed of it to link it in
> the cover letter; for ACPI I could simply use acpi_register_gsi() since
> that uses the right domain by default, but for DT I ended up adding a DT
> entry and a match table.
>
> I'll see about unifying this and I'll send it out your way.

Scratch the unification, but at least I cleaned up some of the
initialization horrors. Patches + benchmark module are at:

https://git.gitlab.arm.com/linux-arm/linux-vs.git -b mainline/irq/eoimodness-v2

Note: I re-ran that on Juno/eMAG to make sure I didn't bust anything, and
while the eMAG improvements are still there, now I get pretty much zilch on
the Juno :/

I use the below script to drive the testing

---
#!/bin/bash

get_irq_count () {
    cat /proc/interrupts | grep irq-prod | awk '{ print $2; }'
}

for f in $(find /sys/devices/system/cpu/cpufreq/ -name "policy*"); do
    echo "performance" > "$f"/scaling_governor
done

KTHREAD_PID=$(ps -aux | grep irq-prod/ | head -n 1 | awk '{ print $2; }')
taskset -pc 0 $KTHREAD_PID

for ((i=0; i < 20; i++)); do
    base_val=$(get_irq_count)

    now=$(date +%s%3N)
    echo 1 > /sys/kernel/irq_prod/active
    sleep 5
    echo 0 > /sys/kernel/irq_prod/active
    end=$(date +%s%3N)

    end_val=$(get_irq_count)
    delta=$((end_val - base_val))
    duration=$((end - now))

    echo $((delta / (duration / 1000))) > $1/$i
done
---

This gives you a file per iteration with irqs/sec in it, and you can
collate that however you wish - I use python + pandas:

---
#!/usr/bin/env python3

import pandas as pd

keys = ["tip", "patch"]
data = {k : [] for k in keys}

for i in range(20):
    for k in keys:
        with open("/path/to/results/{}/{}".format(k, i), "r") as fh:
            data[k].append(int(fh.read()))

df = pd.DataFrame(data)
df_stats = df.describe(percentiles=[.5, .9, .99])
df_stats["delta"] = (df_stats["patch"] - df_stats["tip"]) / df_stats["tip"]
print(df_stats)
---

i.e.

<load tip/irq/core>
./bench_irq.sh tip
<load series>
./bench_irq.sh patch

./compare.py

_______________________________________________
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] 25+ messages in thread

* Re: [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1
  2021-06-01 10:25   ` Valentin Schneider
  2021-06-03 15:32     ` Valentin Schneider
@ 2021-06-08 15:29     ` Marc Zyngier
  2021-06-08 17:58       ` Lorenzo Pieralisi
  1 sibling, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2021-06-08 15:29 UTC (permalink / raw)
  To: Valentin Schneider
  Cc: linux-kernel, linux-arm-kernel, Thomas Gleixner,
	Lorenzo Pieralisi, Vincenzo Frascino, Mark Rutland

[+Mark, since we discussed about this on IRC]

Hi Valentin,

On Tue, 01 Jun 2021 11:25:01 +0100,
Valentin Schneider <valentin.schneider@arm.com> wrote:
> 
> On 27/05/21 12:17, Marc Zyngier wrote:
> > On Tue, 25 May 2021 18:32:45 +0100,
> > Valentin Schneider <valentin.schneider@arm.com> wrote:
> >> 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.
> >
> > That's interesting/worrying. As far as I remember, this machine uses
> > GIC500, which is a well known quantity. If pNMIs are causing issues,
> > that'd probably be a CPU interface problem. Can you elaborate on how
> > you tried to test that part? Just using the below benchmark?
> >
> 
> Not even that, it would hang somewhere at boot. Julien suggested offline
> that it might be a problem with the secondaries' PMR initial value, but I
> really never got to do dig into it.

I just hit a similar problem on an Altra box, which seems to be
related to using PSCI for idle. PSCI has no idea about priority
masking, and enters CPU suspend with interrupt masked at the PMR
level. Good luck waking up from that.

I've pushed a test branch at [1]. It'd be really good if you could
have a quick look and let me know if that helps in your case (it
certainly does on the box I have access to).

Thanks,

	M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=arm64/nmi-idle

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
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] 25+ messages in thread

* Re: [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1
  2021-06-08 15:29     ` Marc Zyngier
@ 2021-06-08 17:58       ` Lorenzo Pieralisi
  0 siblings, 0 replies; 25+ messages in thread
From: Lorenzo Pieralisi @ 2021-06-08 17:58 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Valentin Schneider, linux-kernel, linux-arm-kernel,
	Thomas Gleixner, Vincenzo Frascino, Mark Rutland, sudeep.holla

[+Sudeep]

On Tue, Jun 08, 2021 at 04:29:14PM +0100, Marc Zyngier wrote:
> [+Mark, since we discussed about this on IRC]
> 
> Hi Valentin,
> 
> On Tue, 01 Jun 2021 11:25:01 +0100,
> Valentin Schneider <valentin.schneider@arm.com> wrote:
> > 
> > On 27/05/21 12:17, Marc Zyngier wrote:
> > > On Tue, 25 May 2021 18:32:45 +0100,
> > > Valentin Schneider <valentin.schneider@arm.com> wrote:
> > >> 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.
> > >
> > > That's interesting/worrying. As far as I remember, this machine uses
> > > GIC500, which is a well known quantity. If pNMIs are causing issues,
> > > that'd probably be a CPU interface problem. Can you elaborate on how
> > > you tried to test that part? Just using the below benchmark?
> > >
> > 
> > Not even that, it would hang somewhere at boot. Julien suggested offline
> > that it might be a problem with the secondaries' PMR initial value, but I
> > really never got to do dig into it.
> 
> I just hit a similar problem on an Altra box, which seems to be
> related to using PSCI for idle. PSCI has no idea about priority
> masking, and enters CPU suspend with interrupt masked at the PMR
> level. Good luck waking up from that.

Gah. If we can manage to understand which path in
psci_cpu_suspend_enter() is causing this problem that'd
be great too (it can be both, for different reasons):

if (!psci_power_state_loses_context(state))
(1)	ret = psci_ops.cpu_suspend(state, 0);
else
(2)	ret = cpu_suspend(state, psci_suspend_finisher);

I'd like to understand if the problem is on idle entry or
exit (or both depending on the state we are entering).

On (1) we would return from the call with the CPU state
retained on (2) with CPU context restored (but it rebooted
from reset - so the PMR value is gone).

I am asking about (2) because I am trying to understand what
the power controller does wrt PMR and wake-up IRQs (ie and
whether the PMR plays a role in that). Reworded: trying to
understand how the PMR behaviour is playing with the power
controller wake-up capabilities.

Thoughts appreciated.

I am sorry that you had to debug this, thank you for that.

Lorenzo

> I've pushed a test branch at [1]. It'd be really good if you could
> have a quick look and let me know if that helps in your case (it
> certainly does on the box I have access to).
> 
> Thanks,
> 
> 	M.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=arm64/nmi-idle
> 
> -- 
> Without deviation from the norm, progress is not possible.

_______________________________________________
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] 25+ messages in thread

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


On 01/06/21 11:25, Valentin Schneider wrote:
> On 27/05/21 13:21, Marc Zyngier wrote:
>> On Tue, 25 May 2021 18:32:54 +0100,
>> Valentin Schneider <valentin.schneider@arm.com> wrote:
>>> @@ -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]) {
>>
>> How about GICv2M, GICv3-MBI, and the collection of widget that build a
>> domain on top of a GIC domain? I'm worried that they now all need
>> updating one way or another...
>>
>
> Hmph, that's a good point. It's been a while since I've last stared at the
> v2m, I'll go try to page that back in.
>

It's taken me a while to get back to this, apologies. Here's where I'm at:

At the very least these need the +.irq_ack() treatment, same as the ITS
chips. We can get around this by giving msi_domain_update_chip_ops() some
invoke-first-non-NULL default callbacks, as you've suggested in:

  http://lore.kernel.org/r/87y2c0s748.wl-maz@kernel.org


Now, looking at this made me think about which irq_chip flags are being
used where, and, well...

PCI-MSI IRQs are deemed 'oneshot safe', but platform-MSI ones aren't. So
for instance, if a GICv2M pMSI IRQ gets force-threaded, we'll make it
IRQS_ONESHOT. However, this is still just a glorified SPI as all mask, ack
and eoi operations will be the root chip's, so we should be able to apply
the eoimode=1 automask trickery to it. This won't happen with the current
patches, since the ->chip we'll seeing in handle_strict_flow_irq() will be
gicv2m_pmsi_irq_chip.

We *could* give that one the required flags, but what actually matters for
the automask thing are the flags of first chip in the hiearachy that has
"proper" ack+eoi callbacks. I don't see a nice way of handling this right
now...

>>       M.
>>
>> --
>> Without deviation from the norm, progress is not possible.

_______________________________________________
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] 25+ messages in thread

end of thread, other threads:[~2021-06-15 19:43 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25 17:32 [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 01/10] genirq: Add chip flag to denote automatic IRQ (un)masking Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 02/10] genirq: Define irq_ack() and irq_eoi() helpers Valentin Schneider
2021-05-27 10:55   ` Marc Zyngier
2021-05-27 10:58     ` Marc Zyngier
2021-06-01 10:25       ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 03/10] genirq: Employ ack_irq() and eoi_irq() where relevant Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 04/10] genirq: Add handle_strict_flow_irq() flow handler Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 05/10] genirq: Let purely flow-masked ONESHOT irqs through unmask_threaded_irq() Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 06/10] genirq: Don't mask IRQ within flow handler if IRQ is flow-masked Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 07/10] genirq, irq-gic-v3: Make NMI flow handlers use ->irq_ack() if available Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 08/10] irqchip/gic-v3-its: Use irq_chip_ack_parent() Valentin Schneider
2021-05-27 12:17   ` Marc Zyngier
2021-06-01 10:25     ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 09/10] irqchip/gic: Convert to handle_strict_flow_irq() Valentin Schneider
2021-05-27 12:21   ` Marc Zyngier
2021-06-01 10:25     ` Valentin Schneider
2021-06-15 15:20       ` Valentin Schneider
2021-05-25 17:32 ` [RFC PATCH v2 10/10] irqchip/gic-v3: " Valentin Schneider
2021-05-25 17:34 ` [RFC PATCH v2 00/10] irqchip/irq-gic: Optimize masking by leveraging EOImode=1 Valentin Schneider
2021-05-27 11:17 ` Marc Zyngier
2021-06-01 10:25   ` Valentin Schneider
2021-06-03 15:32     ` Valentin Schneider
2021-06-08 15:29     ` Marc Zyngier
2021-06-08 17:58       ` Lorenzo Pieralisi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).