* [Qemu-devel] [PULL 1/4] arm: HVC and SMC encodings don't exist for M profile
2017-03-20 12:54 [Qemu-devel] [PULL for-2.9 0/4] target-arm queue Peter Maydell
@ 2017-03-20 12:54 ` Peter Maydell
2017-03-20 12:54 ` [Qemu-devel] [PULL 2/4] arm: Don't decode MRS(banked) or MSR(banked) " Peter Maydell
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2017-03-20 12:54 UTC (permalink / raw)
To: qemu-devel
M profile doesn't have the HVC or SMC encodings, so make them always
UNDEF rather than generating calls to helper functions that assume
A/R profile.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 1487616072-9226-2-git-send-email-peter.maydell@linaro.org
---
target/arm/translate.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index b859f10..216852b 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -10377,6 +10377,9 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
goto illegal_op;
if (insn & (1 << 26)) {
+ if (arm_dc_feature(s, ARM_FEATURE_M)) {
+ goto illegal_op;
+ }
if (!(insn & (1 << 20))) {
/* Hypervisor call (v7) */
int imm16 = extract32(insn, 16, 4) << 12
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 2/4] arm: Don't decode MRS(banked) or MSR(banked) for M profile
2017-03-20 12:54 [Qemu-devel] [PULL for-2.9 0/4] target-arm queue Peter Maydell
2017-03-20 12:54 ` [Qemu-devel] [PULL 1/4] arm: HVC and SMC encodings don't exist for M profile Peter Maydell
@ 2017-03-20 12:54 ` Peter Maydell
2017-03-20 12:54 ` [Qemu-devel] [PULL 3/4] arm: Enforce should-be-1 bits in MRS decoding Peter Maydell
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2017-03-20 12:54 UTC (permalink / raw)
To: qemu-devel
M profile doesn't have the MSR(banked) and MRS(banked) instructions
and uses the encodings for different kinds of M-profile MRS/MSR.
Guard the relevant bits of the decode logic to make sure we don't
accidentally fall into them by accident on M-profile.
(The bit being checked for this (bit 5) is part of the SYSm field on
M-profile, but since no currently allocated system registers have
encodings with bit 5 of SYSm set, this hasn't been a problem in
practice.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 1487616072-9226-3-git-send-email-peter.maydell@linaro.org
---
target/arm/translate.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 216852b..a5f5a28 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -10500,7 +10500,8 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
gen_exception_return(s, tmp);
break;
case 6: /* MRS */
- if (extract32(insn, 5, 1)) {
+ if (extract32(insn, 5, 1) &&
+ !arm_dc_feature(s, ARM_FEATURE_M)) {
/* MRS (banked) */
int sysm = extract32(insn, 16, 4) |
(extract32(insn, 4, 1) << 4);
@@ -10521,7 +10522,8 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
store_reg(s, rd, tmp);
break;
case 7: /* MRS */
- if (extract32(insn, 5, 1)) {
+ if (extract32(insn, 5, 1) &&
+ !arm_dc_feature(s, ARM_FEATURE_M)) {
/* MRS (banked) */
int sysm = extract32(insn, 16, 4) |
(extract32(insn, 4, 1) << 4);
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 3/4] arm: Enforce should-be-1 bits in MRS decoding
2017-03-20 12:54 [Qemu-devel] [PULL for-2.9 0/4] target-arm queue Peter Maydell
2017-03-20 12:54 ` [Qemu-devel] [PULL 1/4] arm: HVC and SMC encodings don't exist for M profile Peter Maydell
2017-03-20 12:54 ` [Qemu-devel] [PULL 2/4] arm: Don't decode MRS(banked) or MSR(banked) " Peter Maydell
@ 2017-03-20 12:54 ` Peter Maydell
2017-03-20 12:54 ` [Qemu-devel] [PULL 4/4] arm: Fix APSR writes via M profile MSR Peter Maydell
2017-03-20 13:52 ` [Qemu-devel] [PULL for-2.9 0/4] target-arm queue Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2017-03-20 12:54 UTC (permalink / raw)
To: qemu-devel
The MRS instruction requires that bits [19..16] are all 1s, and for
A/R profile also that bits [7..0] are all 0s. At this point in the
decode tree we have checked all of the rest of the instruction but
were allowing these to be any value. If these bits are not set then
the result is architecturally UNPREDICTABLE, but choosing to UNDEF is
more helpful to the user and avoids unexpected odd behaviour if the
encodings are used for some purpose in future architecture versions.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 1487616072-9226-4-git-send-email-peter.maydell@linaro.org
---
target/arm/translate.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index a5f5a28..c4acff5 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -10510,6 +10510,14 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
break;
}
+ if (extract32(insn, 16, 4) != 0xf) {
+ goto illegal_op;
+ }
+ if (!arm_dc_feature(s, ARM_FEATURE_M) &&
+ extract32(insn, 0, 8) != 0) {
+ goto illegal_op;
+ }
+
/* mrs cpsr */
tmp = tcg_temp_new_i32();
if (arm_dc_feature(s, ARM_FEATURE_M)) {
@@ -10537,6 +10545,12 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
if (IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) {
goto illegal_op;
}
+
+ if (extract32(insn, 16, 4) != 0xf ||
+ extract32(insn, 0, 8) != 0) {
+ goto illegal_op;
+ }
+
tmp = load_cpu_field(spsr);
store_reg(s, rd, tmp);
break;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 4/4] arm: Fix APSR writes via M profile MSR
2017-03-20 12:54 [Qemu-devel] [PULL for-2.9 0/4] target-arm queue Peter Maydell
` (2 preceding siblings ...)
2017-03-20 12:54 ` [Qemu-devel] [PULL 3/4] arm: Enforce should-be-1 bits in MRS decoding Peter Maydell
@ 2017-03-20 12:54 ` Peter Maydell
2017-03-20 13:52 ` [Qemu-devel] [PULL for-2.9 0/4] target-arm queue Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2017-03-20 12:54 UTC (permalink / raw)
To: qemu-devel
Our implementation of writes to the APSR for M-profile via the MSR
instruction was badly broken.
First and worst, we had the sense wrong on the test of bit 2 of the
SYSm field -- this is supposed to request an APSR write if bit 2 is 0
but we were doing it if bit 2 was 1. This bug was introduced in
commit 58117c9bb429cd, so hasn't been in a QEMU release.
Secondly, the choice of exactly which parts of APSR should be written
is defined by bits in the 'mask' field. We were not passing these
through from instruction decode, making it impossible to check them
in the helper.
Pass the mask bits through from the instruction decode to the helper
function and process them appropriately; fix the wrong sense of the
SYSm bit 2 check.
Invalid mask values and invalid combinations of mask and register
number are UNPREDICTABLE; we choose to treat them as if the mask
values were valid.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1487616072-9226-5-git-send-email-peter.maydell@linaro.org
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
target/arm/helper.c | 26 ++++++++++++++++++++++----
target/arm/translate.c | 3 ++-
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 8646a7a..8cb7a94 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8548,8 +8548,18 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
}
}
-void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val)
-{
+void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val)
+{
+ /* We're passed bits [11..0] of the instruction; extract
+ * SYSm and the mask bits.
+ * Invalid combinations of SYSm and mask are UNPREDICTABLE;
+ * we choose to treat them as if the mask bits were valid.
+ * NB that the pseudocode 'mask' variable is bits [11..10],
+ * whereas ours is [11..8].
+ */
+ uint32_t mask = extract32(maskreg, 8, 4);
+ uint32_t reg = extract32(maskreg, 0, 8);
+
if (arm_current_el(env) == 0 && reg > 7) {
/* only xPSR sub-fields may be written by unprivileged */
return;
@@ -8558,8 +8568,16 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val)
switch (reg) {
case 0 ... 7: /* xPSR sub-fields */
/* only APSR is actually writable */
- if (reg & 4) {
- xpsr_write(env, val, 0xf8000000); /* APSR */
+ if (!(reg & 4)) {
+ uint32_t apsrmask = 0;
+
+ if (mask & 8) {
+ apsrmask |= 0xf8000000; /* APSR NZCVQ */
+ }
+ if ((mask & 4) && arm_feature(env, ARM_FEATURE_THUMB_DSP)) {
+ apsrmask |= 0x000f0000; /* APSR GE[3:0] */
+ }
+ xpsr_write(env, val, apsrmask);
}
break;
case 8: /* MSP */
diff --git a/target/arm/translate.c b/target/arm/translate.c
index c4acff5..e32e38c 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -10403,7 +10403,8 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
case 0: /* msr cpsr. */
if (arm_dc_feature(s, ARM_FEATURE_M)) {
tmp = load_reg(s, rn);
- addr = tcg_const_i32(insn & 0xff);
+ /* the constant is the mask and SYSm fields */
+ addr = tcg_const_i32(insn & 0xfff);
gen_helper_v7m_msr(cpu_env, addr, tmp);
tcg_temp_free_i32(addr);
tcg_temp_free_i32(tmp);
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PULL for-2.9 0/4] target-arm queue
2017-03-20 12:54 [Qemu-devel] [PULL for-2.9 0/4] target-arm queue Peter Maydell
` (3 preceding siblings ...)
2017-03-20 12:54 ` [Qemu-devel] [PULL 4/4] arm: Fix APSR writes via M profile MSR Peter Maydell
@ 2017-03-20 13:52 ` Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2017-03-20 13:52 UTC (permalink / raw)
To: QEMU Developers
On 20 March 2017 at 12:54, Peter Maydell <peter.maydell@linaro.org> wrote:
> Small target-arm queue for 2.9: just the patches
> which fix bugs in our MRS/MSR decoding for M profile,
> including a fix for a regression introduced in commit
> 58117c9bb429cd.
>
> thanks
> -- PMM
>
> The following changes since commit 00e7c07b06d004cf54b19724f82afde8a7a37f37:
>
> Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20170320' into staging (2017-03-20 10:51:30 +0000)
>
> are available in the git repository at:
>
> git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20170320
>
> for you to fetch changes up to b28b3377d7e9ba35611d454d5a63ef50cab1f8c5:
>
> arm: Fix APSR writes via M profile MSR (2017-03-20 12:41:44 +0000)
>
> ----------------------------------------------------------------
> target-arm queue:
> * fix MSR/MRS decoding for M profile CPUs
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread