The GICv5 CPU interface implements support for PE-Private Peripheral
Interrupts (PPI), that are handled (enabled/prioritized/delivered)
entirely within the CPU interface hardware.
To enable PPI interrupts, implement the baseline GICv5 host kernel
driver infrastructure required to handle interrupts on a GICv5 system.
Add the exception handling code path and definitions for GICv5
instructions.
Add GICv5 PPI handling code as a specific IRQ domain to:
- Set-up PPI priority
- Manage PPI configuration and state
- Manage IRQ flow handler
- IRQs allocation/free
- Hook-up a PPI specific IRQchip to provide the relevant methods
PPI IRQ priority is chosen as the minimum allowed priority by the
system design (after probing the number of priority bits implemented
by the CPU interface).
Co-developed-by: Sascha Bischoff <sascha.bischoff@arm.com>
Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
Co-developed-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Will Deacon <will@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
---
MAINTAINERS | 2 +
arch/arm64/include/asm/sysreg.h | 19 ++
drivers/irqchip/Kconfig | 5 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-gic-v5.c | 461 +++++++++++++++++++++++++++++++++++++
include/linux/irqchip/arm-gic-v5.h | 19 ++
6 files changed, 507 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index f02a768adecb..222a668f3fb6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1971,6 +1971,8 @@ M: Marc Zyngier <maz@kernel.org>
L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
S: Maintained
F: Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5*.yaml
+F: drivers/irqchip/irq-gic-v5*.[ch]
+F: include/linux/irqchip/arm-gic-v5.h
ARM HDLCD DRM DRIVER
M: Liviu Dudau <liviu.dudau@arm.com>
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 9b5fc6389715..36b82d74db37 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -1082,6 +1082,25 @@
#define GCS_CAP(x) ((((unsigned long)x) & GCS_CAP_ADDR_MASK) | \
GCS_CAP_VALID_TOKEN)
+/*
+ * Definitions for GICv5 instructions
+ */
+#define GICV5_OP_GIC_CDDI sys_insn(1, 0, 12, 2, 0)
+#define GICV5_OP_GIC_CDEOI sys_insn(1, 0, 12, 1, 7)
+#define GICV5_OP_GICR_CDIA sys_insn(1, 0, 12, 3, 0)
+
+/* Definitions for GIC CDDI */
+#define GICV5_GIC_CDDI_TYPE_MASK GENMASK_ULL(31, 29)
+#define GICV5_GIC_CDDI_ID_MASK GENMASK_ULL(23, 0)
+
+/* Definitions for GICR CDIA */
+#define GICV5_GIC_CDIA_VALID_MASK BIT_ULL(32)
+#define GICV5_GICR_CDIA_VALID(r) FIELD_GET(GICV5_GIC_CDIA_VALID_MASK, r)
+#define GICV5_GIC_CDIA_TYPE_MASK GENMASK_ULL(31, 29)
+#define GICV5_GIC_CDIA_ID_MASK GENMASK_ULL(23, 0)
+
+#define gicr_insn(insn) read_sysreg_s(GICV5_OP_GICR_##insn)
+#define gic_insn(v, insn) write_sysreg_s(v, GICV5_OP_GIC_##insn)
#define ARM64_FEATURE_FIELD_BITS 4
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 0d196e447142..3e4fb08b7a4d 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -54,6 +54,11 @@ config ARM_GIC_V3_ITS_FSL_MC
depends on FSL_MC_BUS
default ARM_GIC_V3_ITS
+config ARM_GIC_V5
+ bool
+ select IRQ_DOMAIN_HIERARCHY
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+
config ARM_NVIC
bool
select IRQ_DOMAIN_HIERARCHY
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 23ca4959e6ce..3d75659d99eb 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_ARM_GIC_V3) += irq-gic-v3.o irq-gic-v3-mbi.o irq-gic-common.o
obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v4.o irq-gic-v3-its-msi-parent.o
obj-$(CONFIG_ARM_GIC_V3_ITS_FSL_MC) += irq-gic-v3-its-fsl-mc-msi.o
obj-$(CONFIG_PARTITION_PERCPU) += irq-partition-percpu.o
+obj-$(CONFIG_ARM_GIC_V5) += irq-gic-v5.o
obj-$(CONFIG_HISILICON_IRQ_MBIGEN) += irq-mbigen.o
obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
obj-$(CONFIG_ARM_VIC) += irq-vic.o
diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
new file mode 100644
index 000000000000..a08daa562d21
--- /dev/null
+++ b/drivers/irqchip/irq-gic-v5.c
@@ -0,0 +1,461 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024-2025 ARM Limited, All Rights Reserved.
+ */
+
+#define pr_fmt(fmt) "GICv5: " fmt
+
+#include <linux/irqdomain.h>
+#include <linux/wordpart.h>
+
+#include <linux/irqchip.h>
+#include <linux/irqchip/arm-gic-v5.h>
+
+#include <asm/cpufeature.h>
+#include <asm/exception.h>
+
+static u8 pri_bits __ro_after_init = 5;
+
+#define GICV5_IRQ_PRI_MASK 0x1f
+#define GICV5_IRQ_PRI_MI (GICV5_IRQ_PRI_MASK & GENMASK(4, 5 - pri_bits))
+
+#define PPI_NR 128
+
+static bool gicv5_cpuif_has_gcie(void)
+{
+ return this_cpu_has_cap(ARM64_HAS_GICV5_CPUIF);
+}
+
+struct gicv5_chip_data {
+ struct fwnode_handle *fwnode;
+ struct irq_domain *ppi_domain;
+};
+
+static struct gicv5_chip_data gicv5_global_data __read_mostly;
+
+static void gicv5_ppi_priority_init(void)
+{
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR0_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR1_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR2_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR3_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR4_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR5_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR6_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR7_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR8_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR9_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR10_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR11_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR12_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR13_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR14_EL1);
+ write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRI_MI), SYS_ICC_PPI_PRIORITYR15_EL1);
+
+ /*
+ * Context syncronization required to make sure system register writes
+ * effects are synchronised.
+ */
+ isb();
+}
+
+static void gicv5_ppi_irq_mask(struct irq_data *d)
+{
+ u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64);
+
+ if (d->hwirq < 64)
+ sysreg_clear_set_s(SYS_ICC_PPI_ENABLER0_EL1, hwirq_id_bit, 0);
+ else
+ sysreg_clear_set_s(SYS_ICC_PPI_ENABLER1_EL1, hwirq_id_bit, 0);
+
+ /*
+ * We must ensure that the disable takes effect immediately to
+ * guarantee that the lazy-disabled IRQ mechanism works.
+ * A context synchronization event is required to guarantee it.
+ * Reference: I_ZLTKB/R_YRGMH GICv5 specification - section 2.9.1.
+ */
+ isb();
+}
+
+static void gicv5_ppi_irq_unmask(struct irq_data *d)
+{
+ u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64);
+
+ if (d->hwirq < 64)
+ sysreg_clear_set_s(SYS_ICC_PPI_ENABLER0_EL1, 0, hwirq_id_bit);
+ else
+ sysreg_clear_set_s(SYS_ICC_PPI_ENABLER1_EL1, 0, hwirq_id_bit);
+ /*
+ * We must ensure that the enable takes effect in finite time - a
+ * context synchronization event is required to guarantee it, we
+ * can not take for granted that would happen (eg a core going straight
+ * into idle after enabling a PPI).
+ * Reference: I_ZLTKB/R_YRGMH GICv5 specification - section 2.9.1.
+ */
+ isb();
+}
+
+static void gicv5_hwirq_eoi(u32 hwirq_id, u8 hwirq_type)
+{
+ u64 cddi = hwirq_id | FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
+
+ gic_insn(cddi, CDDI);
+
+ gic_insn(0, CDEOI);
+}
+
+static void gicv5_ppi_irq_eoi(struct irq_data *d)
+{
+ gicv5_hwirq_eoi(d->hwirq, GICV5_HWIRQ_TYPE_PPI);
+}
+
+enum ppi_reg {
+ PPI_PENDING,
+ PPI_ACTIVE,
+ PPI_HM
+};
+
+static __always_inline u64 read_ppi_sysreg_s(unsigned int irq,
+ const enum ppi_reg which)
+{
+ switch (which) {
+ case PPI_PENDING:
+ return irq < 64 ? read_sysreg_s(SYS_ICC_PPI_SPENDR0_EL1) :
+ read_sysreg_s(SYS_ICC_PPI_SPENDR1_EL1);
+ case PPI_ACTIVE:
+ return irq < 64 ? read_sysreg_s(SYS_ICC_PPI_SACTIVER0_EL1) :
+ read_sysreg_s(SYS_ICC_PPI_SACTIVER1_EL1);
+ case PPI_HM:
+ return irq < 64 ? read_sysreg_s(SYS_ICC_PPI_HMR0_EL1) :
+ read_sysreg_s(SYS_ICC_PPI_HMR1_EL1);
+ default:
+ BUILD_BUG_ON(1);
+ }
+}
+
+static __always_inline void write_ppi_sysreg_s(unsigned int irq, bool set,
+ const enum ppi_reg which)
+{
+ u64 bit = BIT_ULL(irq % 64);
+
+ switch (which) {
+ case PPI_PENDING:
+ if (set) {
+ if (irq < 64)
+ write_sysreg_s(bit, SYS_ICC_PPI_SPENDR0_EL1);
+ else
+ write_sysreg_s(bit, SYS_ICC_PPI_SPENDR1_EL1);
+ } else {
+ if (irq < 64)
+ write_sysreg_s(bit, SYS_ICC_PPI_CPENDR0_EL1);
+ else
+ write_sysreg_s(bit, SYS_ICC_PPI_CPENDR1_EL1);
+ }
+ return;
+ case PPI_ACTIVE:
+ if (set) {
+ if (irq < 64)
+ write_sysreg_s(bit, SYS_ICC_PPI_SACTIVER0_EL1);
+ else
+ write_sysreg_s(bit, SYS_ICC_PPI_SACTIVER1_EL1);
+ } else {
+ if (irq < 64)
+ write_sysreg_s(bit, SYS_ICC_PPI_CACTIVER0_EL1);
+ else
+ write_sysreg_s(bit, SYS_ICC_PPI_CACTIVER1_EL1);
+ }
+ return;
+ default:
+ BUILD_BUG_ON(1);
+ }
+}
+
+static int gicv5_ppi_irq_get_irqchip_state(struct irq_data *d,
+ enum irqchip_irq_state which,
+ bool *state)
+{
+ u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64);
+
+ switch (which) {
+ case IRQCHIP_STATE_PENDING:
+ *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_PENDING) & hwirq_id_bit);
+ return 0;
+ case IRQCHIP_STATE_ACTIVE:
+ *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_ACTIVE) & hwirq_id_bit);
+ return 0;
+ default:
+ pr_debug("Unexpected PPI irqchip state\n");
+ return -EINVAL;
+ }
+}
+
+static int gicv5_ppi_irq_set_irqchip_state(struct irq_data *d,
+ enum irqchip_irq_state which,
+ bool state)
+{
+ switch (which) {
+ case IRQCHIP_STATE_PENDING:
+ write_ppi_sysreg_s(d->hwirq, state, PPI_PENDING);
+ return 0;
+ case IRQCHIP_STATE_ACTIVE:
+ write_ppi_sysreg_s(d->hwirq, state, PPI_ACTIVE);
+ return 0;
+ default:
+ pr_debug("Unexpected PPI irqchip state\n");
+ return -EINVAL;
+ }
+}
+
+static bool gicv5_ppi_irq_is_level(irq_hw_number_t hwirq)
+{
+ u64 bit = BIT_ULL(hwirq % 64);
+
+ return !!(read_ppi_sysreg_s(hwirq, PPI_HM) & bit);
+}
+
+static const struct irq_chip gicv5_ppi_irq_chip = {
+ .name = "GICv5-PPI",
+ .irq_mask = gicv5_ppi_irq_mask,
+ .irq_unmask = gicv5_ppi_irq_unmask,
+ .irq_eoi = gicv5_ppi_irq_eoi,
+ .irq_get_irqchip_state = gicv5_ppi_irq_get_irqchip_state,
+ .irq_set_irqchip_state = gicv5_ppi_irq_set_irqchip_state,
+ .flags = IRQCHIP_SKIP_SET_WAKE |
+ IRQCHIP_MASK_ON_SUSPEND,
+};
+
+static int gicv5_irq_ppi_domain_translate(struct irq_domain *d,
+ struct irq_fwspec *fwspec,
+ irq_hw_number_t *hwirq,
+ unsigned int *type)
+{
+ if (!is_of_node(fwspec->fwnode))
+ return -EINVAL;
+
+ if (fwspec->param_count < 3)
+ return -EINVAL;
+
+ if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI)
+ return -EINVAL;
+
+ *hwirq = fwspec->param[1];
+
+ /*
+ * Handling mode is hardcoded for PPIs, set the type using
+ * HW reported value.
+ */
+ *type = gicv5_ppi_irq_is_level(*hwirq) ? IRQ_TYPE_LEVEL_LOW : IRQ_TYPE_EDGE_RISING;
+
+ return 0;
+}
+
+static int gicv5_irq_ppi_domain_alloc(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ unsigned int type = IRQ_TYPE_NONE;
+ struct irq_fwspec *fwspec = arg;
+ irq_hw_number_t hwirq;
+ int ret;
+
+ if (WARN_ON_ONCE(nr_irqs != 1))
+ return -EINVAL;
+
+ ret = gicv5_irq_ppi_domain_translate(domain, fwspec, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ if (type & IRQ_TYPE_LEVEL_MASK)
+ irq_set_status_flags(virq, IRQ_LEVEL);
+
+ irq_set_percpu_devid(virq);
+ irq_domain_set_info(domain, virq, hwirq, &gicv5_ppi_irq_chip, NULL,
+ handle_percpu_devid_irq, NULL, NULL);
+
+ return 0;
+}
+
+static void gicv5_irq_domain_free(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs)
+{
+ struct irq_data *d;
+
+ if (WARN_ON_ONCE(nr_irqs != 1))
+ return;
+
+ d = irq_domain_get_irq_data(domain, virq);
+
+ irq_set_handler(virq, NULL);
+ irq_domain_reset_irq_data(d);
+}
+
+static int gicv5_irq_ppi_domain_select(struct irq_domain *d, struct irq_fwspec *fwspec,
+ enum irq_domain_bus_token bus_token)
+{
+ if (fwspec->fwnode != d->fwnode)
+ return 0;
+
+ if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI)
+ return 0;
+
+ return (d == gicv5_global_data.ppi_domain);
+}
+
+static const struct irq_domain_ops gicv5_irq_ppi_domain_ops = {
+ .translate = gicv5_irq_ppi_domain_translate,
+ .alloc = gicv5_irq_ppi_domain_alloc,
+ .free = gicv5_irq_domain_free,
+ .select = gicv5_irq_ppi_domain_select
+};
+
+static void handle_irq_per_domain(u32 hwirq)
+{
+ u8 hwirq_type = FIELD_GET(GICV5_HWIRQ_TYPE, hwirq);
+ u32 hwirq_id = FIELD_GET(GICV5_HWIRQ_ID, hwirq);
+ struct irq_domain *domain;
+
+ switch (hwirq_type) {
+ case GICV5_HWIRQ_TYPE_PPI:
+ domain = gicv5_global_data.ppi_domain;
+ break;
+ default:
+ pr_err_once("Unknown IRQ type, bail out\n");
+ return;
+ }
+
+ if (generic_handle_domain_irq(domain, hwirq_id)) {
+ pr_err_once("Could not handle, hwirq = 0x%x", hwirq_id);
+ gicv5_hwirq_eoi(hwirq_id, hwirq_type);
+ }
+}
+
+static void __exception_irq_entry gicv5_handle_irq(struct pt_regs *regs)
+{
+ bool valid;
+ u32 hwirq;
+ u64 ia;
+
+ ia = gicr_insn(CDIA);
+ valid = GICV5_GICR_CDIA_VALID(ia);
+
+ if (!valid)
+ return;
+
+ /*
+ * Ensure that the CDIA instruction effects (ie IRQ activation) are
+ * completed before handling the interrupt.
+ */
+ gsb_ack();
+
+ /*
+ * Ensure instruction ordering between an acknowledgment and subsequent
+ * instructions in the IRQ handler using an ISB.
+ */
+ isb();
+
+ hwirq = FIELD_GET(GICV5_HWIRQ_INTID, ia);
+
+ handle_irq_per_domain(hwirq);
+}
+
+static void gicv5_cpu_disable_interrupts(void)
+{
+ u64 cr0;
+
+ cr0 = FIELD_PREP(ICC_CR0_EL1_EN, 0);
+ write_sysreg_s(cr0, SYS_ICC_CR0_EL1);
+}
+
+static void gicv5_cpu_enable_interrupts(void)
+{
+ u64 cr0, pcr;
+
+ write_sysreg_s(0, SYS_ICC_PPI_ENABLER0_EL1);
+ write_sysreg_s(0, SYS_ICC_PPI_ENABLER1_EL1);
+
+ gicv5_ppi_priority_init();
+
+ pcr = FIELD_PREP(ICC_PCR_EL1_PRIORITY, GICV5_IRQ_PRI_MI);
+ write_sysreg_s(pcr, SYS_ICC_PCR_EL1);
+
+ cr0 = FIELD_PREP(ICC_CR0_EL1_EN, 1);
+ write_sysreg_s(cr0, SYS_ICC_CR0_EL1);
+}
+
+static int gicv5_starting_cpu(unsigned int cpu)
+{
+ if (WARN(!gicv5_cpuif_has_gcie(),
+ "GICv5 system components present but CPU does not have FEAT_GCIE"))
+ return -ENODEV;
+
+ gicv5_cpu_enable_interrupts();
+
+ return 0;
+}
+
+static void __init gicv5_free_domains(void)
+{
+ if (gicv5_global_data.ppi_domain)
+ irq_domain_remove(gicv5_global_data.ppi_domain);
+
+ gicv5_global_data.ppi_domain = NULL;
+}
+
+static int __init gicv5_init_domains(struct fwnode_handle *handle)
+{
+ struct irq_domain *d;
+
+ d = irq_domain_create_linear(handle, PPI_NR, &gicv5_irq_ppi_domain_ops, NULL);
+ if (!d)
+ return -ENOMEM;
+
+ irq_domain_update_bus_token(d, DOMAIN_BUS_WIRED);
+ gicv5_global_data.ppi_domain = d;
+ gicv5_global_data.fwnode = handle;
+
+ return 0;
+}
+
+static void gicv5_set_cpuif_pribits(void)
+{
+ u64 icc_idr0 = read_sysreg_s(SYS_ICC_IDR0_EL1);
+
+ switch (FIELD_GET(ICC_IDR0_EL1_PRI_BITS, icc_idr0)) {
+ case ICC_IDR0_EL1_PRI_BITS_4BITS:
+ pri_bits = 4;
+ break;
+ case ICC_IDR0_EL1_PRI_BITS_5BITS:
+ pri_bits = 5;
+ break;
+ default:
+ pr_err("Unexpected ICC_IDR0_EL1_PRI_BITS value, default to 4");
+ pri_bits = 4;
+ break;
+ }
+}
+
+static int __init gicv5_of_init(struct device_node *node, struct device_node *parent)
+{
+ int ret = gicv5_init_domains(of_fwnode_handle(node));
+ if (ret)
+ return ret;
+
+ gicv5_set_cpuif_pribits();
+
+ ret = gicv5_starting_cpu(smp_processor_id());
+ if (ret)
+ goto out_dom;
+
+ ret = set_handle_irq(gicv5_handle_irq);
+ if (ret)
+ goto out_int;
+
+ return 0;
+
+out_int:
+ gicv5_cpu_disable_interrupts();
+out_dom:
+ gicv5_free_domains();
+
+ return ret;
+}
+IRQCHIP_DECLARE(gic_v5, "arm,gic-v5", gicv5_of_init);
diff --git a/include/linux/irqchip/arm-gic-v5.h b/include/linux/irqchip/arm-gic-v5.h
new file mode 100644
index 000000000000..b08ec0308a9b
--- /dev/null
+++ b/include/linux/irqchip/arm-gic-v5.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2025 ARM Limited, All Rights Reserved.
+ */
+#ifndef __LINUX_IRQCHIP_ARM_GIC_V5_H
+#define __LINUX_IRQCHIP_ARM_GIC_V5_H
+
+#include <asm/sysreg.h>
+
+/*
+ * INTID handling
+ */
+#define GICV5_HWIRQ_ID GENMASK(23, 0)
+#define GICV5_HWIRQ_TYPE GENMASK(31, 29)
+#define GICV5_HWIRQ_INTID GENMASK_ULL(31, 0)
+
+#define GICV5_HWIRQ_TYPE_PPI UL(0x1)
+
+#endif
--
2.48.0On Thu, 26 Jun 2025 12:26:11 +0200
Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> The GICv5 CPU interface implements support for PE-Private Peripheral
> Interrupts (PPI), that are handled (enabled/prioritized/delivered)
> entirely within the CPU interface hardware.
I can't remember where I got to last time so if I repeat stuff that
you already responded to, feel free to just ignore me this time ;)
All superficial stuff. Feel free to completely ignore if you like.
> diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
> new file mode 100644
> index 000000000000..a08daa562d21
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-v5.c
> @@ -0,0 +1,461 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024-2025 ARM Limited, All Rights Reserved.
> + */
> +
> +#define pr_fmt(fmt) "GICv5: " fmt
> +
> +#include <linux/irqdomain.h>
> +#include <linux/wordpart.h>
> +
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/arm-gic-v5.h>
> +
> +#include <asm/cpufeature.h>
> +#include <asm/exception.h>
> +
> +static u8 pri_bits __ro_after_init = 5;
> +
> +#define GICV5_IRQ_PRI_MASK 0x1f
> +#define GICV5_IRQ_PRI_MI (GICV5_IRQ_PRI_MASK & GENMASK(4, 5 - pri_bits))
> +
> +#define PPI_NR 128
> +
> +static bool gicv5_cpuif_has_gcie(void)
> +{
> + return this_cpu_has_cap(ARM64_HAS_GICV5_CPUIF);
> +}
> +
> +struct gicv5_chip_data {
> + struct fwnode_handle *fwnode;
> + struct irq_domain *ppi_domain;
> +};
> +
> +static struct gicv5_chip_data gicv5_global_data __read_mostly;
> +static void gicv5_hwirq_eoi(u32 hwirq_id, u8 hwirq_type)
> +{
> + u64 cddi = hwirq_id | FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
Slight preference for not needing to care where hwirq_id goes in CDDI or how big
it is (other than when I checked the header defines).
u64 cddi = FIELD_PREP(GICV5_GIC_CDDI_ID_MASK, hwirq_id) |
FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
> +
> + gic_insn(cddi, CDDI);
> +
> + gic_insn(0, CDEOI);
> +}
> +static int gicv5_ppi_irq_get_irqchip_state(struct irq_data *d,
> + enum irqchip_irq_state which,
> + bool *state)
> +{
> + u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64);
> +
> + switch (which) {
> + case IRQCHIP_STATE_PENDING:
> + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_PENDING) & hwirq_id_bit);
Technically don't need the !! but if you really like it I don't mind that much.
> + return 0;
> + case IRQCHIP_STATE_ACTIVE:
> + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_ACTIVE) & hwirq_id_bit);
> + return 0;
> + default:
> + pr_debug("Unexpected PPI irqchip state\n");
> + return -EINVAL;
> + }
> +}
> +static int gicv5_irq_ppi_domain_translate(struct irq_domain *d,
> + struct irq_fwspec *fwspec,
> + irq_hw_number_t *hwirq,
> + unsigned int *type)
> +{
> + if (!is_of_node(fwspec->fwnode))
> + return -EINVAL;
> +
> + if (fwspec->param_count < 3)
I don't care that much, but could relax this seeing as fwspec->param[2]
isn't used anyway? Maybe a tiny comment on why it matters?
> + return -EINVAL;
> +
> + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI)
> + return -EINVAL;
> +
> + *hwirq = fwspec->param[1];
> +
> + /*
> + * Handling mode is hardcoded for PPIs, set the type using
> + * HW reported value.
> + */
> + *type = gicv5_ppi_irq_is_level(*hwirq) ? IRQ_TYPE_LEVEL_LOW : IRQ_TYPE_EDGE_RISING;
> +
> + return 0;
> +static int __init gicv5_of_init(struct device_node *node, struct device_node *parent)
> +{
> + int ret = gicv5_init_domains(of_fwnode_handle(node));
> + if (ret)
> + return ret;
> +
> + gicv5_set_cpuif_pribits();
> +
> + ret = gicv5_starting_cpu(smp_processor_id());
> + if (ret)
> + goto out_dom;
> +
> + ret = set_handle_irq(gicv5_handle_irq);
> + if (ret)
> + goto out_int;
> +
> + return 0;
> +
> +out_int:
> + gicv5_cpu_disable_interrupts();
> +out_dom:
> + gicv5_free_domains();
Naming is always tricky but I'd not really expect gicv5_free_domains() as the
pair of gicv5_init_domains() (which is doing creation rather than just initializing).
Ah well, names are never prefect and I don't really mind.
> +
> + return ret;
> +}
> +IRQCHIP_DECLARE(gic_v5, "arm,gic-v5", gicv5_of_init);
On Wed, Jul 02, 2025 at 12:40:19PM +0100, Jonathan Cameron wrote:
> On Thu, 26 Jun 2025 12:26:11 +0200
> Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>
> > The GICv5 CPU interface implements support for PE-Private Peripheral
> > Interrupts (PPI), that are handled (enabled/prioritized/delivered)
> > entirely within the CPU interface hardware.
>
> I can't remember where I got to last time so if I repeat stuff that
> you already responded to, feel free to just ignore me this time ;)
>
> All superficial stuff. Feel free to completely ignore if you like.
We are at v6.16-rc4, series has been on the lists for 3 months, it has
been reviewed and we would like to get it into v6.17 if possible and
deemed reasonable, I am asking you folks please, what should I do ?
I can send a v7 with the changes requested below (no bug fixes there)
- it is fine by me - but I need to know please asap if we have a
plan to get this upstream this cycle.
Thanks,
Lorenzo
> > diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
> > new file mode 100644
> > index 000000000000..a08daa562d21
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-gic-v5.c
> > @@ -0,0 +1,461 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2024-2025 ARM Limited, All Rights Reserved.
> > + */
> > +
> > +#define pr_fmt(fmt) "GICv5: " fmt
> > +
> > +#include <linux/irqdomain.h>
> > +#include <linux/wordpart.h>
> > +
> > +#include <linux/irqchip.h>
> > +#include <linux/irqchip/arm-gic-v5.h>
> > +
> > +#include <asm/cpufeature.h>
> > +#include <asm/exception.h>
> > +
> > +static u8 pri_bits __ro_after_init = 5;
> > +
> > +#define GICV5_IRQ_PRI_MASK 0x1f
> > +#define GICV5_IRQ_PRI_MI (GICV5_IRQ_PRI_MASK & GENMASK(4, 5 - pri_bits))
> > +
> > +#define PPI_NR 128
> > +
> > +static bool gicv5_cpuif_has_gcie(void)
> > +{
> > + return this_cpu_has_cap(ARM64_HAS_GICV5_CPUIF);
> > +}
> > +
> > +struct gicv5_chip_data {
> > + struct fwnode_handle *fwnode;
> > + struct irq_domain *ppi_domain;
> > +};
> > +
> > +static struct gicv5_chip_data gicv5_global_data __read_mostly;
>
> > +static void gicv5_hwirq_eoi(u32 hwirq_id, u8 hwirq_type)
> > +{
> > + u64 cddi = hwirq_id | FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
>
> Slight preference for not needing to care where hwirq_id goes in CDDI or how big
> it is (other than when I checked the header defines).
>
> u64 cddi = FIELD_PREP(GICV5_GIC_CDDI_ID_MASK, hwirq_id) |
> FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
>
>
> > +
> > + gic_insn(cddi, CDDI);
> > +
> > + gic_insn(0, CDEOI);
> > +}
>
> > +static int gicv5_ppi_irq_get_irqchip_state(struct irq_data *d,
> > + enum irqchip_irq_state which,
> > + bool *state)
> > +{
> > + u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64);
> > +
> > + switch (which) {
> > + case IRQCHIP_STATE_PENDING:
> > + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_PENDING) & hwirq_id_bit);
>
> Technically don't need the !! but if you really like it I don't mind that much.
>
> > + return 0;
> > + case IRQCHIP_STATE_ACTIVE:
> > + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_ACTIVE) & hwirq_id_bit);
> > + return 0;
> > + default:
> > + pr_debug("Unexpected PPI irqchip state\n");
> > + return -EINVAL;
> > + }
> > +}
>
>
> > +static int gicv5_irq_ppi_domain_translate(struct irq_domain *d,
> > + struct irq_fwspec *fwspec,
> > + irq_hw_number_t *hwirq,
> > + unsigned int *type)
> > +{
> > + if (!is_of_node(fwspec->fwnode))
> > + return -EINVAL;
> > +
> > + if (fwspec->param_count < 3)
>
> I don't care that much, but could relax this seeing as fwspec->param[2]
> isn't used anyway? Maybe a tiny comment on why it matters?
>
> > + return -EINVAL;
> > +
> > + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI)
> > + return -EINVAL;
> > +
> > + *hwirq = fwspec->param[1];
> > +
> > + /*
> > + * Handling mode is hardcoded for PPIs, set the type using
> > + * HW reported value.
> > + */
> > + *type = gicv5_ppi_irq_is_level(*hwirq) ? IRQ_TYPE_LEVEL_LOW : IRQ_TYPE_EDGE_RISING;
> > +
> > + return 0;
>
>
> > +static int __init gicv5_of_init(struct device_node *node, struct device_node *parent)
> > +{
> > + int ret = gicv5_init_domains(of_fwnode_handle(node));
> > + if (ret)
> > + return ret;
> > +
> > + gicv5_set_cpuif_pribits();
> > +
> > + ret = gicv5_starting_cpu(smp_processor_id());
> > + if (ret)
> > + goto out_dom;
> > +
> > + ret = set_handle_irq(gicv5_handle_irq);
> > + if (ret)
> > + goto out_int;
> > +
> > + return 0;
> > +
> > +out_int:
> > + gicv5_cpu_disable_interrupts();
> > +out_dom:
> > + gicv5_free_domains();
>
> Naming is always tricky but I'd not really expect gicv5_free_domains() as the
> pair of gicv5_init_domains() (which is doing creation rather than just initializing).
>
> Ah well, names are never prefect and I don't really mind.
>
> > +
> > + return ret;
> > +}
> > +IRQCHIP_DECLARE(gic_v5, "arm,gic-v5", gicv5_of_init);
>
On Wed, Jul 2, 2025, at 14:46, Lorenzo Pieralisi wrote:
> On Wed, Jul 02, 2025 at 12:40:19PM +0100, Jonathan Cameron wrote:
>> On Thu, 26 Jun 2025 12:26:11 +0200
>> Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>>
>> > The GICv5 CPU interface implements support for PE-Private Peripheral
>> > Interrupts (PPI), that are handled (enabled/prioritized/delivered)
>> > entirely within the CPU interface hardware.
>>
>> I can't remember where I got to last time so if I repeat stuff that
>> you already responded to, feel free to just ignore me this time ;)
>>
>> All superficial stuff. Feel free to completely ignore if you like.
>
> We are at v6.16-rc4, series has been on the lists for 3 months, it has
> been reviewed and we would like to get it into v6.17 if possible and
> deemed reasonable, I am asking you folks please, what should I do ?
>
> I can send a v7 with the changes requested below (no bug fixes there)
> - it is fine by me - but I need to know please asap if we have a
> plan to get this upstream this cycle.
I think the priority right now should be to get the series into
linux-next, as there is a good chance that the added regression
testing will uncover some problem that you should fix.
I had another look at all your patches, mainly to see how much
of them actually change existing code, and there is thankfully
very little of that. Without actual gicv5 hardware implementations
there is very low risk in adding the new driver as well: anything
that still comes up can be fixed on top of v6 or a v7 if you send
it again.
I assume that Thomas will make this a separate branch in the tip
tree, given the size of the series. If he still wants to wait for
more feedback or changes before adding it to tip, I would suggest
you ask Stephen to add your latest branch to linux-next in the
meantime.
Arnd
On Wed, 2 Jul 2025 14:46:10 +0200
Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> On Wed, Jul 02, 2025 at 12:40:19PM +0100, Jonathan Cameron wrote:
> > On Thu, 26 Jun 2025 12:26:11 +0200
> > Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> >
> > > The GICv5 CPU interface implements support for PE-Private Peripheral
> > > Interrupts (PPI), that are handled (enabled/prioritized/delivered)
> > > entirely within the CPU interface hardware.
> >
> > I can't remember where I got to last time so if I repeat stuff that
> > you already responded to, feel free to just ignore me this time ;)
> >
> > All superficial stuff. Feel free to completely ignore if you like.
>
> We are at v6.16-rc4, series has been on the lists for 3 months, it has
> been reviewed and we would like to get it into v6.17 if possible and
> deemed reasonable, I am asking you folks please, what should I do ?
>
> I can send a v7 with the changes requested below (no bug fixes there)
> - it is fine by me - but I need to know please asap if we have a
> plan to get this upstream this cycle.
I'm absolutely fine with leaving these be. The mask stuff I would like
to clean up as it applies quite widely in the series but that
can be a follow up as no bugs (so far!).
As Marc said, these are in a good state.
Jonathan
>
> Thanks,
> Lorenzo
>
> > > diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
> > > new file mode 100644
> > > index 000000000000..a08daa562d21
> > > --- /dev/null
> > > +++ b/drivers/irqchip/irq-gic-v5.c
> > > @@ -0,0 +1,461 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright (C) 2024-2025 ARM Limited, All Rights Reserved.
> > > + */
> > > +
> > > +#define pr_fmt(fmt) "GICv5: " fmt
> > > +
> > > +#include <linux/irqdomain.h>
> > > +#include <linux/wordpart.h>
> > > +
> > > +#include <linux/irqchip.h>
> > > +#include <linux/irqchip/arm-gic-v5.h>
> > > +
> > > +#include <asm/cpufeature.h>
> > > +#include <asm/exception.h>
> > > +
> > > +static u8 pri_bits __ro_after_init = 5;
> > > +
> > > +#define GICV5_IRQ_PRI_MASK 0x1f
> > > +#define GICV5_IRQ_PRI_MI (GICV5_IRQ_PRI_MASK & GENMASK(4, 5 - pri_bits))
> > > +
> > > +#define PPI_NR 128
> > > +
> > > +static bool gicv5_cpuif_has_gcie(void)
> > > +{
> > > + return this_cpu_has_cap(ARM64_HAS_GICV5_CPUIF);
> > > +}
> > > +
> > > +struct gicv5_chip_data {
> > > + struct fwnode_handle *fwnode;
> > > + struct irq_domain *ppi_domain;
> > > +};
> > > +
> > > +static struct gicv5_chip_data gicv5_global_data __read_mostly;
> >
> > > +static void gicv5_hwirq_eoi(u32 hwirq_id, u8 hwirq_type)
> > > +{
> > > + u64 cddi = hwirq_id | FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
> >
> > Slight preference for not needing to care where hwirq_id goes in CDDI or how big
> > it is (other than when I checked the header defines).
> >
> > u64 cddi = FIELD_PREP(GICV5_GIC_CDDI_ID_MASK, hwirq_id) |
> > FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
> >
> >
> > > +
> > > + gic_insn(cddi, CDDI);
> > > +
> > > + gic_insn(0, CDEOI);
> > > +}
> >
> > > +static int gicv5_ppi_irq_get_irqchip_state(struct irq_data *d,
> > > + enum irqchip_irq_state which,
> > > + bool *state)
> > > +{
> > > + u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64);
> > > +
> > > + switch (which) {
> > > + case IRQCHIP_STATE_PENDING:
> > > + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_PENDING) & hwirq_id_bit);
> >
> > Technically don't need the !! but if you really like it I don't mind that much.
> >
> > > + return 0;
> > > + case IRQCHIP_STATE_ACTIVE:
> > > + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_ACTIVE) & hwirq_id_bit);
> > > + return 0;
> > > + default:
> > > + pr_debug("Unexpected PPI irqchip state\n");
> > > + return -EINVAL;
> > > + }
> > > +}
> >
> >
> > > +static int gicv5_irq_ppi_domain_translate(struct irq_domain *d,
> > > + struct irq_fwspec *fwspec,
> > > + irq_hw_number_t *hwirq,
> > > + unsigned int *type)
> > > +{
> > > + if (!is_of_node(fwspec->fwnode))
> > > + return -EINVAL;
> > > +
> > > + if (fwspec->param_count < 3)
> >
> > I don't care that much, but could relax this seeing as fwspec->param[2]
> > isn't used anyway? Maybe a tiny comment on why it matters?
> >
> > > + return -EINVAL;
> > > +
> > > + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI)
> > > + return -EINVAL;
> > > +
> > > + *hwirq = fwspec->param[1];
> > > +
> > > + /*
> > > + * Handling mode is hardcoded for PPIs, set the type using
> > > + * HW reported value.
> > > + */
> > > + *type = gicv5_ppi_irq_is_level(*hwirq) ? IRQ_TYPE_LEVEL_LOW : IRQ_TYPE_EDGE_RISING;
> > > +
> > > + return 0;
> >
> >
> > > +static int __init gicv5_of_init(struct device_node *node, struct device_node *parent)
> > > +{
> > > + int ret = gicv5_init_domains(of_fwnode_handle(node));
> > > + if (ret)
> > > + return ret;
> > > +
> > > + gicv5_set_cpuif_pribits();
> > > +
> > > + ret = gicv5_starting_cpu(smp_processor_id());
> > > + if (ret)
> > > + goto out_dom;
> > > +
> > > + ret = set_handle_irq(gicv5_handle_irq);
> > > + if (ret)
> > > + goto out_int;
> > > +
> > > + return 0;
> > > +
> > > +out_int:
> > > + gicv5_cpu_disable_interrupts();
> > > +out_dom:
> > > + gicv5_free_domains();
> >
> > Naming is always tricky but I'd not really expect gicv5_free_domains() as the
> > pair of gicv5_init_domains() (which is doing creation rather than just initializing).
> >
> > Ah well, names are never prefect and I don't really mind.
> >
> > > +
> > > + return ret;
> > > +}
> > > +IRQCHIP_DECLARE(gic_v5, "arm,gic-v5", gicv5_of_init);
> >
>
On Wed, Jul 02, 2025 at 02:00:22PM +0100, Jonathan Cameron wrote:
> On Wed, 2 Jul 2025 14:46:10 +0200
> Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>
> > On Wed, Jul 02, 2025 at 12:40:19PM +0100, Jonathan Cameron wrote:
> > > On Thu, 26 Jun 2025 12:26:11 +0200
> > > Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > >
> > > > The GICv5 CPU interface implements support for PE-Private Peripheral
> > > > Interrupts (PPI), that are handled (enabled/prioritized/delivered)
> > > > entirely within the CPU interface hardware.
> > >
> > > I can't remember where I got to last time so if I repeat stuff that
> > > you already responded to, feel free to just ignore me this time ;)
> > >
> > > All superficial stuff. Feel free to completely ignore if you like.
> >
> > We are at v6.16-rc4, series has been on the lists for 3 months, it has
> > been reviewed and we would like to get it into v6.17 if possible and
> > deemed reasonable, I am asking you folks please, what should I do ?
> >
> > I can send a v7 with the changes requested below (no bug fixes there)
> > - it is fine by me - but I need to know please asap if we have a
> > plan to get this upstream this cycle.
>
> I'm absolutely fine with leaving these be. The mask stuff I would like
> to clean up as it applies quite widely in the series but that
> can be a follow up as no bugs (so far!).
I am certain that at a given state in the development I used the
FIELD_PREP() on the hwirq_id and then was asked to remove it because
it does not serve any purpose - this, for the records.
Thanks,
Lorenzo
> As Marc said, these are in a good state.
>
> Jonathan
>
> >
> > Thanks,
> > Lorenzo
> >
> > > > diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
> > > > new file mode 100644
> > > > index 000000000000..a08daa562d21
> > > > --- /dev/null
> > > > +++ b/drivers/irqchip/irq-gic-v5.c
> > > > @@ -0,0 +1,461 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +/*
> > > > + * Copyright (C) 2024-2025 ARM Limited, All Rights Reserved.
> > > > + */
> > > > +
> > > > +#define pr_fmt(fmt) "GICv5: " fmt
> > > > +
> > > > +#include <linux/irqdomain.h>
> > > > +#include <linux/wordpart.h>
> > > > +
> > > > +#include <linux/irqchip.h>
> > > > +#include <linux/irqchip/arm-gic-v5.h>
> > > > +
> > > > +#include <asm/cpufeature.h>
> > > > +#include <asm/exception.h>
> > > > +
> > > > +static u8 pri_bits __ro_after_init = 5;
> > > > +
> > > > +#define GICV5_IRQ_PRI_MASK 0x1f
> > > > +#define GICV5_IRQ_PRI_MI (GICV5_IRQ_PRI_MASK & GENMASK(4, 5 - pri_bits))
> > > > +
> > > > +#define PPI_NR 128
> > > > +
> > > > +static bool gicv5_cpuif_has_gcie(void)
> > > > +{
> > > > + return this_cpu_has_cap(ARM64_HAS_GICV5_CPUIF);
> > > > +}
> > > > +
> > > > +struct gicv5_chip_data {
> > > > + struct fwnode_handle *fwnode;
> > > > + struct irq_domain *ppi_domain;
> > > > +};
> > > > +
> > > > +static struct gicv5_chip_data gicv5_global_data __read_mostly;
> > >
> > > > +static void gicv5_hwirq_eoi(u32 hwirq_id, u8 hwirq_type)
> > > > +{
> > > > + u64 cddi = hwirq_id | FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
> > >
> > > Slight preference for not needing to care where hwirq_id goes in CDDI or how big
> > > it is (other than when I checked the header defines).
> > >
> > > u64 cddi = FIELD_PREP(GICV5_GIC_CDDI_ID_MASK, hwirq_id) |
> > > FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
> > >
> > >
> > > > +
> > > > + gic_insn(cddi, CDDI);
> > > > +
> > > > + gic_insn(0, CDEOI);
> > > > +}
> > >
> > > > +static int gicv5_ppi_irq_get_irqchip_state(struct irq_data *d,
> > > > + enum irqchip_irq_state which,
> > > > + bool *state)
> > > > +{
> > > > + u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64);
> > > > +
> > > > + switch (which) {
> > > > + case IRQCHIP_STATE_PENDING:
> > > > + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_PENDING) & hwirq_id_bit);
> > >
> > > Technically don't need the !! but if you really like it I don't mind that much.
> > >
> > > > + return 0;
> > > > + case IRQCHIP_STATE_ACTIVE:
> > > > + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_ACTIVE) & hwirq_id_bit);
> > > > + return 0;
> > > > + default:
> > > > + pr_debug("Unexpected PPI irqchip state\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +}
> > >
> > >
> > > > +static int gicv5_irq_ppi_domain_translate(struct irq_domain *d,
> > > > + struct irq_fwspec *fwspec,
> > > > + irq_hw_number_t *hwirq,
> > > > + unsigned int *type)
> > > > +{
> > > > + if (!is_of_node(fwspec->fwnode))
> > > > + return -EINVAL;
> > > > +
> > > > + if (fwspec->param_count < 3)
> > >
> > > I don't care that much, but could relax this seeing as fwspec->param[2]
> > > isn't used anyway? Maybe a tiny comment on why it matters?
> > >
> > > > + return -EINVAL;
> > > > +
> > > > + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI)
> > > > + return -EINVAL;
> > > > +
> > > > + *hwirq = fwspec->param[1];
> > > > +
> > > > + /*
> > > > + * Handling mode is hardcoded for PPIs, set the type using
> > > > + * HW reported value.
> > > > + */
> > > > + *type = gicv5_ppi_irq_is_level(*hwirq) ? IRQ_TYPE_LEVEL_LOW : IRQ_TYPE_EDGE_RISING;
> > > > +
> > > > + return 0;
> > >
> > >
> > > > +static int __init gicv5_of_init(struct device_node *node, struct device_node *parent)
> > > > +{
> > > > + int ret = gicv5_init_domains(of_fwnode_handle(node));
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + gicv5_set_cpuif_pribits();
> > > > +
> > > > + ret = gicv5_starting_cpu(smp_processor_id());
> > > > + if (ret)
> > > > + goto out_dom;
> > > > +
> > > > + ret = set_handle_irq(gicv5_handle_irq);
> > > > + if (ret)
> > > > + goto out_int;
> > > > +
> > > > + return 0;
> > > > +
> > > > +out_int:
> > > > + gicv5_cpu_disable_interrupts();
> > > > +out_dom:
> > > > + gicv5_free_domains();
> > >
> > > Naming is always tricky but I'd not really expect gicv5_free_domains() as the
> > > pair of gicv5_init_domains() (which is doing creation rather than just initializing).
> > >
> > > Ah well, names are never prefect and I don't really mind.
> > >
> > > > +
> > > > + return ret;
> > > > +}
> > > > +IRQCHIP_DECLARE(gic_v5, "arm,gic-v5", gicv5_of_init);
> > >
> >
>
On Wed, 2 Jul 2025 15:21:52 +0200
Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> On Wed, Jul 02, 2025 at 02:00:22PM +0100, Jonathan Cameron wrote:
> > On Wed, 2 Jul 2025 14:46:10 +0200
> > Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> >
> > > On Wed, Jul 02, 2025 at 12:40:19PM +0100, Jonathan Cameron wrote:
> > > > On Thu, 26 Jun 2025 12:26:11 +0200
> > > > Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > > >
> > > > > The GICv5 CPU interface implements support for PE-Private Peripheral
> > > > > Interrupts (PPI), that are handled (enabled/prioritized/delivered)
> > > > > entirely within the CPU interface hardware.
> > > >
> > > > I can't remember where I got to last time so if I repeat stuff that
> > > > you already responded to, feel free to just ignore me this time ;)
> > > >
> > > > All superficial stuff. Feel free to completely ignore if you like.
> > >
> > > We are at v6.16-rc4, series has been on the lists for 3 months, it has
> > > been reviewed and we would like to get it into v6.17 if possible and
> > > deemed reasonable, I am asking you folks please, what should I do ?
> > >
> > > I can send a v7 with the changes requested below (no bug fixes there)
> > > - it is fine by me - but I need to know please asap if we have a
> > > plan to get this upstream this cycle.
> >
> > I'm absolutely fine with leaving these be. The mask stuff I would like
> > to clean up as it applies quite widely in the series but that
> > can be a follow up as no bugs (so far!).
>
> I am certain that at a given state in the development I used the
> FIELD_PREP() on the hwirq_id and then was asked to remove it because
> it does not serve any purpose - this, for the records.
Fair enough. Though on that front the code is inconsistent as
there are places where it is masked. Anyhow, no problem either
way. The bit of feedback I gave on patch 22 might be more useful
to address (comments not matching code).
Jonathan
>
> Thanks,
> Lorenzo
>
> > As Marc said, these are in a good state.
> >
> > Jonathan
> >
> > >
> > > Thanks,
> > > Lorenzo
> > >
> > > > > diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
> > > > > new file mode 100644
> > > > > index 000000000000..a08daa562d21
> > > > > --- /dev/null
> > > > > +++ b/drivers/irqchip/irq-gic-v5.c
> > > > > @@ -0,0 +1,461 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > +/*
> > > > > + * Copyright (C) 2024-2025 ARM Limited, All Rights Reserved.
> > > > > + */
> > > > > +
> > > > > +#define pr_fmt(fmt) "GICv5: " fmt
> > > > > +
> > > > > +#include <linux/irqdomain.h>
> > > > > +#include <linux/wordpart.h>
> > > > > +
> > > > > +#include <linux/irqchip.h>
> > > > > +#include <linux/irqchip/arm-gic-v5.h>
> > > > > +
> > > > > +#include <asm/cpufeature.h>
> > > > > +#include <asm/exception.h>
> > > > > +
> > > > > +static u8 pri_bits __ro_after_init = 5;
> > > > > +
> > > > > +#define GICV5_IRQ_PRI_MASK 0x1f
> > > > > +#define GICV5_IRQ_PRI_MI (GICV5_IRQ_PRI_MASK & GENMASK(4, 5 - pri_bits))
> > > > > +
> > > > > +#define PPI_NR 128
> > > > > +
> > > > > +static bool gicv5_cpuif_has_gcie(void)
> > > > > +{
> > > > > + return this_cpu_has_cap(ARM64_HAS_GICV5_CPUIF);
> > > > > +}
> > > > > +
> > > > > +struct gicv5_chip_data {
> > > > > + struct fwnode_handle *fwnode;
> > > > > + struct irq_domain *ppi_domain;
> > > > > +};
> > > > > +
> > > > > +static struct gicv5_chip_data gicv5_global_data __read_mostly;
> > > >
> > > > > +static void gicv5_hwirq_eoi(u32 hwirq_id, u8 hwirq_type)
> > > > > +{
> > > > > + u64 cddi = hwirq_id | FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
> > > >
> > > > Slight preference for not needing to care where hwirq_id goes in CDDI or how big
> > > > it is (other than when I checked the header defines).
> > > >
> > > > u64 cddi = FIELD_PREP(GICV5_GIC_CDDI_ID_MASK, hwirq_id) |
> > > > FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
> > > >
> > > >
> > > > > +
> > > > > + gic_insn(cddi, CDDI);
> > > > > +
> > > > > + gic_insn(0, CDEOI);
> > > > > +}
> > > >
> > > > > +static int gicv5_ppi_irq_get_irqchip_state(struct irq_data *d,
> > > > > + enum irqchip_irq_state which,
> > > > > + bool *state)
> > > > > +{
> > > > > + u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64);
> > > > > +
> > > > > + switch (which) {
> > > > > + case IRQCHIP_STATE_PENDING:
> > > > > + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_PENDING) & hwirq_id_bit);
> > > >
> > > > Technically don't need the !! but if you really like it I don't mind that much.
> > > >
> > > > > + return 0;
> > > > > + case IRQCHIP_STATE_ACTIVE:
> > > > > + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_ACTIVE) & hwirq_id_bit);
> > > > > + return 0;
> > > > > + default:
> > > > > + pr_debug("Unexpected PPI irqchip state\n");
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +}
> > > >
> > > >
> > > > > +static int gicv5_irq_ppi_domain_translate(struct irq_domain *d,
> > > > > + struct irq_fwspec *fwspec,
> > > > > + irq_hw_number_t *hwirq,
> > > > > + unsigned int *type)
> > > > > +{
> > > > > + if (!is_of_node(fwspec->fwnode))
> > > > > + return -EINVAL;
> > > > > +
> > > > > + if (fwspec->param_count < 3)
> > > >
> > > > I don't care that much, but could relax this seeing as fwspec->param[2]
> > > > isn't used anyway? Maybe a tiny comment on why it matters?
> > > >
> > > > > + return -EINVAL;
> > > > > +
> > > > > + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + *hwirq = fwspec->param[1];
> > > > > +
> > > > > + /*
> > > > > + * Handling mode is hardcoded for PPIs, set the type using
> > > > > + * HW reported value.
> > > > > + */
> > > > > + *type = gicv5_ppi_irq_is_level(*hwirq) ? IRQ_TYPE_LEVEL_LOW : IRQ_TYPE_EDGE_RISING;
> > > > > +
> > > > > + return 0;
> > > >
> > > >
> > > > > +static int __init gicv5_of_init(struct device_node *node, struct device_node *parent)
> > > > > +{
> > > > > + int ret = gicv5_init_domains(of_fwnode_handle(node));
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + gicv5_set_cpuif_pribits();
> > > > > +
> > > > > + ret = gicv5_starting_cpu(smp_processor_id());
> > > > > + if (ret)
> > > > > + goto out_dom;
> > > > > +
> > > > > + ret = set_handle_irq(gicv5_handle_irq);
> > > > > + if (ret)
> > > > > + goto out_int;
> > > > > +
> > > > > + return 0;
> > > > > +
> > > > > +out_int:
> > > > > + gicv5_cpu_disable_interrupts();
> > > > > +out_dom:
> > > > > + gicv5_free_domains();
> > > >
> > > > Naming is always tricky but I'd not really expect gicv5_free_domains() as the
> > > > pair of gicv5_init_domains() (which is doing creation rather than just initializing).
> > > >
> > > > Ah well, names are never prefect and I don't really mind.
> > > >
> > > > > +
> > > > > + return ret;
> > > > > +}
> > > > > +IRQCHIP_DECLARE(gic_v5, "arm,gic-v5", gicv5_of_init);
> > > >
> > >
> >
On Wed, Jul 02, 2025 at 03:09:07PM +0100, Jonathan Cameron wrote:
> On Wed, 2 Jul 2025 15:21:52 +0200
> Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>
> > On Wed, Jul 02, 2025 at 02:00:22PM +0100, Jonathan Cameron wrote:
> > > On Wed, 2 Jul 2025 14:46:10 +0200
> > > Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > >
> > > > On Wed, Jul 02, 2025 at 12:40:19PM +0100, Jonathan Cameron wrote:
> > > > > On Thu, 26 Jun 2025 12:26:11 +0200
> > > > > Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > > > >
> > > > > > The GICv5 CPU interface implements support for PE-Private Peripheral
> > > > > > Interrupts (PPI), that are handled (enabled/prioritized/delivered)
> > > > > > entirely within the CPU interface hardware.
> > > > >
> > > > > I can't remember where I got to last time so if I repeat stuff that
> > > > > you already responded to, feel free to just ignore me this time ;)
> > > > >
> > > > > All superficial stuff. Feel free to completely ignore if you like.
> > > >
> > > > We are at v6.16-rc4, series has been on the lists for 3 months, it has
> > > > been reviewed and we would like to get it into v6.17 if possible and
> > > > deemed reasonable, I am asking you folks please, what should I do ?
> > > >
> > > > I can send a v7 with the changes requested below (no bug fixes there)
> > > > - it is fine by me - but I need to know please asap if we have a
> > > > plan to get this upstream this cycle.
> > >
> > > I'm absolutely fine with leaving these be. The mask stuff I would like
> > > to clean up as it applies quite widely in the series but that
> > > can be a follow up as no bugs (so far!).
> >
> > I am certain that at a given state in the development I used the
> > FIELD_PREP() on the hwirq_id and then was asked to remove it because
> > it does not serve any purpose - this, for the records.
>
> Fair enough. Though on that front the code is inconsistent as
> there are places where it is masked. Anyhow, no problem either
> way. The bit of feedback I gave on patch 22 might be more useful
> to address (comments not matching code).
Yes it is. It is not strictly speaking a bug but the logic should
be changed for SZ_64K PAGE_SIZE (try first 4K, then fallback to 16K).
Well spotted, apologies but again, it is not even a bug, I missed
it.
I think I can do a v7 tomorrow morning for that and include the
other comments as well - definitely not something that should stop
this series from being considered for v6.17 please.
Lorenzo
>
>
> Jonathan
>
> >
> > Thanks,
> > Lorenzo
> >
> > > As Marc said, these are in a good state.
> > >
> > > Jonathan
> > >
> > > >
> > > > Thanks,
> > > > Lorenzo
> > > >
> > > > > > diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
> > > > > > new file mode 100644
> > > > > > index 000000000000..a08daa562d21
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/irqchip/irq-gic-v5.c
> > > > > > @@ -0,0 +1,461 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > > +/*
> > > > > > + * Copyright (C) 2024-2025 ARM Limited, All Rights Reserved.
> > > > > > + */
> > > > > > +
> > > > > > +#define pr_fmt(fmt) "GICv5: " fmt
> > > > > > +
> > > > > > +#include <linux/irqdomain.h>
> > > > > > +#include <linux/wordpart.h>
> > > > > > +
> > > > > > +#include <linux/irqchip.h>
> > > > > > +#include <linux/irqchip/arm-gic-v5.h>
> > > > > > +
> > > > > > +#include <asm/cpufeature.h>
> > > > > > +#include <asm/exception.h>
> > > > > > +
> > > > > > +static u8 pri_bits __ro_after_init = 5;
> > > > > > +
> > > > > > +#define GICV5_IRQ_PRI_MASK 0x1f
> > > > > > +#define GICV5_IRQ_PRI_MI (GICV5_IRQ_PRI_MASK & GENMASK(4, 5 - pri_bits))
> > > > > > +
> > > > > > +#define PPI_NR 128
> > > > > > +
> > > > > > +static bool gicv5_cpuif_has_gcie(void)
> > > > > > +{
> > > > > > + return this_cpu_has_cap(ARM64_HAS_GICV5_CPUIF);
> > > > > > +}
> > > > > > +
> > > > > > +struct gicv5_chip_data {
> > > > > > + struct fwnode_handle *fwnode;
> > > > > > + struct irq_domain *ppi_domain;
> > > > > > +};
> > > > > > +
> > > > > > +static struct gicv5_chip_data gicv5_global_data __read_mostly;
> > > > >
> > > > > > +static void gicv5_hwirq_eoi(u32 hwirq_id, u8 hwirq_type)
> > > > > > +{
> > > > > > + u64 cddi = hwirq_id | FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
> > > > >
> > > > > Slight preference for not needing to care where hwirq_id goes in CDDI or how big
> > > > > it is (other than when I checked the header defines).
> > > > >
> > > > > u64 cddi = FIELD_PREP(GICV5_GIC_CDDI_ID_MASK, hwirq_id) |
> > > > > FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type);
> > > > >
> > > > >
> > > > > > +
> > > > > > + gic_insn(cddi, CDDI);
> > > > > > +
> > > > > > + gic_insn(0, CDEOI);
> > > > > > +}
> > > > >
> > > > > > +static int gicv5_ppi_irq_get_irqchip_state(struct irq_data *d,
> > > > > > + enum irqchip_irq_state which,
> > > > > > + bool *state)
> > > > > > +{
> > > > > > + u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64);
> > > > > > +
> > > > > > + switch (which) {
> > > > > > + case IRQCHIP_STATE_PENDING:
> > > > > > + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_PENDING) & hwirq_id_bit);
> > > > >
> > > > > Technically don't need the !! but if you really like it I don't mind that much.
> > > > >
> > > > > > + return 0;
> > > > > > + case IRQCHIP_STATE_ACTIVE:
> > > > > > + *state = !!(read_ppi_sysreg_s(d->hwirq, PPI_ACTIVE) & hwirq_id_bit);
> > > > > > + return 0;
> > > > > > + default:
> > > > > > + pr_debug("Unexpected PPI irqchip state\n");
> > > > > > + return -EINVAL;
> > > > > > + }
> > > > > > +}
> > > > >
> > > > >
> > > > > > +static int gicv5_irq_ppi_domain_translate(struct irq_domain *d,
> > > > > > + struct irq_fwspec *fwspec,
> > > > > > + irq_hw_number_t *hwirq,
> > > > > > + unsigned int *type)
> > > > > > +{
> > > > > > + if (!is_of_node(fwspec->fwnode))
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + if (fwspec->param_count < 3)
> > > > >
> > > > > I don't care that much, but could relax this seeing as fwspec->param[2]
> > > > > isn't used anyway? Maybe a tiny comment on why it matters?
> > > > >
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI)
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + *hwirq = fwspec->param[1];
> > > > > > +
> > > > > > + /*
> > > > > > + * Handling mode is hardcoded for PPIs, set the type using
> > > > > > + * HW reported value.
> > > > > > + */
> > > > > > + *type = gicv5_ppi_irq_is_level(*hwirq) ? IRQ_TYPE_LEVEL_LOW : IRQ_TYPE_EDGE_RISING;
> > > > > > +
> > > > > > + return 0;
> > > > >
> > > > >
> > > > > > +static int __init gicv5_of_init(struct device_node *node, struct device_node *parent)
> > > > > > +{
> > > > > > + int ret = gicv5_init_domains(of_fwnode_handle(node));
> > > > > > + if (ret)
> > > > > > + return ret;
> > > > > > +
> > > > > > + gicv5_set_cpuif_pribits();
> > > > > > +
> > > > > > + ret = gicv5_starting_cpu(smp_processor_id());
> > > > > > + if (ret)
> > > > > > + goto out_dom;
> > > > > > +
> > > > > > + ret = set_handle_irq(gicv5_handle_irq);
> > > > > > + if (ret)
> > > > > > + goto out_int;
> > > > > > +
> > > > > > + return 0;
> > > > > > +
> > > > > > +out_int:
> > > > > > + gicv5_cpu_disable_interrupts();
> > > > > > +out_dom:
> > > > > > + gicv5_free_domains();
> > > > >
> > > > > Naming is always tricky but I'd not really expect gicv5_free_domains() as the
> > > > > pair of gicv5_init_domains() (which is doing creation rather than just initializing).
> > > > >
> > > > > Ah well, names are never prefect and I don't really mind.
> > > > >
> > > > > > +
> > > > > > + return ret;
> > > > > > +}
> > > > > > +IRQCHIP_DECLARE(gic_v5, "arm,gic-v5", gicv5_of_init);
> > > > >
> > > >
> > >
>
© 2016 - 2025 Red Hat, Inc.