From 60cd8030bfe7da4cd5edd04df25ad359aa22c7c0 Mon Sep 17 00:00:00 2001 From: Heyi Guo Date: Tue, 19 May 2020 11:50:40 +0800 Subject: [PATCH 1/6] drivers/gicv3: use mpidr to probe GICR for current CPU In function gicv3_rdistif_probe(), line #1322 implies gicv3_driver_data->mpidr_to_core_pos() may be null, but the original code uses this interface to get current CPU index unconditionally. It is better to use MPIDR to probe GICR which does not depend on gicv3_driver_data->mpidr_to_core_pos(). Signed-off-by: Heyi Guo Change-Id: I64add055385040fe0a56b977e2299608e2309a6e --- drivers/arm/gic/v3/gicv3_main.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/arm/gic/v3/gicv3_main.c b/drivers/arm/gic/v3/gicv3_main.c index 22efd458d..fb62f4894 100644 --- a/drivers/arm/gic/v3/gicv3_main.c +++ b/drivers/arm/gic/v3/gicv3_main.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015-2020, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2015-2021, ARM Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -1299,8 +1299,8 @@ unsigned int gicv3_set_pmr(unsigned int mask) ******************************************************************************/ int gicv3_rdistif_probe(const uintptr_t gicr_frame) { - u_register_t mpidr; - unsigned int proc_num, proc_self; + u_register_t mpidr, mpidr_self; + unsigned int proc_num; uint64_t typer_val; uintptr_t rdistif_base; bool gicr_frame_found = false; @@ -1314,18 +1314,18 @@ int gicv3_rdistif_probe(const uintptr_t gicr_frame) assert((read_sctlr_el3() & SCTLR_C_BIT) != 0U); #endif /* !__aarch64__ */ - proc_self = gicv3_driver_data->mpidr_to_core_pos(read_mpidr_el1()); + mpidr_self = read_mpidr_el1() & MPIDR_AFFINITY_MASK; rdistif_base = gicr_frame; do { typer_val = gicr_read_typer(rdistif_base); + mpidr = mpidr_from_gicr_typer(typer_val); if (gicv3_driver_data->mpidr_to_core_pos != NULL) { - mpidr = mpidr_from_gicr_typer(typer_val); proc_num = gicv3_driver_data->mpidr_to_core_pos(mpidr); } else { proc_num = (unsigned int)(typer_val >> TYPER_PROC_NUM_SHIFT) & TYPER_PROC_NUM_MASK; } - if (proc_num == proc_self) { + if (mpidr == mpidr_self) { /* The base address doesn't need to be initialized on * every warm boot. */ From deb18901d127bf4252e87a4b597ef7bd2b1e58b1 Mon Sep 17 00:00:00 2001 From: Heyi Guo Date: Thu, 14 Jan 2021 22:16:18 +0800 Subject: [PATCH 2/6] drivers/gicv3: fix potential GICD context override with ESPI enabled RESTORE/SAVE_GICD_EREGS uses (int_id - (MIN_ESPI_ID - MIN_SPI_ID)) to get the context array index for ESPI, which will override the space of standard SPI starting from (MIN_SPI_ID + MIN_SPI_ID). However, using TOTAL_SPI_INTR_NUM to replace the above MIN_SPI_ID cannot totally fix the issue, for TOTAL_SPI_INTR_NUM is not well aligned and the array index will be rounded down by the shifting operation if being shifted more than 2 bits. It will cause buffer override again when the existing maximum SPI reaches 1019. So round up TOTAL_SPI_INTR_NUM with (1 << REG##R_SHIFT) for GICD context arrays. Signed-off-by: Heyi Guo Change-Id: I5be2837c42f381a62f8d46a4ecd778009b1fe059 --- drivers/arm/gic/v3/gicv3_main.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/arm/gic/v3/gicv3_main.c b/drivers/arm/gic/v3/gicv3_main.c index fb62f4894..65145d597 100644 --- a/drivers/arm/gic/v3/gicv3_main.c +++ b/drivers/arm/gic/v3/gicv3_main.c @@ -70,7 +70,8 @@ static bool is_sgi_ppi(unsigned int id); for (unsigned int int_id = MIN_ESPI_ID; int_id < (intr_num);\ int_id += (1U << REG##R_SHIFT)) { \ gicd_write_##reg((base), int_id, \ - (ctx)->gicd_##reg[(int_id - (MIN_ESPI_ID - MIN_SPI_ID))\ + (ctx)->gicd_##reg[(int_id - (MIN_ESPI_ID - \ + round_up(TOTAL_SPI_INTR_NUM, 1U << REG##R_SHIFT)))\ >> REG##R_SHIFT]); \ } \ } while (false) @@ -79,7 +80,8 @@ static bool is_sgi_ppi(unsigned int id); do { \ for (unsigned int int_id = MIN_ESPI_ID; int_id < (intr_num);\ int_id += (1U << REG##R_SHIFT)) { \ - (ctx)->gicd_##reg[(int_id - (MIN_ESPI_ID - MIN_SPI_ID))\ + (ctx)->gicd_##reg[(int_id - (MIN_ESPI_ID - \ + round_up(TOTAL_SPI_INTR_NUM, 1U << REG##R_SHIFT)))\ >> REG##R_SHIFT] = gicd_read_##reg((base), int_id);\ } \ } while (false) From 69ae4427f04681ff86e7e09b43815e0f94ad708d Mon Sep 17 00:00:00 2001 From: Heyi Guo Date: Tue, 19 May 2020 14:01:49 +0800 Subject: [PATCH 3/6] drivers/gicv3: fix logical issue for num_eints In function gicv3_spis_config_defaults(), the variable num_ints is set to (maximum SPI INTID + 1), while num_eints is set to (maximum ESPI INTID). It introduces not only inconsistency to the code, but also logical bug in the "for" loops, for the INTID of num_eints is also valid and the check should be inclusive. Fix this by setting num_eints to (maximum ESPI INTID + 1) as well. Fix similar issues in gicv3_distif_save() and gicv3_distif_init_restore(). Signed-off-by: Heyi Guo Change-Id: I4425777d17e84e85f38853603340bd348640154f --- drivers/arm/gic/v3/gicv3_helpers.c | 4 ++-- drivers/arm/gic/v3/gicv3_main.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/arm/gic/v3/gicv3_helpers.c b/drivers/arm/gic/v3/gicv3_helpers.c index ff346f9df..d2cce9607 100644 --- a/drivers/arm/gic/v3/gicv3_helpers.c +++ b/drivers/arm/gic/v3/gicv3_helpers.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015-2020, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2015-2021, ARM Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -117,7 +117,7 @@ void gicv3_spis_config_defaults(uintptr_t gicd_base) * Maximum ESPI INTID is 32 * (GICD_TYPER.ESPI_range + 1) + 4095 */ num_eints = ((((typer_reg >> TYPER_ESPI_RANGE_SHIFT) & - TYPER_ESPI_RANGE_MASK) + 1U) << 5) + MIN_ESPI_ID - 1; + TYPER_ESPI_RANGE_MASK) + 1U) << 5) + MIN_ESPI_ID; for (i = MIN_ESPI_ID; i < num_eints; i += (1U << IGROUPR_SHIFT)) { diff --git a/drivers/arm/gic/v3/gicv3_main.c b/drivers/arm/gic/v3/gicv3_main.c index 65145d597..5a49b4f5e 100644 --- a/drivers/arm/gic/v3/gicv3_main.c +++ b/drivers/arm/gic/v3/gicv3_main.c @@ -757,7 +757,7 @@ void gicv3_distif_save(gicv3_dist_ctx_t * const dist_ctx) * Maximum ESPI INTID is 32 * (GICD_TYPER.ESPI_range + 1) + 4095 */ num_eints = ((((typer_reg >> TYPER_ESPI_RANGE_SHIFT) & - TYPER_ESPI_RANGE_MASK) + 1U) << 5) + MIN_ESPI_ID - 1; + TYPER_ESPI_RANGE_MASK) + 1U) << 5) + MIN_ESPI_ID; } else { num_eints = 0U; } @@ -881,7 +881,7 @@ void gicv3_distif_init_restore(const gicv3_dist_ctx_t * const dist_ctx) * Maximum ESPI INTID is 32 * (GICD_TYPER.ESPI_range + 1) + 4095 */ num_eints = ((((typer_reg >> TYPER_ESPI_RANGE_SHIFT) & - TYPER_ESPI_RANGE_MASK) + 1U) << 5) + MIN_ESPI_ID - 1; + TYPER_ESPI_RANGE_MASK) + 1U) << 5) + MIN_ESPI_ID; } else { num_eints = 0U; } From 4e42c227bf8a8f5eaf441d56c2b4f8ab8458ee59 Mon Sep 17 00:00:00 2001 From: Heyi Guo Date: Tue, 19 May 2020 15:41:14 +0800 Subject: [PATCH 4/6] drivers/gicv3: limit SPI ID to avoid misjudgement in GICD_OFFSET() The GICv3 architecture allows GICD_TYPER.ITLinesNumber to be 31, so the maximum possible value for num_ints is 1024. The value must be limited to (MAX_SPI_ID + 1), or GICD_OFFSET() will consider it as ESPI INTID and return wrong register address. Signed-off-by: Heyi Guo Change-Id: Iddcb83d3e5d241b39f4176c19c2bceaa2c3dd653 --- drivers/arm/gic/v3/gicv3_helpers.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/arm/gic/v3/gicv3_helpers.c b/drivers/arm/gic/v3/gicv3_helpers.c index d2cce9607..8add42af9 100644 --- a/drivers/arm/gic/v3/gicv3_helpers.c +++ b/drivers/arm/gic/v3/gicv3_helpers.c @@ -105,6 +105,15 @@ void gicv3_spis_config_defaults(uintptr_t gicd_base) /* Maximum SPI INTID is 32 * (GICD_TYPER.ITLinesNumber + 1) - 1 */ num_ints = ((typer_reg & TYPER_IT_LINES_NO_MASK) + 1U) << 5; + /* + * The GICv3 architecture allows GICD_TYPER.ITLinesNumber to be 31, so + * the maximum possible value for num_ints is 1024. Limit the value to + * MAX_SPI_ID + 1 to avoid getting wrong address in GICD_OFFSET() macro. + */ + if (num_ints > MAX_SPI_ID + 1U) { + num_ints = MAX_SPI_ID + 1U; + } + /* Treat all (E)SPIs as G1NS by default. We do 32 at a time. */ for (i = MIN_SPI_ID; i < num_ints; i += (1U << IGROUPR_SHIFT)) { gicd_write_igroupr(gicd_base, i, ~0U); From 705032deea7ac031c6d14d124c0aa3d837c73a92 Mon Sep 17 00:00:00 2001 From: Heyi Guo Date: Thu, 21 Jan 2021 10:34:00 +0800 Subject: [PATCH 5/6] drivers/gicv3: add debug log for maximum INTID of SPI and eSPI Add debug log for the maximum supported INTID of SPI and eSPI on the current GIC implementation. Signed-off-by: Heyi Guo Change-Id: Ie45ab1d85b39658c4ca4bc54ee433ac44e41d03f --- drivers/arm/gic/v3/gicv3_helpers.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/arm/gic/v3/gicv3_helpers.c b/drivers/arm/gic/v3/gicv3_helpers.c index 8add42af9..6bb66a02b 100644 --- a/drivers/arm/gic/v3/gicv3_helpers.c +++ b/drivers/arm/gic/v3/gicv3_helpers.c @@ -113,6 +113,7 @@ void gicv3_spis_config_defaults(uintptr_t gicd_base) if (num_ints > MAX_SPI_ID + 1U) { num_ints = MAX_SPI_ID + 1U; } + INFO("Maximum SPI INTID supported: %u\n", num_ints - 1); /* Treat all (E)SPIs as G1NS by default. We do 32 at a time. */ for (i = MIN_SPI_ID; i < num_ints; i += (1U << IGROUPR_SHIFT)) { @@ -127,6 +128,7 @@ void gicv3_spis_config_defaults(uintptr_t gicd_base) */ num_eints = ((((typer_reg >> TYPER_ESPI_RANGE_SHIFT) & TYPER_ESPI_RANGE_MASK) + 1U) << 5) + MIN_ESPI_ID; + INFO("Maximum ESPI INTID supported: %u\n", num_eints - 1); for (i = MIN_ESPI_ID; i < num_eints; i += (1U << IGROUPR_SHIFT)) { @@ -134,6 +136,7 @@ void gicv3_spis_config_defaults(uintptr_t gicd_base) } } else { num_eints = 0U; + INFO("ESPI range is not implemented.\n"); } #endif From 612b4a3f2d2929b6b8fd9dd211c8488050e10183 Mon Sep 17 00:00:00 2001 From: Heyi Guo Date: Tue, 19 May 2020 16:45:17 +0800 Subject: [PATCH 6/6] drivers/gicv3: also shift eSPI register offset in GICD_OFFSET_64() ESPI register offset should also be shifted right by REG##R_SHIFT to keep consistent. It is not a functional issue, for GICD_OFFSET_64() is only used for GICD_IROUTER, and IROUTER_SHIFT is 0. Signed-off-by: Heyi Guo Change-Id: I76eee5c50e4300890e78e80bddde135ce88daa2d --- drivers/arm/gic/v3/gicv3_private.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/arm/gic/v3/gicv3_private.h b/drivers/arm/gic/v3/gicv3_private.h index c5d027da2..416cdd018 100644 --- a/drivers/arm/gic/v3/gicv3_private.h +++ b/drivers/arm/gic/v3/gicv3_private.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015-2020, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2015-2021, ARM Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -48,7 +48,8 @@ #define GICD_OFFSET_64(REG, id) \ (((id) <= MAX_SPI_ID) ? \ GICD_##REG##R + (((uintptr_t)(id) >> REG##R_SHIFT) << 3) : \ - GICD_##REG##RE + (((uintptr_t)(id) - MIN_ESPI_ID) << 3)) + GICD_##REG##RE + ((((uintptr_t)(id) - MIN_ESPI_ID) >> \ + REG##R_SHIFT) << 3)) #else /* GICv3 */ #define GICD_OFFSET_8(REG, id) \