From 7bccb9e3222024470073568d05c2121386564ca3 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 22 Aug 2022 21:57:20 -0700 Subject: target/avr: Support probe argument to tlb_fill While there are no target-specific nonfaulting probes, generic code may grow some uses at some point. Note that the attrs argument was incorrect -- it should have been MEMTXATTRS_UNSPECIFIED. Just use the simpler interface. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/avr/helper.c | 46 +++++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 17 deletions(-) (limited to 'target') diff --git a/target/avr/helper.c b/target/avr/helper.c index db76452f9a..82284f8997 100644 --- a/target/avr/helper.c +++ b/target/avr/helper.c @@ -102,38 +102,50 @@ bool avr_cpu_tlb_fill(CPUState *cs, vaddr address, int size, MMUAccessType access_type, int mmu_idx, bool probe, uintptr_t retaddr) { - int prot = 0; - MemTxAttrs attrs = {}; + int prot, page_size = TARGET_PAGE_SIZE; uint32_t paddr; address &= TARGET_PAGE_MASK; if (mmu_idx == MMU_CODE_IDX) { - /* access to code in flash */ + /* Access to code in flash. */ paddr = OFFSET_CODE + address; prot = PAGE_READ | PAGE_EXEC; - if (paddr + TARGET_PAGE_SIZE > OFFSET_DATA) { + if (paddr >= OFFSET_DATA) { + /* + * This should not be possible via any architectural operations. + * There is certainly not an exception that we can deliver. + * Accept probing that might come from generic code. + */ + if (probe) { + return false; + } error_report("execution left flash memory"); abort(); } - } else if (address < NUMBER_OF_CPU_REGISTERS + NUMBER_OF_IO_REGISTERS) { - /* - * access to CPU registers, exit and rebuilt this TB to use full access - * incase it touches specially handled registers like SREG or SP - */ - AVRCPU *cpu = AVR_CPU(cs); - CPUAVRState *env = &cpu->env; - env->fullacc = 1; - cpu_loop_exit_restore(cs, retaddr); } else { - /* access to memory. nothing special */ + /* Access to memory. */ paddr = OFFSET_DATA + address; prot = PAGE_READ | PAGE_WRITE; + if (address < NUMBER_OF_CPU_REGISTERS + NUMBER_OF_IO_REGISTERS) { + /* + * Access to CPU registers, exit and rebuilt this TB to use + * full access in case it touches specially handled registers + * like SREG or SP. For probing, set page_size = 1, in order + * to force tlb_fill to be called for the next access. + */ + if (probe) { + page_size = 1; + } else { + AVRCPU *cpu = AVR_CPU(cs); + CPUAVRState *env = &cpu->env; + env->fullacc = 1; + cpu_loop_exit_restore(cs, retaddr); + } + } } - tlb_set_page_with_attrs(cs, address, paddr, attrs, prot, - mmu_idx, TARGET_PAGE_SIZE); - + tlb_set_page(cs, address, paddr, prot, mmu_idx, page_size); return true; } -- cgit v1.2.3-55-g7522 From 9e1b2375daabc4d133baf08766676c5d301bade5 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Fri, 26 Aug 2022 13:32:56 -0700 Subject: target/avr: Call avr_cpu_do_interrupt directly There is no need to go through cc->tcg_ops when we know what value that must have. Reviewed-by: Michael Rolnik Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/avr/helper.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'target') diff --git a/target/avr/helper.c b/target/avr/helper.c index 82284f8997..9614ccf3e4 100644 --- a/target/avr/helper.c +++ b/target/avr/helper.c @@ -29,14 +29,13 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request) { bool ret = false; - CPUClass *cc = CPU_GET_CLASS(cs); AVRCPU *cpu = AVR_CPU(cs); CPUAVRState *env = &cpu->env; if (interrupt_request & CPU_INTERRUPT_RESET) { if (cpu_interrupts_enabled(env)) { cs->exception_index = EXCP_RESET; - cc->tcg_ops->do_interrupt(cs); + avr_cpu_do_interrupt(cs); cs->interrupt_request &= ~CPU_INTERRUPT_RESET; @@ -47,7 +46,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request) if (cpu_interrupts_enabled(env) && env->intsrc != 0) { int index = ctz32(env->intsrc); cs->exception_index = EXCP_INT(index); - cc->tcg_ops->do_interrupt(cs); + avr_cpu_do_interrupt(cs); env->intsrc &= env->intsrc - 1; /* clear the interrupt */ if (!env->intsrc) { -- cgit v1.2.3-55-g7522 From cecaad540155927f2faf1b6897c72cc59074cb45 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Fri, 26 Aug 2022 13:35:52 -0700 Subject: target/avr: Only execute one interrupt at a time We cannot deliver two interrupts simultaneously; the first interrupt handler must execute first. Reviewed-by: Michael Rolnik Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/avr/helper.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'target') diff --git a/target/avr/helper.c b/target/avr/helper.c index 9614ccf3e4..34f1cbffb2 100644 --- a/target/avr/helper.c +++ b/target/avr/helper.c @@ -28,7 +28,6 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request) { - bool ret = false; AVRCPU *cpu = AVR_CPU(cs); CPUAVRState *env = &cpu->env; @@ -38,8 +37,7 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request) avr_cpu_do_interrupt(cs); cs->interrupt_request &= ~CPU_INTERRUPT_RESET; - - ret = true; + return true; } } if (interrupt_request & CPU_INTERRUPT_HARD) { @@ -52,11 +50,10 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request) if (!env->intsrc) { cs->interrupt_request &= ~CPU_INTERRUPT_HARD; } - - ret = true; + return true; } } - return ret; + return false; } void avr_cpu_do_interrupt(CPUState *cs) -- cgit v1.2.3-55-g7522 From 36027c70974fef1392e6c73dfb94c3f94f0930bc Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Fri, 26 Aug 2022 13:53:32 -0700 Subject: target/avr: Disable interrupts when env->skip set This bit is not saved across interrupts, so we must delay delivering the interrupt until the skip has been processed. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1118 Reviewed-by: Michael Rolnik Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/avr/helper.c | 9 +++++++++ target/avr/translate.c | 26 ++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) (limited to 'target') diff --git a/target/avr/helper.c b/target/avr/helper.c index 34f1cbffb2..156dde4e92 100644 --- a/target/avr/helper.c +++ b/target/avr/helper.c @@ -31,6 +31,15 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request) AVRCPU *cpu = AVR_CPU(cs); CPUAVRState *env = &cpu->env; + /* + * We cannot separate a skip from the next instruction, + * as the skip would not be preserved across the interrupt. + * Separating the two insn normally only happens at page boundaries. + */ + if (env->skip) { + return false; + } + if (interrupt_request & CPU_INTERRUPT_RESET) { if (cpu_interrupts_enabled(env)) { cs->exception_index = EXCP_RESET; diff --git a/target/avr/translate.c b/target/avr/translate.c index dc9c3d6bcc..026753c963 100644 --- a/target/avr/translate.c +++ b/target/avr/translate.c @@ -2971,8 +2971,18 @@ static void avr_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) if (skip_label) { canonicalize_skip(ctx); gen_set_label(skip_label); - if (ctx->base.is_jmp == DISAS_NORETURN) { + + switch (ctx->base.is_jmp) { + case DISAS_NORETURN: ctx->base.is_jmp = DISAS_CHAIN; + break; + case DISAS_NEXT: + if (ctx->base.tb->flags & TB_FLAGS_SKIP) { + ctx->base.is_jmp = DISAS_TOO_MANY; + } + break; + default: + break; } } @@ -2989,6 +2999,11 @@ static void avr_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs) { DisasContext *ctx = container_of(dcbase, DisasContext, base); bool nonconst_skip = canonicalize_skip(ctx); + /* + * Because we disable interrupts while env->skip is set, + * we must return to the main loop to re-evaluate afterward. + */ + bool force_exit = ctx->base.tb->flags & TB_FLAGS_SKIP; switch (ctx->base.is_jmp) { case DISAS_NORETURN: @@ -2997,7 +3012,7 @@ static void avr_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs) case DISAS_NEXT: case DISAS_TOO_MANY: case DISAS_CHAIN: - if (!nonconst_skip) { + if (!nonconst_skip && !force_exit) { /* Note gen_goto_tb checks singlestep. */ gen_goto_tb(ctx, 1, ctx->npc); break; @@ -3005,8 +3020,11 @@ static void avr_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs) tcg_gen_movi_tl(cpu_pc, ctx->npc); /* fall through */ case DISAS_LOOKUP: - tcg_gen_lookup_and_goto_ptr(); - break; + if (!force_exit) { + tcg_gen_lookup_and_goto_ptr(); + break; + } + /* fall through */ case DISAS_EXIT: tcg_gen_exit_tb(NULL, 0); break; -- cgit v1.2.3-55-g7522