From f80b551ddc8b2d3ffc358f6fc7351e4100ab3dff Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 10 Dec 2018 16:56:34 +0000 Subject: [PATCH 1/6] target/m68k: In dump_address_map() check for memory access failures In dump_address_map(), use address_space_ldl() instead of ldl_phys(). This allows us to check whether the memory access failed. Signed-off-by: Peter Maydell Message-Id: <20181210165636.28366-2-peter.maydell@linaro.org> Signed-off-by: Laurent Vivier --- target/m68k/helper.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/target/m68k/helper.c b/target/m68k/helper.c index d958a34959..5b81995ee7 100644 --- a/target/m68k/helper.c +++ b/target/m68k/helper.c @@ -403,6 +403,7 @@ static void dump_address_map(CPUM68KState *env, uint32_t root_pointer) int last_attr = -1, attr = -1; M68kCPU *cpu = m68k_env_get_cpu(env); CPUState *cs = CPU(cpu); + MemTxResult txres; if (env->mmu.tcr & M68K_TCR_PAGE_8K) { /* 8k page */ @@ -416,22 +417,29 @@ static void dump_address_map(CPUM68KState *env, uint32_t root_pointer) tib_mask = M68K_4K_PAGE_MASK; } for (i = 0; i < M68K_ROOT_POINTER_ENTRIES; i++) { - tia = ldl_phys(cs->as, M68K_POINTER_BASE(root_pointer) + i * 4); - if (!M68K_UDT_VALID(tia)) { + tia = address_space_ldl(cs->as, M68K_POINTER_BASE(root_pointer) + i * 4, + MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK || !M68K_UDT_VALID(tia)) { continue; } for (j = 0; j < M68K_ROOT_POINTER_ENTRIES; j++) { - tib = ldl_phys(cs->as, M68K_POINTER_BASE(tia) + j * 4); - if (!M68K_UDT_VALID(tib)) { + tib = address_space_ldl(cs->as, M68K_POINTER_BASE(tia) + j * 4, + MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK || !M68K_UDT_VALID(tib)) { continue; } for (k = 0; k < tic_size; k++) { - tic = ldl_phys(cs->as, (tib & tib_mask) + k * 4); - if (!M68K_PDT_VALID(tic)) { + tic = address_space_ldl(cs->as, (tib & tib_mask) + k * 4, + MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK || !M68K_PDT_VALID(tic)) { continue; } if (M68K_PDT_INDIRECT(tic)) { - tic = ldl_phys(cs->as, M68K_INDIRECT_POINTER(tic)); + tic = address_space_ldl(cs->as, M68K_INDIRECT_POINTER(tic), + MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK) { + continue; + } } last_logical = logical; From adcf0bf017351776510121e47b9226095836023c Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 10 Dec 2018 16:56:35 +0000 Subject: [PATCH 2/6] target/m68k: In get_physical_address() check for memory access failures In get_physical_address(), use address_space_ldl() and address_space_stl() instead of ldl_phys() and stl_phys(). This allows us to check whether the memory access failed. For the moment, we simply return -1 in this case; add a TODO comment that we should ideally generate the appropriate kind of fault. Signed-off-by: Peter Maydell Message-Id: <20181210165636.28366-3-peter.maydell@linaro.org> Signed-off-by: Laurent Vivier --- target/m68k/helper.c | 62 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 10 deletions(-) diff --git a/target/m68k/helper.c b/target/m68k/helper.c index 5b81995ee7..edd7bb64ed 100644 --- a/target/m68k/helper.c +++ b/target/m68k/helper.c @@ -651,6 +651,7 @@ static int get_physical_address(CPUM68KState *env, hwaddr *physical, bool debug = access_type & ACCESS_DEBUG; int page_bits; int i; + MemTxResult txres; /* Transparent Translation (physical = logical) */ for (i = 0; i < M68K_MAX_TTR; i++) { @@ -680,12 +681,19 @@ static int get_physical_address(CPUM68KState *env, hwaddr *physical, /* Root Index */ entry = M68K_POINTER_BASE(next) | M68K_ROOT_INDEX(address); - next = ldl_phys(cs->as, entry); + next = address_space_ldl(cs->as, entry, MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK) { + goto txfail; + } if (!M68K_UDT_VALID(next)) { return -1; } if (!(next & M68K_DESC_USED) && !debug) { - stl_phys(cs->as, entry, next | M68K_DESC_USED); + address_space_stl(cs->as, entry, next | M68K_DESC_USED, + MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK) { + goto txfail; + } } if (next & M68K_DESC_WRITEPROT) { if (access_type & ACCESS_PTEST) { @@ -700,12 +708,19 @@ static int get_physical_address(CPUM68KState *env, hwaddr *physical, /* Pointer Index */ entry = M68K_POINTER_BASE(next) | M68K_POINTER_INDEX(address); - next = ldl_phys(cs->as, entry); + next = address_space_ldl(cs->as, entry, MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK) { + goto txfail; + } if (!M68K_UDT_VALID(next)) { return -1; } if (!(next & M68K_DESC_USED) && !debug) { - stl_phys(cs->as, entry, next | M68K_DESC_USED); + address_space_stl(cs->as, entry, next | M68K_DESC_USED, + MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK) { + goto txfail; + } } if (next & M68K_DESC_WRITEPROT) { if (access_type & ACCESS_PTEST) { @@ -724,27 +739,46 @@ static int get_physical_address(CPUM68KState *env, hwaddr *physical, entry = M68K_4K_PAGE_BASE(next) | M68K_4K_PAGE_INDEX(address); } - next = ldl_phys(cs->as, entry); + next = address_space_ldl(cs->as, entry, MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK) { + goto txfail; + } if (!M68K_PDT_VALID(next)) { return -1; } if (M68K_PDT_INDIRECT(next)) { - next = ldl_phys(cs->as, M68K_INDIRECT_POINTER(next)); + next = address_space_ldl(cs->as, M68K_INDIRECT_POINTER(next), + MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK) { + goto txfail; + } } if (access_type & ACCESS_STORE) { if (next & M68K_DESC_WRITEPROT) { if (!(next & M68K_DESC_USED) && !debug) { - stl_phys(cs->as, entry, next | M68K_DESC_USED); + address_space_stl(cs->as, entry, next | M68K_DESC_USED, + MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK) { + goto txfail; + } } } else if ((next & (M68K_DESC_MODIFIED | M68K_DESC_USED)) != (M68K_DESC_MODIFIED | M68K_DESC_USED) && !debug) { - stl_phys(cs->as, entry, - next | (M68K_DESC_MODIFIED | M68K_DESC_USED)); + address_space_stl(cs->as, entry, + next | (M68K_DESC_MODIFIED | M68K_DESC_USED), + MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK) { + goto txfail; + } } } else { if (!(next & M68K_DESC_USED) && !debug) { - stl_phys(cs->as, entry, next | M68K_DESC_USED); + address_space_stl(cs->as, entry, next | M68K_DESC_USED, + MEMTXATTRS_UNSPECIFIED, &txres); + if (txres != MEMTX_OK) { + goto txfail; + } } } @@ -776,6 +810,14 @@ static int get_physical_address(CPUM68KState *env, hwaddr *physical, } return 0; + +txfail: + /* + * A page table load/store failed. TODO: we should really raise a + * suitable guest fault here if this is not a debug access. + * For now just return that the translation failed. + */ + return -1; } hwaddr m68k_cpu_get_phys_page_debug(CPUState *cs, vaddr addr) From e1aaf3a88e95ab007445281e2b2f6e3c8da47f22 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 10 Dec 2018 16:56:36 +0000 Subject: [PATCH 3/6] target/m68k: Switch to transaction_failed hook Switch the m68k target from the old unassigned_access hook to the transaction_failed hook. The notable difference is that rather than it being called for all physical memory accesses which fail (including those made by DMA devices or by the gdbstub), it is only called for those made by the CPU via its MMU. (In previous commits we put in explicit checks for the direct physical loads made by the target/m68k code which will no longer be handled by calling the unassigned_access hook.) Signed-off-by: Peter Maydell Message-Id: <20181210165636.28366-4-peter.maydell@linaro.org> Signed-off-by: Laurent Vivier --- target/m68k/cpu.c | 2 +- target/m68k/cpu.h | 7 ++++--- target/m68k/op_helper.c | 20 ++++++++------------ 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c index 582e3a73b3..6d09c630b0 100644 --- a/target/m68k/cpu.c +++ b/target/m68k/cpu.c @@ -271,7 +271,7 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data) cc->gdb_write_register = m68k_cpu_gdb_write_register; cc->handle_mmu_fault = m68k_cpu_handle_mmu_fault; #if defined(CONFIG_SOFTMMU) - cc->do_unassigned_access = m68k_cpu_unassigned_access; + cc->do_transaction_failed = m68k_cpu_transaction_failed; cc->get_phys_page_debug = m68k_cpu_get_phys_page_debug; #endif cc->disas_set_info = m68k_cpu_disas_set_info; diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index ad41608341..6039b47d0c 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -544,9 +544,10 @@ static inline int cpu_mmu_index (CPUM68KState *env, bool ifetch) int m68k_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int size, int rw, int mmu_idx); -void m68k_cpu_unassigned_access(CPUState *cs, hwaddr addr, - bool is_write, bool is_exec, int is_asi, - unsigned size); +void m68k_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr, + unsigned size, MMUAccessType access_type, + int mmu_idx, MemTxAttrs attrs, + MemTxResult response, uintptr_t retaddr); #include "exec/cpu-all.h" diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c index 76f439985a..1c272b4cda 100644 --- a/target/m68k/op_helper.c +++ b/target/m68k/op_helper.c @@ -454,19 +454,15 @@ static inline void do_interrupt_m68k_hardirq(CPUM68KState *env) do_interrupt_all(env, 1); } -void m68k_cpu_unassigned_access(CPUState *cs, hwaddr addr, bool is_write, - bool is_exec, int is_asi, unsigned size) +void m68k_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr, + unsigned size, MMUAccessType access_type, + int mmu_idx, MemTxAttrs attrs, + MemTxResult response, uintptr_t retaddr) { M68kCPU *cpu = M68K_CPU(cs); CPUM68KState *env = &cpu->env; -#ifdef DEBUG_UNASSIGNED - qemu_log_mask(CPU_LOG_INT, "Unassigned " TARGET_FMT_plx " wr=%d exe=%d\n", - addr, is_write, is_exec); -#endif - if (env == NULL) { - /* when called from gdb, env is NULL */ - return; - } + + cpu_restore_state(cs, retaddr, true); if (m68k_feature(env, M68K_FEATURE_M68040)) { env->mmu.mmusr = 0; @@ -476,7 +472,7 @@ void m68k_cpu_unassigned_access(CPUState *cs, hwaddr addr, bool is_write, if (env->sr & SR_S) { /* SUPERVISOR */ env->mmu.ssw |= M68K_TM_040_SUPER; } - if (is_exec) { /* instruction or data */ + if (access_type == MMU_INST_FETCH) { /* instruction or data */ env->mmu.ssw |= M68K_TM_040_CODE; } else { env->mmu.ssw |= M68K_TM_040_DATA; @@ -494,7 +490,7 @@ void m68k_cpu_unassigned_access(CPUState *cs, hwaddr addr, bool is_write, break; } - if (!is_write) { + if (access_type != MMU_DATA_STORE) { env->mmu.ssw |= M68K_RW_040; } From 89fa312be0dfd8b4c539c8763796e785c6b00b46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 10 Mar 2019 01:34:23 +0100 Subject: [PATCH 4/6] target/m68k: Reduce the l1 TCGLabel scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20190310003428.11723-2-f4bug@amsat.org> Signed-off-by: Laurent Vivier --- target/m68k/translate.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 58596278c2..176c5d966c 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -3020,7 +3020,6 @@ DISAS_INSN(branch) int32_t offset; uint32_t base; int op; - TCGLabel *l1; base = s->pc; op = (insn >> 8) & 0xf; @@ -3036,7 +3035,7 @@ DISAS_INSN(branch) } if (op > 1) { /* Bcc */ - l1 = gen_new_label(); + TCGLabel *l1 = gen_new_label(); gen_jmpcc(s, ((insn >> 8) & 0xf) ^ 1, l1); gen_jmp_tb(s, 1, base + offset); gen_set_label(l1); From 44c64e90950adf9efe7f4235a32eb868d1290ebb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 10 Mar 2019 01:34:25 +0100 Subject: [PATCH 5/6] target/m68k: Fix a tcg_temp leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function gen_get_ccr() returns a tcg_temp created with tcg_temp_new(). Free it with tcg_temp_free(). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20190310003428.11723-4-f4bug@amsat.org> Signed-off-by: Laurent Vivier --- target/m68k/translate.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 176c5d966c..bf700c01b1 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -2227,6 +2227,7 @@ static TCGv gen_get_sr(DisasContext *s) sr = tcg_temp_new(); tcg_gen_andi_i32(sr, QREG_SR, 0xffe0); tcg_gen_or_i32(sr, sr, ccr); + tcg_temp_free(ccr); return sr; } From 60d3d0cfeb1658d2827d6a4f0df27252bb36baba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 10 Mar 2019 01:34:27 +0100 Subject: [PATCH 6/6] target/m68k: Optimize rotate_x() using extract_i32() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Optimize rotate_x() using tcg_gen_extract_i32(). We can now free the 'sz' tcg_temp earlier. Since it is allocated with tcg_const_i32(), free it with tcg_temp_free_i32(). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20190310003428.11723-6-f4bug@amsat.org> Signed-off-by: Laurent Vivier --- target/m68k/translate.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index bf700c01b1..f0534a4ba0 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -3693,6 +3693,7 @@ static TCGv rotate_x(TCGv reg, TCGv shift, int left, int size) tcg_gen_sub_i32(shl, shl, shift); /* shl = size + 1 - shift */ tcg_gen_sub_i32(shx, sz, shift); /* shx = size - shift */ } + tcg_temp_free_i32(sz); /* reg = (reg << shl) | (reg >> shr) | (x << shx); */ @@ -3708,9 +3709,7 @@ static TCGv rotate_x(TCGv reg, TCGv shift, int left, int size) /* X = (reg >> size) & 1 */ X = tcg_temp_new(); - tcg_gen_shr_i32(X, reg, sz); - tcg_gen_andi_i32(X, X, 1); - tcg_temp_free(sz); + tcg_gen_extract_i32(X, reg, size, 1); return X; }