| Message ID | 20250408-gicv5-host-v1-18-1f26db465f8d@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Arm GICv5: Host driver implementation | expand |
On Tue, Apr 08 2025 at 12:50, Lorenzo Pieralisi wrote: > + > +static void gicv5_ppi_priority_init(void) > +{ > + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), > + SYS_ICC_PPI_PRIORITYR0_EL1); Just let stick it out. You have 100 characters. All over the place... > +static int gicv5_ppi_irq_set_irqchip_state(struct irq_data *d, > + enum irqchip_irq_state which, > + bool val) > +{ > + u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64); > + > + switch (which) { > + case IRQCHIP_STATE_PENDING: > + if (val) { > + if (d->hwirq < 64) > + write_sysreg_s(hwirq_id_bit, > + SYS_ICC_PPI_SPENDR0_EL1); > + else > + write_sysreg_s(hwirq_id_bit, > + SYS_ICC_PPI_SPENDR1_EL1); > + > + } else { > + if (d->hwirq < 64) > + write_sysreg_s(hwirq_id_bit, > + SYS_ICC_PPI_CPENDR0_EL1); > + else > + write_sysreg_s(hwirq_id_bit, > + SYS_ICC_PPI_CPENDR1_EL1); > + } > + > + return 0; > + case IRQCHIP_STATE_ACTIVE: > + if (val) { > + if (d->hwirq < 64) > + write_sysreg_s(hwirq_id_bit, > + SYS_ICC_PPI_SACTIVER0_EL1); > + else > + write_sysreg_s(hwirq_id_bit, > + SYS_ICC_PPI_SACTIVER1_EL1); > + } else { > + if (d->hwirq < 64) > + write_sysreg_s(hwirq_id_bit, > + SYS_ICC_PPI_CACTIVER0_EL1); > + else > + write_sysreg_s(hwirq_id_bit, > + SYS_ICC_PPI_CACTIVER1_EL1); > + } You already precalculate hwirq_id_bit. Can't you do something similar for the registers? case IRQCHIP_STATE_PENDING: u32 reg = val ? SYS_ICC_PPI_SPENDR1_EL1 : SYS_ICC_PPI_SPENDR0_EL1; write_sysreg_s(hwirq_id_bit, reg); return 0; case IRQCHIP_STATE_ACTIVE: .... Ditto in the get_state() function. No? > +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)) { It'd be way more readable to invert this check if (!is_of_node(...)) return -EINVAL; so that the subsequent checks are just a read through. > + if (fwspec->param_count < 3) > + return -EINVAL; > + > + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI) > + return -EINVAL; > + > + *hwirq = fwspec->param[1]; > + *type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK; > + > + return 0; > + } > + > + return -EINVAL; > +} > +static void gicv5_irq_ppi_domain_free(struct irq_domain *domain, > + unsigned int virq, unsigned int nr_irqs) > +{ > + struct irq_data *d; > + > + if (WARN_ON(nr_irqs != 1)) WARN_ON_ONCE ? > + 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) > +{ > + /* Not for us */ > + if (fwspec->fwnode != d->fwnode) > + return 0; > + > + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI) { > + // only handle PPIs Commenting the obvious? > + 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_ppi_domain_free, > + .select = gicv5_irq_ppi_domain_select > +}; > + > +static inline void handle_irq_per_domain(u32 hwirq) > +{ > + u32 hwirq_id; > + struct irq_domain *domain = NULL; > + u8 hwirq_type = FIELD_GET(GICV5_HWIRQ_TYPE, hwirq); So far you managed to comply with the documented reverse fir tree ordering. https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations Why are you changing coding style in the middle of the code? > + > + hwirq_id = FIELD_GET(GICV5_HWIRQ_ID, hwirq); > + > + if (hwirq_type == GICV5_HWIRQ_TYPE_PPI) > + domain = gicv5_global_data.ppi_domain; > + > + if (generic_handle_domain_irq(domain, hwirq_id)) { > + pr_err("Could not handle, hwirq = 0x%x", hwirq_id); pr_err_once() perhaps? > + gicv5_hwirq_eoi(hwirq_id, hwirq_type); > + } > +} > + > +static asmlinkage void __exception_irq_entry > +gicv5_handle_irq(struct pt_regs *regs) > +{ > + u64 ia; > + bool valid; > + u32 hwirq; See above > + ia = gicr_insn(GICV5_OP_GICR_CDIA); > + valid = GICV5_GIC_CDIA_VALID(ia); And please move that to the declaration lines > +static int __init gicv5_init_domains(struct fwnode_handle *handle) > +{ > + gicv5_global_data.fwnode = handle; > + gicv5_global_data.ppi_domain = irq_domain_create_linear( > + handle, 128, &gicv5_irq_ppi_domain_ops, NULL); The ever changing choice of coding styles across functions is really interesting. Obviously the length of 'gicv5_global_data.ppi_domain' forces ugly, but that does not mean it needs to be that way: struct irqdomain *d; d = irq_domain_create_linear(handle, 128, &gicv5_irq_ppi_domain_ops, NULL); if (!d) return - ENOMEM; irq_domain_update_bus_token(d, DOMAIN_BUS_WIRED); gicv5_global_data.fwnode = handle; gicv5_global_data.ppi_domain = d; return 0; No? > +static int __init gicv5_of_init(struct device_node *node, > + struct device_node *parent) > +{ > + int ret; > + > + ret = gicv5_init_domains(&node->fwnode); > + if (ret) > + return ret; > + > + gicv5_set_cpuif_pribits(); > + > + ret = gicv5_starting_cpu(smp_processor_id()); You invoke the CPU hotplug callback for the boot CPU explicitly, but what the heck installs the actual hotplug callback for the secondary CPUs? Thanks, tglx
On Tue, Apr 08, 2025 at 11:42:29PM +0200, Thomas Gleixner wrote: > On Tue, Apr 08 2025 at 12:50, Lorenzo Pieralisi wrote: > > + > > +static void gicv5_ppi_priority_init(void) > > +{ > > + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), > > + SYS_ICC_PPI_PRIORITYR0_EL1); > > Just let stick it out. You have 100 characters. All over the place... I will do. > > +static int gicv5_ppi_irq_set_irqchip_state(struct irq_data *d, > > + enum irqchip_irq_state which, > > + bool val) > > +{ > > + u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64); > > + > > + switch (which) { > > + case IRQCHIP_STATE_PENDING: > > + if (val) { > > + if (d->hwirq < 64) > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_SPENDR0_EL1); > > + else > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_SPENDR1_EL1); > > + > > + } else { > > + if (d->hwirq < 64) > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_CPENDR0_EL1); > > + else > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_CPENDR1_EL1); > > + } > > + > > + return 0; > > + case IRQCHIP_STATE_ACTIVE: > > + if (val) { > > + if (d->hwirq < 64) > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_SACTIVER0_EL1); > > + else > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_SACTIVER1_EL1); > > + } else { > > + if (d->hwirq < 64) > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_CACTIVER0_EL1); > > + else > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_CACTIVER1_EL1); > > + } > > You already precalculate hwirq_id_bit. Can't you do something similar > for the registers? > > case IRQCHIP_STATE_PENDING: > u32 reg = val ? SYS_ICC_PPI_SPENDR1_EL1 : SYS_ICC_PPI_SPENDR0_EL1; > > write_sysreg_s(hwirq_id_bit, reg); > return 0; > case IRQCHIP_STATE_ACTIVE: > .... > > Ditto in the get_state() function. > > No? Yes, more readable. > > +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)) { > > It'd be way more readable to invert this check > > if (!is_of_node(...)) > return -EINVAL; > > so that the subsequent checks are just a read through. Will do. > > + if (fwspec->param_count < 3) > > + return -EINVAL; > > + > > + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI) > > + return -EINVAL; > > + > > + *hwirq = fwspec->param[1]; > > + *type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK; > > + > > + return 0; > > + } > > + > > + return -EINVAL; > > +} > > > +static void gicv5_irq_ppi_domain_free(struct irq_domain *domain, > > + unsigned int virq, unsigned int nr_irqs) > > +{ > > + struct irq_data *d; > > + > > + if (WARN_ON(nr_irqs != 1)) > > WARN_ON_ONCE ? Yes. > > + 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) > > +{ > > + /* Not for us */ > > + if (fwspec->fwnode != d->fwnode) > > + return 0; > > + > > + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI) { > > + // only handle PPIs > > Commenting the obvious? > Will remove it. > > + 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_ppi_domain_free, > > + .select = gicv5_irq_ppi_domain_select > > +}; > > + > > +static inline void handle_irq_per_domain(u32 hwirq) > > +{ > > + u32 hwirq_id; > > + struct irq_domain *domain = NULL; > > + u8 hwirq_type = FIELD_GET(GICV5_HWIRQ_TYPE, hwirq); > > So far you managed to comply with the documented reverse fir tree > ordering. > > https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations > > Why are you changing coding style in the middle of the code? Mea culpa, don't bother commenting on this further, point taken. > > + hwirq_id = FIELD_GET(GICV5_HWIRQ_ID, hwirq); > > + > > + if (hwirq_type == GICV5_HWIRQ_TYPE_PPI) > > + domain = gicv5_global_data.ppi_domain; > > + > > + if (generic_handle_domain_irq(domain, hwirq_id)) { > > + pr_err("Could not handle, hwirq = 0x%x", hwirq_id); > > pr_err_once() perhaps? > > > + gicv5_hwirq_eoi(hwirq_id, hwirq_type); > > + } > > +} > > + > > +static asmlinkage void __exception_irq_entry > > +gicv5_handle_irq(struct pt_regs *regs) > > +{ > > + u64 ia; > > + bool valid; > > + u32 hwirq; > > See above > > > + ia = gicr_insn(GICV5_OP_GICR_CDIA); > > + valid = GICV5_GIC_CDIA_VALID(ia); > > And please move that to the declaration lines > > > +static int __init gicv5_init_domains(struct fwnode_handle *handle) > > +{ > > + gicv5_global_data.fwnode = handle; > > + gicv5_global_data.ppi_domain = irq_domain_create_linear( > > + handle, 128, &gicv5_irq_ppi_domain_ops, NULL); > > The ever changing choice of coding styles across functions is really > interesting. Obviously the length of 'gicv5_global_data.ppi_domain' > forces ugly, but that does not mean it needs to be that way: > > struct irqdomain *d; > > d = irq_domain_create_linear(handle, 128, &gicv5_irq_ppi_domain_ops, NULL); > if (!d) > return - ENOMEM; > > irq_domain_update_bus_token(d, DOMAIN_BUS_WIRED); > gicv5_global_data.fwnode = handle; > gicv5_global_data.ppi_domain = d; > return 0; > > No? Yes it is better. > > +static int __init gicv5_of_init(struct device_node *node, > > + struct device_node *parent) > > +{ > > + int ret; > > + > > + ret = gicv5_init_domains(&node->fwnode); > > + if (ret) > > + return ret; > > + > > + gicv5_set_cpuif_pribits(); > > + > > + ret = gicv5_starting_cpu(smp_processor_id()); > > You invoke the CPU hotplug callback for the boot CPU explicitly, but > what the heck installs the actual hotplug callback for the secondary > CPUs? That comes with a subsequent patch[21]. I mentioned in the cover letter that I tried to split the functionality into interrupt types to ease review (well, it does not look like I succeeded, sorry) and then in patch [21] (when LPIs backing IPIs are implemented), enable SMP. The point is, we need patches [18-21] to enable SMP booting. I can squash [18-21] all together or I can enable the hotplug callback here but this patch stand alone is not functional for the reasons above, let me know please what's best in your opinion and I will do. Above all, thank you very much for reviewing the series. Lorenzo
On Tue, Apr 08, 2025 at 11:42:29PM +0200, Thomas Gleixner wrote: [...] > > +static int gicv5_ppi_irq_set_irqchip_state(struct irq_data *d, > > + enum irqchip_irq_state which, > > + bool val) > > +{ > > + u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64); > > + > > + switch (which) { > > + case IRQCHIP_STATE_PENDING: > > + if (val) { > > + if (d->hwirq < 64) > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_SPENDR0_EL1); > > + else > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_SPENDR1_EL1); > > + > > + } else { > > + if (d->hwirq < 64) > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_CPENDR0_EL1); > > + else > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_CPENDR1_EL1); > > + } > > + > > + return 0; > > + case IRQCHIP_STATE_ACTIVE: > > + if (val) { > > + if (d->hwirq < 64) > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_SACTIVER0_EL1); > > + else > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_SACTIVER1_EL1); > > + } else { > > + if (d->hwirq < 64) > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_CACTIVER0_EL1); > > + else > > + write_sysreg_s(hwirq_id_bit, > > + SYS_ICC_PPI_CACTIVER1_EL1); > > + } > > You already precalculate hwirq_id_bit. Can't you do something similar > for the registers? > > case IRQCHIP_STATE_PENDING: > u32 reg = val ? SYS_ICC_PPI_SPENDR1_EL1 : SYS_ICC_PPI_SPENDR0_EL1; > > write_sysreg_s(hwirq_id_bit, reg); > return 0; > case IRQCHIP_STATE_ACTIVE: > .... > > Ditto in the get_state() function. > > No? Can't do it like that, write_sysreg_s takes a register encoding, not a u32 offset, I will see if I can wrap it in a macro, even though that might obfuscate a bit rather than clarify. > > +static asmlinkage void __exception_irq_entry > > +gicv5_handle_irq(struct pt_regs *regs) > > +{ > > + u64 ia; > > + bool valid; > > + u32 hwirq; > > See above > > > + ia = gicr_insn(GICV5_OP_GICR_CDIA); > > + valid = GICV5_GIC_CDIA_VALID(ia); > > And please move that to the declaration lines gicr_insn() is an instruction ACK'ing the IRQ, I would leave it like this explicitly if you don't mind lest it gave the impression that it is an initializer. Thanks, Lorenzo > > +static int __init gicv5_init_domains(struct fwnode_handle *handle) > > +{ > > + gicv5_global_data.fwnode = handle; > > + gicv5_global_data.ppi_domain = irq_domain_create_linear( > > + handle, 128, &gicv5_irq_ppi_domain_ops, NULL); > > The ever changing choice of coding styles across functions is really > interesting. Obviously the length of 'gicv5_global_data.ppi_domain' > forces ugly, but that does not mean it needs to be that way: > > struct irqdomain *d; > > d = irq_domain_create_linear(handle, 128, &gicv5_irq_ppi_domain_ops, NULL); > if (!d) > return - ENOMEM; > > irq_domain_update_bus_token(d, DOMAIN_BUS_WIRED); > gicv5_global_data.fwnode = handle; > gicv5_global_data.ppi_domain = d; > return 0; > > No? > > > +static int __init gicv5_of_init(struct device_node *node, > > + struct device_node *parent) > > +{ > > + int ret; > > + > > + ret = gicv5_init_domains(&node->fwnode); > > + if (ret) > > + return ret; > > + > > + gicv5_set_cpuif_pribits(); > > + > > + ret = gicv5_starting_cpu(smp_processor_id()); > > You invoke the CPU hotplug callback for the boot CPU explicitly, but > what the heck installs the actual hotplug callback for the secondary > CPUs? > > Thanks, > > tglx
diff --git a/MAINTAINERS b/MAINTAINERS index f3ed84466da19906953b5396a5f4b50e878c376e..cdeceb6782355a4a18609135bf7f03249d8b0bb5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1907,6 +1907,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: arch/arm64/include/asm/arch_gicv5.h +F: drivers/irqchip/irq-gic-v5*.[ch] ARM HDLCD DRM DRIVER M: Liviu Dudau <liviu.dudau@arm.com> diff --git a/arch/arm64/include/asm/arch_gicv5.h b/arch/arm64/include/asm/arch_gicv5.h new file mode 100644 index 0000000000000000000000000000000000000000..e86cda5e5b3295c4f9c784d92adad1c6df6dbc34 --- /dev/null +++ b/arch/arm64/include/asm/arch_gicv5.h @@ -0,0 +1,38 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (C) 2025 ARM Ltd. + */ +#ifndef __ASM_ARCH_GICV5_H +#define __ASM_ARCH_GICV5_H + +#include <asm/sysreg.h> + +#ifndef __ASSEMBLY__ + +#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) + +#define gicr_insn(insn) read_sysreg_s(insn) +#define gic_insn(v, insn) write_sysreg_s(v, insn) + +#define GSB_ACK __emit_inst(0xd5000000 | sys_insn(1, 0, 12, 0, 1) | 31) + +#define gsb_ack() asm volatile(GSB_ACK : : : "memory") + +/* Shift and mask definitions for GIC CDDI */ +#define GICV5_GIC_CDDI_TYPE_MASK GENMASK_ULL(31, 29) +#define GICV5_GIC_CDDI_TYPE(r) FIELD_GET(GICV5_GIC_CDDI_TYPE_MASK, r) +#define GICV5_GIC_CDDI_ID_MASK GENMASK_ULL(23, 0) +#define GICV5_GIC_CDDI_ID(r) FIELD_GET(GICV5_GIC_CDDI_ID_MASK, r) + +/* Shift and mask definitions for GICR CDIA */ +#define GICV5_GIC_CDIA_VALID_MASK BIT_ULL(32) +#define GICV5_GIC_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_TYPE(r) FIELD_GET(GICV5_GIC_CDIA_TYPE_MASK, r) +#define GICV5_GIC_CDIA_ID_MASK GENMASK_ULL(23, 0) +#define GICV5_GIC_CDIA_ID(r) FIELD_GET(GICV5_GIC_CDIA_ID_MASK, r) + +#endif /* __ASSEMBLY__ */ +#endif /* __ASM_ARCH_GICV5_H */ diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index cec05e443083b8982b3e72f4212d808a22883914..160a4761d5d85f6dbf36f3142fd619c114733e36 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 if SMP + config ARM_NVIC bool select IRQ_DOMAIN_HIERARCHY diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index 365bcea9a61ff89e2cb41034125b3fc8cd494d81..3f8225bba5f0f9ce5dbb629b6d4782eacf85da44 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -35,6 +35,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 0000000000000000000000000000000000000000..996e2c992ef33e5ec8d2680ad4026b725ca39b04 --- /dev/null +++ b/drivers/irqchip/irq-gic-v5.c @@ -0,0 +1,497 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2024-2025 ARM Limited, All Rights Reserved. + */ + +#define pr_fmt(fmt) "GICv5: " fmt + +#include <linux/irqchip.h> +#include <linux/irqdomain.h> +#include <linux/wordpart.h> + +#include <asm/cpufeature.h> +#include <asm/exception.h> + +#include "irq-gic-v5.h" + +static u8 pri_bits = 5; +#define GICV5_IRQ_PRIORITY_MASK 0x1f +#define GICV5_IRQ_PRIORITY_MI \ + (GICV5_IRQ_PRIORITY_MASK & GENMASK(4, 5 - pri_bits)) + +static bool gicv5_cpuif_has_gcie(void) +{ + return this_cpu_has_cap(ARM64_HAS_GCIE); +} + +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_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR0_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR1_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR2_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR3_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR4_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR5_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR6_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR7_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR8_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR9_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR10_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR11_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR12_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR13_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR14_EL1); + write_sysreg_s(REPEAT_BYTE(GICV5_IRQ_PRIORITY_MI), + SYS_ICC_PPI_PRIORITYR15_EL1); + + /* + * Context syncronization required to make sure system + * register writes effects are synchronized + */ + 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); + + /* + * Ensure that the disable takes effect + */ + 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); +} + +static void gicv5_hwirq_eoi(u32 hwirq_id, u32 hwirq_type) +{ + u64 cddi = hwirq_id | FIELD_PREP(GICV5_GIC_CDDI_TYPE_MASK, hwirq_type); + + gic_insn(cddi, GICV5_OP_GIC_CDDI); + + gic_insn(0, GICV5_OP_GIC_CDEOI); +} + +static void gicv5_ppi_irq_eoi(struct irq_data *d) +{ + gicv5_hwirq_eoi(d->hwirq, GICV5_HWIRQ_TYPE_PPI); +} + +static int gicv5_ppi_set_type(struct irq_data *d, unsigned int type) +{ + /* + * The PPI trigger mode is not configurable at runtime, + * therefore this function simply confirms that the `type` + * parameter matches what is present. + */ + u64 hmr; + + if (d->hwirq < 64) + hmr = read_sysreg_s(SYS_ICC_PPI_HMR0_EL1); + else + hmr = read_sysreg_s(SYS_ICC_PPI_HMR1_EL1); + + switch (type) { + case IRQ_TYPE_LEVEL_HIGH: + case IRQ_TYPE_LEVEL_LOW: + if (((hmr >> (d->hwirq % 64)) & 0x1) != GICV5_PPI_HM_LEVEL) + return -EINVAL; + break; + case IRQ_TYPE_EDGE_RISING: + case IRQ_TYPE_EDGE_FALLING: + if (((hmr >> (d->hwirq % 64)) & 0x1) != GICV5_PPI_HM_EDGE) + return -EINVAL; + break; + default: + pr_debug("Unexpected PPI trigger mode"); + return -EINVAL; + } + + return 0; +} + +static int gicv5_ppi_irq_get_irqchip_state(struct irq_data *d, + enum irqchip_irq_state which, + bool *val) +{ + u64 pendr, activer, enabler, hwirq_id_bit = BIT_ULL(d->hwirq % 64); + + switch (which) { + case IRQCHIP_STATE_PENDING: + if (d->hwirq < 64) + pendr = read_sysreg_s(SYS_ICC_PPI_SPENDR0_EL1); + else + pendr = read_sysreg_s(SYS_ICC_PPI_SPENDR1_EL1); + + *val = !!(pendr & hwirq_id_bit); + + return 0; + case IRQCHIP_STATE_ACTIVE: + if (d->hwirq < 64) + activer = read_sysreg_s(SYS_ICC_PPI_SACTIVER0_EL1); + else + activer = read_sysreg_s(SYS_ICC_PPI_SACTIVER1_EL1); + + *val = !!(activer & hwirq_id_bit); + + return 0; + case IRQCHIP_STATE_MASKED: + if (d->hwirq < 64) + enabler = read_sysreg_s(SYS_ICC_PPI_ENABLER0_EL1); + else + enabler = read_sysreg_s(SYS_ICC_PPI_ENABLER1_EL1); + + *val = !(enabler & hwirq_id_bit); + + return 0; + default: + pr_debug("Unexpected PPI irqchip state\n"); + return -EINVAL; + } + + return -EINVAL; +} + +static int gicv5_ppi_irq_set_irqchip_state(struct irq_data *d, + enum irqchip_irq_state which, + bool val) +{ + u64 hwirq_id_bit = BIT_ULL(d->hwirq % 64); + + switch (which) { + case IRQCHIP_STATE_PENDING: + if (val) { + if (d->hwirq < 64) + write_sysreg_s(hwirq_id_bit, + SYS_ICC_PPI_SPENDR0_EL1); + else + write_sysreg_s(hwirq_id_bit, + SYS_ICC_PPI_SPENDR1_EL1); + + } else { + if (d->hwirq < 64) + write_sysreg_s(hwirq_id_bit, + SYS_ICC_PPI_CPENDR0_EL1); + else + write_sysreg_s(hwirq_id_bit, + SYS_ICC_PPI_CPENDR1_EL1); + } + + return 0; + case IRQCHIP_STATE_ACTIVE: + if (val) { + if (d->hwirq < 64) + write_sysreg_s(hwirq_id_bit, + SYS_ICC_PPI_SACTIVER0_EL1); + else + write_sysreg_s(hwirq_id_bit, + SYS_ICC_PPI_SACTIVER1_EL1); + } else { + if (d->hwirq < 64) + write_sysreg_s(hwirq_id_bit, + SYS_ICC_PPI_CACTIVER0_EL1); + else + write_sysreg_s(hwirq_id_bit, + SYS_ICC_PPI_CACTIVER1_EL1); + } + + return 0; + case IRQCHIP_STATE_MASKED: + if (val) + gicv5_ppi_irq_mask(d); + else + gicv5_ppi_irq_unmask(d); + return 0; + default: + pr_debug("Unexpected PPI irqchip state\n"); + return -EINVAL; + } + + return -EINVAL; +} + +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_set_type = gicv5_ppi_set_type, + .irq_get_irqchip_state = gicv5_ppi_irq_get_irqchip_state, + .irq_set_irqchip_state = gicv5_ppi_irq_set_irqchip_state, + .flags = IRQCHIP_SET_TYPE_MASKED | + 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)) { + if (fwspec->param_count < 3) + return -EINVAL; + + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI) + return -EINVAL; + + *hwirq = fwspec->param[1]; + *type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK; + + return 0; + } + + return -EINVAL; +} + +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(nr_irqs != 1)) + return -EINVAL; + + ret = gicv5_irq_ppi_domain_translate(domain, fwspec, &hwirq, &type); + if (ret) + return ret; + + 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_ppi_domain_free(struct irq_domain *domain, + unsigned int virq, unsigned int nr_irqs) +{ + struct irq_data *d; + + if (WARN_ON(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) +{ + /* Not for us */ + if (fwspec->fwnode != d->fwnode) + return 0; + + if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI) { + // only handle PPIs + 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_ppi_domain_free, + .select = gicv5_irq_ppi_domain_select +}; + +static inline void handle_irq_per_domain(u32 hwirq) +{ + u32 hwirq_id; + struct irq_domain *domain = NULL; + u8 hwirq_type = FIELD_GET(GICV5_HWIRQ_TYPE, hwirq); + + hwirq_id = FIELD_GET(GICV5_HWIRQ_ID, hwirq); + + if (hwirq_type == GICV5_HWIRQ_TYPE_PPI) + domain = gicv5_global_data.ppi_domain; + + if (generic_handle_domain_irq(domain, hwirq_id)) { + pr_err("Could not handle, hwirq = 0x%x", hwirq_id); + gicv5_hwirq_eoi(hwirq_id, hwirq_type); + } +} + +static asmlinkage void __exception_irq_entry +gicv5_handle_irq(struct pt_regs *regs) +{ + u64 ia; + bool valid; + u32 hwirq; + + ia = gicr_insn(GICV5_OP_GICR_CDIA); + valid = GICV5_GIC_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); +} + +/* + * Disable IRQs for the executing CPU + */ +static void gicv5_cpu_disable_interrupts(void) +{ + u64 cr0; + + // Disable interrupts for the Interrupt Domain + cr0 = FIELD_PREP(ICC_CR0_EL1_EN, 0); + write_sysreg_s(cr0, SYS_ICC_CR0_EL1); +} + +/* + * Enable IRQs for the executing CPU + */ +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(); + + // Explicitly set the physical interrupt priority of the CPU + pcr = FIELD_PREP(ICC_PCR_EL1_PRIORITY, GICV5_IRQ_PRIORITY_MI); + write_sysreg_s(pcr, SYS_ICC_PCR_EL1); + + // Enable interrupts for the Interrupt Domain + 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); +} + +static int __init gicv5_init_domains(struct fwnode_handle *handle) +{ + gicv5_global_data.fwnode = handle; + gicv5_global_data.ppi_domain = irq_domain_create_linear( + handle, 128, &gicv5_irq_ppi_domain_ops, NULL); + + if (WARN_ON(!gicv5_global_data.ppi_domain)) + return -ENOMEM; + irq_domain_update_bus_token(gicv5_global_data.ppi_domain, + DOMAIN_BUS_WIRED); + + 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; + + ret = gicv5_init_domains(&node->fwnode); + if (ret) + return ret; + + gicv5_set_cpuif_pribits(); + + ret = gicv5_starting_cpu(smp_processor_id()); + if (ret) { + gicv5_free_domains(); + return ret; + } + + ret = set_handle_irq(gicv5_handle_irq); + if (ret) { + gicv5_free_domains(); + gicv5_cpu_disable_interrupts(); + return ret; + } + + return 0; +} +IRQCHIP_DECLARE(gic_v5, "arm,gic-v5", gicv5_of_init); diff --git a/drivers/irqchip/irq-gic-v5.h b/drivers/irqchip/irq-gic-v5.h new file mode 100644 index 0000000000000000000000000000000000000000..d8b797cdea2f786646fd88d9c8f60d483380991c --- /dev/null +++ b/drivers/irqchip/irq-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_GIC_V5_H +#define __LINUX_IRQCHIP_GIC_V5_H + +#include <asm/arch_gicv5.h> + +#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) + +#define GICV5_PPI_HM_EDGE UL(0x0) +#define GICV5_PPI_HM_LEVEL UL(0x1) + +#endif