[2/4] arm: Don't decode MRS(banked) or MSR(banked) for M profile

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>

---
 target/arm/translate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.7.4

Peter Maydell <peter.maydell@linaro.org> writes:

> 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 ARMv7-A documentation talks about banked registers being a feature
of application processors with Virtualisation Extensions which make the
sense of the test a bit weird. But I guess they are functionally
equivalent. Are there in practice any -A cores without virt?

>

> (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>


Anyway digressions aside:

Reviewed-by: Alex Bennée <alex.bennee@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 895b399..0f8548f 100644

> --- a/target/arm/translate.c

> +++ b/target/arm/translate.c

> @@ -10488,7 +10488,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);

> @@ -10509,7 +10510,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);



--
Alex Bennée

On 20 March 2017 at 10:57, Alex Bennée <alex.bennee@linaro.org> wrote:
>

> Peter Maydell <peter.maydell@linaro.org> writes:

>

>> 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 ARMv7-A documentation talks about banked registers being a feature

> of application processors with Virtualisation Extensions which make the

> sense of the test a bit weird. But I guess they are functionally

> equivalent. Are there in practice any -A cores without virt?


You can have an A profile core without EL2 implemented, yes.
In particular in ARMv8 this encoding is legal (and has defined
effects) even if EL2 is not implemented. It's introduced in
ARMv7VE, and there's an argument for allowing it even if we
don't implement EL2 for those CPUs which in real hardware do
implement it, because system software might assume it can use
these encodings.

We should probably UNDEF it for pre-v7VE cores though. For
this patchset what I really wanted was to avoid the different
decode used by M profile getting mixed up with the A profile
decode path.

thanks
-- PMM