[Qemu-devel] [PATCH] target-xtensa: raise an exception for invalid and reserved opcodes

[Qemu-devel] [PATCH] target-xtensa: raise an exception for invalid and reserved opcodes

[Max Filippov]

This includes opcodes from disabled features and those marked reserved in the ISA.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 target-xtensa/translate.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index 1688bb2..792eff1 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -2378,6 +2378,7 @@ static void disas_xtensa_insn(DisasContext *dc)
 
 invalid_opcode:
     qemu_log("INVALID(pc = %08x)\n", dc->pc);
+    gen_exception_cause(dc, ILLEGAL_INSTRUCTION_CAUSE);
     dc->pc = dc->next_pc;
 #undef HAS_OPTION
 }
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH] target-xtensa: raise an exception for invalid and reserved opcodes

[Peter Maydell]

On 25 October 2011 17:44, Max Filippov <jcmvbkbc@gmail.com> wrote:
> This includes opcodes from disabled features and those marked reserved in the ISA.
>
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  target-xtensa/translate.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
> index 1688bb2..792eff1 100644
> --- a/target-xtensa/translate.c
> +++ b/target-xtensa/translate.c
> @@ -2378,6 +2378,7 @@ static void disas_xtensa_insn(DisasContext *dc)
>
>  invalid_opcode:
>     qemu_log("INVALID(pc = %08x)\n", dc->pc);
> +    gen_exception_cause(dc, ILLEGAL_INSTRUCTION_CAUSE);
>     dc->pc = dc->next_pc;
>  #undef HAS_OPTION
>  }

Don't you need to do something to end the TB as well?
gen_exception_cause() doesn't seem to do it for you.

-- PMM

Re: [Qemu-devel] [PATCH] target-xtensa: raise an exception for invalid and reserved opcodes

[Max Filippov]

> > This includes opcodes from disabled features and those marked reserved in the ISA.
> >
> > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> > ---
> >  target-xtensa/translate.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
> > index 1688bb2..792eff1 100644
> > --- a/target-xtensa/translate.c
> > +++ b/target-xtensa/translate.c
> > @@ -2378,6 +2378,7 @@ static void disas_xtensa_insn(DisasContext *dc)
> >
> >  invalid_opcode:
> >     qemu_log("INVALID(pc = %08x)\n", dc->pc);
> > +    gen_exception_cause(dc, ILLEGAL_INSTRUCTION_CAUSE);
> >     dc->pc = dc->next_pc;
> >  #undef HAS_OPTION
> >  }
> 
> Don't you need to do something to end the TB as well?
> gen_exception_cause() doesn't seem to do it for you.

Do you mean that I have to end the TB on every instruction that will definitely raise an exception?
Or on every instruction that potentially raises an exception?
I didn't do any of the above anywhere.

Thanks.
-- Max

Re: [Qemu-devel] [PATCH] target-xtensa: raise an exception for invalid and reserved opcodes

[Richard Henderson]

On 10/25/2011 10:24 AM, Max Filippov wrote:
> Do you mean that I have to end the TB on every instruction that will definitely raise an exception?
> Or on every instruction that potentially raises an exception?
> I didn't do any of the above anywhere.

Normally we do end the TB on every instruction that will definitely
raise an exception.  There's little point to continue translation,
since we know for a fact that control will transfer out of the cpu loop.


r~

[Qemu-devel] [PATCH v2] target-xtensa: raise an exception for invalid and reserved opcodes

[Max Filippov]

This includes opcodes from disabled features and those marked reserved in the ISA.
Also end TB on opcodes that definitely generate an exception: illegal
instructions, syscall and privileged instructions.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
v1 -> v2 changes:
- end TB on opcodes that definitely generate an exception.
---
 target-xtensa/translate.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index 1688bb2..c20c9cc 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -266,6 +266,10 @@ static void gen_exception_cause(DisasContext *dc, uint32_t cause)
     gen_helper_exception_cause(tpc, tcause);
     tcg_temp_free(tpc);
     tcg_temp_free(tcause);
+    if (cause == ILLEGAL_INSTRUCTION_CAUSE ||
+            cause == SYSCALL_CAUSE) {
+        dc->is_jmp = DISAS_UPDATE;
+    }
 }
 
 static void gen_exception_cause_vaddr(DisasContext *dc, uint32_t cause,
@@ -283,6 +287,7 @@ static void gen_check_privilege(DisasContext *dc)
 {
     if (dc->cring) {
         gen_exception_cause(dc, PRIVILEGED_CAUSE);
+        dc->is_jmp = DISAS_UPDATE;
     }
 }
 
@@ -2378,7 +2383,7 @@ static void disas_xtensa_insn(DisasContext *dc)
 
 invalid_opcode:
     qemu_log("INVALID(pc = %08x)\n", dc->pc);
-    dc->pc = dc->next_pc;
+    gen_exception_cause(dc, ILLEGAL_INSTRUCTION_CAUSE);
 #undef HAS_OPTION
 }
 
-- 
1.7.6.4