* [Qemu-devel] [PULL v4 01/12] lm32_sys: increase test case name length limit
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-02-01 17:39 ` Peter Maydell
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 02/12] tests: lm32: new rule for single test cases Michael Walle
` (11 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Anthony Liguori
The new MMU tests use longer names.
Signed-off-by: Michael Walle <michael@walle.cc>
---
hw/misc/lm32_sys.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/misc/lm32_sys.c b/hw/misc/lm32_sys.c
index 9bdb781..8176cdb 100644
--- a/hw/misc/lm32_sys.c
+++ b/hw/misc/lm32_sys.c
@@ -42,7 +42,7 @@ enum {
R_MAX
};
-#define MAX_TESTNAME_LEN 16
+#define MAX_TESTNAME_LEN 32
#define TYPE_LM32_SYS "lm32-sys"
#define LM32_SYS(obj) OBJECT_CHECK(LM32SysState, (obj), TYPE_LM32_SYS)
@@ -80,7 +80,7 @@ static void sys_write(void *opaque, hwaddr addr,
case R_PASSFAIL:
s->regs[addr] = value;
testname = (char *)s->testname;
- qemu_log("TC %-16s %s\n", testname, (value) ? "FAILED" : "OK");
+ qemu_log("TC %-32s %s\n", testname, (value) ? "FAILED" : "OK");
break;
case R_TESTNAME:
s->regs[addr] = value;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 01/12] lm32_sys: increase test case name length limit
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 01/12] lm32_sys: increase test case name length limit Michael Walle
@ 2014-02-01 17:39 ` Peter Maydell
0 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2014-02-01 17:39 UTC (permalink / raw)
To: Michael Walle; +Cc: QEMU Developers, Anthony Liguori
On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
> The new MMU tests use longer names.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> hw/misc/lm32_sys.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/misc/lm32_sys.c b/hw/misc/lm32_sys.c
> index 9bdb781..8176cdb 100644
> --- a/hw/misc/lm32_sys.c
> +++ b/hw/misc/lm32_sys.c
> @@ -42,7 +42,7 @@ enum {
> R_MAX
> };
>
> -#define MAX_TESTNAME_LEN 16
> +#define MAX_TESTNAME_LEN 32
>
> #define TYPE_LM32_SYS "lm32-sys"
> #define LM32_SYS(obj) OBJECT_CHECK(LM32SysState, (obj), TYPE_LM32_SYS)
> @@ -80,7 +80,7 @@ static void sys_write(void *opaque, hwaddr addr,
> case R_PASSFAIL:
> s->regs[addr] = value;
> testname = (char *)s->testname;
> - qemu_log("TC %-16s %s\n", testname, (value) ? "FAILED" : "OK");
> + qemu_log("TC %-32s %s\n", testname, (value) ? "FAILED" : "OK");
You could avoid the duplication of the 32 here by using
qemu_log("TCG %-*s %s\n", MAX_TESTNAME_LEN, testname, value ?
"FAILED" : "OK");
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL v4 02/12] tests: lm32: new rule for single test cases
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 01/12] lm32_sys: increase test case name length limit Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-02-01 17:45 ` Peter Maydell
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 03/12] milkymist-uart: use qemu_chr_fe_write_all() instead of qemu_chr_fe_write() Michael Walle
` (10 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Anthony Liguori
Introduce new target "check_%" to run indiviudal test caes, eg.
make check_mmu
Signed-off-by: Michael Walle <michael@walle.cc>
---
tests/tcg/lm32/Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/tcg/lm32/Makefile b/tests/tcg/lm32/Makefile
index 9a00ef7..19e0664 100644
--- a/tests/tcg/lm32/Makefile
+++ b/tests/tcg/lm32/Makefile
@@ -101,5 +101,8 @@ check: $(CRT) $(SYS) $(TESTCASES)
$(SIM) $(SIMFLAGS) ./$$case; \
done
+check_%: test_%.tst $(CRT) $(SYS)
+ $(SIM) $(SIMFLAGS) $<
+
clean:
$(RM) -fr $(TESTCASES) $(CRT)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 02/12] tests: lm32: new rule for single test cases
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 02/12] tests: lm32: new rule for single test cases Michael Walle
@ 2014-02-01 17:45 ` Peter Maydell
0 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2014-02-01 17:45 UTC (permalink / raw)
To: Michael Walle; +Cc: QEMU Developers, Anthony Liguori
On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
> Introduce new target "check_%" to run indiviudal test caes, eg.
> make check_mmu
"individual". Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> tests/tcg/lm32/Makefile | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tests/tcg/lm32/Makefile b/tests/tcg/lm32/Makefile
> index 9a00ef7..19e0664 100644
> --- a/tests/tcg/lm32/Makefile
> +++ b/tests/tcg/lm32/Makefile
> @@ -101,5 +101,8 @@ check: $(CRT) $(SYS) $(TESTCASES)
> $(SIM) $(SIMFLAGS) ./$$case; \
> done
>
> +check_%: test_%.tst $(CRT) $(SYS)
> + $(SIM) $(SIMFLAGS) $<
> +
> clean:
> $(RM) -fr $(TESTCASES) $(CRT)
You could reimplement the 'check' target to just
depend on $(TESTCASES:test_%.tst=check_%)
rather than doing an explicit loop if you like, but that's optional.
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL v4 03/12] milkymist-uart: use qemu_chr_fe_write_all() instead of qemu_chr_fe_write()
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 01/12] lm32_sys: increase test case name length limit Michael Walle
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 02/12] tests: lm32: new rule for single test cases Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-02-01 17:46 ` Peter Maydell
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 04/12] lm32_uart/lm32_juart: use qemu_chr_fe_write_all() Michael Walle
` (9 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Antony Pavlov, Anthony Liguori
From: Antony Pavlov <antonynpavlov@gmail.com>
qemu_chr_fe_write() is capable of returning 0
to indicate EAGAIN (and friends) and we don't
handle this.
Just change it to qemu_chr_fe_write_all() to fix.
Reported-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Acked-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
Signed-off-by: Michael Walle <michael@walle.cc>
---
hw/char/milkymist-uart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/char/milkymist-uart.c b/hw/char/milkymist-uart.c
index 2c52a0f..da51f82 100644
--- a/hw/char/milkymist-uart.c
+++ b/hw/char/milkymist-uart.c
@@ -124,7 +124,7 @@ static void uart_write(void *opaque, hwaddr addr, uint64_t value,
switch (addr) {
case R_RXTX:
if (s->chr) {
- qemu_chr_fe_write(s->chr, &ch, 1);
+ qemu_chr_fe_write_all(s->chr, &ch, 1);
}
s->regs[R_STAT] |= STAT_TX_EVT;
break;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL v4 04/12] lm32_uart/lm32_juart: use qemu_chr_fe_write_all()
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
` (2 preceding siblings ...)
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 03/12] milkymist-uart: use qemu_chr_fe_write_all() instead of qemu_chr_fe_write() Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-02-01 17:47 ` Peter Maydell
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 05/12] milkymist-vgafb: swap pixel data in source buffer Michael Walle
` (8 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Anthony Liguori
qemu_chr_fe_write() may return EAGAIN. Therefore, use
qemu_chr_fe_write_all().
Signed-off-by: Michael Walle <michael@walle.cc>
---
hw/char/lm32_juart.c | 2 +-
hw/char/lm32_uart.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/char/lm32_juart.c b/hw/char/lm32_juart.c
index 252fe46..380cb5d 100644
--- a/hw/char/lm32_juart.c
+++ b/hw/char/lm32_juart.c
@@ -75,7 +75,7 @@ void lm32_juart_set_jtx(DeviceState *d, uint32_t jtx)
s->jtx = jtx;
if (s->chr) {
- qemu_chr_fe_write(s->chr, &ch, 1);
+ qemu_chr_fe_write_all(s->chr, &ch, 1);
}
}
diff --git a/hw/char/lm32_uart.c b/hw/char/lm32_uart.c
index 85d7265..84c2549 100644
--- a/hw/char/lm32_uart.c
+++ b/hw/char/lm32_uart.c
@@ -177,7 +177,7 @@ static void uart_write(void *opaque, hwaddr addr,
switch (addr) {
case R_RXTX:
if (s->chr) {
- qemu_chr_fe_write(s->chr, &ch, 1);
+ qemu_chr_fe_write_all(s->chr, &ch, 1);
}
break;
case R_IER:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL v4 05/12] milkymist-vgafb: swap pixel data in source buffer
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
` (3 preceding siblings ...)
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 04/12] lm32_uart/lm32_juart: use qemu_chr_fe_write_all() Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-02-01 17:57 ` Peter Maydell
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 06/12] target-lm32: kill cpu_abort() calls Michael Walle
` (7 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Michael Walle, Anthony Liguori
In commit fc97bb5ba3e7239c0b6d24095df6784868dfebbf the lduw_raw() call was
eliminated. But we are reading from the target buffer a 16-bit value, which
is in big-endian format. Therefore, swap the bytes if we are building for a
little-endian host.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Walle <michael@walle.cc>
---
hw/display/milkymist-vgafb_template.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/display/milkymist-vgafb_template.h b/hw/display/milkymist-vgafb_template.h
index e0036e1..3f25484 100644
--- a/hw/display/milkymist-vgafb_template.h
+++ b/hw/display/milkymist-vgafb_template.h
@@ -62,6 +62,7 @@ static void glue(draw_line_, BITS)(void *opaque, uint8_t *d, const uint8_t *s,
while (width--) {
memcpy(&rgb565, s, sizeof(rgb565));
+ rgb565 = be16_to_cpu(rgb565);
r = ((rgb565 >> 11) & 0x1f) << 3;
g = ((rgb565 >> 5) & 0x3f) << 2;
b = ((rgb565 >> 0) & 0x1f) << 3;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 05/12] milkymist-vgafb: swap pixel data in source buffer
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 05/12] milkymist-vgafb: swap pixel data in source buffer Michael Walle
@ 2014-02-01 17:57 ` Peter Maydell
2014-02-03 8:12 ` Paolo Bonzini
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2014-02-01 17:57 UTC (permalink / raw)
To: Michael Walle; +Cc: Paolo Bonzini, QEMU Developers, Anthony Liguori
On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
> In commit fc97bb5ba3e7239c0b6d24095df6784868dfebbf the lduw_raw() call was
> eliminated. But we are reading from the target buffer a 16-bit value, which
> is in big-endian format. Therefore, swap the bytes if we are building for a
> little-endian host.
Paolo, can you remember why you included this change in that commit?
It purports to just be moving the display devices around but it seems to
have included the introduction of this bug, and also a removal of a lduw_raw()
call from (what is now) hw/display/blizzard_template.h which I suspect is
also wrong...
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> hw/display/milkymist-vgafb_template.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/display/milkymist-vgafb_template.h b/hw/display/milkymist-vgafb_template.h
> index e0036e1..3f25484 100644
> --- a/hw/display/milkymist-vgafb_template.h
> +++ b/hw/display/milkymist-vgafb_template.h
> @@ -62,6 +62,7 @@ static void glue(draw_line_, BITS)(void *opaque, uint8_t *d, const uint8_t *s,
>
> while (width--) {
> memcpy(&rgb565, s, sizeof(rgb565));
> + rgb565 = be16_to_cpu(rgb565);
If we know the framebuffer is always bigendian (regardless of the
target CPU endianness) then rather than memcpy and then
byteswap we might as well just
rgb565 = lduw_be_p(s);
I think.
> r = ((rgb565 >> 11) & 0x1f) << 3;
> g = ((rgb565 >> 5) & 0x3f) << 2;
> b = ((rgb565 >> 0) & 0x1f) << 3;
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 05/12] milkymist-vgafb: swap pixel data in source buffer
2014-02-01 17:57 ` Peter Maydell
@ 2014-02-03 8:12 ` Paolo Bonzini
0 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2014-02-03 8:12 UTC (permalink / raw)
To: Peter Maydell, Michael Walle; +Cc: QEMU Developers, Anthony Liguori
Il 01/02/2014 18:57, Peter Maydell ha scritto:
>> > In commit fc97bb5ba3e7239c0b6d24095df6784868dfebbf the lduw_raw() call was
>> > eliminated. But we are reading from the target buffer a 16-bit value, which
>> > is in big-endian format. Therefore, swap the bytes if we are building for a
>> > little-endian host.
> Paolo, can you remember why you included this change in that commit?
> It purports to just be moving the display devices around but it seems to
> have included the introduction of this bug, and also a removal of a lduw_raw()
> call from (what is now) hw/display/blizzard_template.h which I suspect is
> also wrong...
>
Most likely it was an incorrectly squashed patch.
Paolo
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL v4 06/12] target-lm32: kill cpu_abort() calls
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
` (4 preceding siblings ...)
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 05/12] milkymist-vgafb: swap pixel data in source buffer Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 07/12] target-lm32: move model features to LM32CPU Michael Walle
` (6 subsequent siblings)
12 siblings, 0 replies; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Anthony Liguori
Instead of killing QEMU, translate instructions which are not available on
the CPU model as a noop and issue a log message at translation time.
On the real hardware CPU unknown opcodes results in undefined behaviour.
These changes prepare the removal of CPULM32State from DisasContext.
Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Andreas Färber <afaerber@suse.de>
---
target-lm32/translate.c | 72 ++++++++++++++++++++++++++---------------------
1 file changed, 40 insertions(+), 32 deletions(-)
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index 6ea0ecd..e292e1c 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -80,7 +80,6 @@ typedef struct DisasContext {
unsigned int tb_flags, synced_flags; /* tb dependent flags. */
int is_jmp;
- int nr_nops;
struct TranslationBlock *tb;
int singlestep_enabled;
} DisasContext;
@@ -422,7 +421,8 @@ static void dec_divu(DisasContext *dc)
LOG_DIS("divu r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1);
if (!(dc->env->features & LM32_FEATURE_DIVIDE)) {
- cpu_abort(dc->env, "hardware divider is not available\n");
+ qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not available\n");
+ return;
}
l1 = gen_new_label();
@@ -500,7 +500,8 @@ static void dec_modu(DisasContext *dc)
LOG_DIS("modu r%d, r%d, %d\n", dc->r2, dc->r0, dc->r1);
if (!(dc->env->features & LM32_FEATURE_DIVIDE)) {
- cpu_abort(dc->env, "hardware divider is not available\n");
+ qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not available\n");
+ return;
}
l1 = gen_new_label();
@@ -521,7 +522,9 @@ static void dec_mul(DisasContext *dc)
}
if (!(dc->env->features & LM32_FEATURE_MULTIPLY)) {
- cpu_abort(dc->env, "hardware multiplier is not available\n");
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "hardware multiplier is not available\n");
+ return;
}
if (dc->format == OP_FMT_RI) {
@@ -590,7 +593,8 @@ static void dec_scall(DisasContext *dc)
} else if (dc->imm5 == 2) {
LOG_DIS("break\n");
} else {
- cpu_abort(dc->env, "invalid opcode\n");
+ qemu_log_mask(LOG_GUEST_ERROR, "invalid opcode @0x%x", dc->pc);
+ return;
}
if (dc->imm5 == 7) {
@@ -647,10 +651,10 @@ static void dec_rcsr(DisasContext *dc)
case CSR_WP1:
case CSR_WP2:
case CSR_WP3:
- cpu_abort(dc->env, "invalid read access csr=%x\n", dc->csr);
+ qemu_log_mask(LOG_GUEST_ERROR, "invalid read access csr=%x\n", dc->csr);
break;
default:
- cpu_abort(dc->env, "read_csr: unknown csr=%x\n", dc->csr);
+ qemu_log_mask(LOG_GUEST_ERROR, "read_csr: unknown csr=%x\n", dc->csr);
break;
}
}
@@ -672,7 +676,9 @@ static void dec_sextb(DisasContext *dc)
LOG_DIS("sextb r%d, r%d\n", dc->r2, dc->r0);
if (!(dc->env->features & LM32_FEATURE_SIGN_EXTEND)) {
- cpu_abort(dc->env, "hardware sign extender is not available\n");
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "hardware sign extender is not available\n");
+ return;
}
tcg_gen_ext8s_tl(cpu_R[dc->r2], cpu_R[dc->r0]);
@@ -683,7 +689,9 @@ static void dec_sexth(DisasContext *dc)
LOG_DIS("sexth r%d, r%d\n", dc->r2, dc->r0);
if (!(dc->env->features & LM32_FEATURE_SIGN_EXTEND)) {
- cpu_abort(dc->env, "hardware sign extender is not available\n");
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "hardware sign extender is not available\n");
+ return;
}
tcg_gen_ext16s_tl(cpu_R[dc->r2], cpu_R[dc->r0]);
@@ -710,7 +718,8 @@ static void dec_sl(DisasContext *dc)
}
if (!(dc->env->features & LM32_FEATURE_SHIFT)) {
- cpu_abort(dc->env, "hardware shifter is not available\n");
+ qemu_log_mask(LOG_GUEST_ERROR, "hardware shifter is not available\n");
+ return;
}
if (dc->format == OP_FMT_RI) {
@@ -736,7 +745,9 @@ static void dec_sr(DisasContext *dc)
/* TODO: check r1 == 1 during runtime */
} else {
if (dc->imm5 != 1) {
- cpu_abort(dc->env, "hardware shifter is not available\n");
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "hardware shifter is not available\n");
+ return;
}
}
}
@@ -764,7 +775,9 @@ static void dec_sru(DisasContext *dc)
/* TODO: check r1 == 1 during runtime */
} else {
if (dc->imm5 != 1) {
- cpu_abort(dc->env, "hardware shifter is not available\n");
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "hardware shifter is not available\n");
+ return;
}
}
}
@@ -802,7 +815,7 @@ static void dec_user(DisasContext *dc)
{
LOG_DIS("user");
- cpu_abort(dc->env, "user insn undefined\n");
+ qemu_log_mask(LOG_GUEST_ERROR, "user instruction undefined\n");
}
static void dec_wcsr(DisasContext *dc)
@@ -868,7 +881,9 @@ static void dec_wcsr(DisasContext *dc)
case CSR_BP3:
no = dc->csr - CSR_BP0;
if (dc->env->num_bps <= no) {
- cpu_abort(dc->env, "breakpoint #%i is not available\n", no);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "breakpoint #%i is not available\n", no);
+ break;
}
tcg_gen_mov_tl(cpu_bp[no], cpu_R[dc->r1]);
break;
@@ -878,16 +893,20 @@ static void dec_wcsr(DisasContext *dc)
case CSR_WP3:
no = dc->csr - CSR_WP0;
if (dc->env->num_wps <= no) {
- cpu_abort(dc->env, "watchpoint #%i is not available\n", no);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "watchpoint #%i is not available\n", no);
+ break;
}
tcg_gen_mov_tl(cpu_wp[no], cpu_R[dc->r1]);
break;
case CSR_CC:
case CSR_CFG:
- cpu_abort(dc->env, "invalid write access csr=%x\n", dc->csr);
+ qemu_log_mask(LOG_GUEST_ERROR, "invalid write access csr=%x\n",
+ dc->csr);
break;
default:
- cpu_abort(dc->env, "write_csr unknown csr=%x\n", dc->csr);
+ qemu_log_mask(LOG_GUEST_ERROR, "write_csr: unknown csr=%x\n",
+ dc->csr);
break;
}
}
@@ -933,7 +952,7 @@ static void dec_xor(DisasContext *dc)
static void dec_ill(DisasContext *dc)
{
- cpu_abort(dc->env, "unknown opcode 0x%02x\n", dc->opcode);
+ qemu_log_mask(LOG_GUEST_ERROR, "invalid opcode 0x%02x\n", dc->opcode);
}
typedef void (*DecoderInfo)(DisasContext *dc);
@@ -959,18 +978,6 @@ static inline void decode(DisasContext *dc, uint32_t ir)
dc->ir = ir;
LOG_DIS("%8.8x\t", dc->ir);
- /* try guessing 'empty' instruction memory, although it may be a valid
- * instruction sequence (eg. srui r0, r0, 0) */
- if (dc->ir) {
- dc->nr_nops = 0;
- } else {
- LOG_DIS("nr_nops=%d\t", dc->nr_nops);
- dc->nr_nops++;
- if (dc->nr_nops > 4) {
- cpu_abort(dc->env, "fetching nop sequence\n");
- }
- }
-
dc->opcode = EXTRACT_FIELD(ir, 26, 31);
dc->imm5 = EXTRACT_FIELD(ir, 0, 4);
@@ -1034,10 +1041,11 @@ void gen_intermediate_code_internal(LM32CPU *cpu,
dc->is_jmp = DISAS_NEXT;
dc->pc = pc_start;
dc->singlestep_enabled = cs->singlestep_enabled;
- dc->nr_nops = 0;
if (pc_start & 3) {
- cpu_abort(env, "LM32: unaligned PC=%x\n", pc_start);
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "unaligned PC=%x. Ignoring lowest bits.\n", pc_start);
+ pc_start &= ~3;
}
next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL v4 07/12] target-lm32: move model features to LM32CPU
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
` (5 preceding siblings ...)
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 06/12] target-lm32: kill cpu_abort() calls Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 08/12] target-lm32: add breakpoint/watchpoint support Michael Walle
` (5 subsequent siblings)
12 siblings, 0 replies; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Anthony Liguori
This allows us to completely remove CPULM32State from DisasContext.
Instead, copy the fields we need to DisasContext.
Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Michael Walle <michael@walle.cc>
---
target-lm32/cpu-qom.h | 6 ++
target-lm32/cpu.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++-
target-lm32/cpu.h | 7 +-
target-lm32/helper.c | 128 +-------------------------------
target-lm32/translate.c | 29 +++++---
5 files changed, 215 insertions(+), 142 deletions(-)
diff --git a/target-lm32/cpu-qom.h b/target-lm32/cpu-qom.h
index 723f604..9f15093 100644
--- a/target-lm32/cpu-qom.h
+++ b/target-lm32/cpu-qom.h
@@ -60,6 +60,12 @@ typedef struct LM32CPU {
/*< public >*/
CPULM32State env;
+
+ uint32_t revision;
+ uint8_t num_interrupts;
+ uint8_t num_breakpoints;
+ uint8_t num_watchpoints;
+ uint32_t features;
} LM32CPU;
static inline LM32CPU *lm32_env_get_cpu(CPULM32State *env)
diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
index 869878c..2b207ad 100644
--- a/target-lm32/cpu.c
+++ b/target-lm32/cpu.c
@@ -29,6 +29,87 @@ static void lm32_cpu_set_pc(CPUState *cs, vaddr value)
cpu->env.pc = value;
}
+/* Sort alphabetically by type name. */
+static gint lm32_cpu_list_compare(gconstpointer a, gconstpointer b)
+{
+ ObjectClass *class_a = (ObjectClass *)a;
+ ObjectClass *class_b = (ObjectClass *)b;
+ const char *name_a, *name_b;
+
+ name_a = object_class_get_name(class_a);
+ name_b = object_class_get_name(class_b);
+ return strcmp(name_a, name_b);
+}
+
+static void lm32_cpu_list_entry(gpointer data, gpointer user_data)
+{
+ ObjectClass *oc = data;
+ CPUListState *s = user_data;
+ const char *typename = object_class_get_name(oc);
+ char *name;
+
+ name = g_strndup(typename, strlen(typename) - strlen("-" TYPE_LM32_CPU));
+ (*s->cpu_fprintf)(s->file, " %s\n", name);
+ g_free(name);
+}
+
+
+void lm32_cpu_list(FILE *f, fprintf_function cpu_fprintf)
+{
+ CPUListState s = {
+ .file = f,
+ .cpu_fprintf = cpu_fprintf,
+ };
+ GSList *list;
+
+ list = object_class_get_list(TYPE_LM32_CPU, false);
+ list = g_slist_sort(list, lm32_cpu_list_compare);
+ (*cpu_fprintf)(f, "Available CPUs:\n");
+ g_slist_foreach(list, lm32_cpu_list_entry, &s);
+ g_slist_free(list);
+}
+
+static void lm32_cpu_init_cfg_reg(LM32CPU *cpu)
+{
+ CPULM32State *env = &cpu->env;
+ uint32_t cfg = 0;
+
+ if (cpu->features & LM32_FEATURE_MULTIPLY) {
+ cfg |= CFG_M;
+ }
+
+ if (cpu->features & LM32_FEATURE_DIVIDE) {
+ cfg |= CFG_D;
+ }
+
+ if (cpu->features & LM32_FEATURE_SHIFT) {
+ cfg |= CFG_S;
+ }
+
+ if (cpu->features & LM32_FEATURE_SIGN_EXTEND) {
+ cfg |= CFG_X;
+ }
+
+ if (cpu->features & LM32_FEATURE_I_CACHE) {
+ cfg |= CFG_IC;
+ }
+
+ if (cpu->features & LM32_FEATURE_D_CACHE) {
+ cfg |= CFG_DC;
+ }
+
+ if (cpu->features & LM32_FEATURE_CYCLE_COUNT) {
+ cfg |= CFG_CC;
+ }
+
+ cfg |= (cpu->num_interrupts << CFG_INT_SHIFT);
+ cfg |= (cpu->num_breakpoints << CFG_BP_SHIFT);
+ cfg |= (cpu->num_watchpoints << CFG_WP_SHIFT);
+ cfg |= (cpu->revision << CFG_REV_SHIFT);
+
+ env->cfg = cfg;
+}
+
/* CPUClass::reset() */
static void lm32_cpu_reset(CPUState *s)
{
@@ -41,6 +122,7 @@ static void lm32_cpu_reset(CPUState *s)
/* reset cpu state */
memset(env, 0, offsetof(CPULM32State, breakpoints));
+ lm32_cpu_init_cfg_reg(cpu);
tlb_flush(env, 1);
}
@@ -74,6 +156,91 @@ static void lm32_cpu_initfn(Object *obj)
}
}
+static void lm32_basic_cpu_initfn(Object *obj)
+{
+ LM32CPU *cpu = LM32_CPU(obj);
+
+ cpu->revision = 3;
+ cpu->num_interrupts = 32;
+ cpu->num_breakpoints = 4;
+ cpu->num_watchpoints = 4;
+ cpu->features = LM32_FEATURE_SHIFT
+ | LM32_FEATURE_SIGN_EXTEND
+ | LM32_FEATURE_CYCLE_COUNT;
+}
+
+static void lm32_standard_cpu_initfn(Object *obj)
+{
+ LM32CPU *cpu = LM32_CPU(obj);
+
+ cpu->revision = 3;
+ cpu->num_interrupts = 32;
+ cpu->num_breakpoints = 4;
+ cpu->num_watchpoints = 4;
+ cpu->features = LM32_FEATURE_MULTIPLY
+ | LM32_FEATURE_DIVIDE
+ | LM32_FEATURE_SHIFT
+ | LM32_FEATURE_SIGN_EXTEND
+ | LM32_FEATURE_I_CACHE
+ | LM32_FEATURE_CYCLE_COUNT;
+}
+
+static void lm32_full_cpu_initfn(Object *obj)
+{
+ LM32CPU *cpu = LM32_CPU(obj);
+
+ cpu->revision = 3;
+ cpu->num_interrupts = 32;
+ cpu->num_breakpoints = 4;
+ cpu->num_watchpoints = 4;
+ cpu->features = LM32_FEATURE_MULTIPLY
+ | LM32_FEATURE_DIVIDE
+ | LM32_FEATURE_SHIFT
+ | LM32_FEATURE_SIGN_EXTEND
+ | LM32_FEATURE_I_CACHE
+ | LM32_FEATURE_D_CACHE
+ | LM32_FEATURE_CYCLE_COUNT;
+}
+
+typedef struct LM32CPUInfo {
+ const char *name;
+ void (*initfn)(Object *obj);
+} LM32CPUInfo;
+
+static const LM32CPUInfo lm32_cpus[] = {
+ {
+ .name = "lm32-basic",
+ .initfn = lm32_basic_cpu_initfn,
+ },
+ {
+ .name = "lm32-standard",
+ .initfn = lm32_standard_cpu_initfn,
+ },
+ {
+ .name = "lm32-full",
+ .initfn = lm32_full_cpu_initfn,
+ },
+};
+
+static ObjectClass *lm32_cpu_class_by_name(const char *cpu_model)
+{
+ ObjectClass *oc;
+ char *typename;
+
+ if (cpu_model == NULL) {
+ return NULL;
+ }
+
+ typename = g_strdup_printf("%s-" TYPE_LM32_CPU, cpu_model);
+ oc = object_class_by_name(typename);
+ g_free(typename);
+ if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_LM32_CPU) ||
+ object_class_is_abstract(oc))) {
+ oc = NULL;
+ }
+ return oc;
+}
+
static void lm32_cpu_class_init(ObjectClass *oc, void *data)
{
LM32CPUClass *lcc = LM32_CPU_CLASS(oc);
@@ -86,6 +253,7 @@ static void lm32_cpu_class_init(ObjectClass *oc, void *data)
lcc->parent_reset = cc->reset;
cc->reset = lm32_cpu_reset;
+ cc->class_by_name = lm32_cpu_class_by_name;
cc->do_interrupt = lm32_cpu_do_interrupt;
cc->dump_state = lm32_cpu_dump_state;
cc->set_pc = lm32_cpu_set_pc;
@@ -98,19 +266,36 @@ static void lm32_cpu_class_init(ObjectClass *oc, void *data)
cc->gdb_num_core_regs = 32 + 7;
}
+static void lm32_register_cpu_type(const LM32CPUInfo *info)
+{
+ TypeInfo type_info = {
+ .parent = TYPE_LM32_CPU,
+ .instance_init = info->initfn,
+ };
+
+ type_info.name = g_strdup_printf("%s-" TYPE_LM32_CPU, info->name);
+ type_register(&type_info);
+ g_free((void *)type_info.name);
+}
+
static const TypeInfo lm32_cpu_type_info = {
.name = TYPE_LM32_CPU,
.parent = TYPE_CPU,
.instance_size = sizeof(LM32CPU),
.instance_init = lm32_cpu_initfn,
- .abstract = false,
+ .abstract = true,
.class_size = sizeof(LM32CPUClass),
.class_init = lm32_cpu_class_init,
};
static void lm32_cpu_register_types(void)
{
+ int i;
+
type_register_static(&lm32_cpu_type_info);
+ for (i = 0; i < ARRAY_SIZE(lm32_cpus); i++) {
+ lm32_register_cpu_type(&lm32_cpus[i]);
+ }
}
type_init(lm32_cpu_register_types)
diff --git a/target-lm32/cpu.h b/target-lm32/cpu.h
index dbfe043..101df80 100644
--- a/target-lm32/cpu.h
+++ b/target-lm32/cpu.h
@@ -177,23 +177,20 @@ struct CPULM32State {
DeviceState *juart_state;
/* processor core features */
- uint32_t features;
uint32_t flags;
- uint8_t num_bps;
- uint8_t num_wps;
};
#include "cpu-qom.h"
LM32CPU *cpu_lm32_init(const char *cpu_model);
-void cpu_lm32_list(FILE *f, fprintf_function cpu_fprintf);
int cpu_lm32_exec(CPULM32State *s);
/* you can call this signal handler from your SIGBUS and SIGSEGV
signal handlers to inform the virtual CPU of exceptions. non zero
is returned if the signal was handled by the virtual CPU. */
int cpu_lm32_signal_handler(int host_signum, void *pinfo,
void *puc);
+void lm32_cpu_list(FILE *f, fprintf_function cpu_fprintf);
void lm32_translate_init(void);
void cpu_lm32_set_phys_msb_ignore(CPULM32State *env, int value);
@@ -206,7 +203,7 @@ static inline CPULM32State *cpu_init(const char *cpu_model)
return &cpu->env;
}
-#define cpu_list cpu_lm32_list
+#define cpu_list lm32_cpu_list
#define cpu_exec cpu_lm32_exec
#define cpu_gen_code cpu_lm32_gen_code
#define cpu_signal_handler cpu_lm32_signal_handler
diff --git a/target-lm32/helper.c b/target-lm32/helper.c
index 15bc615..f85ff2e 100644
--- a/target-lm32/helper.c
+++ b/target-lm32/helper.c
@@ -90,136 +90,16 @@ void lm32_cpu_do_interrupt(CPUState *cs)
}
}
-typedef struct {
- const char *name;
- uint32_t revision;
- uint8_t num_interrupts;
- uint8_t num_breakpoints;
- uint8_t num_watchpoints;
- uint32_t features;
-} LM32Def;
-
-static const LM32Def lm32_defs[] = {
- {
- .name = "lm32-basic",
- .revision = 3,
- .num_interrupts = 32,
- .num_breakpoints = 4,
- .num_watchpoints = 4,
- .features = (LM32_FEATURE_SHIFT
- | LM32_FEATURE_SIGN_EXTEND
- | LM32_FEATURE_CYCLE_COUNT),
- },
- {
- .name = "lm32-standard",
- .revision = 3,
- .num_interrupts = 32,
- .num_breakpoints = 4,
- .num_watchpoints = 4,
- .features = (LM32_FEATURE_MULTIPLY
- | LM32_FEATURE_DIVIDE
- | LM32_FEATURE_SHIFT
- | LM32_FEATURE_SIGN_EXTEND
- | LM32_FEATURE_I_CACHE
- | LM32_FEATURE_CYCLE_COUNT),
- },
- {
- .name = "lm32-full",
- .revision = 3,
- .num_interrupts = 32,
- .num_breakpoints = 4,
- .num_watchpoints = 4,
- .features = (LM32_FEATURE_MULTIPLY
- | LM32_FEATURE_DIVIDE
- | LM32_FEATURE_SHIFT
- | LM32_FEATURE_SIGN_EXTEND
- | LM32_FEATURE_I_CACHE
- | LM32_FEATURE_D_CACHE
- | LM32_FEATURE_CYCLE_COUNT),
- }
-};
-
-void cpu_lm32_list(FILE *f, fprintf_function cpu_fprintf)
-{
- int i;
-
- cpu_fprintf(f, "Available CPUs:\n");
- for (i = 0; i < ARRAY_SIZE(lm32_defs); i++) {
- cpu_fprintf(f, " %s\n", lm32_defs[i].name);
- }
-}
-
-static const LM32Def *cpu_lm32_find_by_name(const char *name)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(lm32_defs); i++) {
- if (strcasecmp(name, lm32_defs[i].name) == 0) {
- return &lm32_defs[i];
- }
- }
-
- return NULL;
-}
-
-static uint32_t cfg_by_def(const LM32Def *def)
-{
- uint32_t cfg = 0;
-
- if (def->features & LM32_FEATURE_MULTIPLY) {
- cfg |= CFG_M;
- }
-
- if (def->features & LM32_FEATURE_DIVIDE) {
- cfg |= CFG_D;
- }
-
- if (def->features & LM32_FEATURE_SHIFT) {
- cfg |= CFG_S;
- }
-
- if (def->features & LM32_FEATURE_SIGN_EXTEND) {
- cfg |= CFG_X;
- }
-
- if (def->features & LM32_FEATURE_I_CACHE) {
- cfg |= CFG_IC;
- }
-
- if (def->features & LM32_FEATURE_D_CACHE) {
- cfg |= CFG_DC;
- }
-
- if (def->features & LM32_FEATURE_CYCLE_COUNT) {
- cfg |= CFG_CC;
- }
-
- cfg |= (def->num_interrupts << CFG_INT_SHIFT);
- cfg |= (def->num_breakpoints << CFG_BP_SHIFT);
- cfg |= (def->num_watchpoints << CFG_WP_SHIFT);
- cfg |= (def->revision << CFG_REV_SHIFT);
-
- return cfg;
-}
-
LM32CPU *cpu_lm32_init(const char *cpu_model)
{
LM32CPU *cpu;
- CPULM32State *env;
- const LM32Def *def;
+ ObjectClass *oc;
- def = cpu_lm32_find_by_name(cpu_model);
- if (!def) {
+ oc = cpu_class_by_name(TYPE_LM32_CPU, cpu_model);
+ if (oc == NULL) {
return NULL;
}
-
- cpu = LM32_CPU(object_new(TYPE_LM32_CPU));
- env = &cpu->env;
-
- env->features = def->features;
- env->num_bps = def->num_breakpoints;
- env->num_wps = def->num_watchpoints;
- env->cfg = cfg_by_def(def);
+ cpu = LM32_CPU(object_new(object_class_get_name(oc)));
object_property_set_bool(OBJECT(cpu), true, "realized", NULL);
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index e292e1c..93075e4 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -64,7 +64,6 @@ enum {
/* This is the state at translation time. */
typedef struct DisasContext {
- CPULM32State *env;
target_ulong pc;
/* Decoder. */
@@ -82,6 +81,10 @@ typedef struct DisasContext {
struct TranslationBlock *tb;
int singlestep_enabled;
+
+ uint32_t features;
+ uint8_t num_breakpoints;
+ uint8_t num_watchpoints;
} DisasContext;
static const char *regnames[] = {
@@ -420,7 +423,7 @@ static void dec_divu(DisasContext *dc)
LOG_DIS("divu r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1);
- if (!(dc->env->features & LM32_FEATURE_DIVIDE)) {
+ if (!(dc->features & LM32_FEATURE_DIVIDE)) {
qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not available\n");
return;
}
@@ -499,7 +502,7 @@ static void dec_modu(DisasContext *dc)
LOG_DIS("modu r%d, r%d, %d\n", dc->r2, dc->r0, dc->r1);
- if (!(dc->env->features & LM32_FEATURE_DIVIDE)) {
+ if (!(dc->features & LM32_FEATURE_DIVIDE)) {
qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not available\n");
return;
}
@@ -521,7 +524,7 @@ static void dec_mul(DisasContext *dc)
LOG_DIS("mul r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1);
}
- if (!(dc->env->features & LM32_FEATURE_MULTIPLY)) {
+ if (!(dc->features & LM32_FEATURE_MULTIPLY)) {
qemu_log_mask(LOG_GUEST_ERROR,
"hardware multiplier is not available\n");
return;
@@ -675,7 +678,7 @@ static void dec_sextb(DisasContext *dc)
{
LOG_DIS("sextb r%d, r%d\n", dc->r2, dc->r0);
- if (!(dc->env->features & LM32_FEATURE_SIGN_EXTEND)) {
+ if (!(dc->features & LM32_FEATURE_SIGN_EXTEND)) {
qemu_log_mask(LOG_GUEST_ERROR,
"hardware sign extender is not available\n");
return;
@@ -688,7 +691,7 @@ static void dec_sexth(DisasContext *dc)
{
LOG_DIS("sexth r%d, r%d\n", dc->r2, dc->r0);
- if (!(dc->env->features & LM32_FEATURE_SIGN_EXTEND)) {
+ if (!(dc->features & LM32_FEATURE_SIGN_EXTEND)) {
qemu_log_mask(LOG_GUEST_ERROR,
"hardware sign extender is not available\n");
return;
@@ -717,7 +720,7 @@ static void dec_sl(DisasContext *dc)
LOG_DIS("sl r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1);
}
- if (!(dc->env->features & LM32_FEATURE_SHIFT)) {
+ if (!(dc->features & LM32_FEATURE_SHIFT)) {
qemu_log_mask(LOG_GUEST_ERROR, "hardware shifter is not available\n");
return;
}
@@ -740,7 +743,7 @@ static void dec_sr(DisasContext *dc)
LOG_DIS("sr r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1);
}
- if (!(dc->env->features & LM32_FEATURE_SHIFT)) {
+ if (!(dc->features & LM32_FEATURE_SHIFT)) {
if (dc->format == OP_FMT_RI) {
/* TODO: check r1 == 1 during runtime */
} else {
@@ -770,7 +773,7 @@ static void dec_sru(DisasContext *dc)
LOG_DIS("sru r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1);
}
- if (!(dc->env->features & LM32_FEATURE_SHIFT)) {
+ if (!(dc->features & LM32_FEATURE_SHIFT)) {
if (dc->format == OP_FMT_RI) {
/* TODO: check r1 == 1 during runtime */
} else {
@@ -880,7 +883,7 @@ static void dec_wcsr(DisasContext *dc)
case CSR_BP2:
case CSR_BP3:
no = dc->csr - CSR_BP0;
- if (dc->env->num_bps <= no) {
+ if (dc->num_breakpoints <= no) {
qemu_log_mask(LOG_GUEST_ERROR,
"breakpoint #%i is not available\n", no);
break;
@@ -892,7 +895,7 @@ static void dec_wcsr(DisasContext *dc)
case CSR_WP2:
case CSR_WP3:
no = dc->csr - CSR_WP0;
- if (dc->env->num_wps <= no) {
+ if (dc->num_watchpoints <= no) {
qemu_log_mask(LOG_GUEST_ERROR,
"watchpoint #%i is not available\n", no);
break;
@@ -1033,7 +1036,9 @@ void gen_intermediate_code_internal(LM32CPU *cpu,
int max_insns;
pc_start = tb->pc;
- dc->env = env;
+ dc->features = cpu->features;
+ dc->num_breakpoints = cpu->num_breakpoints;
+ dc->num_watchpoints = cpu->num_watchpoints;
dc->tb = tb;
gen_opc_end = tcg_ctx.gen_opc_buf + OPC_MAX_SIZE;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL v4 08/12] target-lm32: add breakpoint/watchpoint support
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
` (6 preceding siblings ...)
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 07/12] target-lm32: move model features to LM32CPU Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-02-01 18:16 ` Peter Maydell
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 09/12] lm32_sys: print test result on stderr Michael Walle
` (4 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Anthony Liguori
This patch adds in-target breakpoint and watchpoint support.
Signed-off-by: Michael Walle <michael@walle.cc>
---
target-lm32/TODO | 2 --
target-lm32/cpu.c | 1 +
target-lm32/cpu.h | 27 ++++++++++++--
target-lm32/helper.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++
target-lm32/helper.h | 3 ++
target-lm32/op_helper.c | 58 +++++++++++++++++++++++++++++-
target-lm32/translate.c | 6 ++--
7 files changed, 178 insertions(+), 8 deletions(-)
diff --git a/target-lm32/TODO b/target-lm32/TODO
index b9ea0c8..e163c42 100644
--- a/target-lm32/TODO
+++ b/target-lm32/TODO
@@ -1,3 +1 @@
-* disassembler (lm32-dis.c)
* linux-user emulation
-* native bp/wp emulation (?)
diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
index 2b207ad..7e716fb 100644
--- a/target-lm32/cpu.c
+++ b/target-lm32/cpu.c
@@ -153,6 +153,7 @@ static void lm32_cpu_initfn(Object *obj)
if (tcg_enabled() && !tcg_initialized) {
tcg_initialized = true;
lm32_translate_init();
+ cpu_set_debug_excp_handler(lm32_debug_excp_handler);
}
}
diff --git a/target-lm32/cpu.h b/target-lm32/cpu.h
index 101df80..18cf348 100644
--- a/target-lm32/cpu.h
+++ b/target-lm32/cpu.h
@@ -163,8 +163,11 @@ struct CPULM32State {
/* debug registers */
uint32_t dc; /* debug control */
- uint32_t bp[4]; /* breakpoint addresses */
- uint32_t wp[4]; /* watchpoint addresses */
+ uint32_t bp[4]; /* breakpoints */
+ uint32_t wp[4]; /* watchpoints */
+
+ CPUBreakpoint * cpu_breakpoint[4];
+ CPUWatchpoint * cpu_watchpoint[4];
CPU_COMMON
@@ -181,6 +184,19 @@ struct CPULM32State {
};
+typedef enum {
+ LM32_WP_DISABLED = 0,
+ LM32_WP_READ,
+ LM32_WP_WRITE,
+ LM32_WP_READ_WRITE,
+} lm32_wp_t;
+
+static inline lm32_wp_t lm32_wp_type(uint32_t dc, int idx)
+{
+ assert(idx < 4);
+ return (dc >> (idx+1)*2) & 0x3;
+}
+
#include "cpu-qom.h"
LM32CPU *cpu_lm32_init(const char *cpu_model);
@@ -193,6 +209,13 @@ int cpu_lm32_signal_handler(int host_signum, void *pinfo,
void lm32_cpu_list(FILE *f, fprintf_function cpu_fprintf);
void lm32_translate_init(void);
void cpu_lm32_set_phys_msb_ignore(CPULM32State *env, int value);
+void QEMU_NORETURN raise_exception(CPULM32State *env, int index);
+void lm32_debug_excp_handler(CPULM32State *env);
+void lm32_breakpoint_insert(CPULM32State *env, int index, target_ulong address);
+void lm32_breakpoint_remove(CPULM32State *env, int index);
+void lm32_watchpoint_insert(CPULM32State *env, int index, target_ulong address,
+ lm32_wp_t wp_type);
+void lm32_watchpoint_remove(CPULM32State *env, int index);
static inline CPULM32State *cpu_init(const char *cpu_model)
{
diff --git a/target-lm32/helper.c b/target-lm32/helper.c
index f85ff2e..cec8e11 100644
--- a/target-lm32/helper.c
+++ b/target-lm32/helper.c
@@ -49,6 +49,95 @@ hwaddr lm32_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
}
}
+void lm32_breakpoint_insert(CPULM32State *env, int idx, target_ulong address)
+{
+ cpu_breakpoint_insert(env, address, BP_CPU, &env->cpu_breakpoint[idx]);
+}
+
+void lm32_breakpoint_remove(CPULM32State *env, int idx)
+{
+ if (!env->cpu_breakpoint[idx]) {
+ return;
+ }
+
+ cpu_breakpoint_remove_by_ref(env, env->cpu_breakpoint[idx]);
+ env->cpu_breakpoint[idx] = NULL;
+}
+
+void lm32_watchpoint_insert(CPULM32State *env, int idx, target_ulong address,
+ lm32_wp_t wp_type)
+{
+ int flags = 0;
+
+ switch (wp_type) {
+ case LM32_WP_DISABLED:
+ /* nothing to to */
+ break;
+ case LM32_WP_READ:
+ flags = BP_CPU | BP_STOP_BEFORE_ACCESS | BP_MEM_READ;
+ break;
+ case LM32_WP_WRITE:
+ flags = BP_CPU | BP_STOP_BEFORE_ACCESS | BP_MEM_WRITE;
+ break;
+ case LM32_WP_READ_WRITE:
+ flags = BP_CPU | BP_STOP_BEFORE_ACCESS | BP_MEM_ACCESS;
+ break;
+ }
+
+ if (flags != 0) {
+ cpu_watchpoint_insert(env, address, 1, flags,
+ &env->cpu_watchpoint[idx]);
+ }
+}
+
+void lm32_watchpoint_remove(CPULM32State *env, int idx)
+{
+ if (!env->cpu_watchpoint[idx]) {
+ return;
+ }
+
+ cpu_watchpoint_remove_by_ref(env, env->cpu_watchpoint[idx]);
+ env->cpu_watchpoint[idx] = NULL;
+}
+
+static bool check_watchpoints(CPULM32State *env)
+{
+ LM32CPU *cpu = lm32_env_get_cpu(env);
+ int i;
+
+ for (i = 0; i < cpu->num_watchpoints; i++) {
+ if (env->cpu_watchpoint[i] &&
+ env->cpu_watchpoint[i]->flags & BP_WATCHPOINT_HIT) {
+ return true;
+ }
+ }
+ return false;
+}
+
+void lm32_debug_excp_handler(CPULM32State *env)
+{
+ CPUBreakpoint *bp;
+
+ if (env->watchpoint_hit) {
+ if (env->watchpoint_hit->flags & BP_CPU) {
+ env->watchpoint_hit = NULL;
+ if (check_watchpoints(env)) {
+ raise_exception(env, EXCP_WATCHPOINT);
+ } else {
+ cpu_resume_from_signal(env, NULL);
+ }
+ }
+ } else {
+ QTAILQ_FOREACH(bp, &env->breakpoints, entry)
+ if (bp->pc == env->pc) {
+ if (bp->flags & BP_CPU) {
+ raise_exception(env, EXCP_BREAKPOINT);
+ }
+ break;
+ }
+ }
+}
+
void lm32_cpu_do_interrupt(CPUState *cs)
{
LM32CPU *cpu = LM32_CPU(cs);
diff --git a/target-lm32/helper.h b/target-lm32/helper.h
index 3ea15a6..ad44fdf 100644
--- a/target-lm32/helper.h
+++ b/target-lm32/helper.h
@@ -2,6 +2,9 @@
DEF_HELPER_2(raise_exception, void, env, i32)
DEF_HELPER_1(hlt, void, env)
+DEF_HELPER_3(wcsr_bp, void, env, i32, i32)
+DEF_HELPER_3(wcsr_wp, void, env, i32, i32)
+DEF_HELPER_2(wcsr_dc, void, env, i32)
DEF_HELPER_2(wcsr_im, void, env, i32)
DEF_HELPER_2(wcsr_ip, void, env, i32)
DEF_HELPER_2(wcsr_jtx, void, env, i32)
diff --git a/target-lm32/op_helper.c b/target-lm32/op_helper.c
index 8f5ef55..71f21d1 100644
--- a/target-lm32/op_helper.c
+++ b/target-lm32/op_helper.c
@@ -19,12 +19,17 @@
#define SHIFT 3
#include "exec/softmmu_template.h"
-void HELPER(raise_exception)(CPULM32State *env, uint32_t index)
+void raise_exception(CPULM32State *env, int index)
{
env->exception_index = index;
cpu_loop_exit(env);
}
+void HELPER(raise_exception)(CPULM32State *env, uint32_t index)
+{
+ raise_exception(env, index);
+}
+
void HELPER(hlt)(CPULM32State *env)
{
CPUState *cs = CPU(lm32_env_get_cpu(env));
@@ -34,6 +39,57 @@ void HELPER(hlt)(CPULM32State *env)
cpu_loop_exit(env);
}
+void HELPER(wcsr_bp)(CPULM32State *env, uint32_t bp, uint32_t idx)
+{
+ uint32_t addr = bp & ~1;
+
+ assert(idx < 4);
+
+ env->bp[idx] = bp;
+ lm32_breakpoint_remove(env, idx);
+ if (bp & 1) {
+ lm32_breakpoint_insert(env, idx, addr);
+ }
+}
+
+void HELPER(wcsr_wp)(CPULM32State *env, uint32_t wp, uint32_t idx)
+{
+ lm32_wp_t wp_type;
+
+ assert(idx < 4);
+
+ env->wp[idx] = wp;
+
+ wp_type = lm32_wp_type(env->dc, idx);
+ lm32_watchpoint_remove(env, idx);
+ if (wp_type != LM32_WP_DISABLED) {
+ lm32_watchpoint_insert(env, idx, wp, wp_type);
+ }
+}
+
+void HELPER(wcsr_dc)(CPULM32State *env, uint32_t dc)
+{
+ uint32_t old_dc;
+ int i;
+ lm32_wp_t old_type;
+ lm32_wp_t new_type;
+
+ old_dc = env->dc;
+ env->dc = dc;
+
+ for (i = 0; i < 4; i++) {
+ old_type = lm32_wp_type(old_dc, i);
+ new_type = lm32_wp_type(dc, i);
+
+ if (old_type != new_type) {
+ lm32_watchpoint_remove(env, i);
+ if (new_type != LM32_WP_DISABLED) {
+ lm32_watchpoint_insert(env, i, env->wp[i], new_type);
+ }
+ }
+ }
+}
+
void HELPER(wcsr_im)(CPULM32State *env, uint32_t im)
{
lm32_pic_set_im(env->pic_state, im);
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index 93075e4..f20460a 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -876,7 +876,7 @@ static void dec_wcsr(DisasContext *dc)
gen_helper_wcsr_jrx(cpu_env, cpu_R[dc->r1]);
break;
case CSR_DC:
- tcg_gen_mov_tl(cpu_dc, cpu_R[dc->r1]);
+ gen_helper_wcsr_dc(cpu_env, cpu_R[dc->r1]);
break;
case CSR_BP0:
case CSR_BP1:
@@ -888,7 +888,7 @@ static void dec_wcsr(DisasContext *dc)
"breakpoint #%i is not available\n", no);
break;
}
- tcg_gen_mov_tl(cpu_bp[no], cpu_R[dc->r1]);
+ gen_helper_wcsr_bp(cpu_env, cpu_R[dc->r1], tcg_const_i32(no));
break;
case CSR_WP0:
case CSR_WP1:
@@ -900,7 +900,7 @@ static void dec_wcsr(DisasContext *dc)
"watchpoint #%i is not available\n", no);
break;
}
- tcg_gen_mov_tl(cpu_wp[no], cpu_R[dc->r1]);
+ gen_helper_wcsr_wp(cpu_env, cpu_R[dc->r1], tcg_const_i32(no));
break;
case CSR_CC:
case CSR_CFG:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 08/12] target-lm32: add breakpoint/watchpoint support
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 08/12] target-lm32: add breakpoint/watchpoint support Michael Walle
@ 2014-02-01 18:16 ` Peter Maydell
2014-02-03 21:27 ` Richard Henderson
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2014-02-01 18:16 UTC (permalink / raw)
To: Michael Walle; +Cc: QEMU Developers, Anthony Liguori, Richard Henderson
On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
> This patch adds in-target breakpoint and watchpoint support.
This looks at least superficially OK, but I'm not really familiar
with QEMU's breakpoint/watchpoint support code. RTH: do
you want to have a look over this one?
A style point, since I'm not really able to review for much
else:
> +void lm32_debug_excp_handler(CPULM32State *env)
> +{
> + CPUBreakpoint *bp;
> +
> + if (env->watchpoint_hit) {
> + if (env->watchpoint_hit->flags & BP_CPU) {
> + env->watchpoint_hit = NULL;
> + if (check_watchpoints(env)) {
> + raise_exception(env, EXCP_WATCHPOINT);
> + } else {
> + cpu_resume_from_signal(env, NULL);
> + }
> + }
> + } else {
> + QTAILQ_FOREACH(bp, &env->breakpoints, entry)
> + if (bp->pc == env->pc) {
> + if (bp->flags & BP_CPU) {
> + raise_exception(env, EXCP_BREAKPOINT);
> + }
> + break;
> + }
Braces around the QTAILQ_FOREACH's body, please.
> + }
> +}
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 08/12] target-lm32: add breakpoint/watchpoint support
2014-02-01 18:16 ` Peter Maydell
@ 2014-02-03 21:27 ` Richard Henderson
2014-02-03 21:35 ` Peter Maydell
0 siblings, 1 reply; 32+ messages in thread
From: Richard Henderson @ 2014-02-03 21:27 UTC (permalink / raw)
To: Peter Maydell, Michael Walle; +Cc: QEMU Developers, Anthony Liguori
On 02/01/2014 10:16 AM, Peter Maydell wrote:
> This looks at least superficially OK, but I'm not really familiar
> with QEMU's breakpoint/watchpoint support code. RTH: do
> you want to have a look over this one?
It's copied verbatim from the i386 port, missing braces and all.
So it certainly passes the sniff test.
r~
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PULL v4 08/12] target-lm32: add breakpoint/watchpoint support
2014-02-03 21:27 ` Richard Henderson
@ 2014-02-03 21:35 ` Peter Maydell
0 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2014-02-03 21:35 UTC (permalink / raw)
To: Richard Henderson; +Cc: Michael Walle, QEMU Developers, Anthony Liguori
On 3 February 2014 21:27, Richard Henderson <rth@twiddle.net> wrote:
> On 02/01/2014 10:16 AM, Peter Maydell wrote:
>> This looks at least superficially OK, but I'm not really familiar
>> with QEMU's breakpoint/watchpoint support code. RTH: do
>> you want to have a look over this one?
>
> It's copied verbatim from the i386 port, missing braces and all.
> So it certainly passes the sniff test.
That sounds like we ought to be generalising our support
code for bp/wp support in targets. Maybe I'll do that if/when
I ever implement bp/wp support for ARM :-)
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL v4 09/12] lm32_sys: print test result on stderr
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
` (7 preceding siblings ...)
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 08/12] target-lm32: add breakpoint/watchpoint support Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-02-01 18:00 ` Peter Maydell
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 10/12] lm32_sys: dump cpu state if test case fails Michael Walle
` (3 subsequent siblings)
12 siblings, 1 reply; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Anthony Liguori
Do not use qemu_log().
Signed-off-by: Michael Walle <michael@walle.cc>
---
hw/misc/lm32_sys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/misc/lm32_sys.c b/hw/misc/lm32_sys.c
index 8176cdb..6af0cca 100644
--- a/hw/misc/lm32_sys.c
+++ b/hw/misc/lm32_sys.c
@@ -80,7 +80,7 @@ static void sys_write(void *opaque, hwaddr addr,
case R_PASSFAIL:
s->regs[addr] = value;
testname = (char *)s->testname;
- qemu_log("TC %-32s %s\n", testname, (value) ? "FAILED" : "OK");
+ fprintf(stderr, "TC %-32s %s\n", testname, (value) ? "FAILED" : "OK");
break;
case R_TESTNAME:
s->regs[addr] = value;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 09/12] lm32_sys: print test result on stderr
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 09/12] lm32_sys: print test result on stderr Michael Walle
@ 2014-02-01 18:00 ` Peter Maydell
2014-02-01 20:31 ` Michael Walle
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2014-02-01 18:00 UTC (permalink / raw)
To: Michael Walle; +Cc: QEMU Developers, Anthony Liguori
On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
> Do not use qemu_log().
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> hw/misc/lm32_sys.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/misc/lm32_sys.c b/hw/misc/lm32_sys.c
> index 8176cdb..6af0cca 100644
> --- a/hw/misc/lm32_sys.c
> +++ b/hw/misc/lm32_sys.c
> @@ -80,7 +80,7 @@ static void sys_write(void *opaque, hwaddr addr,
> case R_PASSFAIL:
> s->regs[addr] = value;
> testname = (char *)s->testname;
> - qemu_log("TC %-32s %s\n", testname, (value) ? "FAILED" : "OK");
> + fprintf(stderr, "TC %-32s %s\n", testname, (value) ? "FAILED" : "OK");
> break;
This looks wrong to me -- devices shouldn't print to stderr, ideally.
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 09/12] lm32_sys: print test result on stderr
2014-02-01 18:00 ` Peter Maydell
@ 2014-02-01 20:31 ` Michael Walle
2014-02-03 22:39 ` Michael Walle
0 siblings, 1 reply; 32+ messages in thread
From: Michael Walle @ 2014-02-01 20:31 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Anthony Liguori
Am Samstag, 1. Februar 2014, 19:00:01 schrieb Peter Maydell:
> On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
> > Do not use qemu_log().
> >
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> >
> > hw/misc/lm32_sys.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/misc/lm32_sys.c b/hw/misc/lm32_sys.c
> > index 8176cdb..6af0cca 100644
> > --- a/hw/misc/lm32_sys.c
> > +++ b/hw/misc/lm32_sys.c
> > @@ -80,7 +80,7 @@ static void sys_write(void *opaque, hwaddr addr,
> >
> > case R_PASSFAIL:
> > s->regs[addr] = value;
> > testname = (char *)s->testname;
> >
> > - qemu_log("TC %-32s %s\n", testname, (value) ? "FAILED" : "OK");
> > + fprintf(stderr, "TC %-32s %s\n", testname, (value) ? "FAILED" :
> > "OK");
> >
> > break;
>
> This looks wrong to me -- devices shouldn't print to stderr, ideally.
lm32_sys is actually no real device. it is just used for unit testing.
-michael
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 09/12] lm32_sys: print test result on stderr
2014-02-01 20:31 ` Michael Walle
@ 2014-02-03 22:39 ` Michael Walle
2014-02-03 22:59 ` Peter Maydell
0 siblings, 1 reply; 32+ messages in thread
From: Michael Walle @ 2014-02-03 22:39 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Anthony Liguori
Am 2014-02-01 21:31, schrieb Michael Walle:
> Am Samstag, 1. Februar 2014, 19:00:01 schrieb Peter Maydell:
>> On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
>> > Do not use qemu_log().
>> >
>> > Signed-off-by: Michael Walle <michael@walle.cc>
>> > ---
>> >
>> > hw/misc/lm32_sys.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/hw/misc/lm32_sys.c b/hw/misc/lm32_sys.c
>> > index 8176cdb..6af0cca 100644
>> > --- a/hw/misc/lm32_sys.c
>> > +++ b/hw/misc/lm32_sys.c
>> > @@ -80,7 +80,7 @@ static void sys_write(void *opaque, hwaddr addr,
>> >
>> > case R_PASSFAIL:
>> > s->regs[addr] = value;
>> > testname = (char *)s->testname;
>> >
>> > - qemu_log("TC %-32s %s\n", testname, (value) ? "FAILED" : "OK");
>> > + fprintf(stderr, "TC %-32s %s\n", testname, (value) ? "FAILED" :
>> > "OK");
>> >
>> > break;
>>
>> This looks wrong to me -- devices shouldn't print to stderr, ideally.
>
> lm32_sys is actually no real device. it is just used for unit testing.
>
Hi Peter,
is this ok? or do you have some better idea? ideally, the lm32 target
should use semihosting and should print to stdout/stderr itself. but
that is not the case atm.
-michael
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 09/12] lm32_sys: print test result on stderr
2014-02-03 22:39 ` Michael Walle
@ 2014-02-03 22:59 ` Peter Maydell
2014-02-04 18:12 ` Michael Walle
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2014-02-03 22:59 UTC (permalink / raw)
To: Michael Walle; +Cc: QEMU Developers, Anthony Liguori
On 3 February 2014 22:39, Michael Walle <michael@walle.cc> wrote:
> Am 2014-02-01 21:31, schrieb Michael Walle:
>
>> Am Samstag, 1. Februar 2014, 19:00:01 schrieb Peter Maydell:
>>>
>>> On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
>>> > Do not use qemu_log().
>>> >
>>> > Signed-off-by: Michael Walle <michael@walle.cc>
>>> > ---
>>> >
>>> > hw/misc/lm32_sys.c | 2 +-
>>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>>> >
>>> > diff --git a/hw/misc/lm32_sys.c b/hw/misc/lm32_sys.c
>>> > index 8176cdb..6af0cca 100644
>>> > --- a/hw/misc/lm32_sys.c
>>> > +++ b/hw/misc/lm32_sys.c
>>> > @@ -80,7 +80,7 @@ static void sys_write(void *opaque, hwaddr addr,
>>> >
>>> > case R_PASSFAIL:
>>> > s->regs[addr] = value;
>>> > testname = (char *)s->testname;
>>> >
>>> > - qemu_log("TC %-32s %s\n", testname, (value) ? "FAILED" :
>>> > "OK");
>>> > + fprintf(stderr, "TC %-32s %s\n", testname, (value) ? "FAILED"
>>> > :
>>> > "OK");
>>> >
>>> > break;
>>>
>>> This looks wrong to me -- devices shouldn't print to stderr, ideally.
>>
>>
>> lm32_sys is actually no real device. it is just used for unit testing.
>>
>
> Hi Peter,
>
> is this ok? or do you have some better idea? ideally, the lm32 target should
> use semihosting and should print to stdout/stderr itself. but that is not
> the case atm.
Well, I guess for a testbench only kind of device printing to stderr is OK.
I've just noticed that the device maps itself into the memory map.
That's pretty foul. I can see why it does it, but really this thing is
bending a lot of the rules.
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 09/12] lm32_sys: print test result on stderr
2014-02-03 22:59 ` Peter Maydell
@ 2014-02-04 18:12 ` Michael Walle
0 siblings, 0 replies; 32+ messages in thread
From: Michael Walle @ 2014-02-04 18:12 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Anthony Liguori
Am 2014-02-03 23:59, schrieb Peter Maydell:
> On 3 February 2014 22:39, Michael Walle <michael@walle.cc> wrote:
>> Am 2014-02-01 21:31, schrieb Michael Walle:
>>
>>> Am Samstag, 1. Februar 2014, 19:00:01 schrieb Peter Maydell:
>>>>
>>>> On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
>>>> > Do not use qemu_log().
>>>> >
>>>> > Signed-off-by: Michael Walle <michael@walle.cc>
>>>> > ---
>>>> >
>>>> > hw/misc/lm32_sys.c | 2 +-
>>>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> >
>>>> > diff --git a/hw/misc/lm32_sys.c b/hw/misc/lm32_sys.c
>>>> > index 8176cdb..6af0cca 100644
>>>> > --- a/hw/misc/lm32_sys.c
>>>> > +++ b/hw/misc/lm32_sys.c
>>>> > @@ -80,7 +80,7 @@ static void sys_write(void *opaque, hwaddr addr,
>>>> >
>>>> > case R_PASSFAIL:
>>>> > s->regs[addr] = value;
>>>> > testname = (char *)s->testname;
>>>> >
>>>> > - qemu_log("TC %-32s %s\n", testname, (value) ? "FAILED" :
>>>> > "OK");
>>>> > + fprintf(stderr, "TC %-32s %s\n", testname, (value) ? "FAILED"
>>>> > :
>>>> > "OK");
>>>> >
>>>> > break;
>>>>
>>>> This looks wrong to me -- devices shouldn't print to stderr,
>>>> ideally.
>>>
>>>
>>> lm32_sys is actually no real device. it is just used for unit
>>> testing.
>>>
>>
>> Hi Peter,
>>
>> is this ok? or do you have some better idea? ideally, the lm32 target
>> should
>> use semihosting and should print to stdout/stderr itself. but that is
>> not
>> the case atm.
>
> Well, I guess for a testbench only kind of device printing to stderr is
> OK.
>
> I've just noticed that the device maps itself into the memory map.
> That's pretty foul. I can see why it does it, but really this thing is
> bending a lot of the rules.
So if it is ok, i'll leave this patch in this pull request. But i'll
also work on the lm32 semihosting so we can get rid of this device
entirely.
-michael
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL v4 10/12] lm32_sys: dump cpu state if test case fails
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
` (8 preceding siblings ...)
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 09/12] lm32_sys: print test result on stderr Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 11/12] target-lm32: stop VM on illegal or unknown instruction Michael Walle
` (2 subsequent siblings)
12 siblings, 0 replies; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Anthony Liguori
This will ease debugging the test cases.
Signed-off-by: Michael Walle <michael@walle.cc>
---
hw/misc/lm32_sys.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/misc/lm32_sys.c b/hw/misc/lm32_sys.c
index 6af0cca..1fd69ff 100644
--- a/hw/misc/lm32_sys.c
+++ b/hw/misc/lm32_sys.c
@@ -81,6 +81,9 @@ static void sys_write(void *opaque, hwaddr addr,
s->regs[addr] = value;
testname = (char *)s->testname;
fprintf(stderr, "TC %-32s %s\n", testname, (value) ? "FAILED" : "OK");
+ if (value) {
+ cpu_dump_state(qemu_get_cpu(0), stderr, fprintf, 0);
+ }
break;
case R_TESTNAME:
s->regs[addr] = value;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PULL v4 11/12] target-lm32: stop VM on illegal or unknown instruction
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
` (9 preceding siblings ...)
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 10/12] lm32_sys: dump cpu state if test case fails Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-02-01 18:06 ` Peter Maydell
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 12/12] hw/lm32: print error if cpu model is not found Michael Walle
2014-02-01 18:21 ` [Qemu-devel] [PULL v4 00/12] target-lm32 updates Peter Maydell
12 siblings, 1 reply; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Anthony Liguori
Instead of translating the instruction to a no-op, pause the VM and display
a message to the user.
As a side effect, this also works for instructions where the operands are
only known at runtime.
Signed-off-by: Michael Walle <michael@walle.cc>
---
target-lm32/helper.h | 1 +
target-lm32/op_helper.c | 17 +++++++++
target-lm32/translate.c | 91 +++++++++++++++++++++++++++++++----------------
3 files changed, 79 insertions(+), 30 deletions(-)
diff --git a/target-lm32/helper.h b/target-lm32/helper.h
index ad44fdf..f4442e0 100644
--- a/target-lm32/helper.h
+++ b/target-lm32/helper.h
@@ -13,5 +13,6 @@ DEF_HELPER_1(rcsr_im, i32, env)
DEF_HELPER_1(rcsr_ip, i32, env)
DEF_HELPER_1(rcsr_jtx, i32, env)
DEF_HELPER_1(rcsr_jrx, i32, env)
+DEF_HELPER_1(ill, void, env)
#include "exec/def-helper.h"
diff --git a/target-lm32/op_helper.c b/target-lm32/op_helper.c
index 71f21d1..7189cb5 100644
--- a/target-lm32/op_helper.c
+++ b/target-lm32/op_helper.c
@@ -8,6 +8,10 @@
#include "exec/softmmu_exec.h"
+#ifndef CONFIG_USER_ONLY
+#include "sysemu/sysemu.h"
+#endif
+
#if !defined(CONFIG_USER_ONLY)
#define MMUSUFFIX _mmu
#define SHIFT 0
@@ -39,6 +43,19 @@ void HELPER(hlt)(CPULM32State *env)
cpu_loop_exit(env);
}
+void HELPER(ill)(CPULM32State *env)
+{
+#ifndef CONFIG_USER_ONLY
+ CPUState *cs = CPU(lm32_env_get_cpu(env));
+ fprintf(stderr, "VM paused due to illegal instruction. "
+ "Connect a debugger or switch to the monitor console "
+ "to find out more.\n");
+ qemu_system_vmstop_request(RUN_STATE_PAUSED);
+ cs->halted = 1;
+ raise_exception(env, EXCP_HALTED);
+#endif
+}
+
void HELPER(wcsr_bp)(CPULM32State *env, uint32_t bp, uint32_t idx)
{
uint32_t addr = bp & ~1;
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index f20460a..43ea4e6 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -122,6 +122,12 @@ static inline void t_gen_raise_exception(DisasContext *dc, uint32_t index)
tcg_temp_free_i32(tmp);
}
+static inline void t_gen_illegal_insn(DisasContext *dc)
+{
+ tcg_gen_movi_tl(cpu_pc, dc->pc);
+ gen_helper_ill(cpu_env);
+}
+
static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
{
TranslationBlock *tb;
@@ -425,6 +431,7 @@ static void dec_divu(DisasContext *dc)
if (!(dc->features & LM32_FEATURE_DIVIDE)) {
qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not available\n");
+ t_gen_illegal_insn(dc);
return;
}
@@ -504,6 +511,7 @@ static void dec_modu(DisasContext *dc)
if (!(dc->features & LM32_FEATURE_DIVIDE)) {
qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not available\n");
+ t_gen_illegal_insn(dc);
return;
}
@@ -527,6 +535,7 @@ static void dec_mul(DisasContext *dc)
if (!(dc->features & LM32_FEATURE_MULTIPLY)) {
qemu_log_mask(LOG_GUEST_ERROR,
"hardware multiplier is not available\n");
+ t_gen_illegal_insn(dc);
return;
}
@@ -595,17 +604,18 @@ static void dec_scall(DisasContext *dc)
LOG_DIS("scall\n");
} else if (dc->imm5 == 2) {
LOG_DIS("break\n");
- } else {
- qemu_log_mask(LOG_GUEST_ERROR, "invalid opcode @0x%x", dc->pc);
- return;
}
if (dc->imm5 == 7) {
tcg_gen_movi_tl(cpu_pc, dc->pc);
t_gen_raise_exception(dc, EXCP_SYSTEMCALL);
- } else {
+ } else if (dc->imm5 == 2) {
tcg_gen_movi_tl(cpu_pc, dc->pc);
t_gen_raise_exception(dc, EXCP_BREAKPOINT);
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR, "invalid opcode @0x%x", dc->pc);
+ t_gen_illegal_insn(dc);
+ return;
}
}
@@ -681,6 +691,7 @@ static void dec_sextb(DisasContext *dc)
if (!(dc->features & LM32_FEATURE_SIGN_EXTEND)) {
qemu_log_mask(LOG_GUEST_ERROR,
"hardware sign extender is not available\n");
+ t_gen_illegal_insn(dc);
return;
}
@@ -694,6 +705,7 @@ static void dec_sexth(DisasContext *dc)
if (!(dc->features & LM32_FEATURE_SIGN_EXTEND)) {
qemu_log_mask(LOG_GUEST_ERROR,
"hardware sign extender is not available\n");
+ t_gen_illegal_insn(dc);
return;
}
@@ -722,6 +734,7 @@ static void dec_sl(DisasContext *dc)
if (!(dc->features & LM32_FEATURE_SHIFT)) {
qemu_log_mask(LOG_GUEST_ERROR, "hardware shifter is not available\n");
+ t_gen_illegal_insn(dc);
return;
}
@@ -743,24 +756,32 @@ static void dec_sr(DisasContext *dc)
LOG_DIS("sr r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1);
}
- if (!(dc->features & LM32_FEATURE_SHIFT)) {
- if (dc->format == OP_FMT_RI) {
- /* TODO: check r1 == 1 during runtime */
- } else {
- if (dc->imm5 != 1) {
- qemu_log_mask(LOG_GUEST_ERROR,
- "hardware shifter is not available\n");
- return;
- }
- }
- }
-
+ /* The real CPU (w/o hardware shifter) only supports right shift by exactly
+ * one bit */
if (dc->format == OP_FMT_RI) {
+ if (!(dc->features & LM32_FEATURE_SHIFT) && (dc->imm5 != 1)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "hardware shifter is not available\n");
+ t_gen_illegal_insn(dc);
+ return;
+ }
tcg_gen_sari_tl(cpu_R[dc->r1], cpu_R[dc->r0], dc->imm5);
} else {
- TCGv t0 = tcg_temp_new();
+ int l1 = gen_new_label();
+ int l2 = gen_new_label();
+ TCGv t0 = tcg_temp_local_new();
tcg_gen_andi_tl(t0, cpu_R[dc->r1], 0x1f);
+
+ if (!(dc->features & LM32_FEATURE_SHIFT)) {
+ tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 1, l1);
+ t_gen_illegal_insn(dc);
+ tcg_gen_br(l2);
+ }
+
+ gen_set_label(l1);
tcg_gen_sar_tl(cpu_R[dc->r2], cpu_R[dc->r0], t0);
+ gen_set_label(l2);
+
tcg_temp_free(t0);
}
}
@@ -773,24 +794,30 @@ static void dec_sru(DisasContext *dc)
LOG_DIS("sru r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1);
}
- if (!(dc->features & LM32_FEATURE_SHIFT)) {
- if (dc->format == OP_FMT_RI) {
- /* TODO: check r1 == 1 during runtime */
- } else {
- if (dc->imm5 != 1) {
- qemu_log_mask(LOG_GUEST_ERROR,
- "hardware shifter is not available\n");
- return;
- }
- }
- }
-
if (dc->format == OP_FMT_RI) {
+ if (!(dc->features & LM32_FEATURE_SHIFT) && (dc->imm5 != 1)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "hardware shifter is not available\n");
+ t_gen_illegal_insn(dc);
+ return;
+ }
tcg_gen_shri_tl(cpu_R[dc->r1], cpu_R[dc->r0], dc->imm5);
} else {
- TCGv t0 = tcg_temp_new();
+ int l1 = gen_new_label();
+ int l2 = gen_new_label();
+ TCGv t0 = tcg_temp_local_new();
tcg_gen_andi_tl(t0, cpu_R[dc->r1], 0x1f);
+
+ if (!(dc->features & LM32_FEATURE_SHIFT)) {
+ tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 1, l1);
+ t_gen_illegal_insn(dc);
+ tcg_gen_br(l2);
+ }
+
+ gen_set_label(l1);
tcg_gen_shr_tl(cpu_R[dc->r2], cpu_R[dc->r0], t0);
+ gen_set_label(l2);
+
tcg_temp_free(t0);
}
}
@@ -819,6 +846,7 @@ static void dec_user(DisasContext *dc)
LOG_DIS("user");
qemu_log_mask(LOG_GUEST_ERROR, "user instruction undefined\n");
+ t_gen_illegal_insn(dc);
}
static void dec_wcsr(DisasContext *dc)
@@ -886,6 +914,7 @@ static void dec_wcsr(DisasContext *dc)
if (dc->num_breakpoints <= no) {
qemu_log_mask(LOG_GUEST_ERROR,
"breakpoint #%i is not available\n", no);
+ t_gen_illegal_insn(dc);
break;
}
gen_helper_wcsr_bp(cpu_env, cpu_R[dc->r1], tcg_const_i32(no));
@@ -898,6 +927,7 @@ static void dec_wcsr(DisasContext *dc)
if (dc->num_watchpoints <= no) {
qemu_log_mask(LOG_GUEST_ERROR,
"watchpoint #%i is not available\n", no);
+ t_gen_illegal_insn(dc);
break;
}
gen_helper_wcsr_wp(cpu_env, cpu_R[dc->r1], tcg_const_i32(no));
@@ -956,6 +986,7 @@ static void dec_xor(DisasContext *dc)
static void dec_ill(DisasContext *dc)
{
qemu_log_mask(LOG_GUEST_ERROR, "invalid opcode 0x%02x\n", dc->opcode);
+ t_gen_illegal_insn(dc);
}
typedef void (*DecoderInfo)(DisasContext *dc);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 11/12] target-lm32: stop VM on illegal or unknown instruction
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 11/12] target-lm32: stop VM on illegal or unknown instruction Michael Walle
@ 2014-02-01 18:06 ` Peter Maydell
2014-02-01 20:53 ` Michael Walle
0 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2014-02-01 18:06 UTC (permalink / raw)
To: Michael Walle; +Cc: QEMU Developers, Anthony Liguori
On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
> Instead of translating the instruction to a no-op, pause the VM and display
> a message to the user.
>
> As a side effect, this also works for instructions where the operands are
> only known at runtime.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> target-lm32/helper.h | 1 +
> target-lm32/op_helper.c | 17 +++++++++
> target-lm32/translate.c | 91 +++++++++++++++++++++++++++++++----------------
> 3 files changed, 79 insertions(+), 30 deletions(-)
>
> diff --git a/target-lm32/helper.h b/target-lm32/helper.h
> index ad44fdf..f4442e0 100644
> --- a/target-lm32/helper.h
> +++ b/target-lm32/helper.h
> @@ -13,5 +13,6 @@ DEF_HELPER_1(rcsr_im, i32, env)
> DEF_HELPER_1(rcsr_ip, i32, env)
> DEF_HELPER_1(rcsr_jtx, i32, env)
> DEF_HELPER_1(rcsr_jrx, i32, env)
> +DEF_HELPER_1(ill, void, env)
>
> #include "exec/def-helper.h"
> diff --git a/target-lm32/op_helper.c b/target-lm32/op_helper.c
> index 71f21d1..7189cb5 100644
> --- a/target-lm32/op_helper.c
> +++ b/target-lm32/op_helper.c
> @@ -8,6 +8,10 @@
>
> #include "exec/softmmu_exec.h"
>
> +#ifndef CONFIG_USER_ONLY
> +#include "sysemu/sysemu.h"
> +#endif
> +
> #if !defined(CONFIG_USER_ONLY)
> #define MMUSUFFIX _mmu
> #define SHIFT 0
> @@ -39,6 +43,19 @@ void HELPER(hlt)(CPULM32State *env)
> cpu_loop_exit(env);
> }
>
> +void HELPER(ill)(CPULM32State *env)
> +{
> +#ifndef CONFIG_USER_ONLY
> + CPUState *cs = CPU(lm32_env_get_cpu(env));
> + fprintf(stderr, "VM paused due to illegal instruction. "
> + "Connect a debugger or switch to the monitor console "
> + "to find out more.\n");
> + qemu_system_vmstop_request(RUN_STATE_PAUSED);
> + cs->halted = 1;
> + raise_exception(env, EXCP_HALTED);
> +#endif
Not really convinced this is a great idea. "This one target CPU
type does something that none of the others do" seems less
than ideal for QEMU as a whole.
> +}
> +
> void HELPER(wcsr_bp)(CPULM32State *env, uint32_t bp, uint32_t idx)
> {
> uint32_t addr = bp & ~1;
> diff --git a/target-lm32/translate.c b/target-lm32/translate.c
> index f20460a..43ea4e6 100644
> --- a/target-lm32/translate.c
> +++ b/target-lm32/translate.c
> @@ -122,6 +122,12 @@ static inline void t_gen_raise_exception(DisasContext *dc, uint32_t index)
> tcg_temp_free_i32(tmp);
> }
>
> +static inline void t_gen_illegal_insn(DisasContext *dc)
> +{
> + tcg_gen_movi_tl(cpu_pc, dc->pc);
> + gen_helper_ill(cpu_env);
> +}
> +
> static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
> {
> TranslationBlock *tb;
> @@ -425,6 +431,7 @@ static void dec_divu(DisasContext *dc)
>
> if (!(dc->features & LM32_FEATURE_DIVIDE)) {
> qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not available\n");
> + t_gen_illegal_insn(dc);
> return;
> }
>
> @@ -504,6 +511,7 @@ static void dec_modu(DisasContext *dc)
>
> if (!(dc->features & LM32_FEATURE_DIVIDE)) {
> qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not available\n");
> + t_gen_illegal_insn(dc);
> return;
> }
>
> @@ -527,6 +535,7 @@ static void dec_mul(DisasContext *dc)
> if (!(dc->features & LM32_FEATURE_MULTIPLY)) {
> qemu_log_mask(LOG_GUEST_ERROR,
> "hardware multiplier is not available\n");
> + t_gen_illegal_insn(dc);
> return;
> }
>
> @@ -595,17 +604,18 @@ static void dec_scall(DisasContext *dc)
> LOG_DIS("scall\n");
> } else if (dc->imm5 == 2) {
> LOG_DIS("break\n");
> - } else {
> - qemu_log_mask(LOG_GUEST_ERROR, "invalid opcode @0x%x", dc->pc);
> - return;
> }
>
> if (dc->imm5 == 7) {
> tcg_gen_movi_tl(cpu_pc, dc->pc);
> t_gen_raise_exception(dc, EXCP_SYSTEMCALL);
> - } else {
> + } else if (dc->imm5 == 2) {
> tcg_gen_movi_tl(cpu_pc, dc->pc);
> t_gen_raise_exception(dc, EXCP_BREAKPOINT);
> + } else {
> + qemu_log_mask(LOG_GUEST_ERROR, "invalid opcode @0x%x", dc->pc);
> + t_gen_illegal_insn(dc);
> + return;
> }
This leaves this function with two consecutive identical if..elseif..else
ladders: why not combine them together? (optionally, use
switch(dc->imm5).)
The rest looks OK.
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 11/12] target-lm32: stop VM on illegal or unknown instruction
2014-02-01 18:06 ` Peter Maydell
@ 2014-02-01 20:53 ` Michael Walle
0 siblings, 0 replies; 32+ messages in thread
From: Michael Walle @ 2014-02-01 20:53 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Anthony Liguori
Am Samstag, 1. Februar 2014, 19:06:40 schrieb Peter Maydell:
> On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
> > Instead of translating the instruction to a no-op, pause the VM and
> > display a message to the user.
> >
> > As a side effect, this also works for instructions where the operands are
> > only known at runtime.
> >
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> >
> > target-lm32/helper.h | 1 +
> > target-lm32/op_helper.c | 17 +++++++++
> > target-lm32/translate.c | 91
> > +++++++++++++++++++++++++++++++---------------- 3 files changed, 79
> > insertions(+), 30 deletions(-)
> >
> > diff --git a/target-lm32/helper.h b/target-lm32/helper.h
> > index ad44fdf..f4442e0 100644
> > --- a/target-lm32/helper.h
> > +++ b/target-lm32/helper.h
> > @@ -13,5 +13,6 @@ DEF_HELPER_1(rcsr_im, i32, env)
> >
> > DEF_HELPER_1(rcsr_ip, i32, env)
> > DEF_HELPER_1(rcsr_jtx, i32, env)
> > DEF_HELPER_1(rcsr_jrx, i32, env)
> >
> > +DEF_HELPER_1(ill, void, env)
> >
> > #include "exec/def-helper.h"
> >
> > diff --git a/target-lm32/op_helper.c b/target-lm32/op_helper.c
> > index 71f21d1..7189cb5 100644
> > --- a/target-lm32/op_helper.c
> > +++ b/target-lm32/op_helper.c
> > @@ -8,6 +8,10 @@
> >
> > #include "exec/softmmu_exec.h"
> >
> > +#ifndef CONFIG_USER_ONLY
> > +#include "sysemu/sysemu.h"
> > +#endif
> > +
> >
> > #if !defined(CONFIG_USER_ONLY)
> > #define MMUSUFFIX _mmu
> > #define SHIFT 0
> >
> > @@ -39,6 +43,19 @@ void HELPER(hlt)(CPULM32State *env)
> >
> > cpu_loop_exit(env);
> >
> > }
> >
> > +void HELPER(ill)(CPULM32State *env)
> > +{
> > +#ifndef CONFIG_USER_ONLY
> > + CPUState *cs = CPU(lm32_env_get_cpu(env));
> > + fprintf(stderr, "VM paused due to illegal instruction. "
> > + "Connect a debugger or switch to the monitor console "
> > + "to find out more.\n");
> > + qemu_system_vmstop_request(RUN_STATE_PAUSED);
> > + cs->halted = 1;
> > + raise_exception(env, EXCP_HALTED);
> > +#endif
>
> Not really convinced this is a great idea. "This one target CPU
> type does something that none of the others do" seems less
> than ideal for QEMU as a whole.
this was discussed some time ago. unfortunately, i don't find the thread atm.
the particular problem with the lm32 target is that there is no defined
behaviour if some unsupported opcode is executed on the real hardware, esp.
there are no exceptions if that happens.
therefore the suggested behaviour was to just stop the emulation and give the
developer the chance to look at it by connecting a debugger.
keep in mind that the lm32 target is more a development aid, rather than a
real virtualization target.
-michael
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL v4 12/12] hw/lm32: print error if cpu model is not found
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
` (10 preceding siblings ...)
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 11/12] target-lm32: stop VM on illegal or unknown instruction Michael Walle
@ 2014-01-20 19:34 ` Michael Walle
2014-02-01 18:11 ` Peter Maydell
2014-02-01 18:21 ` [Qemu-devel] [PULL v4 00/12] target-lm32 updates Peter Maydell
12 siblings, 1 reply; 32+ messages in thread
From: Michael Walle @ 2014-01-20 19:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Walle, Anthony Liguori
QEMU crashed if a the given cpu_model is not found.
Signed-off-by: Michael Walle <michael@walle.cc>
---
hw/lm32/lm32_boards.c | 10 ++++++++++
hw/lm32/milkymist.c | 5 +++++
2 files changed, 15 insertions(+)
diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c
index c032bb8..5e22e9b 100644
--- a/hw/lm32/lm32_boards.c
+++ b/hw/lm32/lm32_boards.c
@@ -101,6 +101,11 @@ static void lm32_evr_init(QEMUMachineInitArgs *args)
cpu_model = "lm32-full";
}
cpu = cpu_lm32_init(cpu_model);
+ if (cpu == NULL) {
+ fprintf(stderr, "qemu: unable to find CPU '%s'\n", cpu_model);
+ exit(1);
+ }
+
env = &cpu->env;
reset_info->cpu = cpu;
@@ -198,6 +203,11 @@ static void lm32_uclinux_init(QEMUMachineInitArgs *args)
cpu_model = "lm32-full";
}
cpu = cpu_lm32_init(cpu_model);
+ if (cpu == NULL) {
+ fprintf(stderr, "qemu: unable to find CPU '%s'\n", cpu_model);
+ exit(1);
+ }
+
env = &cpu->env;
reset_info->cpu = cpu;
diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index 15053c4..baf234c 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -108,6 +108,11 @@ milkymist_init(QEMUMachineInitArgs *args)
cpu_model = "lm32-full";
}
cpu = cpu_lm32_init(cpu_model);
+ if (cpu == NULL) {
+ fprintf(stderr, "qemu: unable to find CPU '%s'\n", cpu_model);
+ exit(1);
+ }
+
env = &cpu->env;
reset_info->cpu = cpu;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 00/12] target-lm32 updates
2014-01-20 19:34 [Qemu-devel] [PULL v4 00/12] target-lm32 updates Michael Walle
` (11 preceding siblings ...)
2014-01-20 19:34 ` [Qemu-devel] [PULL v4 12/12] hw/lm32: print error if cpu model is not found Michael Walle
@ 2014-02-01 18:21 ` Peter Maydell
2014-02-01 20:56 ` Michael Walle
12 siblings, 1 reply; 32+ messages in thread
From: Peter Maydell @ 2014-02-01 18:21 UTC (permalink / raw)
To: Michael Walle; +Cc: QEMU Developers, Anthony Liguori
On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
> Hi Anthony,
>
> this is a pull request for various updates and fixes for the LatticeMico32
> target.
>
> Please pull.
Apologies for doing a review pass on this rather than pulling it.
However given that most of the patches didn't have reviewed-by
tags I gave them a quick eyeball and felt there were enough minor
issues there that I would rather err on the side of caution. If you
address the issues I'll review a respin of this patch series and I'm
happy to apply a pull request once the patches have got review.
thanks
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PULL v4 00/12] target-lm32 updates
2014-02-01 18:21 ` [Qemu-devel] [PULL v4 00/12] target-lm32 updates Peter Maydell
@ 2014-02-01 20:56 ` Michael Walle
0 siblings, 0 replies; 32+ messages in thread
From: Michael Walle @ 2014-02-01 20:56 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Anthony Liguori
Am Samstag, 1. Februar 2014, 19:21:28 schrieb Peter Maydell:
> On 20 January 2014 19:34, Michael Walle <michael@walle.cc> wrote:
> > Hi Anthony,
> >
> > this is a pull request for various updates and fixes for the
> > LatticeMico32 target.
> >
> > Please pull.
>
> Apologies for doing a review pass on this rather than pulling it.
> However given that most of the patches didn't have reviewed-by
> tags I gave them a quick eyeball and felt there were enough minor
> issues there that I would rather err on the side of caution. If you
> address the issues I'll review a respin of this patch series and I'm
> happy to apply a pull request once the patches have got review.
>
> thanks
> -- PMM
no problem and thanks for the review. i'm happy to post a cleaned up v5 ;)
-michael
^ permalink raw reply [flat|nested] 32+ messages in thread