Re: [RFC PATCH] semihosting: suspend recieving CPU when blocked (HACK, WIP)

[Paolo Bonzini]

On 17/12/19 13:14, Alex Bennée wrote:
> [AJB:
> 
> So this at least solves the hang of not being able to quit system
> emulation while blocked. However there are two things we still need to
> ensure:
> 
>  - the PC has not advanced until completion so we can redo the instruction
>  - we actually wake up the CPU in console_read
> 
> In my testcase console_read never seems to get called. I've tried with
> both an external pipe loopback and using the ringbuf:
> 
> qemu-system-aarch64 -M virt --display none -cpu cortex-a57 -kernel 
> systest-a64-with-console.axf -semihosting-config
>  enable=on,chardev=sh0 -serial mon:stdio -chardev ringbuf,logfile=foo,id=sh0
> 
> Signed-off-by: Alex Bennée <address@hidden>
> ---
>  include/exec/cpu-all.h   |  1 +
>  hw/semihosting/console.c | 34 +++++++++++++++++-----------------
>  2 files changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index e96781a4559..093d7a76edd 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -31,6 +31,7 @@
>  #define EXCP_HALTED     0x10003 /* cpu is halted (waiting for external 
> event) */
>  #define EXCP_YIELD      0x10004 /* cpu wants to yield timeslice to another */
>  #define EXCP_ATOMIC     0x10005 /* stop-the-world and emulate atomic */
> +#define EXCP_BLOCKED    0x10006 /* cpu is blocked (semihosting) */
>  
>  /* some important defines:
>   *
> diff --git a/hw/semihosting/console.c b/hw/semihosting/console.c
> index 4db68d62270..bda457a0608 100644
> --- a/hw/semihosting/console.c
> +++ b/hw/semihosting/console.c
> @@ -20,6 +20,7 @@
>  #include "hw/semihosting/semihost.h"
>  #include "hw/semihosting/console.h"
>  #include "exec/gdbstub.h"
> +#include "exec/exec-all.h"
>  #include "qemu/log.h"
>  #include "chardev/char.h"
>  #include <pthread.h>
> @@ -109,50 +110,49 @@ void qemu_semihosting_console_outc(CPUArchState *env, 
> target_ulong addr)
>  
>  typedef struct SemihostingConsole {
>      CharBackend         backend;
> -    pthread_mutex_t     mutex;
> -    pthread_cond_t      cond;
> +    CPUState            *sleeping_cpu;
>      bool                got;
>      Fifo8               fifo;
>  } SemihostingConsole;
>  
> -static SemihostingConsole console = {
> -    .mutex = PTHREAD_MUTEX_INITIALIZER,
> -    .cond = PTHREAD_COND_INITIALIZER
> -};
> +static SemihostingConsole console;
>  
>  static int console_can_read(void *opaque)
>  {
>      SemihostingConsole *c = opaque;
>      int ret;
> -    pthread_mutex_lock(&c->mutex);
> +    g_assert(qemu_mutex_iothread_locked());
>      ret = (int) fifo8_num_free(&c->fifo);
> -    pthread_mutex_unlock(&c->mutex);
>      return ret;
>  }
>  
>  static void console_read(void *opaque, const uint8_t *buf, int size)
>  {
>      SemihostingConsole *c = opaque;
> -    pthread_mutex_lock(&c->mutex);
> +    g_assert(qemu_mutex_iothread_locked());
>      while (size-- && !fifo8_is_full(&c->fifo)) {
>          fifo8_push(&c->fifo, *buf++);
>      }
> -    pthread_cond_broadcast(&c->cond);
> -    pthread_mutex_unlock(&c->mutex);
> +    if (c->sleeping_cpu) {
> +        cpu_resume(c->sleeping_cpu);
> +    }
>  }
>  
>  target_ulong qemu_semihosting_console_inc(CPUArchState *env)
>  {
>      uint8_t ch;
>      SemihostingConsole *c = &console;
> -    qemu_mutex_unlock_iothread();
> -    pthread_mutex_lock(&c->mutex);
> -    while (fifo8_is_empty(&c->fifo)) {
> -        pthread_cond_wait(&c->cond, &c->mutex);
> +    g_assert(qemu_mutex_iothread_locked());
> +    g_assert(current_cpu);
> +    if (fifo8_is_empty(&c->fifo)) {
> +        c->sleeping_cpu = current_cpu;
> +        c->sleeping_cpu->stop = true;
> +        c->sleeping_cpu->exception_index = EXCP_BLOCKED;

Why do you need to set exception_index to something other than -1 (using
cpu_loop_exit_noexc for example)?

Using ->stop here is a bit weird, since ->stop is usually related to
pause_all_vcpus.

Paolo