Hi, I made some changes to TRY TO fix the ARM semihosting issue in SYS_HEAPINFO handling. This problem has been bothering me for quite a while.
A new global variable 'main_ram_base' is added while a new memory API, memory_region_add_subregion_main, is also provided to let SoC/board creator to initialize this variable. I am not sure if this is a good idea (to add a new API) or maybe we just let SoC/board creator to simply set 'main_ram_base' in their 'xxx_realize' functions? As for Cortex-M series, 'main_ram_base' is set during cpu initialization. A64 semihosting handling is also added and use zynqmp as an example. Any comments/reviews are big welcome! Thanks in advance! --- hw/arm/xlnx-zynqmp.c | 2 +- include/exec/cpu-common.h | 1 + include/exec/memory.h | 6 ++++++ memory.c | 8 ++++++++ target-arm/arm-semi.c | 37 ++++++++++++++++++++++++++----------- target-arm/cpu.c | 1 + vl.c | 1 + 7 files changed, 44 insertions(+), 12 deletions(-) diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c index 23c719986715..8124f71992b4 100644 --- a/hw/arm/xlnx-zynqmp.c +++ b/hw/arm/xlnx-zynqmp.c @@ -206,7 +206,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) memory_region_init_alias(&s->ddr_ram_high, NULL, "ddr-ram-high", s->ddr_ram, ddr_low_size, ddr_high_size); - memory_region_add_subregion(get_system_memory(), + memory_region_add_subregion_main(get_system_memory(), XLNX_ZYNQMP_HIGH_RAM_START, &s->ddr_ram_high); } else { diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h index aaee99563464..c345e61ede16 100644 --- a/include/exec/cpu-common.h +++ b/include/exec/cpu-common.h @@ -49,6 +49,7 @@ typedef uintptr_t ram_addr_t; #endif extern ram_addr_t ram_size; +extern ram_addr_t main_ram_base; /* memory API */ diff --git a/include/exec/memory.h b/include/exec/memory.h index 4ab680052f27..d76b0a069c98 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -972,6 +972,8 @@ void memory_region_del_eventfd(MemoryRegion *mr, * may only be added once as a subregion (unless removed with * memory_region_del_subregion()); use memory_region_init_alias() if you * want a region to be a subregion in multiple locations. + * The _main version is used to define the main working ram area, such ddr + * ram region. * * @mr: the region to contain the new subregion; must be a container * initialized with memory_region_init(). @@ -981,6 +983,10 @@ void memory_region_del_eventfd(MemoryRegion *mr, void memory_region_add_subregion(MemoryRegion *mr, hwaddr offset, MemoryRegion *subregion); + +void memory_region_add_subregion_main(MemoryRegion *mr, + hwaddr offset, + MemoryRegion *subregion); /** * memory_region_add_subregion_overlap: Add a subregion to a container * with overlap. diff --git a/memory.c b/memory.c index 8ba496dc7b2a..3221838abefe 100644 --- a/memory.c +++ b/memory.c @@ -1911,6 +1911,14 @@ void memory_region_add_subregion(MemoryRegion *mr, memory_region_add_subregion_common(mr, offset, subregion); } +void memory_region_add_subregion_main(MemoryRegion *mr, + hwaddr offset, + MemoryRegion *subregion) +{ + main_ram_base = offset; + memory_region_add_subregion(mr, offset, subregion); +} + void memory_region_add_subregion_overlap(MemoryRegion *mr, hwaddr offset, MemoryRegion *subregion, diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c index 8be0645eb08b..d30469688b01 100644 --- a/target-arm/arm-semi.c +++ b/target-arm/arm-semi.c @@ -599,17 +599,32 @@ target_ulong do_arm_semihosting(CPUARMState *env) unlock_user(ptr, arg0, 16); #else limit = ram_size; - ptr = lock_user(VERIFY_WRITE, arg0, 16, 0); - if (!ptr) { - /* FIXME - should this error code be -TARGET_EFAULT ? */ - return (uint32_t)-1; - } - /* TODO: Make this use the limit of the loaded application. */ - ptr[0] = tswap32(limit / 2); - ptr[1] = tswap32(limit); - ptr[2] = tswap32(limit); /* Stack base */ - ptr[3] = tswap32(0); /* Stack limit. */ - unlock_user(ptr, arg0, 16); + if (is_a64(env)) { + uint64_t *ptrx; + ptrx = lock_user(VERIFY_WRITE, arg0, 32, 0); + if (!ptrx) { + /* FIXME - should this error code be -TARGET_EFAULT ? */ + return (uint32_t)-1; + } + /* TODO: Make this use the limit of the loaded application. */ + ptrx[0] = tswap64(main_ram_base + ram_size / 2); /* Heap base */ + ptrx[1] = tswap64(main_ram_base + ram_size); /* limit */ + ptrx[2] = tswap64(main_ram_base + ram_size); /* Stack base */ + ptrx[3] = tswap64(main_ram_base + ram_size / 2); /* limit */ + unlock_user(ptrx, arg0, 32); + } else { + ptr = lock_user(VERIFY_WRITE, arg0, 16, 0); + if (!ptr) { + /* FIXME - should this error code be -TARGET_EFAULT ? */ + return (uint32_t)-1; + } + /* TODO: Make this use the limit of the loaded application. */ + ptr[0] = tswap32(main_ram_base + limit / 2); + ptr[1] = tswap32(main_ram_base + limit); + ptr[2] = tswap32(main_ram_base + limit); /* Stack base */ + ptr[3] = tswap32(main_ram_base); /* Stack limit. */ + unlock_user(ptr, arg0, 16); + } #endif return 0; } diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 3fd0743cb391..fbc7d6914694 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -193,6 +193,7 @@ static void arm_cpu_reset(CPUState *s) initial_msp = ldl_phys(s->as, 0); initial_pc = ldl_phys(s->as, 4); } + main_ram_base = 0x20000000; env->regs[13] = initial_msp & 0xFFFFFFFC; env->regs[15] = initial_pc & ~1; diff --git a/vl.c b/vl.c index 0736d8430dc3..ff1eeb50329f 100644 --- a/vl.c +++ b/vl.c @@ -133,6 +133,7 @@ int request_opengl = -1; int display_opengl; const char* keyboard_layout = NULL; ram_addr_t ram_size; +ram_addr_t main_ram_base = 0x0; /* default ram base to 0 */ const char *mem_path = NULL; int mem_prealloc = 0; /* force preallocation of physical target memory */ bool enable_mlock = false; -- 2.7.4