[PATCH v2 0/5] arm64: irqchip/gic-v3: Use compiletime constant PMR values

[PATCH v2 0/5] arm64: irqchip/gic-v3: Use compiletime constant PMR values

[Mark Rutland]

This series optimizes the way regular IRQs are masked/unmasked when
GICv3 pseudo-NMIs are used, removing the need for a static key in fast
paths by using a priority value chosen dynamically at boot time.

Thomas, would you be happy for this series to go through the arm64 tree?
The key part of the series is the final patch which changes both arm64
and irqchip, and I expect merge conflicts or functional fallout to be
contained to arm64.

The GIC distributor and PMR/RPR can present different views of the
interrupt priority space dependent upon the values of GICD_CTLR.DS and
SCR_EL3.FIQ. Currently we treat the distributor's view of the priority
space as canonical, and when the two differ we change the way we handle
values in the PMR/RPR, using the `gic_nonsecure_priorities` static key
to decide what to do.

This approach works, but it's sub-optimal. When using pseudo-NMI we
manipulate the distributor rarely, and we manipulate the PMR/RPR
registers very frequently in code spread out throughout the kernel (e.g.
local_irq_{save,restore}()). It would be nicer if we could use fixed
values for the PMR/RPR, and dynamically choose the values programmed
into the distributor.

This series reworks the GIC code and arm64 architecture code to allow
the use of compiletime-constant PMR values. This simplifies the logic
for PMR management, and when using pseudo-NMI this results in smaller
and better generated code for saving/restoring the irqflags, saving ~4K
of text for defconfig + CONFIG_PSEUDO_NMI=y.

The first patch add a new REPEAT_BYTE_U32() helper which can be useful
for drivers. The second is a preparatory cleanup which I think makes sense
regardless of the rest of the series. The third and fourth patches
rework the GICv3 code to be able to choose priorities at boot time, and
the final patch makes the actual switch.

I've given this some light testing atop v6.10-rc1 with pseudo-NMI
enabled (with priority debugging), along with lockdep on the following
systems:

* M1SDP Morello board, bare metal
  Where GICD_CTRL.DS=0, SCR_EL3.FIQ=0
  Using shifted (NS) values in the distributor

* M1SDP Morello board, KVM guest
  Where GICD_CTRL.DS=1, SCR_EL3.FIQ=0
  Using unshifted values in the distributor

* ThunderX2, KVM guest
  Where GICD_CTRL.DS=1, SCR_EL3.FIQ=0
  Using unshifted values in the distributor

On ThunderX2 bare-metal there is an existing boot-time hang when using
pseudo-NMI which is not solved by this series. With this series applied,
the logging added in patch 3 reports that GICD_CTRL.DS=1, SCR_EL3.FIQ=0,
and so this should be using the same priorities which are seem to work
in a guest.

Since v1 [1]:
* Add REPEAT_BYTE_U32()
* Add MarcZ's Reviewed-by & Tested-by tags
* Cleanup commit titles
* Fix typos

[1] https://lore.kernel.org/linux-arm-kernel/20240529172116.1313498-1-mark.rutland@arm.com/

Mark.

Mark Rutland (5):
  wordpart.h: Add REPEAT_BYTE_U32()
  irqchip/gic-common: Remove sync_access callback
  irqchip/gic-v3: Make distributor priorities variables
  irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier
  arm64: irqchip/gic-v3: Select priorities at boot time

 arch/arm64/include/asm/arch_gicv3.h     |  15 --
 arch/arm64/include/asm/ptrace.h         |  35 +---
 arch/arm64/kernel/image-vars.h          |   5 -
 drivers/irqchip/irq-gic-common.c        |  22 +--
 drivers/irqchip/irq-gic-common.h        |   7 +-
 drivers/irqchip/irq-gic-v3-its.c        |  11 +-
 drivers/irqchip/irq-gic-v3.c            | 225 ++++++++++++------------
 drivers/irqchip/irq-gic.c               |  10 +-
 drivers/irqchip/irq-hip04.c             |   6 +-
 include/linux/irqchip/arm-gic-common.h  |   4 -
 include/linux/irqchip/arm-gic-v3-prio.h |  52 ++++++
 include/linux/irqchip/arm-gic-v3.h      |   2 +-
 include/linux/wordpart.h                |   8 +
 13 files changed, 201 insertions(+), 201 deletions(-)
 create mode 100644 include/linux/irqchip/arm-gic-v3-prio.h

-- 
2.30.2

[PATCH v2 1/5] wordpart.h: Add REPEAT_BYTE_U32()

[Mark Rutland]

In some cases it's necessary to replicate a byte across a u32 value, for
which REPEAT_BYTE() would be helpful. Currently this requires explicit
masking of the result to avoid sparse warnings, as e.g.

    (u32)REPEAT_BYTE(0xa0))

... will result in a warning:

    cast truncates bits from constant value (a0a0a0a0a0a0a0a0 becomes a0a0a0a0)

Add a new REPEAT_BYTE_U32() which does the necessary masking internally,
so that we don't need to duplicate this for every usage.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will@kernel.org>
---
 include/linux/wordpart.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/wordpart.h b/include/linux/wordpart.h
index 4ca1ba66d2f07..5a7b97bb7c956 100644
--- a/include/linux/wordpart.h
+++ b/include/linux/wordpart.h
@@ -39,6 +39,14 @@
  */
 #define REPEAT_BYTE(x)	((~0ul / 0xff) * (x))
 
+/**
+ * REPEAT_BYTE_U32 - repeat the value @x multiple times as a u32 value
+ * @x: value to repeat
+ *
+ * NOTE: @x is not checked for > 0xff; larger values produce odd results.
+ */
+#define REPEAT_BYTE_U32(x)	lower_32_bits(REPEAT_BYTE(x))
+
 /* Set bits in the first 'n' bytes when loaded from memory */
 #ifdef __LITTLE_ENDIAN
 #  define aligned_byte_mask(n) ((1UL << 8*(n))-1)
-- 
2.30.2

[PATCH v2 2/5] irqchip/gic-common: Remove sync_access callback

[Mark Rutland]

The gic_configure_irq(), gic_dist_config(), and gic_cpu_config()
functions each take an optional "sync_access" callback, but in almost
all cases this is not used. The only user is the GICv3 driver's
gic_cpu_init() function, which uses gic_redist_wait_for_rwp() as the
"sync_access" callback for gic_cpu_config().

It would be simpler and clearer to remove the callback and have the
GICv3 driver call gic_redist_wait_for_rwp() explicitly after
gic_cpu_config().

Remove the "sync_access" callback, and call gic_redist_wait_for_rwp()
explicitly in the GICv3 driver.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will@kernel.org>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Tested-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-common.c | 16 +++-------------
 drivers/irqchip/irq-gic-common.h |  7 +++----
 drivers/irqchip/irq-gic-v3.c     |  7 ++++---
 drivers/irqchip/irq-gic.c        |  6 +++---
 drivers/irqchip/irq-hip04.c      |  6 +++---
 5 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index afd6a1841715a..4ed17620dc4d7 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -45,7 +45,7 @@ void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
 }
 
 int gic_configure_irq(unsigned int irq, unsigned int type,
-		       void __iomem *base, void (*sync_access)(void))
+		       void __iomem *base)
 {
 	u32 confmask = 0x2 << ((irq % 16) * 2);
 	u32 confoff = (irq / 16) * 4;
@@ -84,14 +84,10 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
 
 	raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
 
-	if (sync_access)
-		sync_access();
-
 	return ret;
 }
 
-void gic_dist_config(void __iomem *base, int gic_irqs,
-		     void (*sync_access)(void))
+void gic_dist_config(void __iomem *base, int gic_irqs)
 {
 	unsigned int i;
 
@@ -118,12 +114,9 @@ void gic_dist_config(void __iomem *base, int gic_irqs,
 		writel_relaxed(GICD_INT_EN_CLR_X32,
 			       base + GIC_DIST_ENABLE_CLEAR + i / 8);
 	}
-
-	if (sync_access)
-		sync_access();
 }
 
-void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void))
+void gic_cpu_config(void __iomem *base, int nr)
 {
 	int i;
 
@@ -144,7 +137,4 @@ void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void))
 	for (i = 0; i < nr; i += 4)
 		writel_relaxed(GICD_INT_DEF_PRI_X4,
 					base + GIC_DIST_PRI + i * 4 / 4);
-
-	if (sync_access)
-		sync_access();
 }
diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
index f407cce9ecaaa..c230175dd584c 100644
--- a/drivers/irqchip/irq-gic-common.h
+++ b/drivers/irqchip/irq-gic-common.h
@@ -20,10 +20,9 @@ struct gic_quirk {
 };
 
 int gic_configure_irq(unsigned int irq, unsigned int type,
-                       void __iomem *base, void (*sync_access)(void));
-void gic_dist_config(void __iomem *base, int gic_irqs,
-		     void (*sync_access)(void));
-void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void));
+                       void __iomem *base);
+void gic_dist_config(void __iomem *base, int gic_irqs);
+void gic_cpu_config(void __iomem *base, int nr);
 void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
 		void *data);
 void gic_enable_of_quirks(const struct device_node *np,
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 6fb276504bcc8..d95dda2383fb5 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -670,7 +670,7 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
 
 	offset = convert_offset_index(d, GICD_ICFGR, &index);
 
-	ret = gic_configure_irq(index, type, base + offset, NULL);
+	ret = gic_configure_irq(index, type, base + offset);
 	if (ret && (range == PPI_RANGE || range == EPPI_RANGE)) {
 		/* Misconfigured PPIs are usually not fatal */
 		pr_warn("GIC: PPI INTID%ld is secure or misconfigured\n", irq);
@@ -940,7 +940,7 @@ static void __init gic_dist_init(void)
 		writel_relaxed(GICD_INT_DEF_PRI_X4, base + GICD_IPRIORITYRnE + i);
 
 	/* Now do the common stuff */
-	gic_dist_config(base, GIC_LINE_NR, NULL);
+	gic_dist_config(base, GIC_LINE_NR);
 
 	val = GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1;
 	if (gic_data.rdists.gicd_typer2 & GICD_TYPER2_nASSGIcap) {
@@ -1282,7 +1282,8 @@ static void gic_cpu_init(void)
 	for (i = 0; i < gic_data.ppi_nr + SGI_NR; i += 32)
 		writel_relaxed(~0, rbase + GICR_IGROUPR0 + i / 8);
 
-	gic_cpu_config(rbase, gic_data.ppi_nr + SGI_NR, gic_redist_wait_for_rwp);
+	gic_cpu_config(rbase, gic_data.ppi_nr + SGI_NR);
+	gic_redist_wait_for_rwp();
 
 	/* initialise system registers */
 	gic_cpu_sys_reg_init();
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 98aa383e39db1..87255bde960fc 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -303,7 +303,7 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
 			    type != IRQ_TYPE_EDGE_RISING)
 		return -EINVAL;
 
-	ret = gic_configure_irq(gicirq, type, base + GIC_DIST_CONFIG, NULL);
+	ret = gic_configure_irq(gicirq, type, base + GIC_DIST_CONFIG);
 	if (ret && gicirq < 32) {
 		/* Misconfigured PPIs are usually not fatal */
 		pr_warn("GIC: PPI%ld is secure or misconfigured\n", gicirq - 16);
@@ -479,7 +479,7 @@ static void gic_dist_init(struct gic_chip_data *gic)
 	for (i = 32; i < gic_irqs; i += 4)
 		writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 4 / 4);
 
-	gic_dist_config(base, gic_irqs, NULL);
+	gic_dist_config(base, gic_irqs);
 
 	writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL);
 }
@@ -516,7 +516,7 @@ static int gic_cpu_init(struct gic_chip_data *gic)
 				gic_cpu_map[i] &= ~cpu_mask;
 	}
 
-	gic_cpu_config(dist_base, 32, NULL);
+	gic_cpu_config(dist_base, 32);
 
 	writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
 	gic_cpu_if_up(gic);
diff --git a/drivers/irqchip/irq-hip04.c b/drivers/irqchip/irq-hip04.c
index 46161f6ff289d..5285150fd9096 100644
--- a/drivers/irqchip/irq-hip04.c
+++ b/drivers/irqchip/irq-hip04.c
@@ -130,7 +130,7 @@ static int hip04_irq_set_type(struct irq_data *d, unsigned int type)
 
 	raw_spin_lock(&irq_controller_lock);
 
-	ret = gic_configure_irq(irq, type, base + GIC_DIST_CONFIG, NULL);
+	ret = gic_configure_irq(irq, type, base + GIC_DIST_CONFIG);
 	if (ret && irq < 32) {
 		/* Misconfigured PPIs are usually not fatal */
 		pr_warn("GIC: PPI%d is secure or misconfigured\n", irq - 16);
@@ -260,7 +260,7 @@ static void __init hip04_irq_dist_init(struct hip04_irq_data *intc)
 	for (i = 32; i < nr_irqs; i += 2)
 		writel_relaxed(cpumask, base + GIC_DIST_TARGET + ((i * 2) & ~3));
 
-	gic_dist_config(base, nr_irqs, NULL);
+	gic_dist_config(base, nr_irqs);
 
 	writel_relaxed(1, base + GIC_DIST_CTRL);
 }
@@ -287,7 +287,7 @@ static void hip04_irq_cpu_init(struct hip04_irq_data *intc)
 		if (i != cpu)
 			hip04_cpu_map[i] &= ~cpu_mask;
 
-	gic_cpu_config(dist_base, 32, NULL);
+	gic_cpu_config(dist_base, 32);
 
 	writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
 	writel_relaxed(1, base + GIC_CPU_CTRL);
-- 
2.30.2

[PATCH v2 3/5] irqchip/gic-v3: Make distributor priorities variables

[Mark Rutland]

In subsequent patches the GICv3 driver will choose the regular interrupt
priority at boot time.

In preparation for using dynamic priorities, place the priorities in
variables and update the code to pass these as parameters. Users of
GICD_INT_DEF_PRI_X4 are modified to replicate the priority byte using
REPEAT_BYTE_U32().

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will@kernel.org>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Tested-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-common.c       | 10 ++++++----
 drivers/irqchip/irq-gic-common.h       |  4 ++--
 drivers/irqchip/irq-gic-v3-its.c       | 11 ++++++-----
 drivers/irqchip/irq-gic-v3.c           | 19 +++++++++++--------
 drivers/irqchip/irq-gic.c              |  8 ++++----
 drivers/irqchip/irq-hip04.c            |  4 ++--
 include/linux/irqchip/arm-gic-common.h |  4 ----
 include/linux/irqchip/arm-gic-v3.h     |  2 +-
 8 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 4ed17620dc4d7..c776f9142610e 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -7,6 +7,7 @@
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/irqchip/arm-gic.h>
+#include <linux/kernel.h>
 
 #include "irq-gic-common.h"
 
@@ -87,7 +88,7 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
 	return ret;
 }
 
-void gic_dist_config(void __iomem *base, int gic_irqs)
+void gic_dist_config(void __iomem *base, int gic_irqs, u8 priority)
 {
 	unsigned int i;
 
@@ -102,7 +103,8 @@ void gic_dist_config(void __iomem *base, int gic_irqs)
 	 * Set priority on all global interrupts.
 	 */
 	for (i = 32; i < gic_irqs; i += 4)
-		writel_relaxed(GICD_INT_DEF_PRI_X4, base + GIC_DIST_PRI + i);
+		writel_relaxed(REPEAT_BYTE_U32(priority),
+			       base + GIC_DIST_PRI + i);
 
 	/*
 	 * Deactivate and disable all SPIs. Leave the PPI and SGIs
@@ -116,7 +118,7 @@ void gic_dist_config(void __iomem *base, int gic_irqs)
 	}
 }
 
-void gic_cpu_config(void __iomem *base, int nr)
+void gic_cpu_config(void __iomem *base, int nr, u8 priority)
 {
 	int i;
 
@@ -135,6 +137,6 @@ void gic_cpu_config(void __iomem *base, int nr)
 	 * Set priority on PPI and SGI interrupts
 	 */
 	for (i = 0; i < nr; i += 4)
-		writel_relaxed(GICD_INT_DEF_PRI_X4,
+		writel_relaxed(REPEAT_BYTE_U32(priority),
 					base + GIC_DIST_PRI + i * 4 / 4);
 }
diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
index c230175dd584c..e8eab72ef1954 100644
--- a/drivers/irqchip/irq-gic-common.h
+++ b/drivers/irqchip/irq-gic-common.h
@@ -21,8 +21,8 @@ struct gic_quirk {
 
 int gic_configure_irq(unsigned int irq, unsigned int type,
                        void __iomem *base);
-void gic_dist_config(void __iomem *base, int gic_irqs);
-void gic_cpu_config(void __iomem *base, int nr);
+void gic_dist_config(void __iomem *base, int gic_irqs, u8 priority);
+void gic_cpu_config(void __iomem *base, int nr, u8 priority);
 void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
 		void *data);
 void gic_enable_of_quirks(const struct device_node *np,
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 3c755d5dad6e6..42e63272154e0 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -59,7 +59,7 @@ static u32 lpi_id_bits;
 #define LPI_PROPBASE_SZ		ALIGN(BIT(LPI_NRBITS), SZ_64K)
 #define LPI_PENDBASE_SZ		ALIGN(BIT(LPI_NRBITS) / 8, SZ_64K)
 
-#define LPI_PROP_DEFAULT_PRIO	GICD_INT_DEF_PRI
+static u8 __ro_after_init lpi_prop_prio;
 
 /*
  * Collection structure - just an ID, and a redistributor address to
@@ -1926,7 +1926,7 @@ static int its_vlpi_unmap(struct irq_data *d)
 	/* and restore the physical one */
 	irqd_clr_forwarded_to_vcpu(d);
 	its_send_mapti(its_dev, d->hwirq, event);
-	lpi_update_config(d, 0xff, (LPI_PROP_DEFAULT_PRIO |
+	lpi_update_config(d, 0xff, (lpi_prop_prio |
 				    LPI_PROP_ENABLED |
 				    LPI_PROP_GROUP1));
 
@@ -2181,8 +2181,8 @@ static void its_lpi_free(unsigned long *bitmap, u32 base, u32 nr_ids)
 
 static void gic_reset_prop_table(void *va)
 {
-	/* Priority 0xa0, Group-1, disabled */
-	memset(va, LPI_PROP_DEFAULT_PRIO | LPI_PROP_GROUP1, LPI_PROPBASE_SZ);
+	/* Regular IRQ priority, Group-1, disabled */
+	memset(va, lpi_prop_prio | LPI_PROP_GROUP1, LPI_PROPBASE_SZ);
 
 	/* Make sure the GIC will observe the written configuration */
 	gic_flush_dcache_to_poc(va, LPI_PROPBASE_SZ);
@@ -5650,7 +5650,7 @@ int __init its_lpi_memreserve_init(void)
 }
 
 int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
-		    struct irq_domain *parent_domain)
+		    struct irq_domain *parent_domain, u8 irq_prio)
 {
 	struct device_node *of_node;
 	struct its_node *its;
@@ -5660,6 +5660,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 
 	gic_rdists = rdists;
 
+	lpi_prop_prio = irq_prio;
 	its_parent = parent_domain;
 	of_node = to_of_node(handle);
 	if (of_node)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index d95dda2383fb5..d884d94c6f4d0 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -12,6 +12,7 @@
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/irqdomain.h>
+#include <linux/kernel.h>
 #include <linux/kstrtox.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
@@ -36,7 +37,8 @@
 
 #include "irq-gic-common.h"
 
-#define GICD_INT_NMI_PRI	(GICD_INT_DEF_PRI & ~0x80)
+static u8 dist_prio_irq __ro_after_init = GICD_INT_DEF_PRI;
+static u8 dist_prio_nmi __ro_after_init = GICD_INT_DEF_PRI & ~0x80;
 
 #define FLAGS_WORKAROUND_GICR_WAKER_MSM8996	(1ULL << 0)
 #define FLAGS_WORKAROUND_CAVIUM_ERRATUM_38539	(1ULL << 1)
@@ -556,7 +558,7 @@ static int gic_irq_nmi_setup(struct irq_data *d)
 		desc->handle_irq = handle_fasteoi_nmi;
 	}
 
-	gic_irq_set_prio(d, GICD_INT_NMI_PRI);
+	gic_irq_set_prio(d, dist_prio_nmi);
 
 	return 0;
 }
@@ -591,7 +593,7 @@ static void gic_irq_nmi_teardown(struct irq_data *d)
 		desc->handle_irq = handle_fasteoi_irq;
 	}
 
-	gic_irq_set_prio(d, GICD_INT_DEF_PRI);
+	gic_irq_set_prio(d, dist_prio_irq);
 }
 
 static bool gic_arm64_erratum_2941627_needed(struct irq_data *d)
@@ -753,7 +755,7 @@ static bool gic_rpr_is_nmi_prio(void)
 	if (!gic_supports_nmi())
 		return false;
 
-	return unlikely(gic_read_rpr() == GICD_INT_RPR_PRI(GICD_INT_NMI_PRI));
+	return unlikely(gic_read_rpr() == GICD_INT_RPR_PRI(dist_prio_nmi));
 }
 
 static bool gic_irqnr_is_special(u32 irqnr)
@@ -937,10 +939,11 @@ static void __init gic_dist_init(void)
 		writel_relaxed(0, base + GICD_ICFGRnE + i / 4);
 
 	for (i = 0; i < GIC_ESPI_NR; i += 4)
-		writel_relaxed(GICD_INT_DEF_PRI_X4, base + GICD_IPRIORITYRnE + i);
+		writel_relaxed(REPEAT_BYTE_U32(dist_prio_irq),
+			       base + GICD_IPRIORITYRnE + i);
 
 	/* Now do the common stuff */
-	gic_dist_config(base, GIC_LINE_NR);
+	gic_dist_config(base, GIC_LINE_NR, dist_prio_irq);
 
 	val = GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1;
 	if (gic_data.rdists.gicd_typer2 & GICD_TYPER2_nASSGIcap) {
@@ -1282,7 +1285,7 @@ static void gic_cpu_init(void)
 	for (i = 0; i < gic_data.ppi_nr + SGI_NR; i += 32)
 		writel_relaxed(~0, rbase + GICR_IGROUPR0 + i / 8);
 
-	gic_cpu_config(rbase, gic_data.ppi_nr + SGI_NR);
+	gic_cpu_config(rbase, gic_data.ppi_nr + SGI_NR, dist_prio_irq);
 	gic_redist_wait_for_rwp();
 
 	/* initialise system registers */
@@ -2066,7 +2069,7 @@ static int __init gic_init_bases(phys_addr_t dist_phys_base,
 	gic_cpu_pm_init();
 
 	if (gic_dist_supports_lpis()) {
-		its_init(handle, &gic_data.rdists, gic_data.domain);
+		its_init(handle, &gic_data.rdists, gic_data.domain, dist_prio_irq);
 		its_cpu_init();
 		its_lpi_memreserve_init();
 	} else {
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 87255bde960fc..3be7bd8cd8cde 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -479,7 +479,7 @@ static void gic_dist_init(struct gic_chip_data *gic)
 	for (i = 32; i < gic_irqs; i += 4)
 		writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 4 / 4);
 
-	gic_dist_config(base, gic_irqs);
+	gic_dist_config(base, gic_irqs, GICD_INT_DEF_PRI);
 
 	writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL);
 }
@@ -516,7 +516,7 @@ static int gic_cpu_init(struct gic_chip_data *gic)
 				gic_cpu_map[i] &= ~cpu_mask;
 	}
 
-	gic_cpu_config(dist_base, 32);
+	gic_cpu_config(dist_base, 32, GICD_INT_DEF_PRI);
 
 	writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
 	gic_cpu_if_up(gic);
@@ -608,7 +608,7 @@ void gic_dist_restore(struct gic_chip_data *gic)
 			dist_base + GIC_DIST_CONFIG + i * 4);
 
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
-		writel_relaxed(GICD_INT_DEF_PRI_X4,
+		writel_relaxed(REPEAT_BYTE_U32(GICD_INT_DEF_PRI),
 			dist_base + GIC_DIST_PRI + i * 4);
 
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
@@ -697,7 +697,7 @@ void gic_cpu_restore(struct gic_chip_data *gic)
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_CONFIG + i * 4);
 
 	for (i = 0; i < DIV_ROUND_UP(32, 4); i++)
-		writel_relaxed(GICD_INT_DEF_PRI_X4,
+		writel_relaxed(REPEAT_BYTE_U32(GICD_INT_DEF_PRI),
 					dist_base + GIC_DIST_PRI + i * 4);
 
 	writel_relaxed(GICC_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK);
diff --git a/drivers/irqchip/irq-hip04.c b/drivers/irqchip/irq-hip04.c
index 5285150fd9096..31c3f70a5d5e1 100644
--- a/drivers/irqchip/irq-hip04.c
+++ b/drivers/irqchip/irq-hip04.c
@@ -260,7 +260,7 @@ static void __init hip04_irq_dist_init(struct hip04_irq_data *intc)
 	for (i = 32; i < nr_irqs; i += 2)
 		writel_relaxed(cpumask, base + GIC_DIST_TARGET + ((i * 2) & ~3));
 
-	gic_dist_config(base, nr_irqs);
+	gic_dist_config(base, nr_irqs, GICD_INT_DEF_PRI);
 
 	writel_relaxed(1, base + GIC_DIST_CTRL);
 }
@@ -287,7 +287,7 @@ static void hip04_irq_cpu_init(struct hip04_irq_data *intc)
 		if (i != cpu)
 			hip04_cpu_map[i] &= ~cpu_mask;
 
-	gic_cpu_config(dist_base, 32);
+	gic_cpu_config(dist_base, 32, GICD_INT_DEF_PRI);
 
 	writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
 	writel_relaxed(1, base + GIC_CPU_CTRL);
diff --git a/include/linux/irqchip/arm-gic-common.h b/include/linux/irqchip/arm-gic-common.h
index 1177f3a1aed5d..fc0246cc05ac2 100644
--- a/include/linux/irqchip/arm-gic-common.h
+++ b/include/linux/irqchip/arm-gic-common.h
@@ -10,10 +10,6 @@
 #include <linux/irqchip/arm-vgic-info.h>
 
 #define GICD_INT_DEF_PRI		0xa0
-#define GICD_INT_DEF_PRI_X4		((GICD_INT_DEF_PRI << 24) |\
-					(GICD_INT_DEF_PRI << 16) |\
-					(GICD_INT_DEF_PRI << 8) |\
-					GICD_INT_DEF_PRI)
 
 struct irq_domain;
 struct fwnode_handle;
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 728691365464c..70c0948f978eb 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -638,7 +638,7 @@ struct fwnode_handle;
 int __init its_lpi_memreserve_init(void);
 int its_cpu_init(void);
 int its_init(struct fwnode_handle *handle, struct rdists *rdists,
-	     struct irq_domain *domain);
+	     struct irq_domain *domain, u8 irq_prio);
 int mbi_init(struct fwnode_handle *fwnode, struct irq_domain *parent);
 
 static inline bool gic_enable_sre(void)
-- 
2.30.2

[PATCH v2 4/5] irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier

[Mark Rutland]

In subsequent patches the GICv3 driver will choose the regular interrupt
priority at boot time, dependent on the configuration of GICD_CTRL.DS
and SCR_EL3.FIQ. This will need to be chosen before we configure the
distributor with default prioirities for all the interrupts, which
happens before we currently detect these in gic_cpu_sys_reg_init().

Add a new gic_prio_init() function to detect these earlier and log them
to the console so that any problems can be debugged more easily. This
also allows the uniformity checks in gic_cpu_sys_reg_init() to be
simplified, as we can compare directly with the boot CPU values which
were recorded earlier.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will@kernel.org>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Tested-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-v3.c | 117 +++++++++++++++++++----------------
 1 file changed, 63 insertions(+), 54 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index d884d94c6f4d0..e262f73b1ce84 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -134,6 +134,62 @@ EXPORT_SYMBOL(gic_nonsecure_priorities);
 		__priority;						\
 	})
 
+static u32 gic_get_pribits(void)
+{
+	u32 pribits;
+
+	pribits = gic_read_ctlr();
+	pribits &= ICC_CTLR_EL1_PRI_BITS_MASK;
+	pribits >>= ICC_CTLR_EL1_PRI_BITS_SHIFT;
+	pribits++;
+
+	return pribits;
+}
+
+static bool gic_has_group0(void)
+{
+	u32 val;
+	u32 old_pmr;
+
+	old_pmr = gic_read_pmr();
+
+	/*
+	 * Let's find out if Group0 is under control of EL3 or not by
+	 * setting the highest possible, non-zero priority in PMR.
+	 *
+	 * If SCR_EL3.FIQ is set, the priority gets shifted down in
+	 * order for the CPU interface to set bit 7, and keep the
+	 * actual priority in the non-secure range. In the process, it
+	 * looses the least significant bit and the actual priority
+	 * becomes 0x80. Reading it back returns 0, indicating that
+	 * we're don't have access to Group0.
+	 */
+	gic_write_pmr(BIT(8 - gic_get_pribits()));
+	val = gic_read_pmr();
+
+	gic_write_pmr(old_pmr);
+
+	return val != 0;
+}
+
+static inline bool gic_dist_security_disabled(void)
+{
+	return readl_relaxed(gic_data.dist_base + GICD_CTLR) & GICD_CTLR_DS;
+}
+
+static bool cpus_have_security_disabled __ro_after_init;
+static bool cpus_have_group0 __ro_after_init;
+
+static void __init gic_prio_init(void)
+{
+	cpus_have_security_disabled = gic_dist_security_disabled();
+	cpus_have_group0 = gic_has_group0();
+
+	pr_info("GICD_CTRL.DS=%d, SCR_EL3.FIQ=%d\n",
+		cpus_have_security_disabled,
+		!cpus_have_group0);
+}
+
 /* rdist_nmi_refs[n] == number of cpus having the rdist interrupt n set as NMI */
 static refcount_t *rdist_nmi_refs;
 
@@ -868,44 +924,6 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs
 		__gic_handle_irq_from_irqson(regs);
 }
 
-static u32 gic_get_pribits(void)
-{
-	u32 pribits;
-
-	pribits = gic_read_ctlr();
-	pribits &= ICC_CTLR_EL1_PRI_BITS_MASK;
-	pribits >>= ICC_CTLR_EL1_PRI_BITS_SHIFT;
-	pribits++;
-
-	return pribits;
-}
-
-static bool gic_has_group0(void)
-{
-	u32 val;
-	u32 old_pmr;
-
-	old_pmr = gic_read_pmr();
-
-	/*
-	 * Let's find out if Group0 is under control of EL3 or not by
-	 * setting the highest possible, non-zero priority in PMR.
-	 *
-	 * If SCR_EL3.FIQ is set, the priority gets shifted down in
-	 * order for the CPU interface to set bit 7, and keep the
-	 * actual priority in the non-secure range. In the process, it
-	 * looses the least significant bit and the actual priority
-	 * becomes 0x80. Reading it back returns 0, indicating that
-	 * we're don't have access to Group0.
-	 */
-	gic_write_pmr(BIT(8 - gic_get_pribits()));
-	val = gic_read_pmr();
-
-	gic_write_pmr(old_pmr);
-
-	return val != 0;
-}
-
 static void __init gic_dist_init(void)
 {
 	unsigned int i;
@@ -1122,12 +1140,6 @@ static void gic_update_rdist_properties(void)
 			gic_data.rdists.has_vpend_valid_dirty ? "Valid+Dirty " : "");
 }
 
-/* Check whether it's single security state view */
-static inline bool gic_dist_security_disabled(void)
-{
-	return readl_relaxed(gic_data.dist_base + GICD_CTLR) & GICD_CTLR_DS;
-}
-
 static void gic_cpu_sys_reg_init(void)
 {
 	int i, cpu = smp_processor_id();
@@ -1155,18 +1167,14 @@ static void gic_cpu_sys_reg_init(void)
 		write_gicreg(DEFAULT_PMR_VALUE, ICC_PMR_EL1);
 	} else if (gic_supports_nmi()) {
 		/*
-		 * Mismatch configuration with boot CPU, the system is likely
-		 * to die as interrupt masking will not work properly on all
-		 * CPUs
+		 * Check that all CPUs use the same priority space.
 		 *
-		 * The boot CPU calls this function before enabling NMI support,
-		 * and as a result we'll never see this warning in the boot path
-		 * for that CPU.
+		 * If there's a mismatch with the boot CPU, the system is
+		 * likely to die as interrupt masking will not work properly on
+		 * all CPUs.
 		 */
-		if (static_branch_unlikely(&gic_nonsecure_priorities))
-			WARN_ON(!group0 || gic_dist_security_disabled());
-		else
-			WARN_ON(group0 && !gic_dist_security_disabled());
+		WARN_ON(group0 != cpus_have_group0);
+		WARN_ON(gic_dist_security_disabled() != cpus_have_security_disabled);
 	}
 
 	/*
@@ -2062,6 +2070,7 @@ static int __init gic_init_bases(phys_addr_t dist_phys_base,
 
 	gic_update_rdist_properties();
 
+	gic_prio_init();
 	gic_dist_init();
 	gic_cpu_init();
 	gic_enable_nmi_support();
-- 
2.30.2

[PATCH v2 5/5] arm64: irqchip/gic-v3: Select priorities at boot time

[Mark Rutland]

The distributor and PMR/RPR can present different views of the interrupt
priority space dependent upon the values of GICD_CTLR.DS and
SCR_EL3.FIQ. Currently we treat the distributor's view of the priority
space as canonical, and when the two differ we change the way we handle
values in the PMR/RPR, using the `gic_nonsecure_priorities` static key
to decide what to do.

This approach works, but it's sub-optimal. When using pseudo-NMI we
manipulate the distributor rarely, and we manipulate the PMR/RPR
registers very frequently in code spread out throughout the kernel (e.g.
local_irq_{save,restore}()). It would be nicer if we could use fixed
values for the PMR/RPR, and dynamically choose the values programmed
into the distributor.

This patch changes the GICv3 driver and arm64 code accordingly. PMR
values are chosen at compile time, and the GICv3 driver determines the
appropriate values to program into the distributor at boot time. This
removes the need for the `gic_nonsecure_priorities` static key and
results in smaller and better generated code for saving/restoring the
irqflags.

Before this patch, local_irq_disable() compiles to:

| 0000000000000000 <outlined_local_irq_disable>:
|    0:   d503201f        nop
|    4:   d50343df        msr     daifset, #0x3
|    8:   d65f03c0        ret
|    c:   d503201f        nop
|   10:   d2800c00        mov     x0, #0x60                       // #96
|   14:   d5184600        msr     icc_pmr_el1, x0
|   18:   d65f03c0        ret
|   1c:   d2801400        mov     x0, #0xa0                       // #160
|   20:   17fffffd        b       14 <outlined_local_irq_disable+0x14>

After this patch, local_irq_disable() compiles to:

| 0000000000000000 <outlined_local_irq_disable>:
|    0:   d503201f        nop
|    4:   d50343df        msr     daifset, #0x3
|    8:   d65f03c0        ret
|    c:   d2801800        mov     x0, #0xc0                       // #192
|   10:   d5184600        msr     icc_pmr_el1, x0
|   14:   d65f03c0        ret

... with 3 fewer instructions per call.

For defconfig + CONFIG_PSEUDO_NMI=y, this results in a minor saving of
~4K of text, and will make it easier to make further improvements to the
way we manipulate irqflags and DAIF bits.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will@kernel.org>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Tested-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/arch_gicv3.h     | 15 ----
 arch/arm64/include/asm/ptrace.h         | 35 ++-------
 arch/arm64/kernel/image-vars.h          |  5 --
 drivers/irqchip/irq-gic-v3.c            | 96 ++++++++++---------------
 include/linux/irqchip/arm-gic-v3-prio.h | 52 ++++++++++++++
 5 files changed, 97 insertions(+), 106 deletions(-)
 create mode 100644 include/linux/irqchip/arm-gic-v3-prio.h

diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index 5f172611654b1..9e96f024b2f19 100644
--- a/arch/arm64/include/asm/arch_gicv3.h
+++ b/arch/arm64/include/asm/arch_gicv3.h
@@ -175,21 +175,6 @@ static inline bool gic_prio_masking_enabled(void)
 
 static inline void gic_pmr_mask_irqs(void)
 {
-	BUILD_BUG_ON(GICD_INT_DEF_PRI < (__GIC_PRIO_IRQOFF |
-					 GIC_PRIO_PSR_I_SET));
-	BUILD_BUG_ON(GICD_INT_DEF_PRI >= GIC_PRIO_IRQON);
-	/*
-	 * Need to make sure IRQON allows IRQs when SCR_EL3.FIQ is cleared
-	 * and non-secure PMR accesses are not subject to the shifts that
-	 * are applied to IRQ priorities
-	 */
-	BUILD_BUG_ON((0x80 | (GICD_INT_DEF_PRI >> 1)) >= GIC_PRIO_IRQON);
-	/*
-	 * Same situation as above, but now we make sure that we can mask
-	 * regular interrupts.
-	 */
-	BUILD_BUG_ON((0x80 | (GICD_INT_DEF_PRI >> 1)) < (__GIC_PRIO_IRQOFF_NS |
-							 GIC_PRIO_PSR_I_SET));
 	gic_write_pmr(GIC_PRIO_IRQOFF);
 }
 
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 47ec58031f11b..0abe975d68a8e 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -21,35 +21,12 @@
 #define INIT_PSTATE_EL2 \
 	(PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT | PSR_MODE_EL2h)
 
-/*
- * PMR values used to mask/unmask interrupts.
- *
- * GIC priority masking works as follows: if an IRQ's priority is a higher value
- * than the value held in PMR, that IRQ is masked. Lowering the value of PMR
- * means masking more IRQs (or at least that the same IRQs remain masked).
- *
- * To mask interrupts, we clear the most significant bit of PMR.
- *
- * Some code sections either automatically switch back to PSR.I or explicitly
- * require to not use priority masking. If bit GIC_PRIO_PSR_I_SET is included
- * in the priority mask, it indicates that PSR.I should be set and
- * interrupt disabling temporarily does not rely on IRQ priorities.
- */
-#define GIC_PRIO_IRQON			0xe0
-#define __GIC_PRIO_IRQOFF		(GIC_PRIO_IRQON & ~0x80)
-#define __GIC_PRIO_IRQOFF_NS		0xa0
-#define GIC_PRIO_PSR_I_SET		(1 << 4)
-
-#define GIC_PRIO_IRQOFF							\
-	({								\
-		extern struct static_key_false gic_nonsecure_priorities;\
-		u8 __prio = __GIC_PRIO_IRQOFF;				\
-									\
-		if (static_branch_unlikely(&gic_nonsecure_priorities))	\
-			__prio = __GIC_PRIO_IRQOFF_NS;			\
-									\
-		__prio;							\
-	})
+#include <linux/irqchip/arm-gic-v3-prio.h>
+
+#define GIC_PRIO_IRQON		GICV3_PRIO_UNMASKED
+#define GIC_PRIO_IRQOFF		GICV3_PRIO_IRQ
+
+#define GIC_PRIO_PSR_I_SET	GICV3_PRIO_PSR_I_SET
 
 /* Additional SPSR bits not exposed in the UABI */
 #define PSR_MODE_THREAD_BIT	(1 << 0)
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index ba4f8f7d6a91a..8f5422ed1b758 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -105,11 +105,6 @@ KVM_NVHE_ALIAS(__hyp_stub_vectors);
 KVM_NVHE_ALIAS(vgic_v2_cpuif_trap);
 KVM_NVHE_ALIAS(vgic_v3_cpuif_trap);
 
-#ifdef CONFIG_ARM64_PSEUDO_NMI
-/* Static key checked in GIC_PRIO_IRQOFF. */
-KVM_NVHE_ALIAS(gic_nonsecure_priorities);
-#endif
-
 /* EL2 exception handling */
 KVM_NVHE_ALIAS(__start___kvm_ex_table);
 KVM_NVHE_ALIAS(__stop___kvm_ex_table);
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index e262f73b1ce84..1b2b5f89f8321 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -25,6 +25,7 @@
 #include <linux/irqchip.h>
 #include <linux/irqchip/arm-gic-common.h>
 #include <linux/irqchip/arm-gic-v3.h>
+#include <linux/irqchip/arm-gic-v3-prio.h>
 #include <linux/irqchip/irq-partition-percpu.h>
 #include <linux/bitfield.h>
 #include <linux/bits.h>
@@ -37,8 +38,8 @@
 
 #include "irq-gic-common.h"
 
-static u8 dist_prio_irq __ro_after_init = GICD_INT_DEF_PRI;
-static u8 dist_prio_nmi __ro_after_init = GICD_INT_DEF_PRI & ~0x80;
+static u8 dist_prio_irq __ro_after_init = GICV3_PRIO_IRQ;
+static u8 dist_prio_nmi __ro_after_init = GICV3_PRIO_NMI;
 
 #define FLAGS_WORKAROUND_GICR_WAKER_MSM8996	(1ULL << 0)
 #define FLAGS_WORKAROUND_CAVIUM_ERRATUM_38539	(1ULL << 1)
@@ -110,30 +111,6 @@ static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key);
  */
 static DEFINE_STATIC_KEY_FALSE(supports_pseudo_nmis);
 
-DEFINE_STATIC_KEY_FALSE(gic_nonsecure_priorities);
-EXPORT_SYMBOL(gic_nonsecure_priorities);
-
-/*
- * When the Non-secure world has access to group 0 interrupts (as a
- * consequence of SCR_EL3.FIQ == 0), reading the ICC_RPR_EL1 register will
- * return the Distributor's view of the interrupt priority.
- *
- * When GIC security is enabled (GICD_CTLR.DS == 0), the interrupt priority
- * written by software is moved to the Non-secure range by the Distributor.
- *
- * If both are true (which is when gic_nonsecure_priorities gets enabled),
- * we need to shift down the priority programmed by software to match it
- * against the value returned by ICC_RPR_EL1.
- */
-#define GICD_INT_RPR_PRI(priority)					\
-	({								\
-		u32 __priority = (priority);				\
-		if (static_branch_unlikely(&gic_nonsecure_priorities))	\
-			__priority = 0x80 | (__priority >> 1);		\
-									\
-		__priority;						\
-	})
-
 static u32 gic_get_pribits(void)
 {
 	u32 pribits;
@@ -185,6 +162,41 @@ static void __init gic_prio_init(void)
 	cpus_have_security_disabled = gic_dist_security_disabled();
 	cpus_have_group0 = gic_has_group0();
 
+	/*
+	 * How priority values are used by the GIC depends on two things:
+	 * the security state of the GIC (controlled by the GICD_CTRL.DS bit)
+	 * and if Group 0 interrupts can be delivered to Linux in the non-secure
+	 * world as FIQs (controlled by the SCR_EL3.FIQ bit). These affect the
+	 * way priorities are presented in ICC_PMR_EL1 and in the distributor:
+	 *
+	 * GICD_CTRL.DS | SCR_EL3.FIQ | ICC_PMR_EL1 | Distributor
+	 * -------------------------------------------------------
+	 *      1       |      -      |  unchanged  |  unchanged
+	 * -------------------------------------------------------
+	 *      0       |      1      |  non-secure |  non-secure
+	 * -------------------------------------------------------
+	 *      0       |      0      |  unchanged  |  non-secure
+	 *
+	 * In the non-secure view reads and writes are modified:
+	 *
+	 * - A value written is right-shifted by one and the MSB is set,
+	 *   forcing the priority into the non-secure range.
+	 *
+	 * - A value read is left-shifted by one.
+	 *
+	 * In the first two cases, where ICC_PMR_EL1 and the interrupt priority
+	 * are both either modified or unchanged, we can use the same set of
+	 * priorities.
+	 *
+	 * In the last case, where only the interrupt priorities are modified to
+	 * be in the non-secure range, we program the non-secure values into
+	 * the distributor to match the PMR values we want.
+	 */
+	if (cpus_have_group0 & !cpus_have_security_disabled) {
+		dist_prio_irq = __gicv3_prio_to_ns(dist_prio_irq);
+		dist_prio_nmi = __gicv3_prio_to_ns(dist_prio_nmi);
+	}
+
 	pr_info("GICD_CTRL.DS=%d, SCR_EL3.FIQ=%d\n",
 		cpus_have_security_disabled,
 		!cpus_have_group0);
@@ -811,7 +823,7 @@ static bool gic_rpr_is_nmi_prio(void)
 	if (!gic_supports_nmi())
 		return false;
 
-	return unlikely(gic_read_rpr() == GICD_INT_RPR_PRI(dist_prio_nmi));
+	return unlikely(gic_read_rpr() == GICV3_PRIO_NMI);
 }
 
 static bool gic_irqnr_is_special(u32 irqnr)
@@ -1960,36 +1972,6 @@ static void gic_enable_nmi_support(void)
 	pr_info("Pseudo-NMIs enabled using %s ICC_PMR_EL1 synchronisation\n",
 		gic_has_relaxed_pmr_sync() ? "relaxed" : "forced");
 
-	/*
-	 * How priority values are used by the GIC depends on two things:
-	 * the security state of the GIC (controlled by the GICD_CTRL.DS bit)
-	 * and if Group 0 interrupts can be delivered to Linux in the non-secure
-	 * world as FIQs (controlled by the SCR_EL3.FIQ bit). These affect the
-	 * ICC_PMR_EL1 register and the priority that software assigns to
-	 * interrupts:
-	 *
-	 * GICD_CTRL.DS | SCR_EL3.FIQ | ICC_PMR_EL1 | Group 1 priority
-	 * -----------------------------------------------------------
-	 *      1       |      -      |  unchanged  |    unchanged
-	 * -----------------------------------------------------------
-	 *      0       |      1      |  non-secure |    non-secure
-	 * -----------------------------------------------------------
-	 *      0       |      0      |  unchanged  |    non-secure
-	 *
-	 * where non-secure means that the value is right-shifted by one and the
-	 * MSB bit set, to make it fit in the non-secure priority range.
-	 *
-	 * In the first two cases, where ICC_PMR_EL1 and the interrupt priority
-	 * are both either modified or unchanged, we can use the same set of
-	 * priorities.
-	 *
-	 * In the last case, where only the interrupt priorities are modified to
-	 * be in the non-secure range, we use a different PMR value to mask IRQs
-	 * and the rest of the values that we use remain unchanged.
-	 */
-	if (gic_has_group0() && !gic_dist_security_disabled())
-		static_branch_enable(&gic_nonsecure_priorities);
-
 	static_branch_enable(&supports_pseudo_nmis);
 
 	if (static_branch_likely(&supports_deactivate_key))
diff --git a/include/linux/irqchip/arm-gic-v3-prio.h b/include/linux/irqchip/arm-gic-v3-prio.h
new file mode 100644
index 0000000000000..44157c9abb78b
--- /dev/null
+++ b/include/linux/irqchip/arm-gic-v3-prio.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __LINUX_IRQCHIP_ARM_GIC_V3_PRIO_H
+#define __LINUX_IRQCHIP_ARM_GIC_V3_PRIO_H
+
+/*
+ * GIC priorities from the view of the PMR/RPR.
+ *
+ * These values are chosen to be valid in either the absolute priority space or
+ * the NS view of the priority space. The value programmed into the distributor
+ * and ITS will be chosen at boot time such that these values appear in the
+ * PMR/RPR.
+ *
+ * GICV3_PRIO_UNMASKED is the PMR view of the priority to use to permit both
+ * IRQs and pseudo-NMIs.
+ *
+ * GICV3_PRIO_IRQ is the PMR view of the priority of regular interrupts. This
+ * can be written to the PMR to mask regular IRQs.
+ *
+ * GICV3_PRIO_NMI is the PMR view of the priority of pseudo-NMIs. This can be
+ * written to the PMR to mask pseudo-NMIs.
+ *
+ * On arm64 some code sections either automatically switch back to PSR.I or
+ * explicitly require to not use priority masking. If bit GICV3_PRIO_PSR_I_SET
+ * is included in the priority mask, it indicates that PSR.I should be set and
+ * interrupt disabling temporarily does not rely on IRQ priorities.
+ */
+#define GICV3_PRIO_UNMASKED	0xe0
+#define GICV3_PRIO_IRQ		0xc0
+#define GICV3_PRIO_NMI		0x80
+
+#define GICV3_PRIO_PSR_I_SET	(1 << 4)
+
+#ifndef __ASSEMBLER__
+
+#define __gicv3_prio_to_ns(p)	(0xff & ((p) << 1))
+#define __gicv3_ns_to_prio(ns)	(0x80 | ((ns) >> 1))
+
+#define __gicv3_prio_valid_ns(p) \
+	(__gicv3_ns_to_prio(__gicv3_prio_to_ns(p)) == (p))
+
+static_assert(__gicv3_prio_valid_ns(GICV3_PRIO_NMI));
+static_assert(__gicv3_prio_valid_ns(GICV3_PRIO_IRQ));
+
+static_assert(GICV3_PRIO_NMI < GICV3_PRIO_IRQ);
+static_assert(GICV3_PRIO_IRQ < GICV3_PRIO_UNMASKED);
+
+static_assert(GICV3_PRIO_IRQ < (GICV3_PRIO_IRQ | GICV3_PRIO_PSR_I_SET));
+
+#endif /* __ASSEMBLER */
+
+#endif /* __LINUX_IRQCHIP_ARM_GIC_V3_PRIO_H */
-- 
2.30.2

Re: [PATCH v2 2/5] irqchip/gic-common: Remove sync_access callback

[Jinjie Ruan]

On 2024/6/17 19:18, Mark Rutland wrote:
> The gic_configure_irq(), gic_dist_config(), and gic_cpu_config()
> functions each take an optional "sync_access" callback, but in almost
> all cases this is not used. The only user is the GICv3 driver's
> gic_cpu_init() function, which uses gic_redist_wait_for_rwp() as the
> "sync_access" callback for gic_cpu_config().
> 
> It would be simpler and clearer to remove the callback and have the
> GICv3 driver call gic_redist_wait_for_rwp() explicitly after
> gic_cpu_config().
> 
> Remove the "sync_access" callback, and call gic_redist_wait_for_rwp()
> explicitly in the GICv3 driver.
> 
> There should be no functional change as a result of this patch.

There seems to be a similar patch already:

https://lore.kernel.org/all/20230902134106.1969-1-yuzenghui@huawei.com/

> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Will Deacon <will@kernel.org>
> Reviewed-by: Marc Zyngier <maz@kernel.org>
> Tested-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/irqchip/irq-gic-common.c | 16 +++-------------
>  drivers/irqchip/irq-gic-common.h |  7 +++----
>  drivers/irqchip/irq-gic-v3.c     |  7 ++++---
>  drivers/irqchip/irq-gic.c        |  6 +++---
>  drivers/irqchip/irq-hip04.c      |  6 +++---
>  5 files changed, 16 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
> index afd6a1841715a..4ed17620dc4d7 100644
> --- a/drivers/irqchip/irq-gic-common.c
> +++ b/drivers/irqchip/irq-gic-common.c
> @@ -45,7 +45,7 @@ void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
>  }
>  
>  int gic_configure_irq(unsigned int irq, unsigned int type,
> -		       void __iomem *base, void (*sync_access)(void))
> +		       void __iomem *base)
>  {
>  	u32 confmask = 0x2 << ((irq % 16) * 2);
>  	u32 confoff = (irq / 16) * 4;
> @@ -84,14 +84,10 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
>  
>  	raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
>  
> -	if (sync_access)
> -		sync_access();
> -
>  	return ret;
>  }
>  
> -void gic_dist_config(void __iomem *base, int gic_irqs,
> -		     void (*sync_access)(void))
> +void gic_dist_config(void __iomem *base, int gic_irqs)
>  {
>  	unsigned int i;
>  
> @@ -118,12 +114,9 @@ void gic_dist_config(void __iomem *base, int gic_irqs,
>  		writel_relaxed(GICD_INT_EN_CLR_X32,
>  			       base + GIC_DIST_ENABLE_CLEAR + i / 8);
>  	}
> -
> -	if (sync_access)
> -		sync_access();
>  }
>  
> -void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void))
> +void gic_cpu_config(void __iomem *base, int nr)
>  {
>  	int i;
>  
> @@ -144,7 +137,4 @@ void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void))
>  	for (i = 0; i < nr; i += 4)
>  		writel_relaxed(GICD_INT_DEF_PRI_X4,
>  					base + GIC_DIST_PRI + i * 4 / 4);
> -
> -	if (sync_access)
> -		sync_access();
>  }
> diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
> index f407cce9ecaaa..c230175dd584c 100644
> --- a/drivers/irqchip/irq-gic-common.h
> +++ b/drivers/irqchip/irq-gic-common.h
> @@ -20,10 +20,9 @@ struct gic_quirk {
>  };
>  
>  int gic_configure_irq(unsigned int irq, unsigned int type,
> -                       void __iomem *base, void (*sync_access)(void));
> -void gic_dist_config(void __iomem *base, int gic_irqs,
> -		     void (*sync_access)(void));
> -void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void));
> +                       void __iomem *base);
> +void gic_dist_config(void __iomem *base, int gic_irqs);
> +void gic_cpu_config(void __iomem *base, int nr);
>  void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
>  		void *data);
>  void gic_enable_of_quirks(const struct device_node *np,
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 6fb276504bcc8..d95dda2383fb5 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -670,7 +670,7 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
>  
>  	offset = convert_offset_index(d, GICD_ICFGR, &index);
>  
> -	ret = gic_configure_irq(index, type, base + offset, NULL);
> +	ret = gic_configure_irq(index, type, base + offset);
>  	if (ret && (range == PPI_RANGE || range == EPPI_RANGE)) {
>  		/* Misconfigured PPIs are usually not fatal */
>  		pr_warn("GIC: PPI INTID%ld is secure or misconfigured\n", irq);
> @@ -940,7 +940,7 @@ static void __init gic_dist_init(void)
>  		writel_relaxed(GICD_INT_DEF_PRI_X4, base + GICD_IPRIORITYRnE + i);
>  
>  	/* Now do the common stuff */
> -	gic_dist_config(base, GIC_LINE_NR, NULL);
> +	gic_dist_config(base, GIC_LINE_NR);
>  
>  	val = GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1;
>  	if (gic_data.rdists.gicd_typer2 & GICD_TYPER2_nASSGIcap) {
> @@ -1282,7 +1282,8 @@ static void gic_cpu_init(void)
>  	for (i = 0; i < gic_data.ppi_nr + SGI_NR; i += 32)
>  		writel_relaxed(~0, rbase + GICR_IGROUPR0 + i / 8);
>  
> -	gic_cpu_config(rbase, gic_data.ppi_nr + SGI_NR, gic_redist_wait_for_rwp);
> +	gic_cpu_config(rbase, gic_data.ppi_nr + SGI_NR);
> +	gic_redist_wait_for_rwp();
>  
>  	/* initialise system registers */
>  	gic_cpu_sys_reg_init();
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 98aa383e39db1..87255bde960fc 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -303,7 +303,7 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
>  			    type != IRQ_TYPE_EDGE_RISING)
>  		return -EINVAL;
>  
> -	ret = gic_configure_irq(gicirq, type, base + GIC_DIST_CONFIG, NULL);
> +	ret = gic_configure_irq(gicirq, type, base + GIC_DIST_CONFIG);
>  	if (ret && gicirq < 32) {
>  		/* Misconfigured PPIs are usually not fatal */
>  		pr_warn("GIC: PPI%ld is secure or misconfigured\n", gicirq - 16);
> @@ -479,7 +479,7 @@ static void gic_dist_init(struct gic_chip_data *gic)
>  	for (i = 32; i < gic_irqs; i += 4)
>  		writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 4 / 4);
>  
> -	gic_dist_config(base, gic_irqs, NULL);
> +	gic_dist_config(base, gic_irqs);
>  
>  	writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL);
>  }
> @@ -516,7 +516,7 @@ static int gic_cpu_init(struct gic_chip_data *gic)
>  				gic_cpu_map[i] &= ~cpu_mask;
>  	}
>  
> -	gic_cpu_config(dist_base, 32, NULL);
> +	gic_cpu_config(dist_base, 32);
>  
>  	writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
>  	gic_cpu_if_up(gic);
> diff --git a/drivers/irqchip/irq-hip04.c b/drivers/irqchip/irq-hip04.c
> index 46161f6ff289d..5285150fd9096 100644
> --- a/drivers/irqchip/irq-hip04.c
> +++ b/drivers/irqchip/irq-hip04.c
> @@ -130,7 +130,7 @@ static int hip04_irq_set_type(struct irq_data *d, unsigned int type)
>  
>  	raw_spin_lock(&irq_controller_lock);
>  
> -	ret = gic_configure_irq(irq, type, base + GIC_DIST_CONFIG, NULL);
> +	ret = gic_configure_irq(irq, type, base + GIC_DIST_CONFIG);
>  	if (ret && irq < 32) {
>  		/* Misconfigured PPIs are usually not fatal */
>  		pr_warn("GIC: PPI%d is secure or misconfigured\n", irq - 16);
> @@ -260,7 +260,7 @@ static void __init hip04_irq_dist_init(struct hip04_irq_data *intc)
>  	for (i = 32; i < nr_irqs; i += 2)
>  		writel_relaxed(cpumask, base + GIC_DIST_TARGET + ((i * 2) & ~3));
>  
> -	gic_dist_config(base, nr_irqs, NULL);
> +	gic_dist_config(base, nr_irqs);
>  
>  	writel_relaxed(1, base + GIC_DIST_CTRL);
>  }
> @@ -287,7 +287,7 @@ static void hip04_irq_cpu_init(struct hip04_irq_data *intc)
>  		if (i != cpu)
>  			hip04_cpu_map[i] &= ~cpu_mask;
>  
> -	gic_cpu_config(dist_base, 32, NULL);
> +	gic_cpu_config(dist_base, 32);
>  
>  	writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
>  	writel_relaxed(1, base + GIC_CPU_CTRL);

Re: [PATCH v2 5/5] arm64: irqchip/gic-v3: Select priorities at boot time

[Liao, Chang]

在 2024/6/17 19:18, Mark Rutland 写道:
>  	cpus_have_group0 = gic_has_group0();

> +#define __gicv3_prio_to_ns(p)	(0xff & ((p) << 1))
> +#define __gicv3_ns_to_prio(ns)	(0x80 | ((ns) >> 1))

What about refactoring the gic_has_group0() using the mapping macros
between PMR priority and GIC priority like this:

---------------%<-----------------
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -882,6 +882,7 @@ static bool gic_has_group0(void)
 {
        u32 val;
        u32 old_pmr;
+       u32 prio = BIT(8 - gic_get_pribits());

        old_pmr = gic_read_pmr();

@@ -896,12 +897,12 @@ static bool gic_has_group0(void)
         * becomes 0x80. Reading it back returns 0, indicating that
         * we're don't have access to Group0.
         */
-       gic_write_pmr(BIT(8 - gic_get_pribits()));
+       gic_write_pmr(prio);
        val = gic_read_pmr();

        gic_write_pmr(old_pmr);

-       return val != 0;
+       return val != (__gicv3_prio_to_ns(__gicv3_ns_to_prio(prio)));
 }
--------------->%-----------------


-- 
BR
Liao, Chang

Re: [PATCH v2 5/5] arm64: irqchip/gic-v3: Select priorities at boot time

[Marc Zyngier]

On Fri, 21 Jun 2024 07:23:54 +0100,
"Liao, Chang" <liaochang1@huawei.com> wrote:
> 
> 
> 
> 在 2024/6/17 19:18, Mark Rutland 写道:
> >  	cpus_have_group0 = gic_has_group0();
> 
> > +#define __gicv3_prio_to_ns(p)	(0xff & ((p) << 1))
> > +#define __gicv3_ns_to_prio(ns)	(0x80 | ((ns) >> 1))
> 
> What about refactoring the gic_has_group0() using the mapping macros
> between PMR priority and GIC priority like this:
> 
> ---------------%<-----------------
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -882,6 +882,7 @@ static bool gic_has_group0(void)
>  {
>         u32 val;
>         u32 old_pmr;
> +       u32 prio = BIT(8 - gic_get_pribits());
> 
>         old_pmr = gic_read_pmr();
> 
> @@ -896,12 +897,12 @@ static bool gic_has_group0(void)
>          * becomes 0x80. Reading it back returns 0, indicating that
>          * we're don't have access to Group0.
>          */
> -       gic_write_pmr(BIT(8 - gic_get_pribits()));
> +       gic_write_pmr(prio);
>         val = gic_read_pmr();
> 
>         gic_write_pmr(old_pmr);
> 
> -       return val != 0;
> +       return val != (__gicv3_prio_to_ns(__gicv3_ns_to_prio(prio)));
>  }
> --------------->%-----------------

No, that's terrible, and makes it simply impossible to understand what
is happening without looking at 3 layers of indirection.

Read the comment, and realise that the code implements exactly that.

	M.

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

Re: [PATCH v2 5/5] arm64: irqchip/gic-v3: Select priorities at boot time

[Liao, Chang]

在 2024/6/21 15:48, Marc Zyngier 写道:
> On Fri, 21 Jun 2024 07:23:54 +0100,
> "Liao, Chang" <liaochang1@huawei.com> wrote:
>>
>>
>>
>> 在 2024/6/17 19:18, Mark Rutland 写道:
>>>  	cpus_have_group0 = gic_has_group0();
>>
>>> +#define __gicv3_prio_to_ns(p)	(0xff & ((p) << 1))
>>> +#define __gicv3_ns_to_prio(ns)	(0x80 | ((ns) >> 1))
>>
>> What about refactoring the gic_has_group0() using the mapping macros
>> between PMR priority and GIC priority like this:
>>
>> ---------------%<-----------------
>> --- a/drivers/irqchip/irq-gic-v3.c
>> +++ b/drivers/irqchip/irq-gic-v3.c
>> @@ -882,6 +882,7 @@ static bool gic_has_group0(void)
>>  {
>>         u32 val;
>>         u32 old_pmr;
>> +       u32 prio = BIT(8 - gic_get_pribits());
>>
>>         old_pmr = gic_read_pmr();
>>
>> @@ -896,12 +897,12 @@ static bool gic_has_group0(void)
>>          * becomes 0x80. Reading it back returns 0, indicating that
>>          * we're don't have access to Group0.
>>          */
>> -       gic_write_pmr(BIT(8 - gic_get_pribits()));
>> +       gic_write_pmr(prio);
>>         val = gic_read_pmr();
>>
>>         gic_write_pmr(old_pmr);
>>
>> -       return val != 0;
>> +       return val != (__gicv3_prio_to_ns(__gicv3_ns_to_prio(prio)));
>>  }
>> --------------->%-----------------
> 
> No, that's terrible, and makes it simply impossible to understand what
> is happening without looking at 3 layers of indirection.
> 
> Read the comment, and realise that the code implements exactly that.

You are right, it is BIT(8-gic_get_pribits()) decides gic_read_pmr() must return zero
if SCR_EL3.FIQ is set, there is no need to overcomplicate things.

> 
> 	M.
> 

-- 
BR
Liao, Chang

Re: [PATCH v2 2/5] irqchip/gic-common: Remove sync_access callback

[Mark Rutland]

On Fri, Jun 21, 2024 at 10:22:26AM +0800, Jinjie Ruan wrote:
> 
> 
> On 2024/6/17 19:18, Mark Rutland wrote:
> > The gic_configure_irq(), gic_dist_config(), and gic_cpu_config()
> > functions each take an optional "sync_access" callback, but in almost
> > all cases this is not used. The only user is the GICv3 driver's
> > gic_cpu_init() function, which uses gic_redist_wait_for_rwp() as the
> > "sync_access" callback for gic_cpu_config().
> > 
> > It would be simpler and clearer to remove the callback and have the
> > GICv3 driver call gic_redist_wait_for_rwp() explicitly after
> > gic_cpu_config().
> > 
> > Remove the "sync_access" callback, and call gic_redist_wait_for_rwp()
> > explicitly in the GICv3 driver.
> > 
> > There should be no functional change as a result of this patch.
> 
> There seems to be a similar patch already:
> 
> https://lore.kernel.org/all/20230902134106.1969-1-yuzenghui@huawei.com/

Ok; looking at the diffstat and the diff, that patch didn't adjust
gic_cpu_config(), and Marc has already reviewed and tested this.

Thanks for the pointer, but I don't think that changes anything?

Mark.

> 
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Will Deacon <will@kernel.org>
> > Reviewed-by: Marc Zyngier <maz@kernel.org>
> > Tested-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  drivers/irqchip/irq-gic-common.c | 16 +++-------------
> >  drivers/irqchip/irq-gic-common.h |  7 +++----
> >  drivers/irqchip/irq-gic-v3.c     |  7 ++++---
> >  drivers/irqchip/irq-gic.c        |  6 +++---
> >  drivers/irqchip/irq-hip04.c      |  6 +++---
> >  5 files changed, 16 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
> > index afd6a1841715a..4ed17620dc4d7 100644
> > --- a/drivers/irqchip/irq-gic-common.c
> > +++ b/drivers/irqchip/irq-gic-common.c
> > @@ -45,7 +45,7 @@ void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
> >  }
> >  
> >  int gic_configure_irq(unsigned int irq, unsigned int type,
> > -		       void __iomem *base, void (*sync_access)(void))
> > +		       void __iomem *base)
> >  {
> >  	u32 confmask = 0x2 << ((irq % 16) * 2);
> >  	u32 confoff = (irq / 16) * 4;
> > @@ -84,14 +84,10 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
> >  
> >  	raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
> >  
> > -	if (sync_access)
> > -		sync_access();
> > -
> >  	return ret;
> >  }
> >  
> > -void gic_dist_config(void __iomem *base, int gic_irqs,
> > -		     void (*sync_access)(void))
> > +void gic_dist_config(void __iomem *base, int gic_irqs)
> >  {
> >  	unsigned int i;
> >  
> > @@ -118,12 +114,9 @@ void gic_dist_config(void __iomem *base, int gic_irqs,
> >  		writel_relaxed(GICD_INT_EN_CLR_X32,
> >  			       base + GIC_DIST_ENABLE_CLEAR + i / 8);
> >  	}
> > -
> > -	if (sync_access)
> > -		sync_access();
> >  }
> >  
> > -void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void))
> > +void gic_cpu_config(void __iomem *base, int nr)
> >  {
> >  	int i;
> >  
> > @@ -144,7 +137,4 @@ void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void))
> >  	for (i = 0; i < nr; i += 4)
> >  		writel_relaxed(GICD_INT_DEF_PRI_X4,
> >  					base + GIC_DIST_PRI + i * 4 / 4);
> > -
> > -	if (sync_access)
> > -		sync_access();
> >  }
> > diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
> > index f407cce9ecaaa..c230175dd584c 100644
> > --- a/drivers/irqchip/irq-gic-common.h
> > +++ b/drivers/irqchip/irq-gic-common.h
> > @@ -20,10 +20,9 @@ struct gic_quirk {
> >  };
> >  
> >  int gic_configure_irq(unsigned int irq, unsigned int type,
> > -                       void __iomem *base, void (*sync_access)(void));
> > -void gic_dist_config(void __iomem *base, int gic_irqs,
> > -		     void (*sync_access)(void));
> > -void gic_cpu_config(void __iomem *base, int nr, void (*sync_access)(void));
> > +                       void __iomem *base);
> > +void gic_dist_config(void __iomem *base, int gic_irqs);
> > +void gic_cpu_config(void __iomem *base, int nr);
> >  void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
> >  		void *data);
> >  void gic_enable_of_quirks(const struct device_node *np,
> > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> > index 6fb276504bcc8..d95dda2383fb5 100644
> > --- a/drivers/irqchip/irq-gic-v3.c
> > +++ b/drivers/irqchip/irq-gic-v3.c
> > @@ -670,7 +670,7 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
> >  
> >  	offset = convert_offset_index(d, GICD_ICFGR, &index);
> >  
> > -	ret = gic_configure_irq(index, type, base + offset, NULL);
> > +	ret = gic_configure_irq(index, type, base + offset);
> >  	if (ret && (range == PPI_RANGE || range == EPPI_RANGE)) {
> >  		/* Misconfigured PPIs are usually not fatal */
> >  		pr_warn("GIC: PPI INTID%ld is secure or misconfigured\n", irq);
> > @@ -940,7 +940,7 @@ static void __init gic_dist_init(void)
> >  		writel_relaxed(GICD_INT_DEF_PRI_X4, base + GICD_IPRIORITYRnE + i);
> >  
> >  	/* Now do the common stuff */
> > -	gic_dist_config(base, GIC_LINE_NR, NULL);
> > +	gic_dist_config(base, GIC_LINE_NR);
> >  
> >  	val = GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1;
> >  	if (gic_data.rdists.gicd_typer2 & GICD_TYPER2_nASSGIcap) {
> > @@ -1282,7 +1282,8 @@ static void gic_cpu_init(void)
> >  	for (i = 0; i < gic_data.ppi_nr + SGI_NR; i += 32)
> >  		writel_relaxed(~0, rbase + GICR_IGROUPR0 + i / 8);
> >  
> > -	gic_cpu_config(rbase, gic_data.ppi_nr + SGI_NR, gic_redist_wait_for_rwp);
> > +	gic_cpu_config(rbase, gic_data.ppi_nr + SGI_NR);
> > +	gic_redist_wait_for_rwp();
> >  
> >  	/* initialise system registers */
> >  	gic_cpu_sys_reg_init();
> > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> > index 98aa383e39db1..87255bde960fc 100644
> > --- a/drivers/irqchip/irq-gic.c
> > +++ b/drivers/irqchip/irq-gic.c
> > @@ -303,7 +303,7 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
> >  			    type != IRQ_TYPE_EDGE_RISING)
> >  		return -EINVAL;
> >  
> > -	ret = gic_configure_irq(gicirq, type, base + GIC_DIST_CONFIG, NULL);
> > +	ret = gic_configure_irq(gicirq, type, base + GIC_DIST_CONFIG);
> >  	if (ret && gicirq < 32) {
> >  		/* Misconfigured PPIs are usually not fatal */
> >  		pr_warn("GIC: PPI%ld is secure or misconfigured\n", gicirq - 16);
> > @@ -479,7 +479,7 @@ static void gic_dist_init(struct gic_chip_data *gic)
> >  	for (i = 32; i < gic_irqs; i += 4)
> >  		writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 4 / 4);
> >  
> > -	gic_dist_config(base, gic_irqs, NULL);
> > +	gic_dist_config(base, gic_irqs);
> >  
> >  	writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL);
> >  }
> > @@ -516,7 +516,7 @@ static int gic_cpu_init(struct gic_chip_data *gic)
> >  				gic_cpu_map[i] &= ~cpu_mask;
> >  	}
> >  
> > -	gic_cpu_config(dist_base, 32, NULL);
> > +	gic_cpu_config(dist_base, 32);
> >  
> >  	writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
> >  	gic_cpu_if_up(gic);
> > diff --git a/drivers/irqchip/irq-hip04.c b/drivers/irqchip/irq-hip04.c
> > index 46161f6ff289d..5285150fd9096 100644
> > --- a/drivers/irqchip/irq-hip04.c
> > +++ b/drivers/irqchip/irq-hip04.c
> > @@ -130,7 +130,7 @@ static int hip04_irq_set_type(struct irq_data *d, unsigned int type)
> >  
> >  	raw_spin_lock(&irq_controller_lock);
> >  
> > -	ret = gic_configure_irq(irq, type, base + GIC_DIST_CONFIG, NULL);
> > +	ret = gic_configure_irq(irq, type, base + GIC_DIST_CONFIG);
> >  	if (ret && irq < 32) {
> >  		/* Misconfigured PPIs are usually not fatal */
> >  		pr_warn("GIC: PPI%d is secure or misconfigured\n", irq - 16);
> > @@ -260,7 +260,7 @@ static void __init hip04_irq_dist_init(struct hip04_irq_data *intc)
> >  	for (i = 32; i < nr_irqs; i += 2)
> >  		writel_relaxed(cpumask, base + GIC_DIST_TARGET + ((i * 2) & ~3));
> >  
> > -	gic_dist_config(base, nr_irqs, NULL);
> > +	gic_dist_config(base, nr_irqs);
> >  
> >  	writel_relaxed(1, base + GIC_DIST_CTRL);
> >  }
> > @@ -287,7 +287,7 @@ static void hip04_irq_cpu_init(struct hip04_irq_data *intc)
> >  		if (i != cpu)
> >  			hip04_cpu_map[i] &= ~cpu_mask;
> >  
> > -	gic_cpu_config(dist_base, 32, NULL);
> > +	gic_cpu_config(dist_base, 32);
> >  
> >  	writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
> >  	writel_relaxed(1, base + GIC_CPU_CTRL);

Re: [PATCH v2 0/5] arm64: irqchip/gic-v3: Use compiletime constant PMR values

[Catalin Marinas]

Hi Thomas,

On Mon, Jun 17, 2024 at 12:18:36PM +0100, Mark Rutland wrote:
> Mark Rutland (5):
>   wordpart.h: Add REPEAT_BYTE_U32()
>   irqchip/gic-common: Remove sync_access callback
>   irqchip/gic-v3: Make distributor priorities variables
>   irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier
>   arm64: irqchip/gic-v3: Select priorities at boot time
> 
>  arch/arm64/include/asm/arch_gicv3.h     |  15 --
>  arch/arm64/include/asm/ptrace.h         |  35 +---
>  arch/arm64/kernel/image-vars.h          |   5 -
>  drivers/irqchip/irq-gic-common.c        |  22 +--
>  drivers/irqchip/irq-gic-common.h        |   7 +-
>  drivers/irqchip/irq-gic-v3-its.c        |  11 +-
>  drivers/irqchip/irq-gic-v3.c            | 225 ++++++++++++------------
>  drivers/irqchip/irq-gic.c               |  10 +-
>  drivers/irqchip/irq-hip04.c             |   6 +-
>  include/linux/irqchip/arm-gic-common.h  |   4 -
>  include/linux/irqchip/arm-gic-v3-prio.h |  52 ++++++
>  include/linux/irqchip/arm-gic-v3.h      |   2 +-
>  include/linux/wordpart.h                |   8 +
>  13 files changed, 201 insertions(+), 201 deletions(-)
>  create mode 100644 include/linux/irqchip/arm-gic-v3-prio.h

Are you ok for these patches to go through the arm64 tree (I can put
them on a stable branch) or you'd rather get them through the irqchip
tree? Either way, I don't expect (major) conflicts with the arm64 tree.

Thanks.

-- 
Catalin

Re: [PATCH v2 0/5] arm64: irqchip/gic-v3: Use compiletime constant PMR values

[Thomas Gleixner]

On Fri, Jun 21 2024 at 18:26, Catalin Marinas wrote:
> On Mon, Jun 17, 2024 at 12:18:36PM +0100, Mark Rutland wrote:
>> Mark Rutland (5):
>>   wordpart.h: Add REPEAT_BYTE_U32()
>>   irqchip/gic-common: Remove sync_access callback
>>   irqchip/gic-v3: Make distributor priorities variables
>>   irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier
>>   arm64: irqchip/gic-v3: Select priorities at boot time
>> 
>>  arch/arm64/include/asm/arch_gicv3.h     |  15 --
>>  arch/arm64/include/asm/ptrace.h         |  35 +---
>>  arch/arm64/kernel/image-vars.h          |   5 -
>>  drivers/irqchip/irq-gic-common.c        |  22 +--
>>  drivers/irqchip/irq-gic-common.h        |   7 +-
>>  drivers/irqchip/irq-gic-v3-its.c        |  11 +-
>>  drivers/irqchip/irq-gic-v3.c            | 225 ++++++++++++------------
>>  drivers/irqchip/irq-gic.c               |  10 +-
>>  drivers/irqchip/irq-hip04.c             |   6 +-
>>  include/linux/irqchip/arm-gic-common.h  |   4 -
>>  include/linux/irqchip/arm-gic-v3-prio.h |  52 ++++++
>>  include/linux/irqchip/arm-gic-v3.h      |   2 +-
>>  include/linux/wordpart.h                |   8 +
>>  13 files changed, 201 insertions(+), 201 deletions(-)
>>  create mode 100644 include/linux/irqchip/arm-gic-v3-prio.h
>
> Are you ok for these patches to go through the arm64 tree (I can put
> them on a stable branch) or you'd rather get them through the irqchip
> tree? Either way, I don't expect (major) conflicts with the arm64 tree.

Take them through your tree with my Acked-by. Yes a branch would be
appreciated just in case.

There is also

      https://lore.kernel.org/all/20240529133446.28446-1-Jonathan.Cameron@huawei.com

which fiddles with the GIC but most of this is not irqchip code. No idea
how that is supposed to find it's way into the tree.

Thanks,

        tglx

Re: [PATCH v2 0/5] arm64: irqchip/gic-v3: Use compiletime constant PMR values

[Catalin Marinas]

On Mon, 17 Jun 2024 12:18:36 +0100, Mark Rutland wrote:
> This series optimizes the way regular IRQs are masked/unmasked when
> GICv3 pseudo-NMIs are used, removing the need for a static key in fast
> paths by using a priority value chosen dynamically at boot time.
> 
> Thomas, would you be happy for this series to go through the arm64 tree?
> The key part of the series is the final patch which changes both arm64
> and irqchip, and I expect merge conflicts or functional fallout to be
> contained to arm64.
> 
> [...]

Applied to arm64 (for-next/gic-v3-pmr), thanks!

[1/5] wordpart.h: Add REPEAT_BYTE_U32()
      https://git.kernel.org/arm64/c/118d777c4cb4
[2/5] irqchip/gic-common: Remove sync_access callback
      https://git.kernel.org/arm64/c/e95c64a7fb71
[3/5] irqchip/gic-v3: Make distributor priorities variables
      https://git.kernel.org/arm64/c/a6156e70316b
[4/5] irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier
      https://git.kernel.org/arm64/c/d447bf09a401
[5/5] arm64: irqchip/gic-v3: Select priorities at boot time
      https://git.kernel.org/arm64/c/18fdb6348c48

-- 
Catalin

Re: [PATCH v2 0/5] arm64: irqchip/gic-v3: Use compiletime constant PMR values

[Catalin Marinas]

On Sat, Jun 22, 2024 at 11:29:50PM +0200, Thomas Gleixner wrote:
> On Fri, Jun 21 2024 at 18:26, Catalin Marinas wrote:
> > On Mon, Jun 17, 2024 at 12:18:36PM +0100, Mark Rutland wrote:
> >> Mark Rutland (5):
> >>   wordpart.h: Add REPEAT_BYTE_U32()
> >>   irqchip/gic-common: Remove sync_access callback
> >>   irqchip/gic-v3: Make distributor priorities variables
> >>   irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier
> >>   arm64: irqchip/gic-v3: Select priorities at boot time
> >> 
> >>  arch/arm64/include/asm/arch_gicv3.h     |  15 --
> >>  arch/arm64/include/asm/ptrace.h         |  35 +---
> >>  arch/arm64/kernel/image-vars.h          |   5 -
> >>  drivers/irqchip/irq-gic-common.c        |  22 +--
> >>  drivers/irqchip/irq-gic-common.h        |   7 +-
> >>  drivers/irqchip/irq-gic-v3-its.c        |  11 +-
> >>  drivers/irqchip/irq-gic-v3.c            | 225 ++++++++++++------------
> >>  drivers/irqchip/irq-gic.c               |  10 +-
> >>  drivers/irqchip/irq-hip04.c             |   6 +-
> >>  include/linux/irqchip/arm-gic-common.h  |   4 -
> >>  include/linux/irqchip/arm-gic-v3-prio.h |  52 ++++++
> >>  include/linux/irqchip/arm-gic-v3.h      |   2 +-
> >>  include/linux/wordpart.h                |   8 +
> >>  13 files changed, 201 insertions(+), 201 deletions(-)
> >>  create mode 100644 include/linux/irqchip/arm-gic-v3-prio.h
> >
> > Are you ok for these patches to go through the arm64 tree (I can put
> > them on a stable branch) or you'd rather get them through the irqchip
> > tree? Either way, I don't expect (major) conflicts with the arm64 tree.
> 
> Take them through your tree with my Acked-by. Yes a branch would be
> appreciated just in case.

Thanks. I added them to the arm64 for-next/gic-v3-pmr branch (on
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git).

> There is also
> 
>       https://lore.kernel.org/all/20240529133446.28446-1-Jonathan.Cameron@huawei.com
> 
> which fiddles with the GIC but most of this is not irqchip code. No idea
> how that is supposed to find it's way into the tree.

I tried to avoid this series so far ;). I'll poke James Morse tomorrow
to see whether he has any outstanding comments (since he started the
work initially). If not, I'll queue them through the arm64 tree (same as
above, on a stable branch in case of conflicts).

Thanks.

-- 
Catalin