* [PATCH 0/3] target/avr: Few fixes
@ 2020-07-07 6:46 Philippe Mathieu-Daudé
2020-07-07 6:46 ` [PATCH 1/3] target/avr: Drop tlb_flush() in avr_cpu_reset() Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-07 6:46 UTC (permalink / raw)
To: qemu-devel
Cc: Sarah Harris, Thomas Huth, Joaquin de Andres, Richard Henderson,
Philippe Mathieu-Daudé,
Michael Rolnik
Few fixes on top of the AVR merger series Thomas sent yesterday:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg720089.html
Based-on: <20200705140315.260514-1-huth@tuxfamily.org>
Philippe Mathieu-Daudé (3):
target/avr: Drop tlb_flush() in avr_cpu_reset()
target/avr: Fix $PC displayed address
target/avr: Fix SBRC/SBRS instructions
target/avr/cpu.c | 4 +---
target/avr/translate.c | 4 ++--
2 files changed, 3 insertions(+), 5 deletions(-)
--
2.21.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] target/avr: Drop tlb_flush() in avr_cpu_reset()
2020-07-07 6:46 [PATCH 0/3] target/avr: Few fixes Philippe Mathieu-Daudé
@ 2020-07-07 6:46 ` Philippe Mathieu-Daudé
2020-07-07 8:27 ` Alex Bennée
2020-07-07 6:46 ` [PATCH 2/3] target/avr: Fix $PC displayed address Philippe Mathieu-Daudé
2020-07-07 6:46 ` [RFC PATCH 3/3] target/avr: Fix SBRC/SBRS instructions Philippe Mathieu-Daudé
2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-07 6:46 UTC (permalink / raw)
To: qemu-devel
Cc: Sarah Harris, Thomas Huth, Joaquin de Andres, Richard Henderson,
Philippe Mathieu-Daudé,
Michael Rolnik
Since commit 1f5c00cfdb tlb_flush() is called from cpu_common_reset().
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
target/avr/cpu.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 4e4dd4f6aa..50fb1c378b 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -78,8 +78,6 @@ static void avr_cpu_reset(DeviceState *ds)
env->skip = 0;
memset(env->r, 0, sizeof(env->r));
-
- tlb_flush(cs);
}
static void avr_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
--
2.21.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] target/avr: Fix $PC displayed address
2020-07-07 6:46 [PATCH 0/3] target/avr: Few fixes Philippe Mathieu-Daudé
2020-07-07 6:46 ` [PATCH 1/3] target/avr: Drop tlb_flush() in avr_cpu_reset() Philippe Mathieu-Daudé
@ 2020-07-07 6:46 ` Philippe Mathieu-Daudé
2020-07-07 8:33 ` Alex Bennée
2020-07-07 6:46 ` [RFC PATCH 3/3] target/avr: Fix SBRC/SBRS instructions Philippe Mathieu-Daudé
2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-07 6:46 UTC (permalink / raw)
To: qemu-devel
Cc: Sarah Harris, Thomas Huth, Joaquin de Andres, Richard Henderson,
Philippe Mathieu-Daudé,
Michael Rolnik
$PC is 16-bit wide. Other registers display addresses on a byte
granularity.
To have a coherent ouput, display $PC using byte granularity too.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
target/avr/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 50fb1c378b..9be464991f 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -151,7 +151,7 @@ static void avr_cpu_dump_state(CPUState *cs, FILE *f, int flags)
int i;
qemu_fprintf(f, "\n");
- qemu_fprintf(f, "PC: %06x\n", env->pc_w);
+ qemu_fprintf(f, "PC: %06x\n", env->pc_w * 2);
qemu_fprintf(f, "SP: %04x\n", env->sp);
qemu_fprintf(f, "rampD: %02x\n", env->rampD >> 16);
qemu_fprintf(f, "rampX: %02x\n", env->rampX >> 16);
--
2.21.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC PATCH 3/3] target/avr: Fix SBRC/SBRS instructions
2020-07-07 6:46 [PATCH 0/3] target/avr: Few fixes Philippe Mathieu-Daudé
2020-07-07 6:46 ` [PATCH 1/3] target/avr: Drop tlb_flush() in avr_cpu_reset() Philippe Mathieu-Daudé
2020-07-07 6:46 ` [PATCH 2/3] target/avr: Fix $PC displayed address Philippe Mathieu-Daudé
@ 2020-07-07 6:46 ` Philippe Mathieu-Daudé
2020-07-07 8:56 ` Alex Bennée
2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-07 6:46 UTC (permalink / raw)
To: qemu-devel
Cc: Sarah Harris, Thomas Huth, Joaquin de Andres, Richard Henderson,
Philippe Mathieu-Daudé,
Michael Rolnik
SBRC/SBRS instructions seem to be inverted.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
target/avr/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index fe03e676df..2f77fe3ba7 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1385,7 +1385,7 @@ static bool trans_SBRC(DisasContext *ctx, arg_SBRC *a)
{
TCGv Rr = cpu_r[a->rr];
- ctx->skip_cond = TCG_COND_EQ;
+ ctx->skip_cond = TCG_COND_NE;
ctx->skip_var0 = tcg_temp_new();
ctx->free_skip_var0 = true;
@@ -1401,7 +1401,7 @@ static bool trans_SBRS(DisasContext *ctx, arg_SBRS *a)
{
TCGv Rr = cpu_r[a->rr];
- ctx->skip_cond = TCG_COND_NE;
+ ctx->skip_cond = TCG_COND_EQ;
ctx->skip_var0 = tcg_temp_new();
ctx->free_skip_var0 = true;
--
2.21.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] target/avr: Drop tlb_flush() in avr_cpu_reset()
2020-07-07 6:46 ` [PATCH 1/3] target/avr: Drop tlb_flush() in avr_cpu_reset() Philippe Mathieu-Daudé
@ 2020-07-07 8:27 ` Alex Bennée
0 siblings, 0 replies; 7+ messages in thread
From: Alex Bennée @ 2020-07-07 8:27 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Sarah Harris, Thomas Huth, Joaquin de Andres, Richard Henderson,
qemu-devel, Michael Rolnik
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> Since commit 1f5c00cfdb tlb_flush() is called from cpu_common_reset().
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> target/avr/cpu.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index 4e4dd4f6aa..50fb1c378b 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -78,8 +78,6 @@ static void avr_cpu_reset(DeviceState *ds)
> env->skip = 0;
>
> memset(env->r, 0, sizeof(env->r));
> -
> - tlb_flush(cs);
> }
>
> static void avr_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
--
Alex Bennée
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] target/avr: Fix $PC displayed address
2020-07-07 6:46 ` [PATCH 2/3] target/avr: Fix $PC displayed address Philippe Mathieu-Daudé
@ 2020-07-07 8:33 ` Alex Bennée
0 siblings, 0 replies; 7+ messages in thread
From: Alex Bennée @ 2020-07-07 8:33 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Sarah Harris, Thomas Huth, Joaquin de Andres, Richard Henderson,
qemu-devel, Michael Rolnik
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> $PC is 16-bit wide. Other registers display addresses on a byte
> granularity.
> To have a coherent ouput, display $PC using byte granularity too.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/avr/cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index 50fb1c378b..9be464991f 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -151,7 +151,7 @@ static void avr_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> int i;
>
> qemu_fprintf(f, "\n");
> - qemu_fprintf(f, "PC: %06x\n", env->pc_w);
> + qemu_fprintf(f, "PC: %06x\n", env->pc_w * 2);
OK this was confusing until I grepped around the code and found the
comment in set pc:
cpu->env.pc_w = value / 2; /* internally PC points to words */
so it makes sense I guess but I didn't pick it up from the name. Maybe
worth adding the comment in CPUAVRstate?
Anyway:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> qemu_fprintf(f, "SP: %04x\n", env->sp);
> qemu_fprintf(f, "rampD: %02x\n", env->rampD >> 16);
> qemu_fprintf(f, "rampX: %02x\n", env->rampX >> 16);
--
Alex Bennée
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 3/3] target/avr: Fix SBRC/SBRS instructions
2020-07-07 6:46 ` [RFC PATCH 3/3] target/avr: Fix SBRC/SBRS instructions Philippe Mathieu-Daudé
@ 2020-07-07 8:56 ` Alex Bennée
0 siblings, 0 replies; 7+ messages in thread
From: Alex Bennée @ 2020-07-07 8:56 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Sarah Harris, Thomas Huth, Joaquin de Andres, Richard Henderson,
qemu-devel, Michael Rolnik
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> SBRC/SBRS instructions seem to be inverted.
I'm having trouble following exactly how the skip logic is meant to
work. Intuitively I would expect a skip if clear to be TCG_COND_EQ
because that is true if bit & mask is compared to 0 but it's not clear
that what happens.
It would be easier if we actually had some instruction tests. I see
gcc-avr is a thing.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/avr/translate.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/avr/translate.c b/target/avr/translate.c
> index fe03e676df..2f77fe3ba7 100644
> --- a/target/avr/translate.c
> +++ b/target/avr/translate.c
> @@ -1385,7 +1385,7 @@ static bool trans_SBRC(DisasContext *ctx, arg_SBRC *a)
> {
> TCGv Rr = cpu_r[a->rr];
>
> - ctx->skip_cond = TCG_COND_EQ;
> + ctx->skip_cond = TCG_COND_NE;
> ctx->skip_var0 = tcg_temp_new();
> ctx->free_skip_var0 = true;
>
> @@ -1401,7 +1401,7 @@ static bool trans_SBRS(DisasContext *ctx, arg_SBRS *a)
> {
> TCGv Rr = cpu_r[a->rr];
>
> - ctx->skip_cond = TCG_COND_NE;
> + ctx->skip_cond = TCG_COND_EQ;
> ctx->skip_var0 = tcg_temp_new();
> ctx->free_skip_var0 = true;
--
Alex Bennée
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-07 8:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-07 6:46 [PATCH 0/3] target/avr: Few fixes Philippe Mathieu-Daudé
2020-07-07 6:46 ` [PATCH 1/3] target/avr: Drop tlb_flush() in avr_cpu_reset() Philippe Mathieu-Daudé
2020-07-07 8:27 ` Alex Bennée
2020-07-07 6:46 ` [PATCH 2/3] target/avr: Fix $PC displayed address Philippe Mathieu-Daudé
2020-07-07 8:33 ` Alex Bennée
2020-07-07 6:46 ` [RFC PATCH 3/3] target/avr: Fix SBRC/SBRS instructions Philippe Mathieu-Daudé
2020-07-07 8:56 ` Alex Bennée
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.