On Mon, Feb 17, 2014 at 04:41:51PM +0000, Marc Zyngier wrote: > Hi Christoffer, > > On 17/02/14 01:19, Christoffer Dall wrote: > > On Wed, Feb 05, 2014 at 01:30:33PM +0000, Marc Zyngier wrote: > >> The Generic Interrupt Controller (version 3) offers services that are > >> similar to GICv2, with a number of additional features: > >> - Affinity routing based on the CPU MPIDR (ARE) > >> - System register for the CPU interfaces (SRE) > >> - Support for more that 8 CPUs > >> - Locality-specific Peripheral Interrupts (LPIs) > >> - Interrupt Translation Services (ITS) > >> > >> This patch adds preliminary support for GICv3 with ARE and SRE, > >> non-secure mode only. It relies on higher exception levels to grant ARE > >> and SRE access. > >> > >> Support for LPI and ITS will be added at a later time. > >> > >> Acked-by: Catalin Marinas <catalin.marinas@xxxxxxx> > >> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> > >> --- > >> arch/arm64/Kconfig | 1 + > >> arch/arm64/kernel/head.S | 14 + > >> arch/arm64/kernel/hyp-stub.S | 41 ++- > >> drivers/irqchip/Kconfig | 5 + > >> drivers/irqchip/Makefile | 1 + > >> drivers/irqchip/irq-gic-v3.c | 665 +++++++++++++++++++++++++++++++++++++ > >> include/linux/irqchip/arm-gic-v3.h | 190 +++++++++++ > >> 7 files changed, 915 insertions(+), 2 deletions(-) > >> create mode 100644 drivers/irqchip/irq-gic-v3.c > >> create mode 100644 include/linux/irqchip/arm-gic-v3.h > >> > >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > >> index dd4327f..3a7e26d 100644 > >> --- a/arch/arm64/Kconfig > >> +++ b/arch/arm64/Kconfig > >> @@ -9,6 +9,7 @@ config ARM64 > >> select ARM_AMBA > >> select ARM_ARCH_TIMER > >> select ARM_GIC > >> + select ARM_GIC_V3 > >> select BUILDTIME_EXTABLE_SORT > >> select CLONE_BACKWARDS > >> select COMMON_CLK > >> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S > >> index 0b281ff..522649b 100644 > >> --- a/arch/arm64/kernel/head.S > >> +++ b/arch/arm64/kernel/head.S > >> @@ -22,6 +22,7 @@ > >> > >> #include <linux/linkage.h> > >> #include <linux/init.h> > >> +#include <linux/irqchip/arm-gic-v3.h> > >> > >> #include <asm/assembler.h> > >> #include <asm/ptrace.h> > >> @@ -183,6 +184,19 @@ CPU_LE( bic x0, x0, #(3 << 24) ) // Clear the EE and E0E bits for EL1 > >> msr cnthctl_el2, x0 > >> msr cntvoff_el2, xzr // Clear virtual offset > >> > >> + /* GICv3 system register access */ > >> + mrs x0, id_aa64pfr0_el1 > >> + ubfx x0, x0, #24, #4 > >> + cmp x0, #1 > >> + b.ne 3f > >> + > >> + mrs x0, ICC_SRE_EL2 // Set ICC_SRE_EL2.SRE==1 > >> + orr x0, x0, #1 > >> + msr ICC_SRE_EL2, x0 > >> + isb // Make sure SRE is now 1 > >> + msr ICH_HCR_EL2, xzr // Reset ICC_HCR_EL2 to defaults > > > > Why do we not just set ICC_SRE_EL2.Enable here, but instead do the whole > > exception dance below? > > That's a very good question. The more I think of it, the more I believe > it to be completely unnecessary. > > I'm going to remove the trapping and see what breaks. Or who screams. > > >> + > >> +3: > >> /* Populate ID registers. */ > >> mrs x0, midr_el1 > >> mrs x1, mpidr_el1 > >> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S > >> index 0959611..c850d1e 100644 > >> --- a/arch/arm64/kernel/hyp-stub.S > >> +++ b/arch/arm64/kernel/hyp-stub.S > >> @@ -19,6 +19,7 @@ > >> > >> #include <linux/init.h> > >> #include <linux/linkage.h> > >> +#include <linux/irqchip/arm-gic-v3.h> > >> > >> #include <asm/assembler.h> > >> #include <asm/ptrace.h> > >> @@ -51,16 +52,52 @@ ENDPROC(__hyp_stub_vectors) > >> > >> .align 11 > >> > >> +#define sysreg_to_iss(op0, op1, crn, crm, op2) \ > >> + (((op0 & 3) << 20) | ((op2 & 7 ) << 17) | \ > >> + ((op1 & 7 ) << 14) | ((crn & 15) << 10) | \ > >> + ((crm & 15) << 1)) > >> + > >> el1_sync: > >> + // Hack alert: we don't have a stack, and we can't use the one > >> + // from EL1 because our MMU is off. Thankfully, we have > >> + // tpidr_el2, which is under our complete control, and > >> + // tpidr_el0, which can be used as long as we haven't reached > >> + // userspace. Oh hum... > > > > Hmmm. I feel like either we should tone down the hack alert claim, > > because this is after all safe and we just explain what's going on, or > > we should just allocate a small stack space following el1_sync and set > > the SP upon entry to this handler? > > Allocating a stack here is something I'd rather avoid (how big?). But > yeah, I can probably tone the horror down a notch - I got used to see > that code... ;-) > > > Hopefully none of the code that will ever call into the sysregs trap > > ever makes the same assumption that user space is not called yet ;) > > Well, I'm going to nuke that code anyway. I'm almost convinced that we > don't need it. > > >> + msr tpidr_el2, x1 // Preserve x1 > >> + > >> mrs x1, esr_el2 > >> lsr x1, x1, #26 > >> cmp x1, #0x16 > >> - b.ne 2f // Not an HVC trap > >> + b.ne 3f // Not an HVC trap > >> + > >> + // Handle getting/setting vbar_el2 > >> cbz x0, 1f > >> msr vbar_el2, x0 // Set vbar_el2 > >> b 2f > >> 1: mrs x0, vbar_el2 // Return vbar_el2 > >> -2: eret > >> +2: mrs x1, tpidr_el2 // Restore x1 > >> + eret > >> + > >> +3: cmp x1, #0x18 // MSR/MRS? > >> + b.ne 2b > >> + > >> + // Handle trapped sys reg access > >> + msr tpidr_el0, x0 // Preserve x0 > >> + mrs x1, esr_el2 > >> + ldr x0, =sysreg_to_iss(3, 7, 15, 15, 7) > >> + and x1, x1, x0 > >> + ldr x0, =sysreg_to_iss(3, 0, 12, 12, 5) > >> + eor x0, x0, x1 // Check for ICC_SRE_EL1 > >> + cbnz x0, 4f // Something bad happened > > > > Shouldn't we try to do something here, like go to panic on the > > exception return here, or not worth it? > > Code is now gone... ;-) > > >> + > >> + // Set ICC_SRE_EL1 access enable > >> + mrs x1, ICC_SRE_EL2 > >> + orr x1, x1, #(1 << 3) > >> + msr ICC_SRE_EL2, x1 > >> + > >> +4: mrs x0, tpidr_el0 // Restore x0 > >> + mrs x1, tpidr_el2 // Restore x1 > >> + eret > >> ENDPROC(el1_sync) > >> > >> .macro invalid_vector label > >> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > >> index 61ffdca..5b55c46 100644 > >> --- a/drivers/irqchip/Kconfig > >> +++ b/drivers/irqchip/Kconfig > >> @@ -10,6 +10,11 @@ config ARM_GIC > >> config GIC_NON_BANKED > >> bool > >> > >> +config ARM_GIC_V3 > >> + bool > >> + select IRQ_DOMAIN > >> + select MULTI_IRQ_HANDLER > >> + > >> config ARM_NVIC > >> bool > >> select IRQ_DOMAIN > >> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > >> index 86b484c..e6ac9fd 100644 > >> --- a/drivers/irqchip/Makefile > >> +++ b/drivers/irqchip/Makefile > >> @@ -14,6 +14,7 @@ obj-$(CONFIG_ORION_IRQCHIP) += irq-orion.o > >> obj-$(CONFIG_ARCH_SUNXI) += irq-sun4i.o > >> obj-$(CONFIG_ARCH_SPEAR3XX) += spear-shirq.o > >> obj-$(CONFIG_ARM_GIC) += irq-gic.o > >> +obj-$(CONFIG_ARM_GIC_V3) += irq-gic-v3.o > >> obj-$(CONFIG_ARM_NVIC) += irq-nvic.o > >> obj-$(CONFIG_ARM_VIC) += irq-vic.o > >> obj-$(CONFIG_IMGPDC_IRQ) += irq-imgpdc.o > >> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > >> new file mode 100644 > >> index 0000000..487e061 > >> --- /dev/null > >> +++ b/drivers/irqchip/irq-gic-v3.c > >> @@ -0,0 +1,665 @@ > >> +/* > >> + * Copyright (C) 2013 ARM Limited, All Rights Reserved. > >> + * Author: Marc Zyngier <marc.zyngier@xxxxxxx> > >> + * > >> + * This program is free software; you can redistribute it and/or modify > >> + * it under the terms of the GNU General Public License version 2 as > >> + * published by the Free Software Foundation. > >> + * > >> + * This program is distributed in the hope that it will be useful, > >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of > >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > >> + * GNU General Public License for more details. > >> + * > >> + * You should have received a copy of the GNU General Public License > >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. > >> + */ > >> + > >> +#include <linux/cpu.h> > >> +#include <linux/interrupt.h> > >> +#include <linux/of.h> > >> +#include <linux/of_address.h> > >> +#include <linux/of_irq.h> > >> +#include <linux/percpu.h> > >> +#include <linux/slab.h> > >> + > >> +#include <linux/irqchip/arm-gic-v3.h> > >> + > >> +#include <asm/cputype.h> > >> +#include <asm/exception.h> > >> +#include <asm/smp_plat.h> > >> + > >> +#include "irqchip.h" > >> + > >> +struct gic_chip_data { > >> + void __iomem *dist_base; > >> + void __iomem **redist_base; > >> + void __percpu __iomem **rdist; > >> + struct irq_domain *domain; > >> + u64 redist_stride; > >> + u32 redist_regions; > >> + unsigned int irq_nr; > >> +}; > >> + > >> +static struct gic_chip_data gic_data __read_mostly; > >> + > >> +#define gic_data_rdist_rd_base() (*__this_cpu_ptr(gic_data.rdist)) > >> +#define gic_data_rdist_sgi_base() (gic_data_rdist_rd_base() + SZ_64K) > >> + > >> +static DEFINE_RAW_SPINLOCK(dist_lock); > >> + > >> +#define DEFAULT_PMR_VALUE 0xf0 > >> + > >> +static inline void __iomem *gic_dist_base(struct irq_data *d) > >> +{ > >> + if (d->hwirq < 32) /* SGI+PPI -> SGI_base for this CPU */ > >> + return gic_data_rdist_sgi_base(); > >> + > >> + if (d->hwirq <= 1023) /* SPI -> dist_base */ > >> + return gic_data.dist_base; > >> + > >> + if (d->hwirq >= 8192) > >> + BUG(); /* LPI Detected!!! */ > >> + > >> + return NULL; > >> +} > >> + > >> +static inline unsigned int gic_irq(struct irq_data *d) > >> +{ > >> + return d->hwirq; > >> +} > >> + > >> +static void gic_do_wait_for_rwp(void __iomem *base) > >> +{ > >> + u32 val; > >> + > >> + do { > >> + val = readl_relaxed(base + GICD_CTLR); > >> + cpu_relax(); > >> + } while (val & GICD_CTLR_RWP); > >> +} > >> + > >> +/* Wait for completion of a distributor change */ > >> +static void gic_dist_wait_for_rwp(void) > >> +{ > >> + gic_do_wait_for_rwp(gic_data.dist_base); > >> +} > >> + > >> +/* Wait for completion of a redistributor change */ > >> +static void gic_redist_wait_for_rwp(void) > >> +{ > >> + gic_do_wait_for_rwp(gic_data_rdist_rd_base()); > >> +} > >> + > >> +static void gic_wait_for_rwp(int irq) > >> +{ > >> + if (irq < 32) > >> + gic_redist_wait_for_rwp(); > >> + else > >> + gic_dist_wait_for_rwp(); > >> +} > >> + > >> +/* Low level accessors */ > >> +static u64 gic_read_iar(void) > >> +{ > >> + u64 irqstat; > >> + > >> + asm volatile("mrs %0, " reg(ICC_IAR1_EL1) : "=r" (irqstat)); > >> + return irqstat; > >> +} > >> + > >> +static void gic_write_pmr(u64 val) > >> +{ > >> + asm volatile("msr " reg(ICC_PMR_EL1) ", %0" : : "r" (val)); > >> + isb(); > >> +} > >> + > >> +static void gic_write_ctlr(u64 val) > >> +{ > >> + asm volatile("msr " reg(ICC_CTLR_EL1) ", %0" : : "r" (val)); > >> + isb(); > >> +} > >> + > >> +static void gic_write_grpen1(u64 val) > >> +{ > >> + asm volatile("msr " reg(ICC_GRPEN1_EL1) ", %0" : : "r" (val)); > >> + isb(); > >> +} > >> + > >> +static void gic_write_sgi1r(u64 val) > >> +{ > >> + asm volatile("msr " reg(ICC_SGI1R_EL1) ", %0" : : "r" (val)); > >> + isb(); > >> +} > >> + > >> +static void gic_enable_sre(void) > >> +{ > >> + u64 val; > >> + > >> + /* This access is likely to trap into EL2 */ > >> + asm volatile("mrs %0, " reg(ICC_SRE_EL1) : "=r" (val)); > >> + val |= GICC_SRE_EL1_SRE; > >> + asm volatile("msr " reg(ICC_SRE_EL1) ", %0" : : "r" (val)); > >> + isb(); > >> +} > >> + > >> +static void gic_enable_redist(void) > >> +{ > >> + void __iomem *rbase; > >> + u32 val; > >> + > >> + rbase = gic_data_rdist_rd_base(); > >> + > >> + /* Wake up this CPU redistributor */ > >> + val = readl_relaxed(rbase + GICR_WAKER); > >> + val &= ~GICR_WAKER_ProcessorSleep; > >> + writel_relaxed(val, rbase + GICR_WAKER); > >> + > >> + do { > >> + val = readl_relaxed(rbase + GICR_WAKER); > >> + cpu_relax(); > >> + } while (val & GICR_WAKER_ChildrenAsleep); > >> +} > >> + > >> +/* > >> + * Routines to acknowledge, disable and enable interrupts > >> + */ > >> +static void gic_mask_irq(struct irq_data *d) > >> +{ > >> + u32 mask = 1 << (gic_irq(d) % 32); > >> + > >> + raw_spin_lock(&dist_lock); > >> + writel_relaxed(mask, gic_dist_base(d) + GICD_ICENABLER + (gic_irq(d) / 32) * 4); > >> + gic_wait_for_rwp(gic_irq(d)); > >> + raw_spin_unlock(&dist_lock); > >> +} > >> + > >> +static void gic_unmask_irq(struct irq_data *d) > >> +{ > >> + u32 mask = 1 << (gic_irq(d) % 32); > >> + > >> + raw_spin_lock(&dist_lock); > >> + writel_relaxed(mask, gic_dist_base(d) + GICD_ISENABLER + (gic_irq(d) / 32) * 4); > >> + gic_wait_for_rwp(gic_irq(d)); > >> + raw_spin_unlock(&dist_lock); > >> +} > >> + > >> +static void gic_eoi_irq(struct irq_data *d) > >> +{ > >> + gic_write_eoir(gic_irq(d)); > >> +} > >> + > >> +static int gic_set_type(struct irq_data *d, unsigned int type) > >> +{ > >> + void __iomem *base = gic_dist_base(d); > >> + unsigned int irq = gic_irq(d); > >> + u32 enablemask = 1 << (irq % 32); > >> + u32 enableoff = (irq / 32) * 4; > >> + u32 confmask = 0x2 << ((irq % 16) * 2); > >> + u32 confoff = (irq / 16) * 4; > >> + bool enabled = false; > >> + u32 val; > >> + > >> + /* Interrupt configuration for SGIs can't be changed */ > >> + if (irq < 16) > >> + return -EINVAL; > >> + > >> + if (type != IRQ_TYPE_LEVEL_HIGH && type != IRQ_TYPE_EDGE_RISING) > >> + return -EINVAL; > >> + > >> + raw_spin_lock(&dist_lock); > >> + > >> + val = readl_relaxed(base + GICD_ICFGR + confoff); > >> + if (type == IRQ_TYPE_LEVEL_HIGH) > >> + val &= ~confmask; > >> + else if (type == IRQ_TYPE_EDGE_RISING) > >> + val |= confmask; > >> + > >> + /* > >> + * As recommended by the spec, disable the interrupt before changing > >> + * the configuration > >> + */ > >> + if (readl_relaxed(base + GICD_ISENABLER + enableoff) & enablemask) { > >> + writel_relaxed(enablemask, base + GICD_ICENABLER + enableoff); > >> + gic_wait_for_rwp(irq); > >> + enabled = true; > >> + } > >> + > >> + writel_relaxed(val, base + GICD_ICFGR + confoff); > >> + > >> + if (enabled) { > >> + writel_relaxed(enablemask, base + GICD_ISENABLER + enableoff); > >> + gic_wait_for_rwp(irq); > >> + } > >> + > >> + raw_spin_unlock(&dist_lock); > >> + > >> + return 0; > >> +} > >> + > >> +static int gic_retrigger(struct irq_data *d) > >> +{ > >> + return -ENXIO; > >> +} > >> + > >> +static u64 gic_mpidr_to_affinity(u64 mpidr) > >> +{ > >> + /* Make sure we don't broadcast the interrupt */ > >> + return mpidr & ~GICD_IROUTER_SPI_MODE_ANY; > >> +} > >> + > >> +static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs) > >> +{ > >> + u64 irqnr; > >> + > >> + do { > >> + irqnr = gic_read_iar(); > >> + > >> + if (likely(irqnr > 15 && irqnr < 1021)) { > >> + irqnr = irq_find_mapping(gic_data.domain, irqnr); > >> + handle_IRQ(irqnr, regs); > >> + continue; > >> + } > >> + if (irqnr < 16) { > >> + gic_write_eoir(irqnr); > >> +#ifdef CONFIG_SMP > >> + handle_IPI(irqnr, regs); > >> +#else > >> + WARN_ONCE(true, "Unexpected SGI received!\n"); > >> +#endif > >> + continue; > >> + } > >> + } while (irqnr != 0x3ff); > >> +} > >> + > >> +static void __init gic_dist_init(void) > >> +{ > >> + unsigned int i; > >> + u64 affinity; > >> + int gic_irqs = gic_data.irq_nr; > >> + void __iomem *base = gic_data.dist_base; > >> + > >> + /* Disable the distributor */ > >> + writel_relaxed(0, base + GICD_CTLR); > >> + > >> + /* > >> + * Set all global interrupts to be level triggered, active low. > >> + */ > >> + for (i = 32; i < gic_data.irq_nr; i += 16) > >> + writel_relaxed(0, base + GICD_ICFGR + i / 4); > >> + > >> + /* > >> + * Set priority on all global interrupts. > >> + */ > >> + for (i = 32; i < gic_irqs; i += 4) > >> + writel_relaxed(0xa0a0a0a0, base + GICD_IPRIORITYR + i); > >> + > >> + /* > >> + * Disable all interrupts. Leave the PPI and SGIs alone > >> + * as these enables are banked registers. > > > > as these are enabled through banked registers? > > "The enable bits are banked registers", but I can see where the > confision comes from. I'll rewrite this comment. > > > also, should banked registers be redistributor registers? > > Yup. > > >> + */ > >> + for (i = 32; i < gic_irqs; i += 32) > >> + writel_relaxed(0xffffffff, base + GICD_ICENABLER + i / 8); > >> + > >> + gic_dist_wait_for_rwp(); > >> + > >> + writel_relaxed(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1, > >> + base + GICD_CTLR); > > > > why set GICD_CTLR_ENABLE_G1 ? > > Believe it or not, some people insist on running in secure mode. > Ah, didn't consider this. Maybe because the cover-letter ssays this patch only supports "Non-Secure Group-1 interrupts only". > >> + > >> + /* > >> + * Set all global interrupts to the boot CPU only. ARE must be > >> + * enabled. > >> + */ > >> + affinity = gic_mpidr_to_affinity(read_cpuid_mpidr()); > >> + for (i = 32; i < gic_data.irq_nr; i++) > >> + writeq_relaxed(affinity, base + GICD_IROUTER + i * 8); > >> +} > >> + > >> +static int __init gic_populate_rdist(void) > >> +{ > >> + u64 mpidr = cpu_logical_map(smp_processor_id()); > >> + u64 typer; > >> + u64 aff; > >> + int i; > >> + > >> + aff = mpidr & ((1 << 24) - 1); > >> + aff |= (mpidr >> 8) & (0xffUL << 24); > > > > Some defines would be nice for this if it makes sense in any way. > > There is at least 3 methods GICv3 uses to express affinity. At some > point, I gave up and decided that I needed mental support. > > I'll update it with the appropriate macros (MPIDR_AFFINITY_LEVEL and co). > I don't feel strongly about it, maybe we need giant diagrams on A1 paper instead :) > >> + > >> + for (i = 0; i < gic_data.redist_regions; i++) { > >> + void __iomem *ptr = gic_data.redist_base[i]; > >> + u32 reg; > >> + > >> + reg = readl_relaxed(ptr + GICR_PIDR2) & GIC_PIDR2_ARCH_MASK; > >> + if (reg != 0x30 && reg != 0x40) { /* We're in trouble... */ > >> + pr_warn("No redistributor present @%p\n", ptr); > >> + break; > >> + } > >> + > >> + do { > >> + typer = readq_relaxed(ptr + GICR_TYPER); > >> + if ((typer >> 32) == aff) { > >> + gic_data_rdist_rd_base() = ptr; > >> + pr_info("CPU%d: found redistributor %llx @%p\n", > >> + smp_processor_id(), > >> + (unsigned long long) mpidr, ptr); > >> + return 0; > >> + } > >> + > >> + if (gic_data.redist_stride) { > >> + ptr += gic_data.redist_stride; > >> + } else { > >> + ptr += SZ_64K * 2; /* Skip RD_base + SGI_base */ > >> + if (typer & GICR_TYPER_VLPIS) > >> + ptr += SZ_64K * 2; /* Skip VLPI_base + reserved page */ > >> + } > >> + } while (!(typer & GICR_TYPER_LAST)); > >> + } > >> + > >> + /* We couldn't even deal with ourselves... */ > >> + WARN(true, "CPU%d: mpidr %lx has no re-distributor!\n", > >> + smp_processor_id(), (unsigned long)mpidr); > >> + return -ENODEV; > >> +} > >> + > >> +static void __init gic_cpu_init(void) > >> +{ > >> + void __iomem *rbase; > >> + int i; > >> + > >> + /* Register ourselves with the rest of the world */ > >> + if (gic_populate_rdist()) > >> + return; > >> + > >> + gic_enable_redist(); > >> + > >> + rbase = gic_data_rdist_sgi_base(); > >> + > >> + /* > >> + * Set priority on PPI and SGI interrupts > >> + */ > >> + for (i = 0; i < 32; i += 4) > >> + writel_relaxed(0xa0a0a0a0, rbase + GICR_IPRIORITYR0 + i * 4 / 4); > > > > (i * 4) / 4 == i, last I checked. > > Recurring comment. I decided to keep it inline with the GICv2 practice. > Read it as i * increment / interrupt-per-word. Maybe not worth it if it > causes more problem than anything else... > Yeah, I noticed it in the GICv2 code afterwards, but I don't think you're consistent with this approach throughout the file. I personaly find this notation mind-boggingly-difficult, but that is a personal observation. > >> + > >> + /* > >> + * Disable all PPI interrupts, ensure all SGI interrupts are > >> + * enabled. > > > > Why do we disable all PPIs? > > Because interrupts should default to being disabled? They will get > enabled when you do a request_irq. > Duh, yeah, and SGI don't need to be disabled because we're in control of generating them I guess. Thanks for the explanation. > >> + */ > >> + writel_relaxed(0xffff0000, rbase + GICR_ICENABLER0); > >> + writel_relaxed(0x0000ffff, rbase + GICR_ISENABLER0); > >> + > >> + gic_redist_wait_for_rwp(); > >> + > >> + /* Enable system registers */ > >> + gic_enable_sre(); > >> + > >> + /* Set priority mask register */ > >> + gic_write_pmr(DEFAULT_PMR_VALUE); > >> + > >> + /* EOI drops priority too (mode 0) */ > > > > you mean deactives the interrupt too, right? It always performs a > > priority drop doesn't it? > > Yup. Just my usual brainfart. > > >> + gic_write_ctlr(GICC_CTLR_EL1_EOImode_drop_dir); > >> + > >> + /* ... and let's hit the road... */ > >> + gic_write_grpen1(1); > >> +} > >> + > >> +#ifdef CONFIG_SMP > >> +static int __init gic_secondary_init(struct notifier_block *nfb, > >> + unsigned long action, void *hcpu) > >> +{ > >> + if (action == CPU_STARTING || action == CPU_STARTING_FROZEN) > >> + gic_cpu_init(); > >> + return NOTIFY_OK; > >> +} > >> + > >> +/* > >> + * Notifier for enabling the GIC CPU interface. Set an arbitrarily high > >> + * priority because the GIC needs to be up before the ARM generic timers. > > > > I actually don't understand this bit, but it also sits in the original > > code, so I assume it's correct. > > GIC and timer on the secondary CPUs are intialized using the CPU > notifiers. Since the need the GIC to be up and running before the timers > can enable their interrupts, we use the relative priorities of the > notifiers to order them. > thanks again for the explanation. > >> + */ > >> +static struct notifier_block __initdata gic_cpu_notifier = { > >> + .notifier_call = gic_secondary_init, > >> + .priority = 100, > >> +}; > >> + > >> +static u16 gic_compute_target_list(int *base_cpu, const struct cpumask *mask, > >> + u64 cluster_id) > >> +{ > >> + int cpu = *base_cpu; > >> + u64 mpidr = cpu_logical_map(cpu); > >> + u16 tlist = 0; > >> + > >> + while (cpu < nr_cpu_ids) { > >> + /* > >> + * If we ever get a cluster of more than 16 CPUs, just > >> + * scream and skip that CPU. > >> + */ > >> + if (WARN_ON((mpidr & 0xff) >= 16)) > >> + goto out; > >> + > >> + tlist |= 1 << (mpidr & 0xf); > >> + > >> + cpu = cpumask_next(cpu, mask); > >> + mpidr = cpu_logical_map(cpu); > > > > Are you not accessing memory beyond the __cpu_logical_map array here > > when the cpu is the last one at the beginning of the loop? > > Nice catch! I need to test cpu here before carrying on... > You could also simply use for_each_cpu_mask here and in the beginning of the loop increment the first cpu to *base_cpu if it makes the code cleaner. Not sure. > >> + > >> + if (cluster_id != (mpidr & ~0xffUL)) { > >> + cpu--; > >> + goto out; > >> + } > >> + } > >> +out: > >> + *base_cpu = cpu; > >> + return tlist; > >> +} > >> + > >> +static void gic_send_sgi(u64 cluster_id, u16 tlist, unsigned int irq) > >> +{ > >> + u64 val; > >> + > >> + val = (cluster_id & 0xff00ff0000UL) << 16; /* Aff3 + Aff2 */ > >> + val |= (cluster_id & 0xff00) << 8; /* Aff1 */ > >> + val |= irq << 24; > >> + val |= tlist; > >> + > >> + pr_debug("CPU%d: ICC_SGI1R_EL1 %llx\n", smp_processor_id(), val); > >> + gic_write_sgi1r(val); > >> +} > >> + > >> +static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) > >> +{ > >> + int cpu; > >> + > >> + if (WARN_ON(irq >= 16)) > >> + return; > >> + > >> + /* > >> + * Ensure that stores to Normal memory are visible to the > >> + * other CPUs before issuing the IPI. > >> + */ > >> + dsb(); > >> + > >> + for_each_cpu_mask(cpu, *mask) { > >> + u64 cluster_id = cpu_logical_map(cpu) & ~0xffUL; > >> + u16 tlist; > >> + > >> + tlist = gic_compute_target_list(&cpu, mask, cluster_id); > > > > I think it would be potentially slightly prettier if this function > > returned the last cpu in the cluster so the loop iterator mangling was > > kept local to this function and took a pointer to the tlist it was to > > fill instead, but, this is a minor detail. > > Yeah, maybe. I'll try that and see how it looks. it's quite messy anyway... > yeah, nothing I feel strongly about anyway. > >> + gic_send_sgi(cluster_id, tlist, irq); > >> + } > >> +} > >> + > >> +static void gic_smp_init(void) > >> +{ > >> + set_smp_cross_call(gic_raise_softirq); > >> + register_cpu_notifier(&gic_cpu_notifier); > >> +} > >> + > >> +static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val, > >> + bool force) > >> +{ > >> + unsigned int cpu = cpumask_any_and(mask_val, cpu_online_mask); > >> + void __iomem *reg; > >> + u64 val; > >> + > >> + if (gic_irq(d) < 32) > >> + return -EINVAL; > >> + > >> + reg = gic_dist_base(d) + GICD_IROUTER + (gic_irq(d) * 8); > >> + val = gic_mpidr_to_affinity(cpu_logical_map(cpu)); > >> + > >> + writeq_relaxed(val, reg); > >> + > >> + return IRQ_SET_MASK_OK; > >> +} > >> +#else > >> +#define gic_set_affinity NULL > >> +#define gic_smp_init() do { } while(0) > >> +#endif > >> + > >> +static struct irq_chip gic_chip = { > >> + .name = "GICv3", > >> + .irq_mask = gic_mask_irq, > >> + .irq_unmask = gic_unmask_irq, > >> + .irq_eoi = gic_eoi_irq, > >> + .irq_set_type = gic_set_type, > >> + .irq_retrigger = gic_retrigger, > >> + .irq_set_affinity = gic_set_affinity, > >> +}; > >> + > >> +static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq, > >> + irq_hw_number_t hw) > >> +{ > >> + if (hw < 32) { > >> + irq_set_percpu_devid(irq); > >> + irq_set_chip_and_handler(irq, &gic_chip, > >> + handle_percpu_devid_irq); > >> + set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN); > >> + } else { > >> + irq_set_chip_and_handler(irq, &gic_chip, > >> + handle_fasteoi_irq); > >> + set_irq_flags(irq, IRQF_VALID | IRQF_PROBE); > > > > will this ever be compiled for arch/arm? If not, does it make sense to > > keep the set_irq_flags calls? > > Not impossible at all, by the look of it. That will require some effort, > but if someone feels like it, I don't see the point in actuvely removing > that code. > ok > >> + } > >> + irq_set_chip_data(irq, d->host_data); > >> + return 0; > >> +} > >> + > >> +static int gic_irq_domain_xlate(struct irq_domain *d, > >> + struct device_node *controller, > >> + const u32 *intspec, unsigned int intsize, > >> + unsigned long *out_hwirq, unsigned int *out_type) > >> +{ > >> + if (d->of_node != controller) > >> + return -EINVAL; > >> + if (intsize < 3) > >> + return -EINVAL; > >> + > >> + switch(intspec[0]) { > >> + case 0: /* SPI */ > >> + *out_hwirq = intspec[1] + 32; > >> + break; > >> + case 1: /* PPI */ > >> + *out_hwirq = intspec[1] + 16; > >> + break; > >> + default: > >> + return -EINVAL; > >> + } > >> + > >> + *out_type = intspec[2] & IRQ_TYPE_SENSE_MASK; > >> + return 0; > >> +} > >> + > >> +static const struct irq_domain_ops gic_irq_domain_ops = { > >> + .map = gic_irq_domain_map, > >> + .xlate = gic_irq_domain_xlate, > >> +}; > >> + > >> +static int __init gic_of_init(struct device_node *node, struct device_node *parent) > >> +{ > >> + void __iomem *dist_base; > >> + void __iomem **redist_base; > >> + u64 redist_stride; > >> + u32 redist_regions; > >> + u32 reg; > >> + int gic_irqs; > >> + int err; > >> + int i; > >> + > >> + dist_base = of_iomap(node, 0); > >> + if (!dist_base) { > >> + pr_warn("%s: unable to map gic dist registers\n", > >> + node->full_name); > > > > shouldn't these be pr_err? > > Indeed. > > >> + return -ENXIO; > >> + } > >> + > >> + reg = readl_relaxed(dist_base + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK; > >> + if (reg != 0x30 && reg != 0x40) { > > > > Is everything we do going to work with a GICv4? Anything else we need to > > initialize if it's a GICv4? > > GICv4 is a strict superset of GICv3, and it should all just work as long > as you don't try to use GICv4 specific features. *Should*. > cool > >> + pr_warn("%s: no distributor detected, giving up\n", > >> + node->full_name); > >> + err = -ENODEV; > >> + goto out_unmap_dist; > >> + } > >> + > >> + if (of_property_read_u32(node, "#redistributor-regions", &redist_regions)) > >> + redist_regions = 1; > >> + > >> + redist_base = kzalloc(sizeof(*redist_base) * redist_regions, GFP_KERNEL); > >> + if (!redist_base) { > >> + err = -ENOMEM; > >> + goto out_unmap_dist; > >> + } > >> + > >> + for (i = 0; i < redist_regions; i++) { > >> + redist_base[i] = of_iomap(node, 1 + i); > >> + if (!redist_base[i]) { > >> + pr_warn("%s: couldn't map region %d\n", > >> + node->full_name, i); > >> + err = -ENODEV; > >> + goto out_unmap_rdist; > >> + } > >> + } > >> + > >> + if (of_property_read_u64(node, "redistributor-stride", &redist_stride)) > >> + redist_stride = 0; > > > > The binding doesn't specify this as a two-cell property, so I assume > > this won't work if you follow the example in the bindings doc. > > Yup. I'll update the DT. Nice catch. > > >> + > >> + gic_data.dist_base = dist_base; > >> + gic_data.redist_base = redist_base; > >> + gic_data.redist_regions = redist_regions; > >> + gic_data.redist_stride = redist_stride; > >> + > >> + /* > >> + * Find out how many interrupts are supported. > >> + * The GIC only supports up to 1020 interrupt sources (SGI+PPI+SPI) > >> + */ > >> + gic_irqs = readl_relaxed(gic_data.dist_base + GICD_TYPER) & 0x1f; > >> + gic_irqs = (gic_irqs + 1) * 32; > >> + if (gic_irqs > 1020) > >> + gic_irqs = 1020; > >> + gic_data.irq_nr = gic_irqs; > >> + > >> + gic_data.domain = irq_domain_add_linear(node, gic_irqs - 16, > >> + &gic_irq_domain_ops, &gic_data); > >> + gic_data.rdist = alloc_percpu(typeof(*gic_data.rdist)); > >> + > >> + if (WARN_ON(!gic_data.domain) || WARN_ON(!gic_data.rdist)) { > >> + err = -ENOMEM; > >> + goto out_free; > >> + } > >> + > >> + set_handle_irq(gic_handle_irq); > >> + > >> + gic_smp_init(); > >> + gic_dist_init(); > >> + gic_cpu_init(); > >> + > >> + return 0; > >> + > >> +out_free: > >> + free_percpu(gic_data.rdist); > >> +out_unmap_rdist: > >> + for (i = 0; i < redist_regions; i++) > >> + if (redist_base[i]) > >> + iounmap(redist_base[i]); > >> + kfree(redist_base); > >> +out_unmap_dist: > >> + iounmap(dist_base); > >> + return err; > >> +} > >> + > >> +IRQCHIP_DECLARE(gic_v3, "arm,gic-v3", gic_of_init); > >> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h > >> new file mode 100644 > >> index 0000000..e4d060a > >> --- /dev/null > >> +++ b/include/linux/irqchip/arm-gic-v3.h > >> @@ -0,0 +1,190 @@ > >> +/* > >> + * Copyright (C) 2013 ARM Limited, All Rights Reserved. > >> + * Author: Marc Zyngier <marc.zyngier@xxxxxxx> > >> + * > >> + * > >> + * This program is free software; you can redistribute it and/or modify > >> + * it under the terms of the GNU General Public License version 2 as > >> + * published by the Free Software Foundation. > >> + * > >> + * This program is distributed in the hope that it will be useful, > >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of > >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > >> + * GNU General Public License for more details. > >> + * > >> + * You should have received a copy of the GNU General Public License > >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. > >> + */ > >> +#ifndef __LINUX_IRQCHIP_ARM_GIC_V3_H > >> +#define __LINUX_IRQCHIP_ARM_GIC_V3_H > >> + > >> +/* > >> + * Distributor registers. We assume we're running non-secure, with ARE > >> + * being set. Secure-only and non-ARE registers are not described. > >> + */ > >> +#define GICD_CTLR 0x0000 > >> +#define GICD_TYPER 0x0004 > >> +#define GICD_IIDR 0x0008 > >> +#define GICD_STATUSR 0x0010 > >> +#define GICD_SETSPI_NSR 0x0040 > >> +#define GICD_CLRSPI_NSR 0x0048 > >> +#define GICD_SETSPI_SR 0x0050 > >> +#define GICD_CLRSPI_SR 0x0058 > >> +#define GICD_SEIR 0x0068 > >> +#define GICD_ISENABLER 0x0100 > >> +#define GICD_ICENABLER 0x0180 > >> +#define GICD_ISPENDR 0x0200 > >> +#define GICD_ICPENDR 0x0280 > >> +#define GICD_ISACTIVER 0x0300 > >> +#define GICD_ICACTIVER 0x0380 > >> +#define GICD_IPRIORITYR 0x0400 > >> +#define GICD_ICFGR 0x0C00 > >> +#define GICD_IROUTER 0x6000 > >> +#define GICD_PIDR2 0xFFE8 > >> + > >> +#define GICD_CTLR_RWP (1U << 31) > >> +#define GICD_CTLR_ARE_NS (1U << 4) > >> +#define GICD_CTLR_ENABLE_G1A (1U << 1) > >> +#define GICD_CTLR_ENABLE_G1 (1U << 0) > >> + > >> +#define GICD_IROUTER_SPI_MODE_ONE (0U << 31) > >> +#define GICD_IROUTER_SPI_MODE_ANY (1U << 31) > >> + > >> +#define GIC_PIDR2_ARCH_MASK 0xf0 > >> + > >> +/* > >> + * Re-Distributor registers, offsets from RD_base > >> + */ > >> +#define GICR_CTLR GICD_CTLR > >> +#define GICR_IIDR 0x0004 > >> +#define GICR_TYPER 0x0008 > >> +#define GICR_STATUSR GICD_STATUSR > >> +#define GICR_WAKER 0x0014 > >> +#define GICR_SETLPIR 0x0040 > >> +#define GICR_CLRLPIR 0x0048 > >> +#define GICR_SEIR GICD_SEIR > >> +#define GICR_PROPBASER 0x0070 > >> +#define GICR_PENDBASER 0x0078 > >> +#define GICR_INVLPIR 0x00A0 > >> +#define GICR_INVALLR 0x00B0 > >> +#define GICR_SYNCR 0x00C0 > >> +#define GICR_MOVLPIR 0x0100 > >> +#define GICR_MOVALLR 0x0110 > >> +#define GICR_PIDR2 GICD_PIDR2 > >> + > >> +#define GICR_WAKER_ProcessorSleep (1U << 1) > >> +#define GICR_WAKER_ChildrenAsleep (1U << 2) > >> + > >> +/* > >> + * Re-Distributor registers, offsets from SGI_base > >> + */ > >> +#define GICR_ISENABLER0 GICD_ISENABLER > >> +#define GICR_ICENABLER0 GICD_ICENABLER > >> +#define GICR_ISPENDR0 GICD_ISPENDR > >> +#define GICR_ICPENDR0 GICD_ICPENDR > >> +#define GICR_ISACTIVER0 GICD_ISACTIVER > >> +#define GICR_ICACTIVER0 GICD_ICACTIVER > >> +#define GICR_IPRIORITYR0 GICD_IPRIORITYR > >> +#define GICR_ICFGR0 GICD_ICFGR > >> + > >> +#define GICR_TYPER_VLPIS (1U << 1) > >> +#define GICR_TYPER_LAST (1U << 4) > > > > nit: why are these TYPER definitions down here when the TYPER register > > is defined above and so the WAKER bit definitions? > > I'll group them in the next version. > > >> + > >> +/* > >> + * CPU interface registers > >> + */ > >> +#define GICC_CTLR_EL1_EOImode_drop_dir (0U << 1) > >> +#define GICC_CTLR_EL1_EOImode_drop (1U << 1) > >> +#define GICC_SRE_EL1_SRE (1U << 0) > >> + > >> +/* > >> + * Hypervisor interface registers (SRE only) > >> + */ > >> +#define GICH_LR_VIRTUAL_ID_MASK ((1UL << 32) - 1) > >> + > >> +#define GICH_LR_EOI (1UL << 41) > >> +#define GICH_LR_GROUP (1UL << 60) > >> +#define GICH_LR_STATE (3UL << 62) > >> +#define GICH_LR_PENDING_BIT (1UL << 62) > >> +#define GICH_LR_ACTIVE_BIT (1UL << 63) > >> + > >> +#define GICH_MISR_EOI (1 << 0) > >> +#define GICH_MISR_U (1 << 1) > >> + > >> +#define GICH_HCR_EN (1 << 0) > >> +#define GICH_HCR_UIE (1 << 1) > >> + > >> +#define GICH_VMCR_CTLR_SHIFT 0 > >> +#define GICH_VMCR_CTLR_MASK (0x21f << GICH_VMCR_CTLR_SHIFT) > >> +#define GICH_VMCR_BPR1_SHIFT 18 > >> +#define GICH_VMCR_BPR1_MASK (7 << GICH_VMCR_BPR1_SHIFT) > >> +#define GICH_VMCR_BPR0_SHIFT 21 > >> +#define GICH_VMCR_BPR0_MASK (7 << GICH_VMCR_BPR0_SHIFT) > >> +#define GICH_VMCR_PMR_SHIFT 24 > >> +#define GICH_VMCR_PMR_MASK (0xffUL << GICH_VMCR_PMR_SHIFT) > >> + > >> +#define ICC_EOIR1_EL1 S3_0_C12_C12_1 > >> +#define ICC_IAR1_EL1 S3_0_C12_C12_0 > >> +#define ICC_SGI1R_EL1 S3_0_C12_C11_5 > >> +#define ICC_PMR_EL1 S3_0_C4_C6_0 > >> +#define ICC_CTLR_EL1 S3_0_C12_C12_4 > >> +#define ICC_SRE_EL1 S3_0_C12_C12_5 > >> +#define ICC_GRPEN1_EL1 S3_0_C12_C12_7 > >> + > >> +#define ICC_SRE_EL2 S3_4_C12_C9_5 > >> + > >> +#define ICH_VSEIR_EL2 S3_4_C12_C9_4 > >> +#define ICH_HCR_EL2 S3_4_C12_C11_0 > >> +#define ICH_VTR_EL2 S3_4_C12_C11_1 > >> +#define ICH_MISR_EL2 S3_4_C12_C11_2 > >> +#define ICH_EISR_EL2 S3_4_C12_C11_3 > >> +#define ICH_ELSR_EL2 S3_4_C12_C11_5 > >> +#define ICH_VMCR_EL2 S3_4_C12_C11_7 > >> + > >> +#define __LR0_EL2(x) S3_4_C12_C12_ ## x > >> +#define __LR8_EL2(x) S3_4_C12_C13_ ## x > >> + > >> +#define ICH_LR0_EL2 __LR0_EL2(0) > >> +#define ICH_LR1_EL2 __LR0_EL2(1) > >> +#define ICH_LR2_EL2 __LR0_EL2(2) > >> +#define ICH_LR3_EL2 __LR0_EL2(3) > >> +#define ICH_LR4_EL2 __LR0_EL2(4) > >> +#define ICH_LR5_EL2 __LR0_EL2(5) > >> +#define ICH_LR6_EL2 __LR0_EL2(6) > >> +#define ICH_LR7_EL2 __LR0_EL2(7) > >> +#define ICH_LR8_EL2 __LR8_EL2(0) > >> +#define ICH_LR9_EL2 __LR8_EL2(1) > >> +#define ICH_LR10_EL2 __LR8_EL2(2) > >> +#define ICH_LR11_EL2 __LR8_EL2(3) > >> +#define ICH_LR12_EL2 __LR8_EL2(4) > >> +#define ICH_LR13_EL2 __LR8_EL2(5) > >> +#define ICH_LR14_EL2 __LR8_EL2(6) > >> +#define ICH_LR15_EL2 __LR8_EL2(7) > >> + > >> +#define __AP0Rx_EL2(x) S3_4_C12_C8_ ## x > >> +#define ICH_AP0R0_EL2 __AP0Rx_EL2(0) > >> +#define ICH_AP0R1_EL2 __AP0Rx_EL2(1) > >> +#define ICH_AP0R2_EL2 __AP0Rx_EL2(2) > >> +#define ICH_AP0R3_EL2 __AP0Rx_EL2(3) > >> + > >> +#define __AP1Rx_EL2(x) S3_4_C12_C9_ ## x > >> +#define ICH_AP1R0_EL2 __AP1Rx_EL2(0) > >> +#define ICH_AP1R1_EL2 __AP1Rx_EL2(1) > >> +#define ICH_AP1R2_EL2 __AP1Rx_EL2(2) > >> +#define ICH_AP1R3_EL2 __AP1Rx_EL2(3) > > > > That was fun to review. > > If you think so, then you're ready to join the lucky team of madmen > reviewing DT patches... ;-) > I could not have been more sarcastic in that last comment. It required a full liter of Haagen-Dazs to get through. -Christoffer _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
