* [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions
@ 2018-06-12 20:46 Julia Suvorova
2018-06-13 14:05 ` Stefan Hajnoczi
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Julia Suvorova @ 2018-06-12 20:46 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Stefan Hajnoczi, Joel Stanley, Jim Mussared,
Steffen Görtz, Julia Suvorova
ARMv6-M supports 6 Thumb2 instructions. This patch checks for these
instructions and allows their execution.
Like Thumb2 cores, ARMv6-M always interprets BL instruction as 32-bit.
This patch is required for future Cortex-M0 support.
Signed-off-by: Julia Suvorova <jusual@mail.ru>
---
target/arm/translate.c | 35 ++++++++++++++++++++++++++++++-----
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 0ff5edf2ce..8cae3f5ed0 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -9965,7 +9965,8 @@ static bool thumb_insn_is_16bit(DisasContext *s, uint32_t insn)
* end up actually treating this as two 16-bit insns, though,
* if it's half of a bl/blx pair that might span a page boundary.
*/
- if (arm_dc_feature(s, ARM_FEATURE_THUMB2)) {
+ if (arm_dc_feature(s, ARM_FEATURE_THUMB2) ||
+ arm_dc_feature(s, ARM_FEATURE_M)) {
/* Thumb2 cores (including all M profile ones) always treat
* 32-bit insns as 32-bit.
*/
@@ -10075,6 +10076,11 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
{
uint32_t imm, shift, offset;
uint32_t rd, rn, rm, rs;
+ uint32_t armv6m_insn[] = {0xf3808000 /* msr */, 0xf3b08040 /* dsb */,
+ 0xf3b08050 /* dmb */, 0xf3b08060 /* isb */,
+ 0xf3e08000 /* mrs */, 0xf000d000 /* bl */};
+ uint32_t armv6m_mask[] = {0xffe0d000, 0xfff0d0f0, 0xfff0d0f0,
+ 0xfff0d0f0, 0xffe0d000, 0xf800d000};
TCGv_i32 tmp;
TCGv_i32 tmp2;
TCGv_i32 tmp3;
@@ -10085,10 +10091,25 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
int conds;
int logic_cc;
- /* The only 32 bit insn that's allowed for Thumb1 is the combined
- * BL/BLX prefix and suffix.
+ /*
+ * ARMv6-M supports a limited subset of Thumb2 instructions.
+ * Other Thumb1 architectures allow only 32-bit
+ * combined BL/BLX prefix and suffix.
*/
- if ((insn & 0xf800e800) != 0xf000e800) {
+ if (arm_dc_feature(s, ARM_FEATURE_M) && arm_dc_feature(s, ARM_FEATURE_V6)) {
+ int i;
+ bool found = false;
+
+ for (i = 0; i < ARRAY_SIZE(armv6m_insn); i++) {
+ if ((insn & armv6m_mask[i]) == armv6m_insn[i]) {
+ found = true;
+ break;
+ }
+ }
+ if (!found) {
+ goto illegal_op;
+ }
+ } else if ((insn & 0xf800e800) != 0xf000e800) {
ARCH(6T2);
}
@@ -11009,7 +11030,11 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
}
break;
case 3: /* Special control operations. */
- ARCH(7);
+ if (!arm_dc_feature(s, ARM_FEATURE_V7) &&
+ !(arm_dc_feature(s, ARM_FEATURE_V6) &&
+ arm_dc_feature(s, ARM_FEATURE_M))) {
+ goto illegal_op;
+ }
op = (insn >> 4) & 0xf;
switch (op) {
case 2: /* clrex */
--
2.17.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions
2018-06-12 20:46 [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions Julia Suvorova
@ 2018-06-13 14:05 ` Stefan Hajnoczi
2018-06-13 17:10 ` Julia Suvorova
2018-06-15 10:55 ` Peter Maydell
2018-06-15 13:31 ` Peter Maydell
2 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2018-06-13 14:05 UTC (permalink / raw)
To: Julia Suvorova
Cc: qemu-devel, Peter Maydell, Joel Stanley, Jim Mussared,
Steffen Görtz
[-- Attachment #1: Type: text/plain, Size: 698 bytes --]
On Tue, Jun 12, 2018 at 11:46:32PM +0300, Julia Suvorova wrote:
> ARMv6-M supports 6 Thumb2 instructions. This patch checks for these
> instructions and allows their execution.
> Like Thumb2 cores, ARMv6-M always interprets BL instruction as 32-bit.
>
> This patch is required for future Cortex-M0 support.
>
> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> ---
> target/arm/translate.c | 35 ++++++++++++++++++++++++++++++-----
> 1 file changed, 30 insertions(+), 5 deletions(-)
Does make the undefined instruction test case that I recently posted
pass (including all commented out instructions that weren't working
yet)?
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions
2018-06-13 14:05 ` Stefan Hajnoczi
@ 2018-06-13 17:10 ` Julia Suvorova
2018-06-14 8:17 ` Stefan Hajnoczi
0 siblings, 1 reply; 11+ messages in thread
From: Julia Suvorova @ 2018-06-13 17:10 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, Peter Maydell, Joel Stanley, Jim Mussared,
Steffen Görtz
On 13.06.2018 17:05, Stefan Hajnoczi wrote:
> On Tue, Jun 12, 2018 at 11:46:32PM +0300, Julia Suvorova wrote:
>> ARMv6-M supports 6 Thumb2 instructions. This patch checks for these
>> instructions and allows their execution.
>> Like Thumb2 cores, ARMv6-M always interprets BL instruction as 32-bit.
>>
>> This patch is required for future Cortex-M0 support.
>>
>> Signed-off-by: Julia Suvorova <jusual@mail.ru>
>> ---
>> target/arm/translate.c | 35 ++++++++++++++++++++++++++++++-----
>> 1 file changed, 30 insertions(+), 5 deletions(-)
>
> Does make the undefined instruction test case that I recently posted
> pass (including all commented out instructions that weren't working
> yet)?
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>
Yes, test with all instructions is passed.
Best regards, Julia Suvorova.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions
2018-06-13 17:10 ` Julia Suvorova
@ 2018-06-14 8:17 ` Stefan Hajnoczi
0 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2018-06-14 8:17 UTC (permalink / raw)
To: Julia Suvorova
Cc: Stefan Hajnoczi, Peter Maydell, Jim Mussared, Steffen Görtz,
qemu-devel, Joel Stanley
[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]
On Wed, Jun 13, 2018 at 08:10:23PM +0300, Julia Suvorova via Qemu-devel wrote:
> On 13.06.2018 17:05, Stefan Hajnoczi wrote:
> > On Tue, Jun 12, 2018 at 11:46:32PM +0300, Julia Suvorova wrote:
> > > ARMv6-M supports 6 Thumb2 instructions. This patch checks for these
> > > instructions and allows their execution.
> > > Like Thumb2 cores, ARMv6-M always interprets BL instruction as 32-bit.
> > >
> > > This patch is required for future Cortex-M0 support.
> > >
> > > Signed-off-by: Julia Suvorova <jusual@mail.ru>
> > > ---
> > > target/arm/translate.c | 35 ++++++++++++++++++++++++++++++-----
> > > 1 file changed, 30 insertions(+), 5 deletions(-)
> >
> > Does make the undefined instruction test case that I recently posted
> > pass (including all commented out instructions that weren't working
> > yet)?
> >
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> >
>
> Yes, test with all instructions is passed.
Great, I have sent a v2 of that patch. It now also includes the 6 valid
32-bit instructions to prove that your patch has enabled them.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions
2018-06-12 20:46 [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions Julia Suvorova
2018-06-13 14:05 ` Stefan Hajnoczi
@ 2018-06-15 10:55 ` Peter Maydell
2018-06-15 11:15 ` Julia Suvorova
2018-06-17 5:36 ` Richard Henderson
2018-06-15 13:31 ` Peter Maydell
2 siblings, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2018-06-15 10:55 UTC (permalink / raw)
To: Julia Suvorova
Cc: QEMU Developers, Stefan Hajnoczi, Joel Stanley, Jim Mussared,
Steffen Görtz
On 12 June 2018 at 21:46, Julia Suvorova <jusual@mail.ru> wrote:
> ARMv6-M supports 6 Thumb2 instructions. This patch checks for these
> instructions and allows their execution.
> Like Thumb2 cores, ARMv6-M always interprets BL instruction as 32-bit.
>
> This patch is required for future Cortex-M0 support.
>
> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> @@ -10075,6 +10076,11 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
> {
> uint32_t imm, shift, offset;
> uint32_t rd, rn, rm, rs;
> + uint32_t armv6m_insn[] = {0xf3808000 /* msr */, 0xf3b08040 /* dsb */,
> + 0xf3b08050 /* dmb */, 0xf3b08060 /* isb */,
> + 0xf3e08000 /* mrs */, 0xf000d000 /* bl */};
> + uint32_t armv6m_mask[] = {0xffe0d000, 0xfff0d0f0, 0xfff0d0f0,
> + 0xfff0d0f0, 0xffe0d000, 0xf800d000};
I think these arrays should be 'const'; we can also move them closer
to their point of use, inside the scope of the if() below.
Since those are trivial tweaks, I'm going to put this into
target-arm.next with those changes made, if that's OK.
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions
2018-06-15 10:55 ` Peter Maydell
@ 2018-06-15 11:15 ` Julia Suvorova
2018-06-17 5:36 ` Richard Henderson
1 sibling, 0 replies; 11+ messages in thread
From: Julia Suvorova @ 2018-06-15 11:15 UTC (permalink / raw)
To: Peter Maydell
Cc: QEMU Developers, Stefan Hajnoczi, Joel Stanley, Jim Mussared,
Steffen Görtz
On 15.06.2018 13:55, Peter Maydell wrote:
> On 12 June 2018 at 21:46, Julia Suvorova <jusual@mail.ru> wrote:
>> ARMv6-M supports 6 Thumb2 instructions. This patch checks for these
>> instructions and allows their execution.
>> Like Thumb2 cores, ARMv6-M always interprets BL instruction as 32-bit.
>>
>> This patch is required for future Cortex-M0 support.
>>
>> Signed-off-by: Julia Suvorova <jusual@mail.ru>
>> @@ -10075,6 +10076,11 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
>> {
>> uint32_t imm, shift, offset;
>> uint32_t rd, rn, rm, rs;
>> + uint32_t armv6m_insn[] = {0xf3808000 /* msr */, 0xf3b08040 /* dsb */,
>> + 0xf3b08050 /* dmb */, 0xf3b08060 /* isb */,
>> + 0xf3e08000 /* mrs */, 0xf000d000 /* bl */};
>> + uint32_t armv6m_mask[] = {0xffe0d000, 0xfff0d0f0, 0xfff0d0f0,
>> + 0xfff0d0f0, 0xffe0d000, 0xf800d000};
>
> I think these arrays should be 'const'; we can also move them closer
> to their point of use, inside the scope of the if() below.
>
> Since those are trivial tweaks, I'm going to put this into
> target-arm.next with those changes made, if that's OK.
Sure, thanks.
Best regards, Julia Suvorova.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions
2018-06-12 20:46 [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions Julia Suvorova
2018-06-13 14:05 ` Stefan Hajnoczi
2018-06-15 10:55 ` Peter Maydell
@ 2018-06-15 13:31 ` Peter Maydell
2 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2018-06-15 13:31 UTC (permalink / raw)
To: Julia Suvorova
Cc: QEMU Developers, Stefan Hajnoczi, Joel Stanley, Jim Mussared,
Steffen Görtz
On 12 June 2018 at 21:46, Julia Suvorova <jusual@mail.ru> wrote:
> @@ -10085,10 +10091,25 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
> int conds;
> int logic_cc;
>
> - /* The only 32 bit insn that's allowed for Thumb1 is the combined
> - * BL/BLX prefix and suffix.
> + /*
> + * ARMv6-M supports a limited subset of Thumb2 instructions.
> + * Other Thumb1 architectures allow only 32-bit
> + * combined BL/BLX prefix and suffix.
> */
> - if ((insn & 0xf800e800) != 0xf000e800) {
> + if (arm_dc_feature(s, ARM_FEATURE_M) && arm_dc_feature(s, ARM_FEATURE_V6)) {
I realized during testing that this accidentally breaks v7M and v8M,
because those cores define both ARM_FEATURE_V6 and _V7 (and _V8 for v8M),
so this condition is true and we undef on the non-v6M insns for
v7M and v8M too. I've fixed this in target-arm.next by changing the
condition to
+ if (arm_dc_feature(s, ARM_FEATURE_M) &&
+ !arm_dc_feature(s, ARM_FEATURE_V7)) {
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions
2018-06-15 10:55 ` Peter Maydell
2018-06-15 11:15 ` Julia Suvorova
@ 2018-06-17 5:36 ` Richard Henderson
2018-06-17 16:33 ` Peter Maydell
1 sibling, 1 reply; 11+ messages in thread
From: Richard Henderson @ 2018-06-17 5:36 UTC (permalink / raw)
To: Peter Maydell, Julia Suvorova
Cc: Jim Mussared, Steffen Görtz, QEMU Developers,
Stefan Hajnoczi, Joel Stanley
On 06/15/2018 12:55 AM, Peter Maydell wrote:
>> + uint32_t armv6m_insn[] = {0xf3808000 /* msr */, 0xf3b08040 /* dsb */,
>> + 0xf3b08050 /* dmb */, 0xf3b08060 /* isb */,
>> + 0xf3e08000 /* mrs */, 0xf000d000 /* bl */};
>> + uint32_t armv6m_mask[] = {0xffe0d000, 0xfff0d0f0, 0xfff0d0f0,
>> + 0xfff0d0f0, 0xffe0d000, 0xf800d000};
> I think these arrays should be 'const'; we can also move them closer
> to their point of use, inside the scope of the if() below.
static as well.
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions
2018-06-17 5:36 ` Richard Henderson
@ 2018-06-17 16:33 ` Peter Maydell
2018-06-17 18:48 ` Julia Suvorova
0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2018-06-17 16:33 UTC (permalink / raw)
To: Richard Henderson
Cc: Julia Suvorova, Jim Mussared, Steffen Görtz,
QEMU Developers, Stefan Hajnoczi, Joel Stanley
On 17 June 2018 at 06:36, Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 06/15/2018 12:55 AM, Peter Maydell wrote:
>>> + uint32_t armv6m_insn[] = {0xf3808000 /* msr */, 0xf3b08040 /* dsb */,
>>> + 0xf3b08050 /* dmb */, 0xf3b08060 /* isb */,
>>> + 0xf3e08000 /* mrs */, 0xf000d000 /* bl */};
>>> + uint32_t armv6m_mask[] = {0xffe0d000, 0xfff0d0f0, 0xfff0d0f0,
>>> + 0xfff0d0f0, 0xffe0d000, 0xf800d000};
>> I think these arrays should be 'const'; we can also move them closer
>> to their point of use, inside the scope of the if() below.
>
> static as well.
Mmm; commit is already in master though, will need a followup patch.
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions
2018-06-17 16:33 ` Peter Maydell
@ 2018-06-17 18:48 ` Julia Suvorova
2018-06-18 10:48 ` Peter Maydell
0 siblings, 1 reply; 11+ messages in thread
From: Julia Suvorova @ 2018-06-17 18:48 UTC (permalink / raw)
To: Peter Maydell, Richard Henderson
Cc: Jim Mussared, Steffen Görtz, QEMU Developers,
Stefan Hajnoczi, Joel Stanley
On 17.06.2018 19:33, Peter Maydell wrote:
> On 17 June 2018 at 06:36, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>> On 06/15/2018 12:55 AM, Peter Maydell wrote:
>>>> + uint32_t armv6m_insn[] = {0xf3808000 /* msr */, 0xf3b08040 /* dsb */,
>>>> + 0xf3b08050 /* dmb */, 0xf3b08060 /* isb */,
>>>> + 0xf3e08000 /* mrs */, 0xf000d000 /* bl */};
>>>> + uint32_t armv6m_mask[] = {0xffe0d000, 0xfff0d0f0, 0xfff0d0f0,
>>>> + 0xfff0d0f0, 0xffe0d000, 0xf800d000};
>>> I think these arrays should be 'const'; we can also move them closer
>>> to their point of use, inside the scope of the if() below.
>>
>> static as well.
>
> Mmm; commit is already in master though, will need a followup patch.
I can make it if you wish.
In addition, we can simplify following "if" by removing ARM_FEATURE_V6
since V7M and V8M define V6:
if (!arm_dc_feature(s, ARM_FEATURE_V7) &&
!(arm_dc_feature(s, ARM_FEATURE_V6) &&
arm_dc_feature(s, ARM_FEATURE_M))) {
goto illegal_op;
}
Like this:
if (!arm_dc_feature(s, ARM_FEATURE_V7) &&
!arm_dc_feature(s, ARM_FEATURE_M)) {
goto illegal_op;
}
What do you think?
Best regards, Julia Suvorova.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions
2018-06-17 18:48 ` Julia Suvorova
@ 2018-06-18 10:48 ` Peter Maydell
0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2018-06-18 10:48 UTC (permalink / raw)
To: Julia Suvorova
Cc: Richard Henderson, Jim Mussared, Steffen Görtz,
QEMU Developers, Stefan Hajnoczi, Joel Stanley
On 17 June 2018 at 19:48, Julia Suvorova <jusual@mail.ru> wrote:
> I can make it if you wish.
> In addition, we can simplify following "if" by removing ARM_FEATURE_V6
> since V7M and V8M define V6:
>
> if (!arm_dc_feature(s, ARM_FEATURE_V7) &&
> !(arm_dc_feature(s, ARM_FEATURE_V6) &&
> arm_dc_feature(s, ARM_FEATURE_M))) {
> goto illegal_op;
> }
>
> Like this:
>
> if (!arm_dc_feature(s, ARM_FEATURE_V7) &&
> !arm_dc_feature(s, ARM_FEATURE_M)) {
> goto illegal_op;
> }
>
> What do you think?
Yes; that would be reasonable. I did consider making that
change when I applied the patch, but decided I didn't really
care much either way. So if you want to send a patch for it
that's fine; if you don't, that's also fine.
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-06-18 10:49 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-12 20:46 [Qemu-devel] [PATCH] target/arm: Allow ARMv6-M Thumb2 instructions Julia Suvorova
2018-06-13 14:05 ` Stefan Hajnoczi
2018-06-13 17:10 ` Julia Suvorova
2018-06-14 8:17 ` Stefan Hajnoczi
2018-06-15 10:55 ` Peter Maydell
2018-06-15 11:15 ` Julia Suvorova
2018-06-17 5:36 ` Richard Henderson
2018-06-17 16:33 ` Peter Maydell
2018-06-17 18:48 ` Julia Suvorova
2018-06-18 10:48 ` Peter Maydell
2018-06-15 13:31 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.