From cf6eeb8a60bfb8c6b2e49c3839eed173d814f2ba Mon Sep 17 00:00:00 2001 From: Jon Medhurst Date: Tue, 11 Feb 2014 14:48:56 +0000 Subject: [PATCH] Fix implementation and users of gicd_set_ipriorityr() Make gicd_set_ipriorityr() actually write to the priority register. Also correct callers of this function which want the highest priority to use the value zero as this is the highest priority value according to the ARM Generic Interrupt Controller Architecture Specification. To make this easier to get right, we introduce defines for the lowest and highest priorities for secure and non-secure interrupts. Fixes ARM-software/tf-issues#21 Signed-off-by: Jon Medhurst --- arch/system/gic/gic.h | 6 +++++- arch/system/gic/gic_v2.c | 25 ++++++++++++++++--------- plat/fvp/plat_gic.c | 24 ++++++++++++------------ 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/arch/system/gic/gic.h b/arch/system/gic/gic.h index 9e9fb73c..32660437 100644 --- a/arch/system/gic/gic.h +++ b/arch/system/gic/gic.h @@ -37,7 +37,11 @@ #define GRP0 0 #define GRP1 1 -#define MAX_PRI_VAL 0xff +#define GIC_PRI_MASK 0xff +#define GIC_HIGHEST_SEC_PRIORITY 0 +#define GIC_LOWEST_SEC_PRIORITY 127 +#define GIC_HIGHEST_NS_PRIORITY 128 +#define GIC_LOWEST_NS_PRIORITY 254 /* 255 would disable an interrupt */ #define ENABLE_GRP0 (1 << 0) #define ENABLE_GRP1 (1 << 1) diff --git a/arch/system/gic/gic_v2.c b/arch/system/gic/gic_v2.c index 220ca194..6af03782 100644 --- a/arch/system/gic/gic_v2.c +++ b/arch/system/gic/gic_v2.c @@ -28,6 +28,7 @@ * POSSIBILITY OF SUCH DAMAGE. */ +#include #include #include @@ -264,20 +265,26 @@ void gicd_set_icactiver(unsigned int base, unsigned int id) */ void gicd_set_ipriorityr(unsigned int base, unsigned int id, unsigned int pri) { - unsigned byte_off = id & ((1 << ICACTIVER_SHIFT) - 1); - unsigned int reg_val = gicd_read_icactiver(base, id); + unsigned int reg = base + GICD_IPRIORITYR + (id & ~3); + unsigned int shift = (id & 3) << 3; + unsigned int reg_val = mmio_read_32(reg); /* * Enforce ARM recommendation to manage priority values such * that group1 interrupts always have a lower priority than - * group0 interrupts + * group0 interrupts. + * Note, lower numerical values are higher priorities so the comparison + * checks below are reversed from what might be expected. */ - if (gicd_get_igroupr(base, id) == GRP1) - pri |= 1 << 7; - else - pri &= ~(1 << 7); - - gicd_write_icactiver(base, id, reg_val & ~(pri << (byte_off << 3))); + assert(gicd_get_igroupr(base, id) == GRP1 ? + pri >= GIC_HIGHEST_NS_PRIORITY && + pri <= GIC_LOWEST_NS_PRIORITY : + pri >= GIC_HIGHEST_SEC_PRIORITY && + pri <= GIC_LOWEST_SEC_PRIORITY); + + reg_val &= ~(GIC_PRI_MASK << shift); + reg_val |= (pri & GIC_PRI_MASK) << shift; + mmio_write_32(reg, reg_val); } void gicd_set_itargetsr(unsigned int base, unsigned int id, unsigned int iface) diff --git a/plat/fvp/plat_gic.c b/plat/fvp/plat_gic.c index 6dd13eca..8929172f 100644 --- a/plat/fvp/plat_gic.c +++ b/plat/fvp/plat_gic.c @@ -101,7 +101,7 @@ void gicv3_cpuif_setup(void) val = read_icc_sre_el2(); write_icc_sre_el2(val | ICC_SRE_EN | ICC_SRE_SRE); - write_icc_pmr_el1(MAX_PRI_VAL); + write_icc_pmr_el1(GIC_PRI_MASK); /* Restore SCR_EL3 */ write_scr(scr_val); @@ -166,7 +166,7 @@ void gic_cpuif_setup(unsigned int gicc_base) val = ENABLE_GRP0 | FIQ_EN | FIQ_BYP_DIS_GRP0; val |= IRQ_BYP_DIS_GRP0 | FIQ_BYP_DIS_GRP1 | IRQ_BYP_DIS_GRP1; - gicc_write_pmr(gicc_base, MAX_PRI_VAL); + gicc_write_pmr(gicc_base, GIC_PRI_MASK); gicc_write_ctlr(gicc_base, val); } @@ -214,15 +214,15 @@ void gic_pcpu_distif_setup(unsigned int gicd_base) gicd_clr_igroupr(gicd_base, IRQ_SEC_SGI_6); gicd_clr_igroupr(gicd_base, IRQ_SEC_SGI_7); - gicd_set_ipriorityr(gicd_base, IRQ_SEC_PHY_TIMER, MAX_PRI_VAL); - gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_0, MAX_PRI_VAL); - gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_1, MAX_PRI_VAL); - gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_2, MAX_PRI_VAL); - gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_3, MAX_PRI_VAL); - gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_4, MAX_PRI_VAL); - gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_5, MAX_PRI_VAL); - gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_6, MAX_PRI_VAL); - gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_7, MAX_PRI_VAL); + gicd_set_ipriorityr(gicd_base, IRQ_SEC_PHY_TIMER, GIC_HIGHEST_SEC_PRIORITY); + gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_0, GIC_HIGHEST_SEC_PRIORITY); + gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_1, GIC_HIGHEST_SEC_PRIORITY); + gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_2, GIC_HIGHEST_SEC_PRIORITY); + gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_3, GIC_HIGHEST_SEC_PRIORITY); + gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_4, GIC_HIGHEST_SEC_PRIORITY); + gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_5, GIC_HIGHEST_SEC_PRIORITY); + gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_6, GIC_HIGHEST_SEC_PRIORITY); + gicd_set_ipriorityr(gicd_base, IRQ_SEC_SGI_7, GIC_HIGHEST_SEC_PRIORITY); gicd_set_isenabler(gicd_base, IRQ_SEC_PHY_TIMER); gicd_set_isenabler(gicd_base, IRQ_SEC_SGI_0); @@ -261,7 +261,7 @@ void gic_distif_setup(unsigned int gicd_base) /* Configure secure interrupts now */ gicd_clr_igroupr(gicd_base, IRQ_TZ_WDOG); - gicd_set_ipriorityr(gicd_base, IRQ_TZ_WDOG, MAX_PRI_VAL); + gicd_set_ipriorityr(gicd_base, IRQ_TZ_WDOG, GIC_HIGHEST_SEC_PRIORITY); gicd_set_itargetsr(gicd_base, IRQ_TZ_WDOG, platform_get_core_pos(read_mpidr())); gicd_set_isenabler(gicd_base, IRQ_TZ_WDOG); -- 2.30.2