* [PATCH v2 01/12] target/microblaze: Collected fixes for env->iflags
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-03 7:26 ` [PATCH v2 02/12] target/microblaze: Renumber D_FLAG Richard Henderson
` (11 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
There are several problems here that can result in soft lockup,
depending on exactly where an interrupt or exception is delivered:
Include BIMM_FLAG in IFLAGS_TB_MASK, since it needs to follow D_FLAG.
Ensure that iflags is 0 when entering an interrupt/exception handler.
Add mb_cpu_synchronize_from_tb to restore iflags from tb->flags.
The change to t_sync_flags is cosmetic, but makes the code clearer.
This fixes the reported regression in acceptance/replay_kernel.py.
Fixes: 683a247ed7a4 ("target/microblaze: Store "current" iflags in insn_start")
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/cpu.h | 3 ++-
target/microblaze/cpu.c | 11 +++++++++++
target/microblaze/helper.c | 17 +++++++++++------
target/microblaze/translate.c | 4 ++--
4 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index d11b6fa995..a25a2b427f 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -270,7 +270,8 @@ struct CPUMBState {
#define D_FLAG (1 << 19) /* Bit in ESR. */
/* TB dependent CPUMBState. */
-#define IFLAGS_TB_MASK (D_FLAG | IMM_FLAG | DRTI_FLAG | DRTE_FLAG | DRTB_FLAG)
+#define IFLAGS_TB_MASK (D_FLAG | BIMM_FLAG | IMM_FLAG | \
+ DRTI_FLAG | DRTE_FLAG | DRTB_FLAG)
#define MSR_TB_MASK (MSR_UM | MSR_VM | MSR_EE)
uint32_t iflags;
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 67017ecc33..6392524135 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -80,6 +80,16 @@ static void mb_cpu_set_pc(CPUState *cs, vaddr value)
MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
cpu->env.pc = value;
+ /* Ensure D_FLAG and IMM_FLAG are clear for the new PC */
+ cpu->env.iflags = 0;
+}
+
+static void mb_cpu_synchronize_from_tb(CPUState *cs, TranslationBlock *tb)
+{
+ MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
+
+ cpu->env.pc = tb->pc;
+ cpu->env.iflags = tb->flags & IFLAGS_TB_MASK;
}
static bool mb_cpu_has_work(CPUState *cs)
@@ -321,6 +331,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
cc->cpu_exec_interrupt = mb_cpu_exec_interrupt;
cc->dump_state = mb_cpu_dump_state;
cc->set_pc = mb_cpu_set_pc;
+ cc->synchronize_from_tb = mb_cpu_synchronize_from_tb;
cc->gdb_read_register = mb_cpu_gdb_read_register;
cc->gdb_write_register = mb_cpu_gdb_write_register;
cc->tlb_fill = mb_cpu_tlb_fill;
diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
index 48547385b0..00090526da 100644
--- a/target/microblaze/helper.c
+++ b/target/microblaze/helper.c
@@ -113,7 +113,10 @@ void mb_cpu_do_interrupt(CPUState *cs)
uint32_t t, msr = mb_cpu_read_msr(env);
/* IMM flag cannot propagate across a branch and into the dslot. */
- assert(!((env->iflags & D_FLAG) && (env->iflags & IMM_FLAG)));
+ assert((env->iflags & (D_FLAG | IMM_FLAG)) != (D_FLAG | IMM_FLAG));
+ /* BIMM flag cannot be set without D_FLAG. */
+ assert((env->iflags & (D_FLAG | BIMM_FLAG)) != BIMM_FLAG);
+ /* RTI flags are private to translate. */
assert(!(env->iflags & (DRTI_FLAG | DRTE_FLAG | DRTB_FLAG)));
env->res_addr = RES_ADDR_NONE;
switch (cs->exception_index) {
@@ -146,7 +149,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
env->pc, env->ear,
env->esr, env->iflags);
log_cpu_state_mask(CPU_LOG_INT, cs, 0);
- env->iflags &= ~(IMM_FLAG | D_FLAG);
+ env->iflags = 0;
env->pc = cpu->cfg.base_vectors + 0x20;
break;
@@ -186,14 +189,14 @@ void mb_cpu_do_interrupt(CPUState *cs)
"exception at pc=%x ear=%" PRIx64 " iflags=%x\n",
env->pc, env->ear, env->iflags);
log_cpu_state_mask(CPU_LOG_INT, cs, 0);
- env->iflags &= ~(IMM_FLAG | D_FLAG);
+ env->iflags = 0;
env->pc = cpu->cfg.base_vectors + 0x20;
break;
case EXCP_IRQ:
assert(!(msr & (MSR_EIP | MSR_BIP)));
assert(msr & MSR_IE);
- assert(!(env->iflags & D_FLAG));
+ assert(!(env->iflags & (D_FLAG | IMM_FLAG)));
t = (msr & (MSR_VM | MSR_UM)) << 1;
@@ -226,13 +229,14 @@ void mb_cpu_do_interrupt(CPUState *cs)
mb_cpu_write_msr(env, msr);
env->regs[14] = env->pc;
+ env->iflags = 0;
env->pc = cpu->cfg.base_vectors + 0x10;
//log_cpu_state_mask(CPU_LOG_INT, cs, 0);
break;
case EXCP_HW_BREAK:
- assert(!(env->iflags & IMM_FLAG));
- assert(!(env->iflags & D_FLAG));
+ assert(!(env->iflags & (D_FLAG | IMM_FLAG)));
+
t = (msr & (MSR_VM | MSR_UM)) << 1;
qemu_log_mask(CPU_LOG_INT,
"break at pc=%x msr=%x %x iflags=%x\n",
@@ -242,6 +246,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
msr |= t;
msr |= MSR_BIP;
env->regs[16] = env->pc;
+ env->iflags = 0;
env->pc = cpu->cfg.base_vectors + 0x18;
mb_cpu_write_msr(env, msr);
break;
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index a377818b5e..a8a3249185 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -91,8 +91,8 @@ static int typeb_imm(DisasContext *dc, int x)
static void t_sync_flags(DisasContext *dc)
{
/* Synch the tb dependent flags between translator and runtime. */
- if ((dc->tb_flags ^ dc->base.tb->flags) & ~MSR_TB_MASK) {
- tcg_gen_movi_i32(cpu_iflags, dc->tb_flags & ~MSR_TB_MASK);
+ if ((dc->tb_flags ^ dc->base.tb->flags) & IFLAGS_TB_MASK) {
+ tcg_gen_movi_i32(cpu_iflags, dc->tb_flags & IFLAGS_TB_MASK);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 02/12] target/microblaze: Renumber D_FLAG
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
2020-09-03 7:26 ` [PATCH v2 01/12] target/microblaze: Collected fixes for env->iflags Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-03 7:26 ` [PATCH v2 03/12] target/microblaze: Cleanup mb_cpu_do_interrupt Richard Henderson
` (10 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
ESS[DS] is bit 19 in the manual, but the manual uses big-endian bit
numbering. This corresponds to bit 12 in little-endian numbering.
Let the comment about matching the ESR be true by renumbering it.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/cpu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index a25a2b427f..32811f773d 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -264,10 +264,10 @@ struct CPUMBState {
/* MSR_UM (1 << 11) */
/* MSR_VM (1 << 13) */
/* ESR_ESS_MASK [11:5] -- unwind into iflags for unaligned excp */
+#define D_FLAG (1 << 12) /* Bit in ESR. */
#define DRTI_FLAG (1 << 16)
#define DRTE_FLAG (1 << 17)
#define DRTB_FLAG (1 << 18)
-#define D_FLAG (1 << 19) /* Bit in ESR. */
/* TB dependent CPUMBState. */
#define IFLAGS_TB_MASK (D_FLAG | BIMM_FLAG | IMM_FLAG | \
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 03/12] target/microblaze: Cleanup mb_cpu_do_interrupt
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
2020-09-03 7:26 ` [PATCH v2 01/12] target/microblaze: Collected fixes for env->iflags Richard Henderson
2020-09-03 7:26 ` [PATCH v2 02/12] target/microblaze: Renumber D_FLAG Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-03 7:26 ` [PATCH v2 04/12] target/microblaze: Rename mmu structs Richard Henderson
` (9 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
Reindent; remove dead/commented code.
Use D_FLAG to set ESS[DS].
Sink MSR adjustment for kernel entry, iflags and res_addr clear.
Improve CPU_LOG_INT formatting; report pc and msr before and after.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/helper.c | 209 ++++++++++++++++---------------------
1 file changed, 91 insertions(+), 118 deletions(-)
diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
index 00090526da..27a24bb99a 100644
--- a/target/microblaze/helper.c
+++ b/target/microblaze/helper.c
@@ -111,6 +111,7 @@ void mb_cpu_do_interrupt(CPUState *cs)
MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
CPUMBState *env = &cpu->env;
uint32_t t, msr = mb_cpu_read_msr(env);
+ bool set_esr;
/* IMM flag cannot propagate across a branch and into the dslot. */
assert((env->iflags & (D_FLAG | IMM_FLAG)) != (D_FLAG | IMM_FLAG));
@@ -118,142 +119,114 @@ void mb_cpu_do_interrupt(CPUState *cs)
assert((env->iflags & (D_FLAG | BIMM_FLAG)) != BIMM_FLAG);
/* RTI flags are private to translate. */
assert(!(env->iflags & (DRTI_FLAG | DRTE_FLAG | DRTB_FLAG)));
- env->res_addr = RES_ADDR_NONE;
+
switch (cs->exception_index) {
- case EXCP_HW_EXCP:
- if (!(env->pvr.regs[0] & PVR0_USE_EXC_MASK)) {
- qemu_log_mask(LOG_GUEST_ERROR, "Exception raised on system without exceptions!\n");
- return;
- }
+ case EXCP_HW_EXCP:
+ if (!(env->pvr.regs[0] & PVR0_USE_EXC_MASK)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Exception raised on system without exceptions!\n");
+ return;
+ }
- env->regs[17] = env->pc + 4;
- env->esr &= ~(1 << 12);
+ qemu_log_mask(CPU_LOG_INT,
+ "INT: HWE at pc=%08x msr=%08x iflags=%x\n",
+ env->pc, msr, env->iflags);
- /* Exception breaks branch + dslot sequence? */
- if (env->iflags & D_FLAG) {
- env->esr |= 1 << 12 ;
- env->btr = env->btarget;
- }
+ /* Exception breaks branch + dslot sequence? */
+ set_esr = true;
+ env->esr &= ~D_FLAG;
+ if (env->iflags & D_FLAG) {
+ env->esr |= D_FLAG;
+ env->btr = env->btarget;
+ }
- /* Disable the MMU. */
- t = (msr & (MSR_VM | MSR_UM)) << 1;
- msr &= ~(MSR_VMS | MSR_UMS | MSR_VM | MSR_UM);
- msr |= t;
- /* Exception in progress. */
- msr |= MSR_EIP;
- mb_cpu_write_msr(env, msr);
+ /* Exception in progress. */
+ msr |= MSR_EIP;
+ env->regs[17] = env->pc + 4;
+ env->pc = cpu->cfg.base_vectors + 0x20;
+ break;
- qemu_log_mask(CPU_LOG_INT,
- "hw exception at pc=%x ear=%" PRIx64 " "
- "esr=%x iflags=%x\n",
- env->pc, env->ear,
- env->esr, env->iflags);
- log_cpu_state_mask(CPU_LOG_INT, cs, 0);
- env->iflags = 0;
- env->pc = cpu->cfg.base_vectors + 0x20;
- break;
+ case EXCP_MMU:
+ qemu_log_mask(CPU_LOG_INT,
+ "INT: MMU at pc=%08x msr=%08x "
+ "ear=%" PRIx64 " iflags=%x\n",
+ env->pc, msr, env->ear, env->iflags);
- case EXCP_MMU:
+ /* Exception breaks branch + dslot sequence? */
+ set_esr = true;
+ env->esr &= ~D_FLAG;
+ if (env->iflags & D_FLAG) {
+ env->esr |= D_FLAG;
+ env->btr = env->btarget;
+ /* Reexecute the branch. */
+ env->regs[17] = env->pc - (env->iflags & BIMM_FLAG ? 8 : 4);
+ } else if (env->iflags & IMM_FLAG) {
+ /* Reexecute the imm. */
+ env->regs[17] = env->pc - 4;
+ } else {
env->regs[17] = env->pc;
+ }
- qemu_log_mask(CPU_LOG_INT,
- "MMU exception at pc=%x iflags=%x ear=%" PRIx64 "\n",
- env->pc, env->iflags, env->ear);
+ /* Exception in progress. */
+ msr |= MSR_EIP;
+ env->pc = cpu->cfg.base_vectors + 0x20;
+ break;
- env->esr &= ~(1 << 12);
- /* Exception breaks branch + dslot sequence? */
- if (env->iflags & D_FLAG) {
- env->esr |= 1 << 12 ;
- env->btr = env->btarget;
+ case EXCP_IRQ:
+ assert(!(msr & (MSR_EIP | MSR_BIP)));
+ assert(msr & MSR_IE);
+ assert(!(env->iflags & (D_FLAG | IMM_FLAG)));
- /* Reexecute the branch. */
- env->regs[17] -= 4;
- /* was the branch immprefixed?. */
- if (env->iflags & BIMM_FLAG) {
- env->regs[17] -= 4;
- log_cpu_state_mask(CPU_LOG_INT, cs, 0);
- }
- } else if (env->iflags & IMM_FLAG) {
- env->regs[17] -= 4;
- }
+ qemu_log_mask(CPU_LOG_INT,
+ "INT: DEV at pc=%08x msr=%08x iflags=%x\n",
+ env->pc, msr, env->iflags);
+ set_esr = false;
- /* Disable the MMU. */
- t = (msr & (MSR_VM | MSR_UM)) << 1;
- msr &= ~(MSR_VMS | MSR_UMS | MSR_VM | MSR_UM);
- msr |= t;
- /* Exception in progress. */
- msr |= MSR_EIP;
- mb_cpu_write_msr(env, msr);
+ /* Disable interrupts. */
+ msr &= ~MSR_IE;
+ env->regs[14] = env->pc;
+ env->pc = cpu->cfg.base_vectors + 0x10;
+ break;
- qemu_log_mask(CPU_LOG_INT,
- "exception at pc=%x ear=%" PRIx64 " iflags=%x\n",
- env->pc, env->ear, env->iflags);
- log_cpu_state_mask(CPU_LOG_INT, cs, 0);
- env->iflags = 0;
- env->pc = cpu->cfg.base_vectors + 0x20;
- break;
+ case EXCP_HW_BREAK:
+ assert(!(env->iflags & (D_FLAG | IMM_FLAG)));
- case EXCP_IRQ:
- assert(!(msr & (MSR_EIP | MSR_BIP)));
- assert(msr & MSR_IE);
- assert(!(env->iflags & (D_FLAG | IMM_FLAG)));
+ qemu_log_mask(CPU_LOG_INT,
+ "INT: BRK at pc=%08x msr=%08x iflags=%x\n",
+ env->pc, msr, env->iflags);
+ set_esr = false;
- t = (msr & (MSR_VM | MSR_UM)) << 1;
+ /* Break in progress. */
+ msr |= MSR_BIP;
+ env->regs[16] = env->pc;
+ env->pc = cpu->cfg.base_vectors + 0x18;
+ break;
-#if 0
-#include "disas/disas.h"
+ default:
+ cpu_abort(cs, "unhandled exception type=%d\n", cs->exception_index);
+ /* not reached */
+ }
-/* Useful instrumentation when debugging interrupt issues in either
- the models or in sw. */
- {
- const char *sym;
+ /* Save previous mode, disable mmu, disable user-mode. */
+ t = (msr & (MSR_VM | MSR_UM)) << 1;
+ msr &= ~(MSR_VMS | MSR_UMS | MSR_VM | MSR_UM);
+ msr |= t;
+ mb_cpu_write_msr(env, msr);
- sym = lookup_symbol(env->pc);
- if (sym
- && (!strcmp("netif_rx", sym)
- || !strcmp("process_backlog", sym))) {
+ env->res_addr = RES_ADDR_NONE;
+ env->iflags = 0;
- qemu_log("interrupt at pc=%x msr=%x %x iflags=%x sym=%s\n",
- env->pc, msr, t, env->iflags, sym);
-
- log_cpu_state(cs, 0);
- }
- }
-#endif
- qemu_log_mask(CPU_LOG_INT,
- "interrupt at pc=%x msr=%x %x iflags=%x\n",
- env->pc, msr, t, env->iflags);
-
- msr &= ~(MSR_VMS | MSR_UMS | MSR_VM | MSR_UM | MSR_IE);
- msr |= t;
- mb_cpu_write_msr(env, msr);
-
- env->regs[14] = env->pc;
- env->iflags = 0;
- env->pc = cpu->cfg.base_vectors + 0x10;
- //log_cpu_state_mask(CPU_LOG_INT, cs, 0);
- break;
-
- case EXCP_HW_BREAK:
- assert(!(env->iflags & (D_FLAG | IMM_FLAG)));
-
- t = (msr & (MSR_VM | MSR_UM)) << 1;
- qemu_log_mask(CPU_LOG_INT,
- "break at pc=%x msr=%x %x iflags=%x\n",
- env->pc, msr, t, env->iflags);
- log_cpu_state_mask(CPU_LOG_INT, cs, 0);
- msr &= ~(MSR_VMS | MSR_UMS | MSR_VM | MSR_UM);
- msr |= t;
- msr |= MSR_BIP;
- env->regs[16] = env->pc;
- env->iflags = 0;
- env->pc = cpu->cfg.base_vectors + 0x18;
- mb_cpu_write_msr(env, msr);
- break;
- default:
- cpu_abort(cs, "unhandled exception type=%d\n",
- cs->exception_index);
- break;
+ if (!set_esr) {
+ qemu_log_mask(CPU_LOG_INT,
+ " to pc=%08x msr=%08x\n", env->pc, msr);
+ } else if (env->esr & D_FLAG) {
+ qemu_log_mask(CPU_LOG_INT,
+ " to pc=%08x msr=%08x esr=%04x btr=%08x\n",
+ env->pc, msr, env->esr, env->btr);
+ } else {
+ qemu_log_mask(CPU_LOG_INT,
+ " to pc=%08x msr=%08x esr=%04x\n",
+ env->pc, msr, env->esr);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 04/12] target/microblaze: Rename mmu structs
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
` (2 preceding siblings ...)
2020-09-03 7:26 ` [PATCH v2 03/12] target/microblaze: Cleanup mb_cpu_do_interrupt Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-04 12:10 ` Edgar E. Iglesias
2020-09-04 12:43 ` Philippe Mathieu-Daudé
2020-09-03 7:26 ` [PATCH v2 05/12] target/microblaze: Fill in VMStateDescription for cpu Richard Henderson
` (8 subsequent siblings)
12 siblings, 2 replies; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
Introduce typedefs and follow CODING_STYLE for naming.
Rename struct microblaze_mmu to MicroBlazeMMU.
Rename struct microblaze_mmu_lookup to MicroBlazeMMULookup.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/cpu.h | 2 +-
target/microblaze/mmu.h | 15 ++++++---------
target/microblaze/helper.c | 4 ++--
target/microblaze/mmu.c | 11 +++++------
4 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index 32811f773d..20c2979396 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -278,7 +278,7 @@ struct CPUMBState {
#if !defined(CONFIG_USER_ONLY)
/* Unified MMU. */
- struct microblaze_mmu mmu;
+ MicroBlazeMMU mmu;
#endif
/* Fields up to this point are cleared by a CPU reset */
diff --git a/target/microblaze/mmu.h b/target/microblaze/mmu.h
index 75e5301c79..c1feb811b9 100644
--- a/target/microblaze/mmu.h
+++ b/target/microblaze/mmu.h
@@ -63,8 +63,7 @@
#define TLB_ENTRIES 64
-struct microblaze_mmu
-{
+typedef struct {
/* Data and tag brams. */
uint64_t rams[2][TLB_ENTRIES];
/* We keep a separate ram for the tids to avoid the 48 bit tag width. */
@@ -76,10 +75,9 @@ struct microblaze_mmu
int c_mmu_tlb_access;
int c_mmu_zones;
uint64_t c_addr_mask; /* Mask to apply to physical addresses. */
-};
+} MicroBlazeMMU;
-struct microblaze_mmu_lookup
-{
+typedef struct {
uint32_t paddr;
uint32_t vaddr;
unsigned int size;
@@ -88,13 +86,12 @@ struct microblaze_mmu_lookup
enum {
ERR_PROT, ERR_MISS, ERR_HIT
} err;
-};
+} MicroBlazeMMULookup;
-unsigned int mmu_translate(struct microblaze_mmu *mmu,
- struct microblaze_mmu_lookup *lu,
+unsigned int mmu_translate(MicroBlazeMMU *mmu, MicroBlazeMMULookup *lu,
target_ulong vaddr, int rw, int mmu_idx);
uint32_t mmu_read(CPUMBState *env, bool ea, uint32_t rn);
void mmu_write(CPUMBState *env, bool ea, uint32_t rn, uint32_t v);
-void mmu_init(struct microblaze_mmu *mmu);
+void mmu_init(MicroBlazeMMU *mmu);
#endif
diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
index 27a24bb99a..3c2fd388fb 100644
--- a/target/microblaze/helper.c
+++ b/target/microblaze/helper.c
@@ -52,7 +52,7 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
{
MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
CPUMBState *env = &cpu->env;
- struct microblaze_mmu_lookup lu;
+ MicroBlazeMMULookup lu;
unsigned int hit;
int prot;
@@ -235,7 +235,7 @@ hwaddr mb_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
CPUMBState *env = &cpu->env;
target_ulong vaddr, paddr = 0;
- struct microblaze_mmu_lookup lu;
+ MicroBlazeMMULookup lu;
int mmu_idx = cpu_mmu_index(env, false);
unsigned int hit;
diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
index 6e583d78d9..0546cfd0bc 100644
--- a/target/microblaze/mmu.c
+++ b/target/microblaze/mmu.c
@@ -35,7 +35,7 @@ static unsigned int tlb_decode_size(unsigned int f)
static void mmu_flush_idx(CPUMBState *env, unsigned int idx)
{
CPUState *cs = env_cpu(env);
- struct microblaze_mmu *mmu = &env->mmu;
+ MicroBlazeMMU *mmu = &env->mmu;
unsigned int tlb_size;
uint32_t tlb_tag, end, t;
@@ -55,7 +55,7 @@ static void mmu_flush_idx(CPUMBState *env, unsigned int idx)
static void mmu_change_pid(CPUMBState *env, unsigned int newpid)
{
- struct microblaze_mmu *mmu = &env->mmu;
+ MicroBlazeMMU *mmu = &env->mmu;
unsigned int i;
uint32_t t;
@@ -73,8 +73,7 @@ static void mmu_change_pid(CPUMBState *env, unsigned int newpid)
}
/* rw - 0 = read, 1 = write, 2 = fetch. */
-unsigned int mmu_translate(struct microblaze_mmu *mmu,
- struct microblaze_mmu_lookup *lu,
+unsigned int mmu_translate(MicroBlazeMMU *mmu, MicroBlazeMMULookup *lu,
target_ulong vaddr, int rw, int mmu_idx)
{
unsigned int i, hit = 0;
@@ -290,7 +289,7 @@ void mmu_write(CPUMBState *env, bool ext, uint32_t rn, uint32_t v)
break;
case MMU_R_TLBSX:
{
- struct microblaze_mmu_lookup lu;
+ MicroBlazeMMULookup lu;
int hit;
if (env->mmu.c_mmu_tlb_access <= 1) {
@@ -314,7 +313,7 @@ void mmu_write(CPUMBState *env, bool ext, uint32_t rn, uint32_t v)
}
}
-void mmu_init(struct microblaze_mmu *mmu)
+void mmu_init(MicroBlazeMMU *mmu)
{
int i;
for (i = 0; i < ARRAY_SIZE(mmu->regs); i++) {
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 04/12] target/microblaze: Rename mmu structs
2020-09-03 7:26 ` [PATCH v2 04/12] target/microblaze: Rename mmu structs Richard Henderson
@ 2020-09-04 12:10 ` Edgar E. Iglesias
2020-09-04 12:43 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 21+ messages in thread
From: Edgar E. Iglesias @ 2020-09-04 12:10 UTC (permalink / raw)
To: Richard Henderson; +Cc: thuth, qemu-devel, f4bug
On Thu, Sep 03, 2020 at 12:26:42AM -0700, Richard Henderson wrote:
> Introduce typedefs and follow CODING_STYLE for naming.
> Rename struct microblaze_mmu to MicroBlazeMMU.
> Rename struct microblaze_mmu_lookup to MicroBlazeMMULookup.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> target/microblaze/cpu.h | 2 +-
> target/microblaze/mmu.h | 15 ++++++---------
> target/microblaze/helper.c | 4 ++--
> target/microblaze/mmu.c | 11 +++++------
> 4 files changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index 32811f773d..20c2979396 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -278,7 +278,7 @@ struct CPUMBState {
>
> #if !defined(CONFIG_USER_ONLY)
> /* Unified MMU. */
> - struct microblaze_mmu mmu;
> + MicroBlazeMMU mmu;
> #endif
>
> /* Fields up to this point are cleared by a CPU reset */
> diff --git a/target/microblaze/mmu.h b/target/microblaze/mmu.h
> index 75e5301c79..c1feb811b9 100644
> --- a/target/microblaze/mmu.h
> +++ b/target/microblaze/mmu.h
> @@ -63,8 +63,7 @@
>
> #define TLB_ENTRIES 64
>
> -struct microblaze_mmu
> -{
> +typedef struct {
> /* Data and tag brams. */
> uint64_t rams[2][TLB_ENTRIES];
> /* We keep a separate ram for the tids to avoid the 48 bit tag width. */
> @@ -76,10 +75,9 @@ struct microblaze_mmu
> int c_mmu_tlb_access;
> int c_mmu_zones;
> uint64_t c_addr_mask; /* Mask to apply to physical addresses. */
> -};
> +} MicroBlazeMMU;
>
> -struct microblaze_mmu_lookup
> -{
> +typedef struct {
> uint32_t paddr;
> uint32_t vaddr;
> unsigned int size;
> @@ -88,13 +86,12 @@ struct microblaze_mmu_lookup
> enum {
> ERR_PROT, ERR_MISS, ERR_HIT
> } err;
> -};
> +} MicroBlazeMMULookup;
>
> -unsigned int mmu_translate(struct microblaze_mmu *mmu,
> - struct microblaze_mmu_lookup *lu,
> +unsigned int mmu_translate(MicroBlazeMMU *mmu, MicroBlazeMMULookup *lu,
> target_ulong vaddr, int rw, int mmu_idx);
> uint32_t mmu_read(CPUMBState *env, bool ea, uint32_t rn);
> void mmu_write(CPUMBState *env, bool ea, uint32_t rn, uint32_t v);
> -void mmu_init(struct microblaze_mmu *mmu);
> +void mmu_init(MicroBlazeMMU *mmu);
>
> #endif
> diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
> index 27a24bb99a..3c2fd388fb 100644
> --- a/target/microblaze/helper.c
> +++ b/target/microblaze/helper.c
> @@ -52,7 +52,7 @@ bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> {
> MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> CPUMBState *env = &cpu->env;
> - struct microblaze_mmu_lookup lu;
> + MicroBlazeMMULookup lu;
> unsigned int hit;
> int prot;
>
> @@ -235,7 +235,7 @@ hwaddr mb_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
> CPUMBState *env = &cpu->env;
> target_ulong vaddr, paddr = 0;
> - struct microblaze_mmu_lookup lu;
> + MicroBlazeMMULookup lu;
> int mmu_idx = cpu_mmu_index(env, false);
> unsigned int hit;
>
> diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
> index 6e583d78d9..0546cfd0bc 100644
> --- a/target/microblaze/mmu.c
> +++ b/target/microblaze/mmu.c
> @@ -35,7 +35,7 @@ static unsigned int tlb_decode_size(unsigned int f)
> static void mmu_flush_idx(CPUMBState *env, unsigned int idx)
> {
> CPUState *cs = env_cpu(env);
> - struct microblaze_mmu *mmu = &env->mmu;
> + MicroBlazeMMU *mmu = &env->mmu;
> unsigned int tlb_size;
> uint32_t tlb_tag, end, t;
>
> @@ -55,7 +55,7 @@ static void mmu_flush_idx(CPUMBState *env, unsigned int idx)
>
> static void mmu_change_pid(CPUMBState *env, unsigned int newpid)
> {
> - struct microblaze_mmu *mmu = &env->mmu;
> + MicroBlazeMMU *mmu = &env->mmu;
> unsigned int i;
> uint32_t t;
>
> @@ -73,8 +73,7 @@ static void mmu_change_pid(CPUMBState *env, unsigned int newpid)
> }
>
> /* rw - 0 = read, 1 = write, 2 = fetch. */
> -unsigned int mmu_translate(struct microblaze_mmu *mmu,
> - struct microblaze_mmu_lookup *lu,
> +unsigned int mmu_translate(MicroBlazeMMU *mmu, MicroBlazeMMULookup *lu,
> target_ulong vaddr, int rw, int mmu_idx)
> {
> unsigned int i, hit = 0;
> @@ -290,7 +289,7 @@ void mmu_write(CPUMBState *env, bool ext, uint32_t rn, uint32_t v)
> break;
> case MMU_R_TLBSX:
> {
> - struct microblaze_mmu_lookup lu;
> + MicroBlazeMMULookup lu;
> int hit;
>
> if (env->mmu.c_mmu_tlb_access <= 1) {
> @@ -314,7 +313,7 @@ void mmu_write(CPUMBState *env, bool ext, uint32_t rn, uint32_t v)
> }
> }
>
> -void mmu_init(struct microblaze_mmu *mmu)
> +void mmu_init(MicroBlazeMMU *mmu)
> {
> int i;
> for (i = 0; i < ARRAY_SIZE(mmu->regs); i++) {
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 04/12] target/microblaze: Rename mmu structs
2020-09-03 7:26 ` [PATCH v2 04/12] target/microblaze: Rename mmu structs Richard Henderson
2020-09-04 12:10 ` Edgar E. Iglesias
@ 2020-09-04 12:43 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-04 12:43 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: edgar.iglesias, thuth
On 9/3/20 9:26 AM, Richard Henderson wrote:
> Introduce typedefs and follow CODING_STYLE for naming.
> Rename struct microblaze_mmu to MicroBlazeMMU.
> Rename struct microblaze_mmu_lookup to MicroBlazeMMULookup.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/microblaze/cpu.h | 2 +-
> target/microblaze/mmu.h | 15 ++++++---------
> target/microblaze/helper.c | 4 ++--
> target/microblaze/mmu.c | 11 +++++------
> 4 files changed, 14 insertions(+), 18 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 05/12] target/microblaze: Fill in VMStateDescription for cpu
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
` (3 preceding siblings ...)
2020-09-03 7:26 ` [PATCH v2 04/12] target/microblaze: Rename mmu structs Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-04 12:20 ` Edgar E. Iglesias
2020-09-03 7:26 ` [PATCH v2 06/12] target/microblaze: Rename DISAS_UPDATE to DISAS_EXIT Richard Henderson
` (7 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/cpu.h | 4 ++
target/microblaze/cpu.c | 8 +--
target/microblaze/machine.c | 112 ++++++++++++++++++++++++++++++++++
target/microblaze/meson.build | 5 +-
4 files changed, 121 insertions(+), 8 deletions(-)
create mode 100644 target/microblaze/machine.c
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index 20c2979396..133ebaa4d4 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -419,4 +419,8 @@ static inline int cpu_mmu_index(CPUMBState *env, bool ifetch)
return MMU_KERNEL_IDX;
}
+#ifndef CONFIG_USER_ONLY
+extern const VMStateDescription vmstate_mb_cpu;
+#endif
+
#endif
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 6392524135..388605ccca 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -26,7 +26,6 @@
#include "cpu.h"
#include "qemu/module.h"
#include "hw/qdev-properties.h"
-#include "migration/vmstate.h"
#include "exec/exec-all.h"
#include "fpu/softfloat-helpers.h"
@@ -254,11 +253,6 @@ static void mb_cpu_initfn(Object *obj)
#endif
}
-static const VMStateDescription vmstate_mb_cpu = {
- .name = "cpu",
- .unmigratable = 1,
-};
-
static Property mb_properties[] = {
DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors, 0),
DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackprot,
@@ -338,8 +332,8 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
#ifndef CONFIG_USER_ONLY
cc->do_transaction_failed = mb_cpu_transaction_failed;
cc->get_phys_page_debug = mb_cpu_get_phys_page_debug;
-#endif
dc->vmsd = &vmstate_mb_cpu;
+#endif
device_class_set_props(dc, mb_properties);
cc->gdb_num_core_regs = 32 + 27;
diff --git a/target/microblaze/machine.c b/target/microblaze/machine.c
new file mode 100644
index 0000000000..aad3c5d1d3
--- /dev/null
+++ b/target/microblaze/machine.c
@@ -0,0 +1,112 @@
+/*
+ * Microblaze VMState for qemu.
+ *
+ * Copyright (c) 2020 Linaro, Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "migration/cpu.h"
+
+
+static VMStateField vmstate_mmu_fields[] = {
+ VMSTATE_UINT64_2DARRAY(rams, MicroBlazeMMU, 2, TLB_ENTRIES),
+ VMSTATE_UINT8_ARRAY(tids, MicroBlazeMMU, TLB_ENTRIES),
+ VMSTATE_UINT32_ARRAY(regs, MicroBlazeMMU, 3),
+ VMSTATE_INT32(c_mmu, MicroBlazeMMU),
+ VMSTATE_INT32(c_mmu_tlb_access, MicroBlazeMMU),
+ VMSTATE_INT32(c_mmu_zones, MicroBlazeMMU),
+ VMSTATE_UINT64(c_addr_mask, MicroBlazeMMU),
+ VMSTATE_END_OF_LIST()
+};
+
+static const VMStateDescription vmstate_mmu = {
+ .name = "mmu",
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .fields = vmstate_mmu_fields,
+};
+
+static int get_msr(QEMUFile *f, void *opaque, size_t size,
+ const VMStateField *field)
+{
+ CPUMBState *env = container_of(opaque, CPUMBState, msr);
+
+ mb_cpu_write_msr(env, qemu_get_be32(f));
+ return 0;
+}
+
+static int put_msr(QEMUFile *f, void *opaque, size_t size,
+ const VMStateField *field, QJSON *vmdesc)
+{
+ CPUMBState *env = container_of(opaque, CPUMBState, msr);
+
+ qemu_put_be32(f, mb_cpu_read_msr(env));
+ return 0;
+}
+
+static const VMStateInfo vmstate_msr = {
+ .name = "msr",
+ .get = get_msr,
+ .put = put_msr,
+};
+
+static VMStateField vmstate_env_fields[] = {
+ VMSTATE_UINT32_ARRAY(regs, CPUMBState, 32),
+
+ VMSTATE_UINT32(pc, CPUMBState),
+ VMSTATE_SINGLE(msr, CPUMBState, 0, vmstate_msr, uint32_t),
+ VMSTATE_UINT32(esr, CPUMBState),
+ VMSTATE_UINT32(fsr, CPUMBState),
+ VMSTATE_UINT32(btr, CPUMBState),
+ VMSTATE_UINT32(edr, CPUMBState),
+ VMSTATE_UINT32(slr, CPUMBState),
+ VMSTATE_UINT32(shr, CPUMBState),
+ VMSTATE_UINT64(ear, CPUMBState),
+
+ VMSTATE_UINT32(btarget, CPUMBState),
+ VMSTATE_UINT32(imm, CPUMBState),
+ VMSTATE_UINT32(iflags, CPUMBState),
+
+ VMSTATE_UINT32(res_val, CPUMBState),
+ VMSTATE_UINTTL(res_addr, CPUMBState),
+
+ VMSTATE_UINT32_ARRAY(pvr.regs, CPUMBState, 13),
+
+ VMSTATE_STRUCT(mmu, CPUMBState, 0, vmstate_mmu, MicroBlazeMMU),
+
+ VMSTATE_END_OF_LIST()
+};
+
+static const VMStateDescription vmstate_env = {
+ .name = "env",
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .fields = vmstate_env_fields,
+};
+
+static VMStateField vmstate_cpu_fields[] = {
+ VMSTATE_CPU(),
+ VMSTATE_STRUCT(env, MicroBlazeCPU, 1, vmstate_env, CPUMBState),
+ VMSTATE_END_OF_LIST()
+};
+
+const VMStateDescription vmstate_mb_cpu = {
+ .name = "cpu",
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .fields = vmstate_cpu_fields,
+};
diff --git a/target/microblaze/meson.build b/target/microblaze/meson.build
index 639c3f73a8..05ee0ec163 100644
--- a/target/microblaze/meson.build
+++ b/target/microblaze/meson.build
@@ -11,7 +11,10 @@ microblaze_ss.add(files(
))
microblaze_softmmu_ss = ss.source_set()
-microblaze_softmmu_ss.add(files('mmu.c'))
+microblaze_softmmu_ss.add(files(
+ 'mmu.c',
+ 'machine.c',
+))
target_arch += {'microblaze': microblaze_ss}
target_softmmu_arch += {'microblaze': microblaze_softmmu_ss}
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 05/12] target/microblaze: Fill in VMStateDescription for cpu
2020-09-03 7:26 ` [PATCH v2 05/12] target/microblaze: Fill in VMStateDescription for cpu Richard Henderson
@ 2020-09-04 12:20 ` Edgar E. Iglesias
2020-09-04 15:25 ` Richard Henderson
0 siblings, 1 reply; 21+ messages in thread
From: Edgar E. Iglesias @ 2020-09-04 12:20 UTC (permalink / raw)
To: Richard Henderson; +Cc: thuth, qemu-devel, f4bug
On Thu, Sep 03, 2020 at 12:26:43AM -0700, Richard Henderson wrote:
Hi Richard,
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/microblaze/cpu.h | 4 ++
> target/microblaze/cpu.c | 8 +--
> target/microblaze/machine.c | 112 ++++++++++++++++++++++++++++++++++
> target/microblaze/meson.build | 5 +-
> 4 files changed, 121 insertions(+), 8 deletions(-)
> create mode 100644 target/microblaze/machine.c
>
> diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> index 20c2979396..133ebaa4d4 100644
> --- a/target/microblaze/cpu.h
> +++ b/target/microblaze/cpu.h
> @@ -419,4 +419,8 @@ static inline int cpu_mmu_index(CPUMBState *env, bool ifetch)
> return MMU_KERNEL_IDX;
> }
>
> +#ifndef CONFIG_USER_ONLY
> +extern const VMStateDescription vmstate_mb_cpu;
> +#endif
> +
> #endif
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 6392524135..388605ccca 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -26,7 +26,6 @@
> #include "cpu.h"
> #include "qemu/module.h"
> #include "hw/qdev-properties.h"
> -#include "migration/vmstate.h"
> #include "exec/exec-all.h"
> #include "fpu/softfloat-helpers.h"
>
> @@ -254,11 +253,6 @@ static void mb_cpu_initfn(Object *obj)
> #endif
> }
>
> -static const VMStateDescription vmstate_mb_cpu = {
> - .name = "cpu",
> - .unmigratable = 1,
> -};
> -
> static Property mb_properties[] = {
> DEFINE_PROP_UINT32("base-vectors", MicroBlazeCPU, cfg.base_vectors, 0),
> DEFINE_PROP_BOOL("use-stack-protection", MicroBlazeCPU, cfg.stackprot,
> @@ -338,8 +332,8 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
> #ifndef CONFIG_USER_ONLY
> cc->do_transaction_failed = mb_cpu_transaction_failed;
> cc->get_phys_page_debug = mb_cpu_get_phys_page_debug;
> -#endif
> dc->vmsd = &vmstate_mb_cpu;
> +#endif
> device_class_set_props(dc, mb_properties);
> cc->gdb_num_core_regs = 32 + 27;
>
> diff --git a/target/microblaze/machine.c b/target/microblaze/machine.c
> new file mode 100644
> index 0000000000..aad3c5d1d3
> --- /dev/null
> +++ b/target/microblaze/machine.c
> @@ -0,0 +1,112 @@
> +/*
> + * Microblaze VMState for qemu.
> + *
> + * Copyright (c) 2020 Linaro, Ltd.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "migration/cpu.h"
> +
> +
> +static VMStateField vmstate_mmu_fields[] = {
> + VMSTATE_UINT64_2DARRAY(rams, MicroBlazeMMU, 2, TLB_ENTRIES),
> + VMSTATE_UINT8_ARRAY(tids, MicroBlazeMMU, TLB_ENTRIES),
> + VMSTATE_UINT32_ARRAY(regs, MicroBlazeMMU, 3),
> + VMSTATE_INT32(c_mmu, MicroBlazeMMU),
> + VMSTATE_INT32(c_mmu_tlb_access, MicroBlazeMMU),
> + VMSTATE_INT32(c_mmu_zones, MicroBlazeMMU),
> + VMSTATE_UINT64(c_addr_mask, MicroBlazeMMU),
These last 4 entries are elaboration-time settings, i.e they will not
change during VM runtime. I don't think we need to transfer these since
we expect the peer to setup the same kind of microblaze?
> + VMSTATE_END_OF_LIST()
> +};
> +
> +static const VMStateDescription vmstate_mmu = {
> + .name = "mmu",
> + .version_id = 0,
> + .minimum_version_id = 0,
> + .fields = vmstate_mmu_fields,
> +};
> +
> +static int get_msr(QEMUFile *f, void *opaque, size_t size,
> + const VMStateField *field)
> +{
> + CPUMBState *env = container_of(opaque, CPUMBState, msr);
> +
> + mb_cpu_write_msr(env, qemu_get_be32(f));
> + return 0;
> +}
> +
> +static int put_msr(QEMUFile *f, void *opaque, size_t size,
> + const VMStateField *field, QJSON *vmdesc)
> +{
> + CPUMBState *env = container_of(opaque, CPUMBState, msr);
> +
> + qemu_put_be32(f, mb_cpu_read_msr(env));
> + return 0;
> +}
> +
> +static const VMStateInfo vmstate_msr = {
> + .name = "msr",
> + .get = get_msr,
> + .put = put_msr,
> +};
> +
> +static VMStateField vmstate_env_fields[] = {
> + VMSTATE_UINT32_ARRAY(regs, CPUMBState, 32),
> +
> + VMSTATE_UINT32(pc, CPUMBState),
> + VMSTATE_SINGLE(msr, CPUMBState, 0, vmstate_msr, uint32_t),
> + VMSTATE_UINT32(esr, CPUMBState),
> + VMSTATE_UINT32(fsr, CPUMBState),
> + VMSTATE_UINT32(btr, CPUMBState),
> + VMSTATE_UINT32(edr, CPUMBState),
> + VMSTATE_UINT32(slr, CPUMBState),
> + VMSTATE_UINT32(shr, CPUMBState),
> + VMSTATE_UINT64(ear, CPUMBState),
> +
> + VMSTATE_UINT32(btarget, CPUMBState),
> + VMSTATE_UINT32(imm, CPUMBState),
> + VMSTATE_UINT32(iflags, CPUMBState),
> +
> + VMSTATE_UINT32(res_val, CPUMBState),
> + VMSTATE_UINTTL(res_addr, CPUMBState),
> +
> + VMSTATE_UINT32_ARRAY(pvr.regs, CPUMBState, 13),
pvr.regs are also elaboration time setup and should be read-only during
the VM's lifetime.
If you fix those, this looks good to me.:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 05/12] target/microblaze: Fill in VMStateDescription for cpu
2020-09-04 12:20 ` Edgar E. Iglesias
@ 2020-09-04 15:25 ` Richard Henderson
2020-09-04 15:31 ` Edgar Iglesias
0 siblings, 1 reply; 21+ messages in thread
From: Richard Henderson @ 2020-09-04 15:25 UTC (permalink / raw)
To: Edgar E. Iglesias; +Cc: thuth, qemu-devel, f4bug
On 9/4/20 5:20 AM, Edgar E. Iglesias wrote:
>> +static VMStateField vmstate_mmu_fields[] = {
>> + VMSTATE_UINT64_2DARRAY(rams, MicroBlazeMMU, 2, TLB_ENTRIES),
>> + VMSTATE_UINT8_ARRAY(tids, MicroBlazeMMU, TLB_ENTRIES),
>> + VMSTATE_UINT32_ARRAY(regs, MicroBlazeMMU, 3),
>> + VMSTATE_INT32(c_mmu, MicroBlazeMMU),
>> + VMSTATE_INT32(c_mmu_tlb_access, MicroBlazeMMU),
>> + VMSTATE_INT32(c_mmu_zones, MicroBlazeMMU),
>> + VMSTATE_UINT64(c_addr_mask, MicroBlazeMMU),
>
> These last 4 entries are elaboration-time settings, i.e they will not
> change during VM runtime. I don't think we need to transfer these since
> we expect the peer to setup the same kind of microblaze?
Ah, I see. Yes, migration is only supported between "like" systems.
Though in this case I wasn't thinking so much of migration and the other uses
to which VMState gets put, like record/replay and the follow-on patches for gdb
reverse debugging.
>> + VMSTATE_UINT32_ARRAY(pvr.regs, CPUMBState, 13),
>
> pvr.regs are also elaboration time setup and should be read-only during
> the VM's lifetime.
What do you think about moving all of these to cpu->cfg, so that all of the
configuration-time stuff is together?
r~
>
> If you fix those, this looks good to me.:
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v2 05/12] target/microblaze: Fill in VMStateDescription for cpu
2020-09-04 15:25 ` Richard Henderson
@ 2020-09-04 15:31 ` Edgar Iglesias
0 siblings, 0 replies; 21+ messages in thread
From: Edgar Iglesias @ 2020-09-04 15:31 UTC (permalink / raw)
To: Richard Henderson
Cc: thuth@redhat.com, qemu-devel@nongnu.org, f4bug@amsat.org
[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]
On 4 Sep 2020 17:25, Richard Henderson <richard.henderson@linaro.org> wrote:
>
> On 9/4/20 5:20 AM, Edgar E. Iglesias wrote:
> >> +static VMStateField vmstate_mmu_fields[] = {
> >> + VMSTATE_UINT64_2DARRAY(rams, MicroBlazeMMU, 2, TLB_ENTRIES),
> >> + VMSTATE_UINT8_ARRAY(tids, MicroBlazeMMU, TLB_ENTRIES),
> >> + VMSTATE_UINT32_ARRAY(regs, MicroBlazeMMU, 3),
> >> + VMSTATE_INT32(c_mmu, MicroBlazeMMU),
> >> + VMSTATE_INT32(c_mmu_tlb_access, MicroBlazeMMU),
> >> + VMSTATE_INT32(c_mmu_zones, MicroBlazeMMU),
> >> + VMSTATE_UINT64(c_addr_mask, MicroBlazeMMU),
> >
> > These last 4 entries are elaboration-time settings, i.e they will not
> > change during VM runtime. I don't think we need to transfer these since
> > we expect the peer to setup the same kind of microblaze?
>
> Ah, I see. Yes, migration is only supported between "like" systems.
>
> Though in this case I wasn't thinking so much of migration and the other uses
> to which VMState gets put, like record/replay and the follow-on patches for gdb
> reverse debugging.
>
> >> + VMSTATE_UINT32_ARRAY(pvr.regs, CPUMBState, 13),
> >
> > pvr.regs are also elaboration time setup and should be read-only during
> > the VM's lifetime.
>
> What do you think about moving all of these to cpu->cfg, so that all of the
> configuration-time stuff is together?
That would be great, thanks!
Btw, I was running tests on this series and saw some regressions but I've got other stuff moving in my tree so it may very well not be related but I'd like to make sure. Will probably take me a few days to figure things out...
Best regards,
Edgar
>
>
> r~
>
>
> >
> > If you fix those, this looks good to me.:
> > Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> >
>
[-- Attachment #2: Type: text/html, Size: 3242 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 06/12] target/microblaze: Rename DISAS_UPDATE to DISAS_EXIT
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
` (4 preceding siblings ...)
2020-09-03 7:26 ` [PATCH v2 05/12] target/microblaze: Fill in VMStateDescription for cpu Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-04 12:44 ` Philippe Mathieu-Daudé
2020-09-03 7:26 ` [PATCH v2 07/12] target/microblaze: Introduce DISAS_EXIT_NEXT, DISAS_EXIT_JUMP Richard Henderson
` (6 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
The name "update" suggests that something needs updating, but
this is not the case. Use "exit" to emphasize that nothing
needs doing except to exit.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/translate.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index a8a3249185..8ceb04f4f0 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -37,7 +37,7 @@
/* is_jmp field values */
#define DISAS_JUMP DISAS_TARGET_0 /* only pc was modified dynamically */
-#define DISAS_UPDATE DISAS_TARGET_1 /* cpu state was modified dynamically */
+#define DISAS_EXIT DISAS_TARGET_1 /* all cpu state modified dynamically */
static TCGv_i32 cpu_R[32];
static TCGv_i32 cpu_pc;
@@ -1161,7 +1161,7 @@ static bool trans_brk(DisasContext *dc, arg_typea_br *arg)
tcg_gen_ori_i32(cpu_msr, cpu_msr, MSR_BIP);
tcg_gen_movi_tl(cpu_res_addr, -1);
- dc->base.is_jmp = DISAS_UPDATE;
+ dc->base.is_jmp = DISAS_EXIT;
return true;
}
@@ -1202,7 +1202,7 @@ static bool trans_brki(DisasContext *dc, arg_typeb_br *arg)
~(MSR_VMS | MSR_UMS | MSR_VM | MSR_UM));
}
tcg_gen_ori_i32(cpu_msr, cpu_msr, msr_to_set);
- dc->base.is_jmp = DISAS_UPDATE;
+ dc->base.is_jmp = DISAS_EXIT;
#endif
return true;
@@ -1712,7 +1712,7 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
/* Force an exit if the per-tb cpu state has changed. */
if (dc->base.is_jmp == DISAS_NEXT && dc->cpustate_changed) {
- dc->base.is_jmp = DISAS_UPDATE;
+ dc->base.is_jmp = DISAS_EXIT;
tcg_gen_movi_i32(cpu_pc, dc->base.pc_next);
}
}
@@ -1733,7 +1733,7 @@ static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
gen_goto_tb(dc, 0, dc->base.pc_next);
return;
- case DISAS_UPDATE:
+ case DISAS_EXIT:
if (unlikely(cs->singlestep_enabled)) {
gen_raise_exception(dc, EXCP_DEBUG);
} else {
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 06/12] target/microblaze: Rename DISAS_UPDATE to DISAS_EXIT
2020-09-03 7:26 ` [PATCH v2 06/12] target/microblaze: Rename DISAS_UPDATE to DISAS_EXIT Richard Henderson
@ 2020-09-04 12:44 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-04 12:44 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: edgar.iglesias, thuth
On 9/3/20 9:26 AM, Richard Henderson wrote:
> The name "update" suggests that something needs updating, but
> this is not the case. Use "exit" to emphasize that nothing
> needs doing except to exit.
>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/microblaze/translate.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 07/12] target/microblaze: Introduce DISAS_EXIT_NEXT, DISAS_EXIT_JUMP
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
` (5 preceding siblings ...)
2020-09-03 7:26 ` [PATCH v2 06/12] target/microblaze: Rename DISAS_UPDATE to DISAS_EXIT Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-03 7:26 ` [PATCH v2 08/12] target/microblaze: Replace cpustate_changed with DISAS_EXIT_NEXT Richard Henderson
` (5 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
Like DISAS_EXIT, except we need to update cpu_pc,
either to pc_next or to btarget respectively.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/translate.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 8ceb04f4f0..2abef328a3 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -39,6 +39,11 @@
#define DISAS_JUMP DISAS_TARGET_0 /* only pc was modified dynamically */
#define DISAS_EXIT DISAS_TARGET_1 /* all cpu state modified dynamically */
+/* cpu state besides pc was modified dynamically; update pc to next */
+#define DISAS_EXIT_NEXT DISAS_TARGET_2
+/* cpu state besides pc was modified dynamically; update pc to btarget */
+#define DISAS_EXIT_JUMP DISAS_TARGET_3
+
static TCGv_i32 cpu_R[32];
static TCGv_i32 cpu_pc;
static TCGv_i32 cpu_msr;
@@ -1712,8 +1717,7 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
/* Force an exit if the per-tb cpu state has changed. */
if (dc->base.is_jmp == DISAS_NEXT && dc->cpustate_changed) {
- dc->base.is_jmp = DISAS_EXIT;
- tcg_gen_movi_i32(cpu_pc, dc->base.pc_next);
+ dc->base.is_jmp = DISAS_EXIT_NEXT;
}
}
@@ -1734,12 +1738,14 @@ static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
return;
case DISAS_EXIT:
- if (unlikely(cs->singlestep_enabled)) {
- gen_raise_exception(dc, EXCP_DEBUG);
- } else {
- tcg_gen_exit_tb(NULL, 0);
- }
- return;
+ break;
+ case DISAS_EXIT_NEXT:
+ tcg_gen_movi_i32(cpu_pc, dc->base.pc_next);
+ break;
+ case DISAS_EXIT_JUMP:
+ tcg_gen_mov_i32(cpu_pc, cpu_btarget);
+ tcg_gen_discard_i32(cpu_btarget);
+ break;
case DISAS_JUMP:
if (dc->jmp_dest != -1 && !cs->singlestep_enabled) {
@@ -1781,6 +1787,13 @@ static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
default:
g_assert_not_reached();
}
+
+ /* Finish DISAS_EXIT_* */
+ if (unlikely(cs->singlestep_enabled)) {
+ gen_raise_exception(dc, EXCP_DEBUG);
+ } else {
+ tcg_gen_exit_tb(NULL, 0);
+ }
}
static void mb_tr_disas_log(const DisasContextBase *dcb, CPUState *cs)
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 08/12] target/microblaze: Replace cpustate_changed with DISAS_EXIT_NEXT
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
` (6 preceding siblings ...)
2020-09-03 7:26 ` [PATCH v2 07/12] target/microblaze: Introduce DISAS_EXIT_NEXT, DISAS_EXIT_JUMP Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-03 7:26 ` [PATCH v2 09/12] target/microblaze: Handle DISAS_EXIT_NEXT in delay slot Richard Henderson
` (4 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
Rather than look for the combination of DISAS_NEXT with a separate
variable, go ahead and set is_jmp to the desired state.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/translate.c | 34 ++++++++++------------------------
1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 2abef328a3..6bf299a826 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -70,7 +70,6 @@ typedef struct DisasContext {
/* Decoder. */
uint32_t ext_imm;
- unsigned int cpustate_changed;
unsigned int tb_flags;
unsigned int tb_flags_to_set;
int mem_index;
@@ -1255,7 +1254,7 @@ static bool trans_mbar(DisasContext *dc, arg_mbar *arg)
*
* Therefore, choose to end the TB always.
*/
- dc->cpustate_changed = 1;
+ dc->base.is_jmp = DISAS_EXIT_NEXT;
return true;
}
@@ -1307,19 +1306,6 @@ static void msr_read(DisasContext *dc, TCGv_i32 d)
tcg_temp_free_i32(t);
}
-#ifndef CONFIG_USER_ONLY
-static void msr_write(DisasContext *dc, TCGv_i32 v)
-{
- dc->cpustate_changed = 1;
-
- /* Install MSR_C. */
- tcg_gen_extract_i32(cpu_msr_c, v, 2, 1);
-
- /* Clear MSR_C and MSR_CC; MSR_PVR is not writable, and is always clear. */
- tcg_gen_andi_i32(cpu_msr, v, ~(MSR_C | MSR_CC | MSR_PVR));
-}
-#endif
-
static bool do_msrclrset(DisasContext *dc, arg_type_msr *arg, bool set)
{
uint32_t imm = arg->imm;
@@ -1352,7 +1338,7 @@ static bool do_msrclrset(DisasContext *dc, arg_type_msr *arg, bool set)
} else {
tcg_gen_andi_i32(cpu_msr, cpu_msr, ~imm);
}
- dc->cpustate_changed = 1;
+ dc->base.is_jmp = DISAS_EXIT_NEXT;
}
return true;
}
@@ -1385,7 +1371,13 @@ static bool trans_mts(DisasContext *dc, arg_mts *arg)
TCGv_i32 src = reg_for_read(dc, arg->ra);
switch (arg->rs) {
case SR_MSR:
- msr_write(dc, src);
+ /* Install MSR_C. */
+ tcg_gen_extract_i32(cpu_msr_c, src, 2, 1);
+ /*
+ * Clear MSR_C and MSR_CC;
+ * MSR_PVR is not writable, and is always clear.
+ */
+ tcg_gen_andi_i32(cpu_msr, src, ~(MSR_C | MSR_CC | MSR_PVR));
break;
case SR_FSR:
tcg_gen_st_i32(src, cpu_env, offsetof(CPUMBState, fsr));
@@ -1417,7 +1409,7 @@ static bool trans_mts(DisasContext *dc, arg_mts *arg)
qemu_log_mask(LOG_GUEST_ERROR, "Invalid mts reg 0x%x\n", arg->rs);
return true;
}
- dc->cpustate_changed = 1;
+ dc->base.is_jmp = DISAS_EXIT_NEXT;
return true;
#endif
}
@@ -1629,7 +1621,6 @@ static void mb_tr_init_disas_context(DisasContextBase *dcb, CPUState *cs)
dc->cpu = cpu;
dc->tb_flags = dc->base.tb->flags;
- dc->cpustate_changed = 0;
dc->ext_imm = dc->base.tb->cs_base;
dc->r0 = NULL;
dc->r0_set = false;
@@ -1714,11 +1705,6 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
}
dc->base.is_jmp = DISAS_JUMP;
}
-
- /* Force an exit if the per-tb cpu state has changed. */
- if (dc->base.is_jmp == DISAS_NEXT && dc->cpustate_changed) {
- dc->base.is_jmp = DISAS_EXIT_NEXT;
- }
}
static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 09/12] target/microblaze: Handle DISAS_EXIT_NEXT in delay slot
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
` (7 preceding siblings ...)
2020-09-03 7:26 ` [PATCH v2 08/12] target/microblaze: Replace cpustate_changed with DISAS_EXIT_NEXT Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-03 7:26 ` [PATCH v2 10/12] target/microblaze: Force rtid, rted, rtbd to exit Richard Henderson
` (3 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
It is legal to put an mts instruction into a delay slot.
We should continue to return to the main loop in that
case so that we recognize any pending interrupts.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/translate.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 6bf299a826..608d413c83 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1696,6 +1696,10 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
dc->base.pc_next += 4;
if (dc->jmp_cond != TCG_COND_NEVER && !(dc->tb_flags & D_FLAG)) {
+ /*
+ * Finish any return-from branch.
+ * TODO: Diagnose rtXd in delay slot of rtYd earlier.
+ */
if (dc->tb_flags & DRTI_FLAG) {
do_rti(dc);
} else if (dc->tb_flags & DRTB_FLAG) {
@@ -1703,7 +1707,35 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
} else if (dc->tb_flags & DRTE_FLAG) {
do_rte(dc);
}
- dc->base.is_jmp = DISAS_JUMP;
+
+ /* Complete the branch, ending the TB. */
+ switch (dc->base.is_jmp) {
+ case DISAS_NORETURN:
+ /*
+ * E.g. illegal insn in a delay slot. We've already exited
+ * and will handle D_FLAG in mb_cpu_do_interrupt.
+ */
+ break;
+ case DISAS_EXIT:
+ /*
+ * TODO: diagnose brk/brki in delay slot earlier.
+ * This would then fold into the illegal insn case above.
+ */
+ break;
+ case DISAS_NEXT:
+ /* Normal insn a delay slot. */
+ dc->base.is_jmp = DISAS_JUMP;
+ break;
+ case DISAS_EXIT_NEXT:
+ /*
+ * E.g. mts insn in a delay slot. Continue with btarget,
+ * but still return to the main loop.
+ */
+ dc->base.is_jmp = DISAS_EXIT_JUMP;
+ break;
+ default:
+ g_assert_not_reached();
+ }
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 10/12] target/microblaze: Force rtid, rted, rtbd to exit
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
` (8 preceding siblings ...)
2020-09-03 7:26 ` [PATCH v2 09/12] target/microblaze: Handle DISAS_EXIT_NEXT in delay slot Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-03 7:26 ` [PATCH v2 11/12] target/microblaze: Use tcg_gen_lookup_and_goto_ptr Richard Henderson
` (2 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
These return-from-exception type instructions have modified
MSR to re-enable various forms of interrupt. Force a return
to the main loop.
Consolidate the cleanup of tb_flags into mb_tr_translate_insn.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/translate.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 608d413c83..da84fdb20b 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1518,7 +1518,6 @@ static void do_rti(DisasContext *dc)
tcg_gen_or_i32(cpu_msr, cpu_msr, tmp);
tcg_temp_free_i32(tmp);
- dc->tb_flags &= ~DRTI_FLAG;
}
static void do_rtb(DisasContext *dc)
@@ -1531,7 +1530,6 @@ static void do_rtb(DisasContext *dc)
tcg_gen_or_i32(cpu_msr, cpu_msr, tmp);
tcg_temp_free_i32(tmp);
- dc->tb_flags &= ~DRTB_FLAG;
}
static void do_rte(DisasContext *dc)
@@ -1545,7 +1543,6 @@ static void do_rte(DisasContext *dc)
tcg_gen_or_i32(cpu_msr, cpu_msr, tmp);
tcg_temp_free_i32(tmp);
- dc->tb_flags &= ~DRTE_FLAG;
}
/* Insns connected to FSL or AXI stream attached devices. */
@@ -1700,12 +1697,16 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
* Finish any return-from branch.
* TODO: Diagnose rtXd in delay slot of rtYd earlier.
*/
- if (dc->tb_flags & DRTI_FLAG) {
- do_rti(dc);
- } else if (dc->tb_flags & DRTB_FLAG) {
- do_rtb(dc);
- } else if (dc->tb_flags & DRTE_FLAG) {
- do_rte(dc);
+ uint32_t rt_ibe = dc->tb_flags & (DRTI_FLAG | DRTB_FLAG | DRTE_FLAG);
+ if (unlikely(rt_ibe != 0)) {
+ dc->tb_flags &= ~(DRTI_FLAG | DRTB_FLAG | DRTE_FLAG);
+ if (rt_ibe & DRTI_FLAG) {
+ do_rti(dc);
+ } else if (rt_ibe & DRTB_FLAG) {
+ do_rtb(dc);
+ } else {
+ do_rte(dc);
+ }
}
/* Complete the branch, ending the TB. */
@@ -1723,8 +1724,12 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
*/
break;
case DISAS_NEXT:
- /* Normal insn a delay slot. */
- dc->base.is_jmp = DISAS_JUMP;
+ /*
+ * Normal insn a delay slot.
+ * However, the return-from-exception type insns should
+ * return to the main loop, as they have adjusted MSR.
+ */
+ dc->base.is_jmp = (rt_ibe ? DISAS_EXIT_JUMP : DISAS_JUMP);
break;
case DISAS_EXIT_NEXT:
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 11/12] target/microblaze: Use tcg_gen_lookup_and_goto_ptr
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
` (9 preceding siblings ...)
2020-09-03 7:26 ` [PATCH v2 10/12] target/microblaze: Force rtid, rted, rtbd to exit Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-03 7:26 ` [PATCH v2 12/12] target/microblaze: Diagnose invalid insns in delay slots Richard Henderson
2020-09-03 12:24 ` [PATCH v2 00/12] target/microblaze improvements Thomas Huth
12 siblings, 0 replies; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
Normal indirect jumps, or page-crossing direct jumps, can use
tcg_gen_lookup_and_goto_ptr to avoid returning to the main loop
simply to find an existing TB for the next pc.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index da84fdb20b..d98572fab9 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -147,7 +147,7 @@ static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
tcg_gen_exit_tb(dc->base.tb, n);
} else {
tcg_gen_movi_i32(cpu_pc, dest);
- tcg_gen_exit_tb(NULL, 0);
+ tcg_gen_lookup_and_goto_ptr();
}
dc->base.is_jmp = DISAS_NORETURN;
}
@@ -1803,7 +1803,7 @@ static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
if (unlikely(cs->singlestep_enabled)) {
gen_raise_exception(dc, EXCP_DEBUG);
} else {
- tcg_gen_exit_tb(NULL, 0);
+ tcg_gen_lookup_and_goto_ptr();
}
return;
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 12/12] target/microblaze: Diagnose invalid insns in delay slots
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
` (10 preceding siblings ...)
2020-09-03 7:26 ` [PATCH v2 11/12] target/microblaze: Use tcg_gen_lookup_and_goto_ptr Richard Henderson
@ 2020-09-03 7:26 ` Richard Henderson
2020-09-04 12:27 ` Edgar E. Iglesias
2020-09-03 12:24 ` [PATCH v2 00/12] target/microblaze improvements Thomas Huth
12 siblings, 1 reply; 21+ messages in thread
From: Richard Henderson @ 2020-09-03 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, thuth, f4bug
These cases result in undefined and undocumented behaviour but the
behaviour is deterministic, i.e cores will not lock-up or expose
security issues. However, RTL will not raise exceptions either.
Therefore, log a GUEST_ERROR and treat these cases as nops, to
avoid corner cases which could put qemu into an invalid state.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v2: Log pc as well.
---
target/microblaze/translate.c | 48 ++++++++++++++++++++++++++++++-----
1 file changed, 41 insertions(+), 7 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index d98572fab9..ff0cb7dbb6 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -179,6 +179,21 @@ static bool trap_userspace(DisasContext *dc, bool cond)
return cond_user;
}
+/*
+ * Return true, and log an error, if the current insn is
+ * within a delay slot.
+ */
+static bool invalid_delay_slot(DisasContext *dc, const char *insn_type)
+{
+ if (dc->tb_flags & D_FLAG) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Invalid insn in delay slot: %s at %08x\n",
+ insn_type, (uint32_t)dc->base.pc_next);
+ return true;
+ }
+ return false;
+}
+
static TCGv_i32 reg_for_read(DisasContext *dc, int reg)
{
if (likely(reg != 0)) {
@@ -500,6 +515,9 @@ DO_TYPEA_CFG(idivu, use_div, true, gen_idivu)
static bool trans_imm(DisasContext *dc, arg_imm *arg)
{
+ if (invalid_delay_slot(dc, "imm")) {
+ return true;
+ }
dc->ext_imm = arg->imm << 16;
tcg_gen_movi_i32(cpu_imm, dc->ext_imm);
dc->tb_flags_to_set = IMM_FLAG;
@@ -1067,6 +1085,9 @@ static bool do_branch(DisasContext *dc, int dest_rb, int dest_imm,
{
uint32_t add_pc;
+ if (invalid_delay_slot(dc, "branch")) {
+ return true;
+ }
if (delay) {
setup_dslot(dc, dest_rb < 0);
}
@@ -1106,6 +1127,9 @@ static bool do_bcc(DisasContext *dc, int dest_rb, int dest_imm,
{
TCGv_i32 zero, next;
+ if (invalid_delay_slot(dc, "bcc")) {
+ return true;
+ }
if (delay) {
setup_dslot(dc, dest_rb < 0);
}
@@ -1158,6 +1182,10 @@ static bool trans_brk(DisasContext *dc, arg_typea_br *arg)
if (trap_userspace(dc, true)) {
return true;
}
+ if (invalid_delay_slot(dc, "brk")) {
+ return true;
+ }
+
tcg_gen_mov_i32(cpu_pc, reg_for_read(dc, arg->rb));
if (arg->rd) {
tcg_gen_movi_i32(cpu_R[arg->rd], dc->base.pc_next);
@@ -1176,6 +1204,10 @@ static bool trans_brki(DisasContext *dc, arg_typeb_br *arg)
if (trap_userspace(dc, imm != 0x8 && imm != 0x18)) {
return true;
}
+ if (invalid_delay_slot(dc, "brki")) {
+ return true;
+ }
+
tcg_gen_movi_i32(cpu_pc, imm);
if (arg->rd) {
tcg_gen_movi_i32(cpu_R[arg->rd], dc->base.pc_next);
@@ -1216,6 +1248,11 @@ static bool trans_mbar(DisasContext *dc, arg_mbar *arg)
{
int mbar_imm = arg->imm;
+ /* Note that mbar is a specialized branch instruction. */
+ if (invalid_delay_slot(dc, "mbar")) {
+ return true;
+ }
+
/* Data access memory barrier. */
if ((mbar_imm & 2) == 0) {
tcg_gen_mb(TCG_BAR_SC | TCG_MO_ALL);
@@ -1263,6 +1300,10 @@ static bool do_rts(DisasContext *dc, arg_typeb_bc *arg, int to_set)
if (trap_userspace(dc, to_set)) {
return true;
}
+ if (invalid_delay_slot(dc, "rts")) {
+ return true;
+ }
+
dc->tb_flags_to_set |= to_set;
setup_dslot(dc, true);
@@ -1695,7 +1736,6 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
if (dc->jmp_cond != TCG_COND_NEVER && !(dc->tb_flags & D_FLAG)) {
/*
* Finish any return-from branch.
- * TODO: Diagnose rtXd in delay slot of rtYd earlier.
*/
uint32_t rt_ibe = dc->tb_flags & (DRTI_FLAG | DRTB_FLAG | DRTE_FLAG);
if (unlikely(rt_ibe != 0)) {
@@ -1717,12 +1757,6 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
* and will handle D_FLAG in mb_cpu_do_interrupt.
*/
break;
- case DISAS_EXIT:
- /*
- * TODO: diagnose brk/brki in delay slot earlier.
- * This would then fold into the illegal insn case above.
- */
- break;
case DISAS_NEXT:
/*
* Normal insn a delay slot.
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v2 12/12] target/microblaze: Diagnose invalid insns in delay slots
2020-09-03 7:26 ` [PATCH v2 12/12] target/microblaze: Diagnose invalid insns in delay slots Richard Henderson
@ 2020-09-04 12:27 ` Edgar E. Iglesias
0 siblings, 0 replies; 21+ messages in thread
From: Edgar E. Iglesias @ 2020-09-04 12:27 UTC (permalink / raw)
To: Richard Henderson; +Cc: thuth, qemu-devel, f4bug
On Thu, Sep 03, 2020 at 12:26:50AM -0700, Richard Henderson wrote:
> These cases result in undefined and undocumented behaviour but the
> behaviour is deterministic, i.e cores will not lock-up or expose
> security issues. However, RTL will not raise exceptions either.
>
> Therefore, log a GUEST_ERROR and treat these cases as nops, to
> avoid corner cases which could put qemu into an invalid state.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v2: Log pc as well.
> ---
> target/microblaze/translate.c | 48 ++++++++++++++++++++++++++++++-----
> 1 file changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index d98572fab9..ff0cb7dbb6 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -179,6 +179,21 @@ static bool trap_userspace(DisasContext *dc, bool cond)
> return cond_user;
> }
>
> +/*
> + * Return true, and log an error, if the current insn is
> + * within a delay slot.
> + */
> +static bool invalid_delay_slot(DisasContext *dc, const char *insn_type)
> +{
> + if (dc->tb_flags & D_FLAG) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "Invalid insn in delay slot: %s at %08x\n",
> + insn_type, (uint32_t)dc->base.pc_next);
> + return true;
> + }
> + return false;
> +}
> +
> static TCGv_i32 reg_for_read(DisasContext *dc, int reg)
> {
> if (likely(reg != 0)) {
> @@ -500,6 +515,9 @@ DO_TYPEA_CFG(idivu, use_div, true, gen_idivu)
>
> static bool trans_imm(DisasContext *dc, arg_imm *arg)
> {
> + if (invalid_delay_slot(dc, "imm")) {
> + return true;
> + }
> dc->ext_imm = arg->imm << 16;
> tcg_gen_movi_i32(cpu_imm, dc->ext_imm);
> dc->tb_flags_to_set = IMM_FLAG;
> @@ -1067,6 +1085,9 @@ static bool do_branch(DisasContext *dc, int dest_rb, int dest_imm,
> {
> uint32_t add_pc;
>
> + if (invalid_delay_slot(dc, "branch")) {
> + return true;
> + }
> if (delay) {
> setup_dslot(dc, dest_rb < 0);
> }
> @@ -1106,6 +1127,9 @@ static bool do_bcc(DisasContext *dc, int dest_rb, int dest_imm,
> {
> TCGv_i32 zero, next;
>
> + if (invalid_delay_slot(dc, "bcc")) {
> + return true;
> + }
> if (delay) {
> setup_dslot(dc, dest_rb < 0);
> }
> @@ -1158,6 +1182,10 @@ static bool trans_brk(DisasContext *dc, arg_typea_br *arg)
> if (trap_userspace(dc, true)) {
> return true;
> }
> + if (invalid_delay_slot(dc, "brk")) {
> + return true;
> + }
> +
> tcg_gen_mov_i32(cpu_pc, reg_for_read(dc, arg->rb));
> if (arg->rd) {
> tcg_gen_movi_i32(cpu_R[arg->rd], dc->base.pc_next);
> @@ -1176,6 +1204,10 @@ static bool trans_brki(DisasContext *dc, arg_typeb_br *arg)
> if (trap_userspace(dc, imm != 0x8 && imm != 0x18)) {
> return true;
> }
> + if (invalid_delay_slot(dc, "brki")) {
> + return true;
> + }
> +
> tcg_gen_movi_i32(cpu_pc, imm);
> if (arg->rd) {
> tcg_gen_movi_i32(cpu_R[arg->rd], dc->base.pc_next);
> @@ -1216,6 +1248,11 @@ static bool trans_mbar(DisasContext *dc, arg_mbar *arg)
> {
> int mbar_imm = arg->imm;
>
> + /* Note that mbar is a specialized branch instruction. */
> + if (invalid_delay_slot(dc, "mbar")) {
> + return true;
> + }
> +
> /* Data access memory barrier. */
> if ((mbar_imm & 2) == 0) {
> tcg_gen_mb(TCG_BAR_SC | TCG_MO_ALL);
> @@ -1263,6 +1300,10 @@ static bool do_rts(DisasContext *dc, arg_typeb_bc *arg, int to_set)
> if (trap_userspace(dc, to_set)) {
> return true;
> }
> + if (invalid_delay_slot(dc, "rts")) {
> + return true;
> + }
> +
> dc->tb_flags_to_set |= to_set;
> setup_dslot(dc, true);
>
> @@ -1695,7 +1736,6 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
> if (dc->jmp_cond != TCG_COND_NEVER && !(dc->tb_flags & D_FLAG)) {
> /*
> * Finish any return-from branch.
> - * TODO: Diagnose rtXd in delay slot of rtYd earlier.
> */
> uint32_t rt_ibe = dc->tb_flags & (DRTI_FLAG | DRTB_FLAG | DRTE_FLAG);
> if (unlikely(rt_ibe != 0)) {
> @@ -1717,12 +1757,6 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
> * and will handle D_FLAG in mb_cpu_do_interrupt.
> */
> break;
> - case DISAS_EXIT:
> - /*
> - * TODO: diagnose brk/brki in delay slot earlier.
> - * This would then fold into the illegal insn case above.
> - */
> - break;
> case DISAS_NEXT:
> /*
> * Normal insn a delay slot.
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 00/12] target/microblaze improvements
2020-09-03 7:26 [PATCH v2 00/12] target/microblaze improvements Richard Henderson
` (11 preceding siblings ...)
2020-09-03 7:26 ` [PATCH v2 12/12] target/microblaze: Diagnose invalid insns in delay slots Richard Henderson
@ 2020-09-03 12:24 ` Thomas Huth
12 siblings, 0 replies; 21+ messages in thread
From: Thomas Huth @ 2020-09-03 12:24 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: edgar.iglesias, f4bug
On 03/09/2020 09.26, Richard Henderson wrote:
> Version 2 includes fixes for iflags that could cause lockups.
>
> It seems it was easier to do so with icount=7, which is what we do during
> the replay acceptance tests. This causes TBs to contain no more than 7
> insns, and often less to make up for an incomplete count elsewhere.
> Which stressed the iflags bits around delay slots and imm in ways that
> pure single-step doesn't.
>
> In addition, cpu vmstate is filled in and interrupt logging is tidied.
Thanks, this fixes the failing acceptance test in the Gitlab-CI for me:
https://gitlab.com/huth/qemu/-/pipelines/185245009
Tested-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 21+ messages in thread