[PATCH 0/3] target/unicore32: Semihosting cleanup

[PATCH 0/3] target/unicore32: Semihosting cleanup

[Philippe Mathieu-Daudé]

Trivial cleanups around semihosting code.

Guan: Do you have binaries to run testing?

I couldn't rebuild anything from https://github.com/gxt/UniCore32
that you referenced here:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg608054.html
- u-boot patches are missing
- which gnu-toolchain-unicore should we use?
- your webpage is down: http://mprc.pku.edu.cn/~guanxuetao/
- there are too many warnings reported for QEMU 2.7-stable.

Philippe Mathieu-Daudé (3):
  target/unicore32: Remove unused headers
  target/unicore32: Replace DPRINTF() by qemu_log_mask(GUEST_ERROR)
  target/unicore32: Prefer qemu_semihosting_log_out() over curses

 default-configs/unicore32-softmmu.mak |  1 +
 target/unicore32/helper.c             | 70 +++++----------------------
 2 files changed, 13 insertions(+), 58 deletions(-)

-- 
2.21.3

[PATCH 1/3] target/unicore32: Remove unused headers

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/unicore32/helper.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target/unicore32/helper.c b/target/unicore32/helper.c
index 7d538e2144..53292ae311 100644
--- a/target/unicore32/helper.c
+++ b/target/unicore32/helper.c
@@ -12,9 +12,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
-#include "exec/gdbstub.h"
 #include "exec/helper-proto.h"
-#include "qemu/host-utils.h"
 #ifndef CONFIG_USER_ONLY
 #include "ui/console.h"
 #endif
-- 
2.21.3

Re: [PATCH 1/3] target/unicore32: Remove unused headers

[Richard Henderson]

On 6/3/20 5:37 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/unicore32/helper.c | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 1/3] target/unicore32: Remove unused headers

[Laurent Vivier]

Le 03/06/2020 à 14:37, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/unicore32/helper.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/target/unicore32/helper.c b/target/unicore32/helper.c
> index 7d538e2144..53292ae311 100644
> --- a/target/unicore32/helper.c
> +++ b/target/unicore32/helper.c
> @@ -12,9 +12,7 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "exec/exec-all.h"
> -#include "exec/gdbstub.h"
>  #include "exec/helper-proto.h"
> -#include "qemu/host-utils.h"
>  #ifndef CONFIG_USER_ONLY
>  #include "ui/console.h"
>  #endif
> 

Applied to my trivial-patches branch.

Thanks,
Laurent

[PATCH 2/3] target/unicore32: Replace DPRINTF() by qemu_log_mask(GUEST_ERROR)

[Philippe Mathieu-Daudé]

Replace disabled DPRINTF() by qemu_log_mask(GUEST_ERROR).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/unicore32/helper.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/target/unicore32/helper.c b/target/unicore32/helper.c
index 53292ae311..00371a7da6 100644
--- a/target/unicore32/helper.c
+++ b/target/unicore32/helper.c
@@ -10,6 +10,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
@@ -106,8 +107,9 @@ void helper_cp0_set(CPUUniCore32State *env, uint32_t val, uint32_t creg,
     }
     return;
 unrecognized:
-    DPRINTF("Wrong register (%d) or wrong operation (%d) in cp0_set!\n",
-            creg, cop);
+    qemu_log_mask(LOG_GUEST_ERROR,
+                  "Wrong register (%d) or wrong operation (%d) in cp0_set!\n",
+                  creg, cop);
 }
 
 uint32_t helper_cp0_get(CPUUniCore32State *env, uint32_t creg, uint32_t cop)
@@ -153,8 +155,9 @@ uint32_t helper_cp0_get(CPUUniCore32State *env, uint32_t creg, uint32_t cop)
         }
         break;
     }
-    DPRINTF("Wrong register (%d) or wrong operation (%d) in cp0_set!\n",
-            creg, cop);
+    qemu_log_mask(LOG_GUEST_ERROR,
+                  "Wrong register (%d) or wrong operation (%d) in cp0_set!\n",
+                  creg, cop);
     return 0;
 }
 
-- 
2.21.3

Re: [PATCH 2/3] target/unicore32: Replace DPRINTF() by qemu_log_mask(GUEST_ERROR)

[Richard Henderson]

On 6/3/20 5:37 AM, Philippe Mathieu-Daudé wrote:
> Replace disabled DPRINTF() by qemu_log_mask(GUEST_ERROR).
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/unicore32/helper.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 2/3] target/unicore32: Replace DPRINTF() by qemu_log_mask(GUEST_ERROR)

[Laurent Vivier]

Le 03/06/2020 à 14:37, Philippe Mathieu-Daudé a écrit :
> Replace disabled DPRINTF() by qemu_log_mask(GUEST_ERROR).
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/unicore32/helper.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/target/unicore32/helper.c b/target/unicore32/helper.c
> index 53292ae311..00371a7da6 100644
> --- a/target/unicore32/helper.c
> +++ b/target/unicore32/helper.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/log.h"
>  #include "cpu.h"
>  #include "exec/exec-all.h"
>  #include "exec/helper-proto.h"
> @@ -106,8 +107,9 @@ void helper_cp0_set(CPUUniCore32State *env, uint32_t val, uint32_t creg,
>      }
>      return;
>  unrecognized:
> -    DPRINTF("Wrong register (%d) or wrong operation (%d) in cp0_set!\n",
> -            creg, cop);
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "Wrong register (%d) or wrong operation (%d) in cp0_set!\n",
> +                  creg, cop);
>  }
>  
>  uint32_t helper_cp0_get(CPUUniCore32State *env, uint32_t creg, uint32_t cop)
> @@ -153,8 +155,9 @@ uint32_t helper_cp0_get(CPUUniCore32State *env, uint32_t creg, uint32_t cop)
>          }
>          break;
>      }
> -    DPRINTF("Wrong register (%d) or wrong operation (%d) in cp0_set!\n",
> -            creg, cop);
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "Wrong register (%d) or wrong operation (%d) in cp0_set!\n",
> +                  creg, cop);
>      return 0;
>  }
>  
> 

Applied to my trivial-patches branch.

Thanks,
Laurent

[PATCH 3/3] target/unicore32: Prefer qemu_semihosting_log_out() over curses

[Philippe Mathieu-Daudé]

Use the common API for semihosting logging.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 default-configs/unicore32-softmmu.mak |  1 +
 target/unicore32/helper.c             | 57 +++------------------------
 2 files changed, 6 insertions(+), 52 deletions(-)

diff --git a/default-configs/unicore32-softmmu.mak b/default-configs/unicore32-softmmu.mak
index 0bfce48c6d..899288e3d7 100644
--- a/default-configs/unicore32-softmmu.mak
+++ b/default-configs/unicore32-softmmu.mak
@@ -3,3 +3,4 @@
 # Boards:
 #
 CONFIG_PUV3=y
+CONFIG_SEMIHOSTING=y
diff --git a/target/unicore32/helper.c b/target/unicore32/helper.c
index 00371a7da6..54c26871fe 100644
--- a/target/unicore32/helper.c
+++ b/target/unicore32/helper.c
@@ -14,9 +14,7 @@
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
-#ifndef CONFIG_USER_ONLY
-#include "ui/console.h"
-#endif
+#include "hw/semihosting/console.h"
 
 #undef DEBUG_UC32
 
@@ -161,58 +159,13 @@ uint32_t helper_cp0_get(CPUUniCore32State *env, uint32_t creg, uint32_t cop)
     return 0;
 }
 
-#ifdef CONFIG_CURSES
-
-/* KEY_EVENT is defined in wincon.h and in curses.h. Avoid redefinition. */
-#undef KEY_EVENT
-#include <curses.h>
-#undef KEY_EVENT
-
-/*
- * FIXME:
- *     1. curses windows will be blank when switching back
- *     2. backspace is not handled yet
- */
-static void putc_on_screen(unsigned char ch)
+void helper_cp1_putc(target_ulong regval)
 {
-    static WINDOW *localwin;
-    static int init;
+    const char c = regval;
 
-    if (!init) {
-        /* Assume 80 * 30 screen to minimize the implementation */
-        localwin = newwin(30, 80, 0, 0);
-        scrollok(localwin, TRUE);
-        init = TRUE;
-    }
-
-    if (isprint(ch)) {
-        wprintw(localwin, "%c", ch);
-    } else {
-        switch (ch) {
-        case '\n':
-            wprintw(localwin, "%c", ch);
-            break;
-        case '\r':
-            /* If '\r' is put before '\n', the curses window will destroy the
-             * last print line. And meanwhile, '\n' implifies '\r' inside. */
-            break;
-        default: /* Not handled, so just print it hex code */
-            wprintw(localwin, "-- 0x%x --", ch);
-        }
-    }
-
-    wrefresh(localwin);
+    qemu_semihosting_log_out(&c, sizeof(c));
 }
-#else
-#define putc_on_screen(c)               do { } while (0)
-#endif
-
-void helper_cp1_putc(target_ulong x)
-{
-    putc_on_screen((unsigned char)x);   /* Output to screen */
-    DPRINTF("%c", x);                   /* Output to stdout */
-}
-#endif
+#endif /* !CONFIG_USER_ONLY */
 
 bool uc32_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 {
-- 
2.21.3

Re: [PATCH 3/3] target/unicore32: Prefer qemu_semihosting_log_out() over curses

[Richard Henderson]

On 6/3/20 5:37 AM, Philippe Mathieu-Daudé wrote:
> Use the common API for semihosting logging.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  default-configs/unicore32-softmmu.mak |  1 +
>  target/unicore32/helper.c             | 57 +++------------------------
>  2 files changed, 6 insertions(+), 52 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/3] target/unicore32: Prefer qemu_semihosting_log_out() over curses

[Laurent Vivier]

Le 03/06/2020 à 14:37, Philippe Mathieu-Daudé a écrit :
> Use the common API for semihosting logging.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  default-configs/unicore32-softmmu.mak |  1 +
>  target/unicore32/helper.c             | 57 +++------------------------
>  2 files changed, 6 insertions(+), 52 deletions(-)
> 
> diff --git a/default-configs/unicore32-softmmu.mak b/default-configs/unicore32-softmmu.mak
> index 0bfce48c6d..899288e3d7 100644
> --- a/default-configs/unicore32-softmmu.mak
> +++ b/default-configs/unicore32-softmmu.mak
> @@ -3,3 +3,4 @@
>  # Boards:
>  #
>  CONFIG_PUV3=y
> +CONFIG_SEMIHOSTING=y
> diff --git a/target/unicore32/helper.c b/target/unicore32/helper.c
> index 00371a7da6..54c26871fe 100644
> --- a/target/unicore32/helper.c
> +++ b/target/unicore32/helper.c
> @@ -14,9 +14,7 @@
>  #include "cpu.h"
>  #include "exec/exec-all.h"
>  #include "exec/helper-proto.h"
> -#ifndef CONFIG_USER_ONLY
> -#include "ui/console.h"
> -#endif
> +#include "hw/semihosting/console.h"
>  
>  #undef DEBUG_UC32
>  
> @@ -161,58 +159,13 @@ uint32_t helper_cp0_get(CPUUniCore32State *env, uint32_t creg, uint32_t cop)
>      return 0;
>  }
>  
> -#ifdef CONFIG_CURSES
> -
> -/* KEY_EVENT is defined in wincon.h and in curses.h. Avoid redefinition. */
> -#undef KEY_EVENT
> -#include <curses.h>
> -#undef KEY_EVENT
> -
> -/*
> - * FIXME:
> - *     1. curses windows will be blank when switching back
> - *     2. backspace is not handled yet
> - */
> -static void putc_on_screen(unsigned char ch)
> +void helper_cp1_putc(target_ulong regval)
>  {
> -    static WINDOW *localwin;
> -    static int init;
> +    const char c = regval;
>  
> -    if (!init) {
> -        /* Assume 80 * 30 screen to minimize the implementation */
> -        localwin = newwin(30, 80, 0, 0);
> -        scrollok(localwin, TRUE);
> -        init = TRUE;
> -    }
> -
> -    if (isprint(ch)) {
> -        wprintw(localwin, "%c", ch);
> -    } else {
> -        switch (ch) {
> -        case '\n':
> -            wprintw(localwin, "%c", ch);
> -            break;
> -        case '\r':
> -            /* If '\r' is put before '\n', the curses window will destroy the
> -             * last print line. And meanwhile, '\n' implifies '\r' inside. */
> -            break;
> -        default: /* Not handled, so just print it hex code */
> -            wprintw(localwin, "-- 0x%x --", ch);
> -        }
> -    }
> -
> -    wrefresh(localwin);
> +    qemu_semihosting_log_out(&c, sizeof(c));
>  }
> -#else
> -#define putc_on_screen(c)               do { } while (0)
> -#endif
> -
> -void helper_cp1_putc(target_ulong x)
> -{
> -    putc_on_screen((unsigned char)x);   /* Output to screen */
> -    DPRINTF("%c", x);                   /* Output to stdout */
> -}
> -#endif
> +#endif /* !CONFIG_USER_ONLY */
>  
>  bool uc32_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  {
> 

Applied to my trivial-patches branch.

Thanks,
Laurent