[Qemu-devel] [PATCH] target-mips: Fix ALIGN instruction when bp=0

[Miodrag Dinic]

Hello to everyone,

We have uncovered a use-case with ALIGN instruction which is
not handled correctly by QEMU. It impacts both, user and system mode emulation.

Using ALIGN instruction with bp=0 as the last argument, should behave
as a register to register move with sign extension if running on a mips64 system.
The problem is that the sign extension is not preformed.

Taken from the official documentation :

Operation

ALIGN:
    tmp_rt_hi = unsigned_word(GPR[rt]) << (8*bp)
    tmp_rs_lo =unsigned_word(GPR[rs]) >> (8*(4-bp))
    tmp = tmp_rt_hi || tmp_rt_lo
    GPR[rd] = sign_extend.32(tmp)

DALIGN
    tmp_rt_hi = unsigned_doubleword(GPR[rt]) << (8*bp)
    tmp_rs_lo =unsigned_doubleword(GPR[rs]) >> (8*(8-bp))
    tmp = tmp_rt_hi || tmp_rt_lo
    GPR[rd] = tmp


Here is a simple test for reproducing the problem :

#include <stdio.h>
#include <limits>
#include <stdint.h>


int main(int argc, char * argv[]) {
 
  int64_t param1, param2, result, resultd;
  int64_t *param1_ptr = &param1, *param2_ptr = &param2, *result_ptr = &result, *resultd_ptr = &resultd;
 
  uint64_t param1_arr[] = { 0xfedcba98, 0x01234567, 0xaabbccdd, 0x004488bb };
  uint64_t param2_arr[] = { 0x01234567, 0xaabbccdd, 0xfedcba98, 0x004488bb };
  uint64_t result_arr[] = { 0xfffffffffedcba98, 0x1234567, 0xffffffffaabbccdd, 0x4488bb };
  uint64_t resultd_arr[] = { 0xfedcba98, 0x1234567, 0xaabbccdd, 0x4488bb };

  printf("\n\n");

  for (int i = 0; i < 4; i++) {
    param1 = param1_arr[i];
    param2 = param2_arr[i];
  __asm__ volatile(
      "move          $t0, %0\n\t"
      "move          $t1, %1\n\t"
      "move          $t2, %2\n\t"
      "move          $t3, %3\n\t"
    
      "ld            $a0, 0($t0)\n\t"
      "ld            $a1, 0($t0)\n\t"
      "align         $a2, $a1, $a0, 0\n\t"
      "dalign        $a3, $a1, $a0, 0\n\t"
      "sd            $a2, 0($t2)\n\t"
      "sd            $a3, 0($t3)\n\t"
      :
      : "r" (param1_ptr), "r" (param2_ptr), "r" (result_ptr), "r"(resultd_ptr)
      : "a0", "a1", "a2", "a3", "t0", "t1", "t2", "t3", "cc", "memory");
      printf("ALIGN %s: ", result == result_arr[i] ? "PASS" : "FAIL");
      printf("Expected = %lx, actual = %lx\n", result_arr[i], result);
      printf("DALIGN %s: ", resultd == resultd_arr[i] ? "PASS" : "FAIL");
      printf("Expected = %lx, actual = %lx\n", resultd_arr[i], resultd);
  }
}

Compile it with any available R6 enabled toolchain :

mips-img-linux-gnu-gcc -static -mips64r6 -mabi=64 align-instr.cc -o align-instr-test

Running the test :

<QEMU>/mips64el-linux-user/qemu-mips64el -cpu MIPS64R6-generic ./align-instr-test

ALIGN FAIL: Expected = fffffffffedcba98, actual = fedcba98
DALIGN PASS: Expected = fedcba98, actual = fedcba98
ALIGN PASS: Expected = 1234567, actual = 1234567
DALIGN PASS: Expected = 1234567, actual = 1234567
ALIGN FAIL: Expected = ffffffffaabbccdd, actual = aabbccdd
DALIGN PASS: Expected = aabbccdd, actual = aabbccdd
ALIGN PASS: Expected = 4488bb, actual = 4488bb
DALIGN PASS: Expected = 4488bb, actual = 4488bb


Test includes dalign instruction checking for verification
there is no regression after retesting with a patch applied.

A fix for this issue is to add sign extension for the bp=0
case when running within a 64-bit target.

After applying a patch, the test output is :
<QEMU>/mips64el-linux-user/qemu-mips64el -cpu MIPS64R6-generic ./align-instr-test

ALIGN PASS: Expected = fffffffffedcba98, actual = fffffffffedcba98
DALIGN PASS: Expected = fedcba98, actual = fedcba98
ALIGN PASS: Expected = 1234567, actual = 1234567
DALIGN PASS: Expected = 1234567, actual = 1234567
ALIGN PASS: Expected = ffffffffaabbccdd, actual = ffffffffaabbccdd
DALIGN PASS: Expected = aabbccdd, actual = aabbccdd
ALIGN PASS: Expected = 4488bb, actual = 4488bb
DALIGN PASS: Expected = 4488bb, actual = 4488bb


Patch is in the attachment.

Regards,
Miodrag

0001-target-mips-Fix-ALIGN-instruction-when-bp-0.patch
Description: 0001-target-mips-Fix-ALIGN-instruction-when-bp-0.patch