From 8d761517dcba7874655cabe06ffaf2fc761abbc0 Mon Sep 17 00:00:00 2001 From: Boris Mittelberg Date: Thu, 10 Nov 2022 16:36:34 -0800 Subject: cortex-m mpu: illegal shift fix Shifting left by 32 is undefined behavior. An AND operation with mask of 0xffffffff is meaningless, so just avoid it. BUG=b:64477774 BRANCH=none TEST=none Signed-off-by: Boris Mittelberg Change-Id: Ibcb3359f453345caee01936c074a9c0ae5aff7dc Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4021135 Tested-by: Peter Marheine Reviewed-by: Peter Marheine Code-Coverage: Zoss --- core/cortex-m/include/mpu_private.h | 1 + core/cortex-m/mpu.c | 16 ++++++++++++++-- test/mpu.c | 23 +++++++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/core/cortex-m/include/mpu_private.h b/core/cortex-m/include/mpu_private.h index eca474e14d..eb9de4c395 100644 --- a/core/cortex-m/include/mpu_private.h +++ b/core/cortex-m/include/mpu_private.h @@ -21,5 +21,6 @@ int mpu_update_region(uint8_t region, uint32_t addr, uint8_t size_bit, int mpu_config_region(uint8_t region, uint32_t addr, uint32_t size, uint16_t attr, uint8_t enable); struct mpu_rw_regions mpu_get_rw_regions(void); +uint32_t align_down_to_bits(uint32_t addr, uint8_t addr_bits); #endif /* __CROS_EC_MPU_PRIVATE_H */ diff --git a/core/cortex-m/mpu.c b/core/cortex-m/mpu.c index c0793180dc..db03936dfa 100644 --- a/core/cortex-m/mpu.c +++ b/core/cortex-m/mpu.c @@ -94,6 +94,17 @@ int mpu_update_region(uint8_t region, uint32_t addr, uint8_t size_bit, return EC_SUCCESS; } +/* + * Align address to a maximum of 31 bits + */ +uint32_t align_down_to_bits(uint32_t addr, uint8_t addr_bits) +{ + if (addr_bits < 32) + return addr & ~((1u << addr_bits) - 1); + else + return addr; +} + /* * Greedily configure the largest possible part of the given region from the * base address. @@ -140,7 +151,7 @@ static int mpu_config_region_greedy(uint8_t region, uint32_t addr, * disabling if it is not completely contained in the requested * range. */ - subregion_base = addr & ~((1 << natural_alignment) - 1); + subregion_base = align_down_to_bits(addr, natural_alignment); subregion_size = 1 << (natural_alignment - 3); *consumed = 0; for (sr_idx = 0; sr_idx < 8; sr_idx++) { @@ -159,7 +170,8 @@ static int mpu_config_region_greedy(uint8_t region, uint32_t addr, *consumed = 1 << natural_alignment; } - return mpu_update_region(region, addr & ~((1 << natural_alignment) - 1), + return mpu_update_region(region, + align_down_to_bits(addr, natural_alignment), natural_alignment, attr, enable, subregion_disable); } diff --git a/test/mpu.c b/test/mpu.c index 2193c0c617..957111b901 100644 --- a/test/mpu.c +++ b/test/mpu.c @@ -3,6 +3,14 @@ * found in the LICENSE file. */ +/* This test file meant to be executed on a real device. Example: + * 1. make tests BOARD=bloonchipper + * 2. servod --board=bloonchipper + * 3. flash_ec --board bloonchipper --image build/bloonchipper/test-mpu.bin + * 4. Open console via dut-control raw_fpmcu_console_uart_pty + * 5. runtest on console + */ + #include #include "mpu.h" #include "mpu_private.h" @@ -172,6 +180,19 @@ test_static int test_mpu_get_rw_regions(void) return EC_SUCCESS; } +test_static int test_align_down_to_bits(void) +{ + uint32_t addr = 0x87654321; + + TEST_EQ(align_down_to_bits(addr, 0), addr, "%d"); + TEST_EQ(align_down_to_bits(addr, 1), 0x87654320, "%d"); + TEST_EQ(align_down_to_bits(addr, 30), 0x80000000, "%d"); + TEST_EQ(align_down_to_bits(addr, 31), 0x80000000, "%d"); + TEST_EQ(align_down_to_bits(addr, 32), addr, "%d"); + TEST_EQ(align_down_to_bits(addr, 33), addr, "%d"); + return EC_SUCCESS; +} + void run_test(int argc, const char **argv) { enum ec_image cur_image = system_get_image_copy(); @@ -211,6 +232,8 @@ void run_test(int argc, const char **argv) RUN_TEST(reset_mpu); RUN_TEST(test_mpu_get_rw_regions); RUN_TEST(reset_mpu); + RUN_TEST(test_align_down_to_bits); + RUN_TEST(reset_mpu); /* This test must be last because it generates a panic */ RUN_TEST(test_mpu_update_region_valid_region); RUN_TEST(reset_mpu); -- cgit v1.2.1