From 16b913cdedfd3c236fe2dba2f99b743fd1ed3a53 Mon Sep 17 00:00:00 2001 From: kettenis Date: Wed, 15 Aug 2018 21:46:29 +0000 Subject: [PATCH] Turns out the integration of the GIC-500 on the Rockchip RK3399 is busted. It treats all access to the memory mapped registers as "secure" even if we're running in non-secure mode. As a result, during bringup of OpenBSD on the RK3399, I got confused and tweaked the interrupt priorities in a way that is wrong (but worked on the RK3399. Fix those priorities to match what they should be according to the documentation (and works on other hardware that includes a GICv3) and add code that detects the broken RK3399 GIC and adjusts the priorities accordingly. Also remove (broken) code that tries to mess around with group 0 interrupts and fix setting bits in the GICD_CTLR register on the broken RK3399 GIC. --- sys/arch/arm64/dev/agintc.c | 85 +++++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 28 deletions(-) diff --git a/sys/arch/arm64/dev/agintc.c b/sys/arch/arm64/dev/agintc.c index 030ac25082d..6be7ca13077 100644 --- a/sys/arch/arm64/dev/agintc.c +++ b/sys/arch/arm64/dev/agintc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: agintc.c,v 1.12 2018/08/15 20:27:56 kettenis Exp $ */ +/* $OpenBSD: agintc.c,v 1.13 2018/08/15 21:46:29 kettenis Exp $ */ /* * Copyright (c) 2007, 2009, 2011, 2017 Dale Rahn * Copyright (c) 2018 Mark Kettenis @@ -80,6 +80,7 @@ #define GICD_ICACTIVER(i) (0x0380 + (IRQ_TO_REG32(i) * 4)) #define GICD_IPRIORITYR(i) (0x0400 + (i)) #define GICD_ICFGR(i) (0x0c00 + (IRQ_TO_REG16(i) * 4)) +#define GICD_NSACR(i) (0x0e00 + (IRQ_TO_REG16(i) * 4)) #define GICD_IROUTER(i) (0x6000 + ((i) * 8)) /* redistributor registers */ @@ -114,7 +115,8 @@ #define GICR_ICFGR1 0x10c04 #define GICR_PROP_SIZE (64 * 1024) -#define GICR_PROP_ENABLE (1 << 0) +#define GICR_PROP_GROUP1 (1 << 1) +#define GICR_PROP_ENABLE (1 << 0) #define GICR_PEND_SIZE (64 * 1024) #define PPI_BASE 16 @@ -151,6 +153,7 @@ struct agintc_softc { int sc_cpuremap[MAXCPUS]; int sc_nintr; int sc_nlpi; + int sc_rk3399_quirk; struct evcount sc_spur; int sc_ncells; int sc_num_redist; @@ -269,10 +272,11 @@ agintc_attach(struct device *parent, struct device *self, void *aux) struct agintc_softc *sc = (struct agintc_softc *)self; struct fdt_attach_args *faa = aux; uint32_t typer; + uint32_t nsacr, oldnsacr; + uint32_t ctrl, bits; int i, j, nintr; int psw; int offset, nredist; - int grp1enable; #ifdef MULTIPROCESSOR int nipi, ipiirq[2]; #endif @@ -314,6 +318,31 @@ agintc_attach(struct device *parent, struct device *self, void *aux) sc->sc_nlpi = 8192; } + /* + * The Rockchip RK3399 is busted. Its GIC-500 treats all + * access to its memory mapped registers as "secure". As a + * result, several registers don't behave as expected. For + * example, the GICD_IPRIORITYRn and GICR_IPRIORITYRn + * registers expose the full priority range available to + * secure interrupts. We need to be aware of this and write + * an adjusted priority value into these registers. We also + * need to be careful not to touch any bits that shouldn't be + * writable in non-secure mode. + * + * We check whether we have secure mode access to these + * registers by attempting to write to the GICD_NSACR register + * and check whether its contents actually change. + */ + oldnsacr = bus_space_read_4(sc->sc_iot, sc->sc_d_ioh, GICD_NSACR(32)); + bus_space_write_4(sc->sc_iot, sc->sc_d_ioh, GICD_NSACR(32), + oldnsacr ^ 0xffffffff); + nsacr = bus_space_read_4(sc->sc_iot, sc->sc_d_ioh, GICD_NSACR(32)); + if (nsacr != oldnsacr) { + bus_space_write_4(sc->sc_iot, sc->sc_d_ioh, GICD_NSACR(32), + oldnsacr); + sc->sc_rk3399_quirk = 1; + } + evcount_attach(&sc->sc_spur, "irq1023/spur", NULL); __asm volatile("msr "STR(ICC_SRE_EL1)", %x0" : : "r" (ICC_SRE_EL1_EN)); @@ -416,13 +445,17 @@ agintc_attach(struct device *parent, struct device *self, void *aux) agintc_setipl, agintc_irq_handler); /* enable interrupts */ - bus_space_write_4(sc->sc_iot, sc->sc_d_ioh, GICD_CTLR, - GICD_CTRL_ARE_NS|GICD_CTRL_EnableGrp1A|GICD_CTRL_EnableGrp1); + ctrl = bus_space_read_4(sc->sc_iot, sc->sc_d_ioh, GICD_CTLR); + bits = GICD_CTRL_ARE_NS | GICD_CTRL_EnableGrp1A | GICD_CTRL_EnableGrp1; + if (sc->sc_rk3399_quirk) { + bits &= ~GICD_CTRL_EnableGrp1A; + bits <<= 1; + } + bus_space_write_4(sc->sc_iot, sc->sc_d_ioh, GICD_CTLR, ctrl | bits); - grp1enable = 1; __asm volatile("msr "STR(ICC_PMR)", %x0" :: "r"(0xff)); __asm volatile("msr "STR(ICC_BPR1)", %x0" :: "r"(0)); - __asm volatile("msr "STR(ICC_IGRPEN1)", %x0" :: "r"(grp1enable)); + __asm volatile("msr "STR(ICC_IGRPEN1)", %x0" :: "r"(1)); #ifdef MULTIPROCESSOR /* setup IPI interrupts */ @@ -587,14 +620,15 @@ agintc_set_priority(struct agintc_softc *sc, int irq, int pri) uint32_t prival; /* - * The interrupt priority registers expose the full range of - * priorities available in secure mode, and at least bit 3-7 - * must be implemented. For non-secure interrupts the top bit - * must be one. We only use 16 (13 really) interrupt - * priorities, so shift into bits 3-6. - * also low values are higher priority thus NIPL - pri + * The interrupt priority registers only expose the priorities + * available in non-secure mode, so the top bit is hidden. So + * here we shift into bits 4-7. + * Also low values are higher priority thus NIPL - pri */ - prival = 0x80 | ((NIPL - pri) << 3); + prival = ((NIPL - pri) << 4); + if (sc->sc_rk3399_quirk) + prival = 0x80 | (prival >> 1); + if (irq >= SPI_BASE) { bus_space_write_1(sc->sc_iot, sc->sc_d_ioh, GICD_IPRIORITYR(irq), prival); @@ -617,15 +651,17 @@ agintc_setipl(int new) ci->ci_cpl = new; /* - * The priority mask register only exposes the priorities - * available in non-secure mode, so the top bit is hidden. So - * here we shift into bits 4-7. - * low values are higher priority thus NIPL - pri + * The priority mask register exposes the full range of + * priorities available in secure mode, and at least bit 3-7 + * must be implemented. For non-secure interrupts the top bit + * must be one. We only use 16 (13 really) interrupt + * priorities, so shift into bits 3-6. + * Low values are higher priority thus NIPL - pri */ if (new == IPL_NONE) prival = 0xff; /* minimum priority */ else - prival = ((NIPL - new) << 4); + prival = 0x80 | ((NIPL - new) << 3); __asm volatile("msr "STR(ICC_PMR)", %x0" : : "r" (prival)); __isb(); @@ -639,7 +675,6 @@ agintc_intr_enable(struct agintc_softc *sc, int irq) struct cpu_info *ci = curcpu(); int hwcpu = sc->sc_cpuremap[ci->ci_cpuid]; int bit = 1 << IRQ_TO_REG32BIT(irq); - uint32_t enable; if (irq >= 32) { bus_space_write_4(sc->sc_iot, sc->sc_d_ioh, @@ -647,12 +682,6 @@ agintc_intr_enable(struct agintc_softc *sc, int irq) } else { bus_space_write_4(sc->sc_iot, sc->sc_r_ioh[hwcpu], GICR_ISENABLE0, bit); - /* enable group1 as well */ - bus_space_read_4(sc->sc_iot, sc->sc_r_ioh[hwcpu], - GICR_IGROUP0); - enable |= 1 << IRQ_TO_REG32BIT(irq); - bus_space_write_4(sc->sc_iot, sc->sc_r_ioh[hwcpu], - GICR_IGROUP0, enable); } } @@ -960,8 +989,8 @@ agintc_intr_establish(int irqno, int level, int (*func)(void *), } else { uint8_t *prop = AGINTC_DMA_KVA(sc->sc_prop); - prop[irqno - LPI_BASE] = - 0x80 | ((NIPL - ih->ih_ipl) << 3) | GICR_PROP_ENABLE; + prop[irqno - LPI_BASE] = ((NIPL - ih->ih_ipl) << 4) | + GICR_PROP_GROUP1 | GICR_PROP_ENABLE; /* Make globally visible. */ cpu_dcache_wb_range((vaddr_t)prop, 1); -- 2.20.1