Re: [Qemu-devel] [PATCH] target/arm: Remove 5J architecture

[Peter Maydell]

On 28 August 2017 at 23:19, Portia Stephens <address@hidden> wrote:
> This fixes the issue that any BXJ instruction will result in an illegal_op.
> This is because the 5J archiecture is always unsupported.
> 5J architecture doesn't have a feature set and ENABLE_ARCH_5J is hardcoded
> to 0, causing any ARCH(5J) to result in an illegal_op. The only use of
> ARCH(5J) is in the BXJ instruction disassembly.
>
> This patch replaces that ARCH(5J) with ARCH(6) and removes the 5J 
> architecture,
> this isn't technically correct since the v5J ISA does support the BXJ
> instruction. This change means that running a BXJ instruction on any v5 will
> cause an illegal_op but it is better than the current state where any
> architecture running a BXJ would cause an illegal_op. The correct solution
> would be to create a feature set for v5J but that doesn't seem worth it as the
> v5J is so old.
>
> Signed-off-by: Portia Stephens <address@hidden>
> Reviewed-by: Alistair Francis <address@hidden>
> ---
>  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 d1a5f56998..4a30c0d7e0 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -41,7 +41,6 @@
>  #define ENABLE_ARCH_5     arm_dc_feature(s, ARM_FEATURE_V5)
>  /* currently all emulated v5 cores are also v5TE, so don't bother */
>  #define ENABLE_ARCH_5TE   arm_dc_feature(s, ARM_FEATURE_V5)
> -#define ENABLE_ARCH_5J    0
>  #define ENABLE_ARCH_6     arm_dc_feature(s, ARM_FEATURE_V6)
>  #define ENABLE_ARCH_6K    arm_dc_feature(s, ARM_FEATURE_V6K)
>  #define ENABLE_ARCH_6T2   arm_dc_feature(s, ARM_FEATURE_THUMB2)
> @@ -8389,7 +8388,10 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>              break;
>          case 0x2:
>              if (op1 == 1) {
> -                ARCH(5J); /* bxj */
> +                /* This should actually be ARCH(5J) but there is currently no
> +                 * 5J architecture in QEMU.
> +                 */
> +                ARCH(6); /* bxj */
>                  /* Trivial implementation equivalent to bx.  */
>                  tmp = load_reg(s, rm);
>                  gen_bx(s, tmp);

Thanks for this patch. However we do have both v5-no-J
(arm946, all the pxa2xx cores) and v5-with-J CPUs (arm926, arm1026),
so I think it would be better to fix this bug by adding an
extra ARM_FEATURE_JAZELLE, which would be set in
arm_cpu_realizefn() if ARM_FEATURE_V6 is set, and set in
the per-core realize functions for arm926 and arm1026.
It should be a fairly small patch overall I think.

thanks
-- PMM