* [Qemu-devel] [PATCH] target-arm: Reinsert SRS missing return statements.
@ 2013-04-15 4:50 Peter Chubb
2013-04-15 7:23 ` Peter Maydell
0 siblings, 1 reply; 6+ messages in thread
From: Peter Chubb @ 2013-04-15 4:50 UTC (permalink / raw)
To: Peter Maydell; +Cc: Japheth.Lim, qemu-devel
Since patch
81465888c5306cd94abb9847e560796fd13d3c2f
target-arm: factor out handling of SRS instruction
the SRS instruction has not worked in QEMU.
The problem is a return directive that was removed in the
refactoring, so after decoding the instruction, qemu would fall
through to do stuff that it should not have done.
Signed-off-by: Peter Chubb <peter.chubb@nicta.com.au>
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 35a21be..c870246 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -6762,6 +6762,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
}
ARCH(6);
gen_srs(s, (insn & 0x1f), (insn >> 23) & 3, insn & (1 << 21));
+ return;
} else if ((insn & 0x0e50ffe0) == 0x08100a00) {
/* rfe */
int32_t offset;
@@ -8209,6 +8210,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
/* srs */
gen_srs(s, (insn & 0x1f), (insn & (1 << 24)) ? 1 : 2,
insn & (1 << 21));
+ return;
}
} else {
int i, loaded_base = 0;
--
1.7.10.4
Dr Peter Chubb peter.chubb AT nicta.com.au
http://www.ssrg.nicta.com.au Software Systems Research Group/NICTA
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] target-arm: Reinsert SRS missing return statements.
2013-04-15 4:50 [Qemu-devel] [PATCH] target-arm: Reinsert SRS missing return statements Peter Chubb
@ 2013-04-15 7:23 ` Peter Maydell
2013-04-15 7:29 ` Peter Chubb
0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2013-04-15 7:23 UTC (permalink / raw)
To: Peter Chubb; +Cc: Japheth.Lim, qemu-devel
On 15 April 2013 05:50, Peter Chubb <peter.chubb@nicta.com.au> wrote:
>
>
> Since patch
> 81465888c5306cd94abb9847e560796fd13d3c2f
> target-arm: factor out handling of SRS instruction
> the SRS instruction has not worked in QEMU.
>
> The problem is a return directive that was removed in the
> refactoring, so after decoding the instruction, qemu would fall
> through to do stuff that it should not have done.
Nice catch for the ARM decoder, but not needed for thumb2
I think?
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target-arm: Reinsert SRS missing return statements.
2013-04-15 7:23 ` Peter Maydell
@ 2013-04-15 7:29 ` Peter Chubb
2013-04-15 7:32 ` Peter Maydell
0 siblings, 1 reply; 6+ messages in thread
From: Peter Chubb @ 2013-04-15 7:29 UTC (permalink / raw)
To: Peter Maydell; +Cc: Japheth.Lim, Peter Chubb, qemu-devel
>>>>> "Peter" == Peter Maydell <peter.maydell@linaro.org> writes:
Peter> On 15 April 2013 05:50, Peter Chubb <peter.chubb@nicta.com.au>
Peter> wrote:
>>
>>
>> Since patch 81465888c5306cd94abb9847e560796fd13d3c2f target-arm:
>> factor out handling of SRS instruction the SRS instruction has not
>> worked in QEMU.
>>
>> The problem is a return directive that was removed in the
>> refactoring, so after decoding the instruction, qemu would fall
>> through to do stuff that it should not have done.
Peter> Nice catch for the ARM decoder, but not needed for thumb2 I
Peter> think?
It was there in the code that was removed.
I didn't analyse too deeply, as nothing we do uses the thumb version.
Peter C
--
Dr Peter Chubb peter.chubb AT nicta.com.au
http://www.ssrg.nicta.com.au Software Systems Research Group/NICTA
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target-arm: Reinsert SRS missing return statements.
2013-04-15 7:29 ` Peter Chubb
@ 2013-04-15 7:32 ` Peter Maydell
2013-04-15 9:53 ` [Qemu-devel] [PATCH v2] target-arm: Reinsert SRS missing return statement Peter Chubb
0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2013-04-15 7:32 UTC (permalink / raw)
To: Peter Chubb; +Cc: Japheth.Lim, qemu-devel
On 15 April 2013 08:29, Peter Chubb <peter.chubb@nicta.com.au> wrote:
>>>>>> "Peter" == Peter Maydell <peter.maydell@linaro.org> writes:
> Peter> On 15 April 2013 05:50, Peter Chubb <peter.chubb@nicta.com.au>
> Peter> wrote:
>>> The problem is a return directive that was removed in the
>>> refactoring, so after decoding the instruction, qemu would fall
>>> through to do stuff that it should not have done.
>
> Peter> Nice catch for the ARM decoder, but not needed for thumb2 I
> Peter> think?
>
> It was there in the code that was removed.
No it wasn't, I just looked at the broken commit which is why I
pointed it out. (Also you can see that the gen_rfe() above isn't
followed by a return, so I checked code flow to be sure it was OK.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2] target-arm: Reinsert SRS missing return statement.
2013-04-15 7:32 ` Peter Maydell
@ 2013-04-15 9:53 ` Peter Chubb
2013-04-15 10:08 ` Peter Maydell
0 siblings, 1 reply; 6+ messages in thread
From: Peter Chubb @ 2013-04-15 9:53 UTC (permalink / raw)
To: Peter Maydell; +Cc: Japheth.Lim, Peter Chubb, qemu-devel
Since patch
81465888c5306cd94abb9847e560796fd13d3c2f
target-arm: factor out handling of SRS instruction
the SRS instruction has not worked in QEMU.
The problem is a missing return directive that was removed in the
refactoring, so after decoding the instruction, qemu would do all the
generic stuff that it should do for most instructions -- but not SRS.
Signed-off-by: Peter Chubb <peter.chubb@nicta.com.au>
---
target-arm/translate.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 35a21be..a1b7b8c 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -6762,6 +6762,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
}
ARCH(6);
gen_srs(s, (insn & 0x1f), (insn >> 23) & 3, insn & (1 << 21));
+ return;
} else if ((insn & 0x0e50ffe0) == 0x08100a00) {
/* rfe */
int32_t offset;
--
1.7.10.4
--
Dr Peter Chubb peter.chubb AT nicta.com.au
http://www.ssrg.nicta.com.au Software Systems Research Group/NICTA
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v2] target-arm: Reinsert SRS missing return statement.
2013-04-15 9:53 ` [Qemu-devel] [PATCH v2] target-arm: Reinsert SRS missing return statement Peter Chubb
@ 2013-04-15 10:08 ` Peter Maydell
0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2013-04-15 10:08 UTC (permalink / raw)
To: Peter Chubb; +Cc: Japheth.Lim, qemu-devel
On 15 April 2013 10:53, Peter Chubb <peter.chubb@nicta.com.au> wrote:
>
> Since patch
> 81465888c5306cd94abb9847e560796fd13d3c2f
> target-arm: factor out handling of SRS instruction
> the SRS instruction has not worked in QEMU.
>
> The problem is a missing return directive that was removed in the
> refactoring, so after decoding the instruction, qemu would do all the
> generic stuff that it should do for most instructions -- but not SRS.
Actually, the problem is not doing generic stuff, but that
we hit the 'goto illegal_op' at the bottom of the if..elseif
ladder.
> Signed-off-by: Peter Chubb <peter.chubb@nicta.com.au>
> ---
> target-arm/translate.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 35a21be..a1b7b8c 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -6762,6 +6762,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> }
> ARCH(6);
> gen_srs(s, (insn & 0x1f), (insn >> 23) & 3, insn & (1 << 21));
> + return;
> } else if ((insn & 0x0e50ffe0) == 0x08100a00) {
> /* rfe */
> int32_t offset;
> --
> 1.7.10.4
Code is ok, though:
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
I'll pull it into the target-arm tree and tweak the commit message.
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-15 10:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-15 4:50 [Qemu-devel] [PATCH] target-arm: Reinsert SRS missing return statements Peter Chubb
2013-04-15 7:23 ` Peter Maydell
2013-04-15 7:29 ` Peter Chubb
2013-04-15 7:32 ` Peter Maydell
2013-04-15 9:53 ` [Qemu-devel] [PATCH v2] target-arm: Reinsert SRS missing return statement Peter Chubb
2013-04-15 10:08 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).