From 593b377073194cf1698965e5e0582684d295ea6f Mon Sep 17 00:00:00 2001 From: Mark Zhuang Date: Fri, 11 Oct 2024 16:57:48 +0800 Subject: [PATCH 1/3] target/riscv: fix read/write virtual memory across page boundaries When read/write virtual addresses cross page boundaries, the physical addresses are not necessarily contiguous and need to call virt2phys again. --- src/target/riscv/riscv.c | 77 +++++++++++++++++++++++++++++++++------- src/target/riscv/riscv.h | 3 ++ 2 files changed, 68 insertions(+), 12 deletions(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 59717fd14..c727a9d7c 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -3061,6 +3061,27 @@ static int riscv_virt2phys(struct target *target, target_addr_t virtual, target_ virtual, physical); } +static int check_access_request(struct target *target, target_addr_t address, + uint32_t size, uint32_t count, bool is_write) +{ + const bool is_misaligned = address % size != 0; + // TODO: This assumes that size of each page is 4 KiB, which is not necessarily the case. + const bool crosses_page_boundary = RISCV_PGBASE(address + size * count - 1) != RISCV_PGBASE(address); + if (is_misaligned && crosses_page_boundary) { + int mmu_enabled; + int result = riscv_mmu(target, &mmu_enabled); + if (result != ERROR_OK) + return result; + if (mmu_enabled) { + LOG_TARGET_ERROR(target, "Mis-aligned memory %s (address=0x%" TARGET_PRIxADDR ", size=%d, count=%d)" + " would access an element across page boundary. This is not supported.", + is_write ? "write" : "read", address, size, count); + return ERROR_FAIL; + } + } + return ERROR_OK; +} + static int riscv_read_phys_memory(struct target *target, target_addr_t phys_address, uint32_t size, uint32_t count, uint8_t *buffer) { @@ -3076,15 +3097,32 @@ static int riscv_read_memory(struct target *target, target_addr_t address, return ERROR_OK; } - target_addr_t physical_addr; - int result = target->type->virt2phys(target, address, &physical_addr); - if (result != ERROR_OK) { - LOG_TARGET_ERROR(target, "Address translation failed."); + int result = check_access_request(target, address, size, count, false); + if (result != ERROR_OK) return result; - } RISCV_INFO(r); - return r->read_memory(target, physical_addr, size, count, buffer, size); + uint32_t current_count = 0; + while (current_count < count) { + target_addr_t physical_addr; + result = target->type->virt2phys(target, address, &physical_addr); + if (result != ERROR_OK) { + LOG_TARGET_ERROR(target, "Address translation failed."); + return result; + } + + /* TODO: For simplicity, this algorithm assumes the worst case - the smallest possible page size, + * which is 4 KiB. The algorithm can be improved to detect the real page size, and allow to use larger + * memory transfers and avoid extra unnecessary virt2phys address translations. */ + uint32_t chunk_count = MIN(count - current_count, (RISCV_PGSIZE - RISCV_PGOFFSET(address)) / size); + result = r->read_memory(target, physical_addr, size, chunk_count, buffer + current_count * size, size); + if (result != ERROR_OK) + return result; + + current_count += chunk_count; + address += chunk_count * size; + } + return ERROR_OK; } static int riscv_write_phys_memory(struct target *target, target_addr_t phys_address, @@ -3104,17 +3142,32 @@ static int riscv_write_memory(struct target *target, target_addr_t address, return ERROR_OK; } - target_addr_t physical_addr; - int result = target->type->virt2phys(target, address, &physical_addr); - if (result != ERROR_OK) { - LOG_TARGET_ERROR(target, "Address translation failed."); + int result = check_access_request(target, address, size, count, true); + if (result != ERROR_OK) return result; - } struct target_type *tt = get_target_type(target); if (!tt) return ERROR_FAIL; - return tt->write_memory(target, physical_addr, size, count, buffer); + + uint32_t current_count = 0; + while (current_count < count) { + target_addr_t physical_addr; + result = target->type->virt2phys(target, address, &physical_addr); + if (result != ERROR_OK) { + LOG_TARGET_ERROR(target, "Address translation failed."); + return result; + } + + uint32_t chunk_count = MIN(count - current_count, (RISCV_PGSIZE - RISCV_PGOFFSET(address)) / size); + result = tt->write_memory(target, physical_addr, size, chunk_count, buffer + current_count * size); + if (result != ERROR_OK) + return result; + + current_count += chunk_count; + address += chunk_count * size; + } + return ERROR_OK; } static const char *riscv_get_gdb_arch(const struct target *target) diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 4ac10fa76..9170304d2 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -29,6 +29,9 @@ struct riscv_program; #define RISCV_HGATP_MODE(xlen) ((xlen) == 32 ? HGATP32_MODE : HGATP64_MODE) #define RISCV_HGATP_PPN(xlen) ((xlen) == 32 ? HGATP32_PPN : HGATP64_PPN) #define RISCV_PGSHIFT 12 +#define RISCV_PGSIZE BIT(RISCV_PGSHIFT) +#define RISCV_PGBASE(addr) ((addr) & ~(RISCV_PGSIZE - 1)) +#define RISCV_PGOFFSET(addr) ((addr) & (RISCV_PGSIZE - 1)) #define PG_MAX_LEVEL 5 From 4a1bd80842388e7225bf295a2446a3eb1c31cf32 Mon Sep 17 00:00:00 2001 From: Mark Zhuang Date: Mon, 14 Oct 2024 00:18:11 +0800 Subject: [PATCH 2/3] target/riscv: fix cross-page misaligned access when mmu not enabled When mmu is disabled, simply call the physical read/write function --- src/target/riscv/riscv.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index c727a9d7c..873b267ae 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -3061,23 +3061,17 @@ static int riscv_virt2phys(struct target *target, target_addr_t virtual, target_ virtual, physical); } -static int check_access_request(struct target *target, target_addr_t address, +static int check_virt_memory_access(struct target *target, target_addr_t address, uint32_t size, uint32_t count, bool is_write) { const bool is_misaligned = address % size != 0; // TODO: This assumes that size of each page is 4 KiB, which is not necessarily the case. const bool crosses_page_boundary = RISCV_PGBASE(address + size * count - 1) != RISCV_PGBASE(address); if (is_misaligned && crosses_page_boundary) { - int mmu_enabled; - int result = riscv_mmu(target, &mmu_enabled); - if (result != ERROR_OK) - return result; - if (mmu_enabled) { - LOG_TARGET_ERROR(target, "Mis-aligned memory %s (address=0x%" TARGET_PRIxADDR ", size=%d, count=%d)" - " would access an element across page boundary. This is not supported.", - is_write ? "write" : "read", address, size, count); - return ERROR_FAIL; - } + LOG_TARGET_ERROR(target, "Mis-aligned memory %s (address=0x%" TARGET_PRIxADDR ", size=%d, count=%d)" + " would access an element across page boundary. This is not supported.", + is_write ? "write" : "read", address, size, count); + return ERROR_FAIL; } return ERROR_OK; } @@ -3097,11 +3091,20 @@ static int riscv_read_memory(struct target *target, target_addr_t address, return ERROR_OK; } - int result = check_access_request(target, address, size, count, false); + int mmu_enabled; + int result = riscv_mmu(target, &mmu_enabled); if (result != ERROR_OK) return result; RISCV_INFO(r); + + if (!mmu_enabled) + return r->read_memory(target, address, size, count, buffer, size); + + result = check_virt_memory_access(target, address, size, count, false); + if (result != ERROR_OK) + return result; + uint32_t current_count = 0; while (current_count < count) { target_addr_t physical_addr; @@ -3142,7 +3145,8 @@ static int riscv_write_memory(struct target *target, target_addr_t address, return ERROR_OK; } - int result = check_access_request(target, address, size, count, true); + int mmu_enabled; + int result = riscv_mmu(target, &mmu_enabled); if (result != ERROR_OK) return result; @@ -3150,6 +3154,13 @@ static int riscv_write_memory(struct target *target, target_addr_t address, if (!tt) return ERROR_FAIL; + if (!mmu_enabled) + return tt->write_memory(target, address, size, count, buffer); + + result = check_virt_memory_access(target, address, size, count, true); + if (result != ERROR_OK) + return result; + uint32_t current_count = 0; while (current_count < count) { target_addr_t physical_addr; From 92d7a5798dceee9687376e627905a86b7ca43dcb Mon Sep 17 00:00:00 2001 From: Mark Zhuang Date: Tue, 15 Oct 2024 20:13:02 +0800 Subject: [PATCH 3/3] [NFC] target/riscv: refactor riscv_read_memory,riscv_write_memory Reduce duplicate code --- src/target/riscv/riscv.c | 87 ++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 52 deletions(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 873b267ae..299df5333 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -3083,11 +3083,25 @@ static int riscv_read_phys_memory(struct target *target, target_addr_t phys_addr return r->read_memory(target, phys_address, size, count, buffer, size); } -static int riscv_read_memory(struct target *target, target_addr_t address, - uint32_t size, uint32_t count, uint8_t *buffer) +static int riscv_write_phys_memory(struct target *target, target_addr_t phys_address, + uint32_t size, uint32_t count, const uint8_t *buffer) { + struct target_type *tt = get_target_type(target); + if (!tt) + return ERROR_FAIL; + return tt->write_memory(target, phys_address, size, count, buffer); +} + +static int riscv_rw_memory(struct target *target, target_addr_t address, uint32_t size, + uint32_t count, uint8_t *read_buffer, const uint8_t *write_buffer) +{ + /* Exactly one of the buffers must be set, the other must be NULL */ + assert(!!read_buffer != !!write_buffer); + + const bool is_write = write_buffer ? true : false; if (count == 0) { - LOG_TARGET_WARNING(target, "0-length read from 0x%" TARGET_PRIxADDR, address); + LOG_TARGET_WARNING(target, "0-length %s 0x%" TARGET_PRIxADDR, + is_write ? "write to" : "read from", address); return ERROR_OK; } @@ -3097,11 +3111,18 @@ static int riscv_read_memory(struct target *target, target_addr_t address, return result; RISCV_INFO(r); + struct target_type *tt = get_target_type(target); + if (!tt) + return ERROR_FAIL; - if (!mmu_enabled) - return r->read_memory(target, address, size, count, buffer, size); + if (!mmu_enabled) { + if (is_write) + return tt->write_memory(target, address, size, count, write_buffer); + else + return r->read_memory(target, address, size, count, read_buffer, size); + } - result = check_virt_memory_access(target, address, size, count, false); + result = check_virt_memory_access(target, address, size, count, is_write); if (result != ERROR_OK) return result; @@ -3118,7 +3139,10 @@ static int riscv_read_memory(struct target *target, target_addr_t address, * which is 4 KiB. The algorithm can be improved to detect the real page size, and allow to use larger * memory transfers and avoid extra unnecessary virt2phys address translations. */ uint32_t chunk_count = MIN(count - current_count, (RISCV_PGSIZE - RISCV_PGOFFSET(address)) / size); - result = r->read_memory(target, physical_addr, size, chunk_count, buffer + current_count * size, size); + if (is_write) + result = tt->write_memory(target, physical_addr, size, chunk_count, write_buffer + current_count * size); + else + result = r->read_memory(target, physical_addr, size, chunk_count, read_buffer + current_count * size, size); if (result != ERROR_OK) return result; @@ -3128,57 +3152,16 @@ static int riscv_read_memory(struct target *target, target_addr_t address, return ERROR_OK; } -static int riscv_write_phys_memory(struct target *target, target_addr_t phys_address, - uint32_t size, uint32_t count, const uint8_t *buffer) +static int riscv_read_memory(struct target *target, target_addr_t address, + uint32_t size, uint32_t count, uint8_t *buffer) { - struct target_type *tt = get_target_type(target); - if (!tt) - return ERROR_FAIL; - return tt->write_memory(target, phys_address, size, count, buffer); + return riscv_rw_memory(target, address, size, count, buffer, NULL); } static int riscv_write_memory(struct target *target, target_addr_t address, uint32_t size, uint32_t count, const uint8_t *buffer) { - if (count == 0) { - LOG_TARGET_WARNING(target, "0-length write to 0x%" TARGET_PRIxADDR, address); - return ERROR_OK; - } - - int mmu_enabled; - int result = riscv_mmu(target, &mmu_enabled); - if (result != ERROR_OK) - return result; - - struct target_type *tt = get_target_type(target); - if (!tt) - return ERROR_FAIL; - - if (!mmu_enabled) - return tt->write_memory(target, address, size, count, buffer); - - result = check_virt_memory_access(target, address, size, count, true); - if (result != ERROR_OK) - return result; - - uint32_t current_count = 0; - while (current_count < count) { - target_addr_t physical_addr; - result = target->type->virt2phys(target, address, &physical_addr); - if (result != ERROR_OK) { - LOG_TARGET_ERROR(target, "Address translation failed."); - return result; - } - - uint32_t chunk_count = MIN(count - current_count, (RISCV_PGSIZE - RISCV_PGOFFSET(address)) / size); - result = tt->write_memory(target, physical_addr, size, chunk_count, buffer + current_count * size); - if (result != ERROR_OK) - return result; - - current_count += chunk_count; - address += chunk_count * size; - } - return ERROR_OK; + return riscv_rw_memory(target, address, size, count, NULL, buffer); } static const char *riscv_get_gdb_arch(const struct target *target)