* [PATCH for-4.2 0/2] target/arm: two fixes for ldrex/strex @ 2019-11-17 9:06 Richard Henderson 2019-11-17 9:06 ` [PATCH 1/2] target/arm: Do not reject rt == rt2 for strexd Richard Henderson 2019-11-17 9:06 ` [PATCH 2/2] target/arm: Relax r13 restriction for ldrex/strex for v8.0 Richard Henderson 0 siblings, 2 replies; 9+ messages in thread From: Richard Henderson @ 2019-11-17 9:06 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell During this cycle I added checks for UNPREDICTABLE behavior, but didn't quite get it all right. r~ Richard Henderson (2): target/arm: Do not reject rt == rt2 for strexd target/arm: Relax r13 restriction for ldrex/strex for v8.0 target/arm/translate.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] target/arm: Do not reject rt == rt2 for strexd 2019-11-17 9:06 [PATCH for-4.2 0/2] target/arm: two fixes for ldrex/strex Richard Henderson @ 2019-11-17 9:06 ` Richard Henderson 2019-11-18 13:17 ` Peter Maydell 2019-11-17 9:06 ` [PATCH 2/2] target/arm: Relax r13 restriction for ldrex/strex for v8.0 Richard Henderson 1 sibling, 1 reply; 9+ messages in thread From: Richard Henderson @ 2019-11-17 9:06 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell There was too much cut and paste between ldrexd and strexd, as ldrexd does prohibit two output registers the same. Fixes: af288228995 Reported-by: Michael Goffioul <michael.goffioul@gmail.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index 2ea9da7637..b285b23858 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -8934,7 +8934,7 @@ static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel) || (s->thumb && (a->rd == 13 || a->rt == 13)) || (mop == MO_64 && (a->rt2 == 15 - || a->rd == a->rt2 || a->rt == a->rt2 + || a->rd == a->rt2 || (s->thumb && a->rt2 == 13)))) { unallocated_encoding(s); return true; -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] target/arm: Do not reject rt == rt2 for strexd 2019-11-17 9:06 ` [PATCH 1/2] target/arm: Do not reject rt == rt2 for strexd Richard Henderson @ 2019-11-18 13:17 ` Peter Maydell 0 siblings, 0 replies; 9+ messages in thread From: Peter Maydell @ 2019-11-18 13:17 UTC (permalink / raw) To: Richard Henderson; +Cc: QEMU Developers On Sun, 17 Nov 2019 at 09:06, Richard Henderson <richard.henderson@linaro.org> wrote: > > There was too much cut and paste between ldrexd and strexd, > as ldrexd does prohibit two output registers the same. > > Fixes: af288228995 > Reported-by: Michael Goffioul <michael.goffioul@gmail.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/translate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/arm/translate.c b/target/arm/translate.c > index 2ea9da7637..b285b23858 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c > @@ -8934,7 +8934,7 @@ static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel) > || (s->thumb && (a->rd == 13 || a->rt == 13)) > || (mop == MO_64 > && (a->rt2 == 15 > - || a->rd == a->rt2 || a->rt == a->rt2 > + || a->rd == a->rt2 > || (s->thumb && a->rt2 == 13)))) { > unallocated_encoding(s); > return true; > -- > 2.17.1 Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] target/arm: Relax r13 restriction for ldrex/strex for v8.0 2019-11-17 9:06 [PATCH for-4.2 0/2] target/arm: two fixes for ldrex/strex Richard Henderson 2019-11-17 9:06 ` [PATCH 1/2] target/arm: Do not reject rt == rt2 for strexd Richard Henderson @ 2019-11-17 9:06 ` Richard Henderson 2019-11-18 13:10 ` Peter Maydell 1 sibling, 1 reply; 9+ messages in thread From: Richard Henderson @ 2019-11-17 9:06 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell Armv8-A removes UNPREDICTABLE for R13 for these cases. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/translate.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index b285b23858..3db8103966 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -8931,11 +8931,11 @@ static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel) /* We UNDEF for these UNPREDICTABLE cases. */ if (a->rd == 15 || a->rn == 15 || a->rt == 15 || a->rd == a->rn || a->rd == a->rt - || (s->thumb && (a->rd == 13 || a->rt == 13)) + || (!ENABLE_ARCH_8 && s->thumb && (a->rd == 13 || a->rt == 13)) || (mop == MO_64 && (a->rt2 == 15 || a->rd == a->rt2 - || (s->thumb && a->rt2 == 13)))) { + || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) { unallocated_encoding(s); return true; } @@ -9087,10 +9087,10 @@ static bool op_ldrex(DisasContext *s, arg_LDREX *a, MemOp mop, bool acq) /* We UNDEF for these UNPREDICTABLE cases. */ if (a->rn == 15 || a->rt == 15 - || (s->thumb && a->rt == 13) + || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13) || (mop == MO_64 && (a->rt2 == 15 || a->rt == a->rt2 - || (s->thumb && a->rt2 == 13)))) { + || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) { unallocated_encoding(s); return true; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] target/arm: Relax r13 restriction for ldrex/strex for v8.0 2019-11-17 9:06 ` [PATCH 2/2] target/arm: Relax r13 restriction for ldrex/strex for v8.0 Richard Henderson @ 2019-11-18 13:10 ` Peter Maydell 2019-11-18 13:15 ` Richard Henderson 0 siblings, 1 reply; 9+ messages in thread From: Peter Maydell @ 2019-11-18 13:10 UTC (permalink / raw) To: Richard Henderson; +Cc: QEMU Developers On Sun, 17 Nov 2019 at 09:06, Richard Henderson <richard.henderson@linaro.org> wrote: > > Armv8-A removes UNPREDICTABLE for R13 for these cases. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/translate.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/target/arm/translate.c b/target/arm/translate.c > index b285b23858..3db8103966 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c > @@ -8931,11 +8931,11 @@ static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel) > /* We UNDEF for these UNPREDICTABLE cases. */ > if (a->rd == 15 || a->rn == 15 || a->rt == 15 > || a->rd == a->rn || a->rd == a->rt > - || (s->thumb && (a->rd == 13 || a->rt == 13)) > + || (!ENABLE_ARCH_8 && s->thumb && (a->rd == 13 || a->rt == 13)) > || (mop == MO_64 > && (a->rt2 == 15 > || a->rd == a->rt2 > - || (s->thumb && a->rt2 == 13)))) { > + || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) { > unallocated_encoding(s); > return true; > } > @@ -9087,10 +9087,10 @@ static bool op_ldrex(DisasContext *s, arg_LDREX *a, MemOp mop, bool acq) > > /* We UNDEF for these UNPREDICTABLE cases. */ > if (a->rn == 15 || a->rt == 15 > - || (s->thumb && a->rt == 13) > + || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13) > || (mop == MO_64 > && (a->rt2 == 15 || a->rt == a->rt2 > - || (s->thumb && a->rt2 == 13)))) { > + || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) { > unallocated_encoding(s); > return true; > } These cases for r13 are indeed no longer UNPREDICTABLE in v8A, but they are still marked as UNPREDICTABLE for v8M... thanks -- PMM ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] target/arm: Relax r13 restriction for ldrex/strex for v8.0 2019-11-18 13:10 ` Peter Maydell @ 2019-11-18 13:15 ` Richard Henderson 2019-11-18 17:53 ` Peter Maydell 0 siblings, 1 reply; 9+ messages in thread From: Richard Henderson @ 2019-11-18 13:15 UTC (permalink / raw) To: Peter Maydell; +Cc: QEMU Developers On 11/18/19 2:10 PM, Peter Maydell wrote: >> /* We UNDEF for these UNPREDICTABLE cases. */ >> if (a->rn == 15 || a->rt == 15 >> - || (s->thumb && a->rt == 13) >> + || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13) >> || (mop == MO_64 >> && (a->rt2 == 15 || a->rt == a->rt2 >> - || (s->thumb && a->rt2 == 13)))) { >> + || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) { >> unallocated_encoding(s); >> return true; >> } > > These cases for r13 are indeed no longer UNPREDICTABLE in > v8A, but they are still marked as UNPREDICTABLE for v8M... Ho hum. I knew I should have looked at that doc as well... r~ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] target/arm: Relax r13 restriction for ldrex/strex for v8.0 2019-11-18 13:15 ` Richard Henderson @ 2019-11-18 17:53 ` Peter Maydell 2019-11-18 20:02 ` Richard Henderson 0 siblings, 1 reply; 9+ messages in thread From: Peter Maydell @ 2019-11-18 17:53 UTC (permalink / raw) To: Richard Henderson; +Cc: QEMU Developers On Mon, 18 Nov 2019 at 13:16, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 11/18/19 2:10 PM, Peter Maydell wrote: > >> /* We UNDEF for these UNPREDICTABLE cases. */ > >> if (a->rn == 15 || a->rt == 15 > >> - || (s->thumb && a->rt == 13) > >> + || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13) > >> || (mop == MO_64 > >> && (a->rt2 == 15 || a->rt == a->rt2 > >> - || (s->thumb && a->rt2 == 13)))) { > >> + || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) { > >> unallocated_encoding(s); > >> return true; > >> } > > > > These cases for r13 are indeed no longer UNPREDICTABLE in > > v8A, but they are still marked as UNPREDICTABLE for v8M... > > Ho hum. I knew I should have looked at that doc as well... I would like to get this in for rc2 tomorrow, so I propose to squash in changes to give the following result (basically turning the ENABLE_ARCH_8 checks into checks on a new bool 'v8a'): diff --git a/target/arm/translate.c b/target/arm/translate.c index b285b23858e..4d5d4bd8886 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -8927,15 +8927,17 @@ static bool trans_SWPB(DisasContext *s, arg_SWP *a) static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel) { TCGv_i32 addr; + /* Some cases stopped being UNPREDICTABLE in v8A (but not v8M) */ + bool v8a = ENABLE_ARCH_8 && !arm_dc_feature(s, ARM_FEATURE_M); /* We UNDEF for these UNPREDICTABLE cases. */ if (a->rd == 15 || a->rn == 15 || a->rt == 15 || a->rd == a->rn || a->rd == a->rt - || (s->thumb && (a->rd == 13 || a->rt == 13)) + || (!v8a && s->thumb && (a->rd == 13 || a->rt == 13)) || (mop == MO_64 && (a->rt2 == 15 || a->rd == a->rt2 - || (s->thumb && a->rt2 == 13)))) { + || (!v8a && s->thumb && a->rt2 == 13)))) { unallocated_encoding(s); return true; } @@ -9084,13 +9086,15 @@ static bool trans_STLH(DisasContext *s, arg_STL *a) static bool op_ldrex(DisasContext *s, arg_LDREX *a, MemOp mop, bool acq) { TCGv_i32 addr; + /* Some cases stopped being UNPREDICTABLE in v8A (but not v8M) */ + bool v8a = ENABLE_ARCH_8 && !arm_dc_feature(s, ARM_FEATURE_M); /* We UNDEF for these UNPREDICTABLE cases. */ if (a->rn == 15 || a->rt == 15 - || (s->thumb && a->rt == 13) + || (!v8a && s->thumb && a->rt == 13) || (mop == MO_64 && (a->rt2 == 15 || a->rt == a->rt2 - || (s->thumb && a->rt2 == 13)))) { + || (!v8a && s->thumb && a->rt2 == 13)))) { unallocated_encoding(s); return true; } thanks -- PMM ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] target/arm: Relax r13 restriction for ldrex/strex for v8.0 2019-11-18 17:53 ` Peter Maydell @ 2019-11-18 20:02 ` Richard Henderson 2019-11-18 21:22 ` Peter Maydell 0 siblings, 1 reply; 9+ messages in thread From: Richard Henderson @ 2019-11-18 20:02 UTC (permalink / raw) To: Peter Maydell; +Cc: QEMU Developers On 11/18/19 6:53 PM, Peter Maydell wrote: > On Mon, 18 Nov 2019 at 13:16, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> On 11/18/19 2:10 PM, Peter Maydell wrote: >>>> /* We UNDEF for these UNPREDICTABLE cases. */ >>>> if (a->rn == 15 || a->rt == 15 >>>> - || (s->thumb && a->rt == 13) >>>> + || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13) >>>> || (mop == MO_64 >>>> && (a->rt2 == 15 || a->rt == a->rt2 >>>> - || (s->thumb && a->rt2 == 13)))) { >>>> + || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) { >>>> unallocated_encoding(s); >>>> return true; >>>> } >>> >>> These cases for r13 are indeed no longer UNPREDICTABLE in >>> v8A, but they are still marked as UNPREDICTABLE for v8M... >> >> Ho hum. I knew I should have looked at that doc as well... > > I would like to get this in for rc2 tomorrow, so I propose > to squash in changes to give the following result (basically > turning the ENABLE_ARCH_8 checks into checks on a new bool 'v8a'): > > > diff --git a/target/arm/translate.c b/target/arm/translate.c > index b285b23858e..4d5d4bd8886 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c > @@ -8927,15 +8927,17 @@ static bool trans_SWPB(DisasContext *s, arg_SWP *a) > static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel) > { > TCGv_i32 addr; > + /* Some cases stopped being UNPREDICTABLE in v8A (but not v8M) */ > + bool v8a = ENABLE_ARCH_8 && !arm_dc_feature(s, ARM_FEATURE_M); Sorry, I wrote the patch but got distracted with other bugs without getting around to posting. I had solved this with a new ENABLE_ARCH_8A, but this version works for me as well. Thanks, r~ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] target/arm: Relax r13 restriction for ldrex/strex for v8.0 2019-11-18 20:02 ` Richard Henderson @ 2019-11-18 21:22 ` Peter Maydell 0 siblings, 0 replies; 9+ messages in thread From: Peter Maydell @ 2019-11-18 21:22 UTC (permalink / raw) To: Richard Henderson; +Cc: QEMU Developers On Mon, 18 Nov 2019 at 20:02, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 11/18/19 6:53 PM, Peter Maydell wrote: > > On Mon, 18 Nov 2019 at 13:16, Richard Henderson > > <richard.henderson@linaro.org> wrote: > >> > >> On 11/18/19 2:10 PM, Peter Maydell wrote: > >>>> /* We UNDEF for these UNPREDICTABLE cases. */ > >>>> if (a->rn == 15 || a->rt == 15 > >>>> - || (s->thumb && a->rt == 13) > >>>> + || (!ENABLE_ARCH_8 && s->thumb && a->rt == 13) > >>>> || (mop == MO_64 > >>>> && (a->rt2 == 15 || a->rt == a->rt2 > >>>> - || (s->thumb && a->rt2 == 13)))) { > >>>> + || (!ENABLE_ARCH_8 && s->thumb && a->rt2 == 13)))) { > >>>> unallocated_encoding(s); > >>>> return true; > >>>> } > >>> > >>> These cases for r13 are indeed no longer UNPREDICTABLE in > >>> v8A, but they are still marked as UNPREDICTABLE for v8M... > >> > >> Ho hum. I knew I should have looked at that doc as well... > > > > I would like to get this in for rc2 tomorrow, so I propose > > to squash in changes to give the following result (basically > > turning the ENABLE_ARCH_8 checks into checks on a new bool 'v8a'): > > > > > > diff --git a/target/arm/translate.c b/target/arm/translate.c > > index b285b23858e..4d5d4bd8886 100644 > > --- a/target/arm/translate.c > > +++ b/target/arm/translate.c > > @@ -8927,15 +8927,17 @@ static bool trans_SWPB(DisasContext *s, arg_SWP *a) > > static bool op_strex(DisasContext *s, arg_STREX *a, MemOp mop, bool rel) > > { > > TCGv_i32 addr; > > + /* Some cases stopped being UNPREDICTABLE in v8A (but not v8M) */ > > + bool v8a = ENABLE_ARCH_8 && !arm_dc_feature(s, ARM_FEATURE_M); > > Sorry, I wrote the patch but got distracted with other bugs without getting > around to posting. I had solved this with a new ENABLE_ARCH_8A, but this > version works for me as well. At some point we should decide whether we prefer these ENABLE macros or just to open-code arm_dc_feature() calls, because the current mix is a bit odd... (for code I've written I've tended to the arm_dc_feature() approach). thanks -- PMM ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-11-18 21:25 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-17 9:06 [PATCH for-4.2 0/2] target/arm: two fixes for ldrex/strex Richard Henderson 2019-11-17 9:06 ` [PATCH 1/2] target/arm: Do not reject rt == rt2 for strexd Richard Henderson 2019-11-18 13:17 ` Peter Maydell 2019-11-17 9:06 ` [PATCH 2/2] target/arm: Relax r13 restriction for ldrex/strex for v8.0 Richard Henderson 2019-11-18 13:10 ` Peter Maydell 2019-11-18 13:15 ` Richard Henderson 2019-11-18 17:53 ` Peter Maydell 2019-11-18 20:02 ` Richard Henderson 2019-11-18 21:22 ` 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).