[PATCH v2 15/55] target/sparc: Remove DEBUG_UNALIGNED

Richard Henderson posted 55 patches 4 years, 6 months ago
Maintainers: Cornelia Huck <cohuck@redhat.com>, Greg Kurz <groug@kaod.org>, Bin Meng <bin.meng@windriver.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Alistair Francis <alistair.francis@wdc.com>, Eduardo Habkost <ehabkost@redhat.com>, Marek Vasut <marex@denx.de>, Richard Henderson <richard.henderson@linaro.org>, Riku Voipio <riku.voipio@iki.fi>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Alexandre Iooss <erdnaxe@crans.org>, Stefan Hajnoczi <stefanha@redhat.com>, Jiaxun Yang <jiaxun.yang@flygoat.com>, David Hildenbrand <david@redhat.com>, Palmer Dabbelt <palmer@dabbelt.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Aurelien Jarno <aurelien@aurel32.net>, Chris Wulff <crwulff@gmail.com>, Laurent Vivier <laurent@vivier.eu>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Yoshinori Sato <ysato@users.sourceforge.jp>, Max Filippov <jcmvbkbc@gmail.com>, Mahmoud Mandour <ma.mandourr@gmail.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>
There is a newer version of this series
[PATCH v2 15/55] target/sparc: Remove DEBUG_UNALIGNED
Posted by Richard Henderson 4 years, 6 months ago
The printf should have been qemu_log_mask, the parameters
themselves no longer compile, and because this is placed
before unwinding the PC is actively wrong.

We get better (and correct) logging on the other side of
raising the exception, in sparc_cpu_do_interrupt.

Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sparc/ldst_helper.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
index 22327d7d72..974afea041 100644
--- a/target/sparc/ldst_helper.c
+++ b/target/sparc/ldst_helper.c
@@ -27,7 +27,6 @@
 
 //#define DEBUG_MMU
 //#define DEBUG_MXCC
-//#define DEBUG_UNALIGNED
 //#define DEBUG_UNASSIGNED
 //#define DEBUG_ASI
 //#define DEBUG_CACHE_CONTROL
@@ -364,10 +363,6 @@ static void do_check_align(CPUSPARCState *env, target_ulong addr,
                            uint32_t align, uintptr_t ra)
 {
     if (addr & align) {
-#ifdef DEBUG_UNALIGNED
-        printf("Unaligned access to 0x" TARGET_FMT_lx " from 0x" TARGET_FMT_lx
-               "\n", addr, env->pc);
-#endif
         cpu_raise_exception_ra(env, TT_UNALIGNED, ra);
     }
 }
@@ -1968,10 +1963,6 @@ void QEMU_NORETURN sparc_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     SPARCCPU *cpu = SPARC_CPU(cs);
     CPUSPARCState *env = &cpu->env;
 
-#ifdef DEBUG_UNALIGNED
-    printf("Unaligned access to 0x" TARGET_FMT_lx " from 0x" TARGET_FMT_lx
-           "\n", addr, env->pc);
-#endif
     cpu_raise_exception_ra(env, TT_UNALIGNED, retaddr);
 }
 #endif
-- 
2.25.1


Re: [PATCH v2 15/55] target/sparc: Remove DEBUG_UNALIGNED
Posted by Mark Cave-Ayland 4 years, 5 months ago
On 03/08/2021 05:14, Richard Henderson wrote:

> The printf should have been qemu_log_mask, the parameters
> themselves no longer compile, and because this is placed
> before unwinding the PC is actively wrong.
> 
> We get better (and correct) logging on the other side of
> raising the exception, in sparc_cpu_do_interrupt.
> 
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/sparc/ldst_helper.c | 9 ---------
>   1 file changed, 9 deletions(-)
> 
> diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
> index 22327d7d72..974afea041 100644
> --- a/target/sparc/ldst_helper.c
> +++ b/target/sparc/ldst_helper.c
> @@ -27,7 +27,6 @@
>   
>   //#define DEBUG_MMU
>   //#define DEBUG_MXCC
> -//#define DEBUG_UNALIGNED
>   //#define DEBUG_UNASSIGNED
>   //#define DEBUG_ASI
>   //#define DEBUG_CACHE_CONTROL
> @@ -364,10 +363,6 @@ static void do_check_align(CPUSPARCState *env, target_ulong addr,
>                              uint32_t align, uintptr_t ra)
>   {
>       if (addr & align) {
> -#ifdef DEBUG_UNALIGNED
> -        printf("Unaligned access to 0x" TARGET_FMT_lx " from 0x" TARGET_FMT_lx
> -               "\n", addr, env->pc);
> -#endif
>           cpu_raise_exception_ra(env, TT_UNALIGNED, ra);
>       }
>   }
> @@ -1968,10 +1963,6 @@ void QEMU_NORETURN sparc_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>       SPARCCPU *cpu = SPARC_CPU(cs);
>       CPUSPARCState *env = &cpu->env;
>   
> -#ifdef DEBUG_UNALIGNED
> -    printf("Unaligned access to 0x" TARGET_FMT_lx " from 0x" TARGET_FMT_lx
> -           "\n", addr, env->pc);
> -#endif
>       cpu_raise_exception_ra(env, TT_UNALIGNED, retaddr);
>   }
>   #endif

Indeed. I suspect that this was originally intended for developers to capture that an 
unaligned access occurred rather than processing it correctly.

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.