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

Alex Bennée posted 1 patch 4 years, 3 months ago
Failed in applying to current master (apply log)
include/exec/cpu-all.h   |  1 +
hw/semihosting/console.c | 34 +++++++++++++++++-----------------
2 files changed, 18 insertions(+), 17 deletions(-)
[RFC PATCH] semihosting: suspend recieving CPU when blocked (HACK, WIP)
Posted by Alex Bennée 4 years, 3 months ago
Sleeping in the semihosting code is problematic as we deadlock the
whole system. This includes issuing a "quit" via the HMP or presumably
if gdbserver got involved. What we really want is to return to the
main loop and gt woken up when there is data to process. We can then
re-execute the instruction which will succeed this time.

[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 <alex.bennee@linaro.org>
---
 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;
+        cpu_loop_exit(c->sleeping_cpu);
+        /* never returns */
     }
+    c->sleeping_cpu = NULL;
     ch = fifo8_pop(&c->fifo);
-    pthread_mutex_unlock(&c->mutex);
-    qemu_mutex_lock_iothread();
     return (target_ulong) ch;
 }
 
-- 
2.20.1


Re: [RFC PATCH] semihosting: suspend recieving CPU when blocked (HACK, WIP)
Posted by Paolo Bonzini 4 years, 3 months ago
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 <alex.bennee@linaro.org>
> ---
>  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


Re: [RFC PATCH] semihosting: suspend recieving CPU when blocked (HACK, WIP)
Posted by Alex Bennée 4 years, 3 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> 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 <alex.bennee@linaro.org>
>> ---
>>  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)?

If there is no exception to process we won't exit the main loop which we
need to do if we want to wait until there is data to read.

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

Arguably we could come up with a better API to cpu.c but this allows us
to use cpu_resume(c->sleeping_cpu) when waking up rather than hand
rolling our own wake-up mechanism.

>
> Paolo


-- 
Alex Bennée

Re: [RFC PATCH] semihosting: suspend recieving CPU when blocked (HACK, WIP)
Posted by Paolo Bonzini 4 years, 3 months ago
On 17/12/19 14:42, Alex Bennée wrote:
>> Why do you need to set exception_index to something other than -1 (using
>> cpu_loop_exit_noexc for example)?
> If there is no exception to process we won't exit the main loop which we
> need to do if we want to wait until there is data to read.

Okay.

>> Using ->stop here is a bit weird, since ->stop is usually related to
>> pause_all_vcpus.
> 
> Arguably we could come up with a better API to cpu.c but this allows us
> to use cpu_resume(c->sleeping_cpu) when waking up rather than hand
> rolling our own wake-up mechanism.

But we already have the right wake-up mechanism, which is
cpu->halted/cpu_has_work.  That also makes it possible to just use
EXCP_HALTED instead of adding a new EXCP_BLOCKED.

Paolo


Re: [RFC PATCH] semihosting: suspend recieving CPU when blocked (HACK, WIP)
Posted by Alex Bennée 4 years, 3 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 17/12/19 14:42, Alex Bennée wrote:
>>> Why do you need to set exception_index to something other than -1 (using
>>> cpu_loop_exit_noexc for example)?
>> If there is no exception to process we won't exit the main loop which we
>> need to do if we want to wait until there is data to read.
>
> Okay.
>
>>> Using ->stop here is a bit weird, since ->stop is usually related to
>>> pause_all_vcpus.
>> 
>> Arguably we could come up with a better API to cpu.c but this allows us
>> to use cpu_resume(c->sleeping_cpu) when waking up rather than hand
>> rolling our own wake-up mechanism.
>
> But we already have the right wake-up mechanism, which is
> cpu->halted/cpu_has_work.

cpu_has_work is a guest function though and semihosting_console is a
common hw module. It can't peek into the guests internal state. This all
comes back to cpu_thread_is_idle anyway in making our decision about if
we do or do not sleep on the halt_cond.

> That also makes it possible to just use
> EXCP_HALTED instead of adding a new EXCP_BLOCKED.

We can certainly use EXCP_HALTED but maybe come up with a common way of
entering the state? There seems to be a combination of messing around
with special interrupts and direct poking of cs->halted = 1 while
setting the exception. Maybe this could finally clear up the #if
defined(TARGET_I386) hacking in cpus.c?

>
> Paolo


-- 
Alex Bennée

Re: [RFC PATCH] semihosting: suspend recieving CPU when blocked (HACK, WIP)
Posted by Paolo Bonzini 4 years, 3 months ago
On 17/12/19 15:18, Alex Bennée wrote:
> 
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 17/12/19 14:42, Alex Bennée wrote:
>>>> Why do you need to set exception_index to something other than -1 (using
>>>> cpu_loop_exit_noexc for example)?
>>> If there is no exception to process we won't exit the main loop which we
>>> need to do if we want to wait until there is data to read.
>>
>> Okay.
>>
>>>> Using ->stop here is a bit weird, since ->stop is usually related to
>>>> pause_all_vcpus.
>>>
>>> Arguably we could come up with a better API to cpu.c but this allows us
>>> to use cpu_resume(c->sleeping_cpu) when waking up rather than hand
>>> rolling our own wake-up mechanism.
>>
>> But we already have the right wake-up mechanism, which is
>> cpu->halted/cpu_has_work.
> 
> cpu_has_work is a guest function though and semihosting_console is a
> common hw module. It can't peek into the guests internal state.

semihosting_console only needs to something like
cpu_interrupt(cpu->stopped_cpu, CPU_INTERRUPT_SEMIHOST).  (By the way,
the stopped_cpu should probably be a list to mimic the condition
variable---for example a GList).

> This all
> comes back to cpu_thread_is_idle anyway in making our decision about if
> we do or do not sleep on the halt_cond.
> 
>> That also makes it possible to just use
>> EXCP_HALTED instead of adding a new EXCP_BLOCKED.
> 
> We can certainly use EXCP_HALTED but maybe come up with a common way of
> entering the state? There seems to be a combination of messing around
> with special interrupts and direct poking of cs->halted = 1 while
> setting the exception. Maybe this could finally clear up the #if
> defined(TARGET_I386) hacking in cpus.c?

If you're talking accel/tcg/cpu-exec.c, that's different; the issue
there is that x86 has a kind of warm reset pin that is not equivalent to
cpu_reset.  Removing that would only entail adding a new member function
to CPUClass.

Paolo


Re: [RFC PATCH] semihosting: suspend recieving CPU when blocked (HACK, WIP)
Posted by Paolo Bonzini 4 years, 3 months ago
On 17/12/19 15:18, Alex Bennée wrote:
> 
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 17/12/19 14:42, Alex Bennée wrote:
>>>> Why do you need to set exception_index to something other than -1 (using
>>>> cpu_loop_exit_noexc for example)?
>>> If there is no exception to process we won't exit the main loop which we
>>> need to do if we want to wait until there is data to read.
>>
>> Okay.
>>
>>>> Using ->stop here is a bit weird, since ->stop is usually related to
>>>> pause_all_vcpus.
>>>
>>> Arguably we could come up with a better API to cpu.c but this allows us
>>> to use cpu_resume(c->sleeping_cpu) when waking up rather than hand
>>> rolling our own wake-up mechanism.
>>
>> But we already have the right wake-up mechanism, which is
>> cpu->halted/cpu_has_work.
> 
> cpu_has_work is a guest function though and semihosting_console is a
> common hw module. It can't peek into the guests internal state.

semihosting_console only needs to something like
cpu_interrupt(cpu->stopped_cpu, CPU_INTERRUPT_SEMIHOST).  (By the way,
the stopped_cpu should probably be a list to mimic the condition
variable---for example a GSList).

> This all
> comes back to cpu_thread_is_idle anyway in making our decision about if
> we do or do not sleep on the halt_cond.
> 
>> That also makes it possible to just use
>> EXCP_HALTED instead of adding a new EXCP_BLOCKED.
> 
> We can certainly use EXCP_HALTED but maybe come up with a common way of
> entering the state? There seems to be a combination of messing around
> with special interrupts and direct poking of cs->halted = 1 while
> setting the exception. Maybe this could finally clear up the #if
> defined(TARGET_I386) hacking in cpus.c?

If you're talking accel/tcg/cpu-exec.c, that's different; the issue
there is that x86 has a kind of warm reset pin that is not equivalent to
cpu_reset.  Removing that would only entail adding a new member function
to CPUClass.

Paolo


Re: [RFC PATCH] semihosting: suspend recieving CPU when blocked (HACK, WIP)
Posted by Alex Bennée 4 years, 3 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 17/12/19 15:18, Alex Bennée wrote:
>> 
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> On 17/12/19 14:42, Alex Bennée wrote:
>>>>> Why do you need to set exception_index to something other than -1 (using
>>>>> cpu_loop_exit_noexc for example)?
>>>> If there is no exception to process we won't exit the main loop which we
>>>> need to do if we want to wait until there is data to read.
>>>
>>> Okay.
>>>
>>>>> Using ->stop here is a bit weird, since ->stop is usually related to
>>>>> pause_all_vcpus.
>>>>
>>>> Arguably we could come up with a better API to cpu.c but this allows us
>>>> to use cpu_resume(c->sleeping_cpu) when waking up rather than hand
>>>> rolling our own wake-up mechanism.
>>>
>>> But we already have the right wake-up mechanism, which is
>>> cpu->halted/cpu_has_work.
>> 
>> cpu_has_work is a guest function though and semihosting_console is a
>> common hw module. It can't peek into the guests internal state.
>
> semihosting_console only needs to something like
> cpu_interrupt(cpu->stopped_cpu, CPU_INTERRUPT_SEMIHOST).

As an exception is being delivered we just end up re-executing the
EXCP_SEMIHOST. I still don't see why using cpu_interrupt is an
improvement seeing as it is secondary to exception processing.

> (By the way,
> the stopped_cpu should probably be a list to mimic the condition
> variable---for example a GSList).

ok

>
>> This all
>> comes back to cpu_thread_is_idle anyway in making our decision about if
>> we do or do not sleep on the halt_cond.
>> 
>>> That also makes it possible to just use
>>> EXCP_HALTED instead of adding a new EXCP_BLOCKED.
>> 
>> We can certainly use EXCP_HALTED but maybe come up with a common way of
>> entering the state? There seems to be a combination of messing around
>> with special interrupts and direct poking of cs->halted = 1 while
>> setting the exception. Maybe this could finally clear up the #if
>> defined(TARGET_I386) hacking in cpus.c?
>
> If you're talking accel/tcg/cpu-exec.c, that's different; the issue
> there is that x86 has a kind of warm reset pin that is not equivalent to
> cpu_reset.  Removing that would only entail adding a new member function
> to CPUClass.
>
> Paolo


-- 
Alex Bennée

Re: [RFC PATCH] semihosting: suspend recieving CPU when blocked (HACK, WIP)
Posted by Paolo Bonzini 4 years, 3 months ago
Il mer 18 dic 2019, 18:36 Alex Bennée <alex.bennee@linaro.org> ha scritto:

>
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > On 17/12/19 15:18, Alex Bennée wrote:
> >> cpu_has_work is a guest function though and semihosting_console is a
> >> common hw module. It can't peek into the guests internal state.
> >
> > semihosting_console only needs to something like
> > cpu_interrupt(cpu->stopped_cpu, CPU_INTERRUPT_SEMIHOST).
>
> As an exception is being delivered we just end up re-executing the
> EXCP_SEMIHOST. I still don't see why using cpu_interrupt is an
> improvement seeing as it is secondary to exception processing.
>

FWIW I skimmed your patch and yes an interrupt is not needed since you are
delaying the update of the program counter; that's nicer.

Paolo