From a91e12fbeac527df82b39d763a68f00d1d890cdc Mon Sep 17 00:00:00 2001 From: Soby Mathew Date: Fri, 15 Jan 2016 14:05:37 +0000 Subject: [PATCH] Fix race in GIC IPRIORITY and ITARGET accessors GICD_IPRIORITYR and GICD_ITARGETSR specifically support byte addressing so that individual interrupt priorities can be atomically updated by issuing a single byte write. The previous implementation of gicd_set_ipriority() and gicd_set_itargetsr() used 32-bit register accesses, modifying values for 4 interrupts at a time, using a read-modify-write approach. This potentially may cause concurrent changes by other CPUs to the adjacent interrupts to be corrupted. This patch fixes the issue by modifying these accessors to use byte addressing. Fixes ARM-software/tf-issues#343 Change-Id: Iec28b5f5074045b00dfb8d5f5339b685f9425915 --- drivers/arm/gic/gic_v2.c | 15 +++------------ drivers/arm/gic/v2/gicv2_helpers.c | 7 ++----- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/drivers/arm/gic/gic_v2.c b/drivers/arm/gic/gic_v2.c index dc5dc08cb..05399c3a7 100644 --- a/drivers/arm/gic/gic_v2.c +++ b/drivers/arm/gic/gic_v2.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2014, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2013-2016, ARM Limited and Contributors. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: @@ -261,10 +261,6 @@ void gicd_set_icactiver(uintptr_t base, unsigned int id) */ void gicd_set_ipriorityr(uintptr_t base, unsigned int id, unsigned int pri) { - 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 @@ -278,17 +274,12 @@ void gicd_set_ipriorityr(uintptr_t base, unsigned int id, unsigned int pri) 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); + mmio_write_8(base + GICD_IPRIORITYR + id, pri & GIC_PRI_MASK); } void gicd_set_itargetsr(uintptr_t base, unsigned int id, unsigned int target) { - unsigned byte_off = id & ((1 << ITARGETSR_SHIFT) - 1); - unsigned int reg_val = gicd_read_itargetsr(base, id); - - gicd_write_itargetsr(base, id, reg_val | (target << (byte_off << 3))); + mmio_write_8(base + GICD_ITARGETSR + id, target & GIC_TARGET_CPU_MASK); } /******************************************************************************* diff --git a/drivers/arm/gic/v2/gicv2_helpers.c b/drivers/arm/gic/v2/gicv2_helpers.c index 1f904c516..8c4e2f0f1 100644 --- a/drivers/arm/gic/v2/gicv2_helpers.c +++ b/drivers/arm/gic/v2/gicv2_helpers.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: @@ -101,10 +101,7 @@ void gicd_write_spendsgir(uintptr_t base, unsigned int id, unsigned int val) */ void gicd_set_itargetsr(uintptr_t base, unsigned int id, unsigned int target) { - unsigned byte_off = id & ((1 << ITARGETSR_SHIFT) - 1); - unsigned int reg_val = gicd_read_itargetsr(base, id); - - gicd_write_itargetsr(base, id, reg_val | (target << (byte_off << 3))); + mmio_write_8(base + GICD_ITARGETSR + id, target & GIC_TARGET_CPU_MASK); } /*******************************************************************************