diff mbox

[PULL,2/5] target/sh4: fix reset when using a kernel and an initrd

Message ID 20170529193016.6888-3-aurelien@aurel32.net
State New
Headers show

Commit Message

Aurelien Jarno May 29, 2017, 7:30 p.m. UTC
When a masked exception happens, the SH4 CPU generates a non-masked
reset exception, which then jumps to the reset vector at address
0xA0000000. While this is emulated correctly in QEMU, this does not
work when using a kernel and initrd as this address then contain an
illegal instruction (and there is no guarantee the kernel and initrd
haven't been overwritten).

Therefore call qemu_system_reset_request to reload the kernel and initrd
and load the program counter to the kernel entry point.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/sh4/helper.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Stefan Hajnoczi May 30, 2017, 10:17 a.m. UTC | #1
On Mon, May 29, 2017 at 09:30:13PM +0200, Aurelien Jarno wrote:
> When a masked exception happens, the SH4 CPU generates a non-masked
> reset exception, which then jumps to the reset vector at address
> 0xA0000000. While this is emulated correctly in QEMU, this does not
> work when using a kernel and initrd as this address then contain an
> illegal instruction (and there is no guarantee the kernel and initrd
> haven't been overwritten).
> 
> Therefore call qemu_system_reset_request to reload the kernel and initrd
> and load the program counter to the kernel entry point.
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target/sh4/helper.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/target/sh4/helper.c b/target/sh4/helper.c
> index 4c024f9529..5296e7cf4e 100644
> --- a/target/sh4/helper.c
> +++ b/target/sh4/helper.c
> @@ -21,6 +21,7 @@
>  #include "cpu.h"
>  #include "exec/exec-all.h"
>  #include "exec/log.h"
> +#include "sysemu/sysemu.h"
>  
>  #if !defined(CONFIG_USER_ONLY)
>  #include "hw/sh4/sh_intc.h"
> @@ -92,7 +93,14 @@ void superh_cpu_do_interrupt(CPUState *cs)
>  
>      if (env->sr & (1u << SR_BL)) {
>          if (do_exp && cs->exception_index != 0x1e0) {
> -            cs->exception_index = 0x000; /* masked exception -> reset */
> +            /* In theory a masked exception generates a reset exception,
> +               which in turn jumps to the reset vector. However this only
> +               works when using a bootloader. When using a kernel and an
> +               initrd, they need to be reloaded and the program counter
> +               should be loaded with the kernel entry point.
> +               qemu_system_reset_request takes care of that.  */
> +            qemu_system_reset_request();
> +            return;

The qemu_system_reset_request() prototype is changing.  There is a
conflict with another merged pull request.  Please rebase onto
https://github.com/stefanha/qemu.git staging.

target/sh4/helper.c:102:39: error: too few arguments to function call, single argument 'reason' was not specified
            qemu_system_reset_request();
            ~~~~~~~~~~~~~~~~~~~~~~~~~ ^
Eric Blake May 30, 2017, 3:02 p.m. UTC | #2
On 05/30/2017 05:17 AM, Stefan Hajnoczi wrote:
> On Mon, May 29, 2017 at 09:30:13PM +0200, Aurelien Jarno wrote:
>> When a masked exception happens, the SH4 CPU generates a non-masked
>> reset exception, which then jumps to the reset vector at address
>> 0xA0000000. While this is emulated correctly in QEMU, this does not
>> work when using a kernel and initrd as this address then contain an
>> illegal instruction (and there is no guarantee the kernel and initrd
>> haven't been overwritten).
>>

>> +            qemu_system_reset_request();
>> +            return;
> 
> The qemu_system_reset_request() prototype is changing.  There is a
> conflict with another merged pull request.  Please rebase onto
> https://github.com/stefanha/qemu.git staging.
> 
> target/sh4/helper.c:102:39: error: too few arguments to function call, single argument 'reason' was not specified
>             qemu_system_reset_request();
>             ~~~~~~~~~~~~~~~~~~~~~~~~~ ^

You'll probably want SHUTDOWN_CAUSE_GUEST_RESET as the reason, based on
the changes made to other target/ files.
Aurelien Jarno May 30, 2017, 4:21 p.m. UTC | #3
On 2017-05-30 10:02, Eric Blake wrote:
> On 05/30/2017 05:17 AM, Stefan Hajnoczi wrote:
> > On Mon, May 29, 2017 at 09:30:13PM +0200, Aurelien Jarno wrote:
> >> When a masked exception happens, the SH4 CPU generates a non-masked
> >> reset exception, which then jumps to the reset vector at address
> >> 0xA0000000. While this is emulated correctly in QEMU, this does not
> >> work when using a kernel and initrd as this address then contain an
> >> illegal instruction (and there is no guarantee the kernel and initrd
> >> haven't been overwritten).
> >>
> 
> >> +            qemu_system_reset_request();
> >> +            return;
> > 
> > The qemu_system_reset_request() prototype is changing.  There is a
> > conflict with another merged pull request.  Please rebase onto
> > https://github.com/stefanha/qemu.git staging.
> > 
> > target/sh4/helper.c:102:39: error: too few arguments to function call, single argument 'reason' was not specified
> >             qemu_system_reset_request();
> >             ~~~~~~~~~~~~~~~~~~~~~~~~~ ^
> 
> You'll probably want SHUTDOWN_CAUSE_GUEST_RESET as the reason, based on
> the changes made to other target/ files.
 
Indeed I confirm. This is the same as the triple fault on x86, which
uses SHUTDOWN_CAUSE_GUEST_RESET.
diff mbox

Patch

diff --git a/target/sh4/helper.c b/target/sh4/helper.c
index 4c024f9529..5296e7cf4e 100644
--- a/target/sh4/helper.c
+++ b/target/sh4/helper.c
@@ -21,6 +21,7 @@ 
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "exec/log.h"
+#include "sysemu/sysemu.h"
 
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/sh4/sh_intc.h"
@@ -92,7 +93,14 @@  void superh_cpu_do_interrupt(CPUState *cs)
 
     if (env->sr & (1u << SR_BL)) {
         if (do_exp && cs->exception_index != 0x1e0) {
-            cs->exception_index = 0x000; /* masked exception -> reset */
+            /* In theory a masked exception generates a reset exception,
+               which in turn jumps to the reset vector. However this only
+               works when using a bootloader. When using a kernel and an
+               initrd, they need to be reloaded and the program counter
+               should be loaded with the kernel entry point.
+               qemu_system_reset_request takes care of that.  */
+            qemu_system_reset_request();
+            return;
         }
         if (do_irq && !env->in_sleep) {
             return; /* masked */