From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MSCS9-00052M-5K
	for qemu-devel@nongnu.org; Sat, 18 Jul 2009 12:08:01 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MSCS4-00050H-N4
	for qemu-devel@nongnu.org; Sat, 18 Jul 2009 12:08:00 -0400
Received: from [199.232.76.173] (port=36320 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MSCS4-000503-8K
	for qemu-devel@nongnu.org; Sat, 18 Jul 2009 12:07:56 -0400
Received: from mail-ew0-f217.google.com ([209.85.219.217]:62072)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <filip.navara@gmail.com>) id 1MSCS3-0001ca-Md
	for qemu-devel@nongnu.org; Sat, 18 Jul 2009 12:07:56 -0400
Received: by ewy17 with SMTP id 17so1221280ewy.34
	for <qemu-devel@nongnu.org>; Sat, 18 Jul 2009 09:07:54 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <066878C1-6EDC-4395-8EBB-16D52C4F1822@ok-labs.com>
References: <066878C1-6EDC-4395-8EBB-16D52C4F1822@ok-labs.com>
Date: Sat, 18 Jul 2009 18:07:54 +0200
Message-ID: <5b31733c0907180907o2ea71b44m8deea95651cfc014@mail.gmail.com>
Subject: Re: [Qemu-devel] ARMv6: Fix SRS/RFE instruction
From: Filip Navara <filip.navara@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: qemu-devel@nongnu.org

On Tue, Jul 22, 2008 at 4:20 AM, Hans Jang<hsjang@ok-labs.com> wrote:
> We are in the process of implementing a KZM (i.MX31) ARMv6 machine port i=
n
> order to run the OKL4 kernel. We found the new CPS/SRS/RFE instructions w=
ere
> broken.
> Vincent Palatin released a patch recently which fixes the CPS problem.
> Attached is a patch to fix the SRS/RFE bugs. Could this patch please be
> applied to the main trunk.
> Thanks
> - Hyeonsung Jang.
>
>
> - The encoding of 'IA' condition must be '01' instead of '02'.
> - SRS instruction must store banked SPSR instead of CPSR at the
> specific address.
> - 'return' statements are missing
>
>
> Index: target-arm/translate.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- target-arm/translate.c =A0 =A0 =A0(revision 4921)
> +++ target-arm/translate.c =A0 =A0 =A0(working copy)
> @@ -5702,7 +5702,7 @@
> =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0} else if ((insn & 0x0e5fffe0) =3D=3D 0x084d0500) {
> =A0 =A0 =A0 =A0 =A0 =A0/* srs */
> - =A0 =A0 =A0 =A0 =A0 =A0uint32_t offset;
> + =A0 =A0 =A0 =A0 =A0 =A0int32_t offset;
> =A0 =A0 =A0 =A0 =A0 =A0if (IS_USER(s))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto illegal_op;
> =A0 =A0 =A0 =A0 =A0 =A0ARCH(6);
> @@ -5716,8 +5716,8 @@
> =A0 =A0 =A0 =A0 =A0 =A0i =3D (insn >> 23) & 3;
> =A0 =A0 =A0 =A0 =A0 =A0switch (i) {
> =A0 =A0 =A0 =A0 =A0 =A0case 0: offset =3D -4; break; /* DA */
> - =A0 =A0 =A0 =A0 =A0 =A0case 1: offset =3D -8; break; /* DB */
> - =A0 =A0 =A0 =A0 =A0 =A0case 2: offset =3D 0; break; /* IA */
> + =A0 =A0 =A0 =A0 =A0 =A0case 1: offset =3D 0; break; /* IA */
> + =A0 =A0 =A0 =A0 =A0 =A0case 2: offset =3D -8; break; /* DB */
> =A0 =A0 =A0 =A0 =A0 =A0case 3: offset =3D 4; break; /* IB */
> =A0 =A0 =A0 =A0 =A0 =A0default: abort();
> =A0 =A0 =A0 =A0 =A0 =A0}
> @@ -5725,32 +5725,33 @@
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_addi_i32(addr, addr, offset);
> =A0 =A0 =A0 =A0 =A0 =A0tmp =3D load_reg(s, 14);
> =A0 =A0 =A0 =A0 =A0 =A0gen_st32(tmp, addr, 0);
> - =A0 =A0 =A0 =A0 =A0 =A0tmp =3D new_tmp();
> - =A0 =A0 =A0 =A0 =A0 =A0gen_helper_cpsr_read(tmp);
> + =A0 =A0 =A0 =A0 =A0 =A0tmp =3D load_cpu_field(spsr);
> =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_addi_i32(addr, addr, 4);
> =A0 =A0 =A0 =A0 =A0 =A0gen_st32(tmp, addr, 0);
> =A0 =A0 =A0 =A0 =A0 =A0if (insn & (1 << 21)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Base writeback. =A0*/
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0switch (i) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 0: offset =3D -8; break;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 1: offset =3D -4; break;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 2: offset =3D 4; break;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 1: offset =3D 4; break;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 2: offset =3D -4; break;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 3: offset =3D 0; break;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0default: abort();
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (offset)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_addi_i32(addr, tmp, offs=
et);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_addi_i32(addr, addr, off=
set);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (op1 =3D=3D (env->uncached_cpsr & CPSR_=
M)) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0gen_movl_reg_T1(s, 13);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0store_reg(s, 13, addr);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0gen_helper_set_r13_banked(cpu_en=
v, tcg_const_i32(op1),
> cpu_T[1]);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0gen_helper_set_r13_banked(cpu_en=
v, tcg_const_i32(op1),
> addr);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dead_tmp(addr);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0} else {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dead_tmp(addr);
> =A0 =A0 =A0 =A0 =A0 =A0}
> + =A0 =A0 =A0 =A0 =A0 =A0return;
> =A0 =A0 =A0 =A0} else if ((insn & 0x0e5fffe0) =3D=3D 0x081d0a00) {
> =A0 =A0 =A0 =A0 =A0 =A0/* rfe */
> - =A0 =A0 =A0 =A0 =A0 =A0uint32_t offset;
> + =A0 =A0 =A0 =A0 =A0 =A0int32_t offset;
> =A0 =A0 =A0 =A0 =A0 =A0if (IS_USER(s))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto illegal_op;
> =A0 =A0 =A0 =A0 =A0 =A0ARCH(6);
> @@ -5759,8 +5760,8 @@
> =A0 =A0 =A0 =A0 =A0 =A0i =3D (insn >> 23) & 3;
> =A0 =A0 =A0 =A0 =A0 =A0switch (i) {
> =A0 =A0 =A0 =A0 =A0 =A0case 0: offset =3D -4; break; /* DA */
> - =A0 =A0 =A0 =A0 =A0 =A0case 1: offset =3D -8; break; /* DB */
> - =A0 =A0 =A0 =A0 =A0 =A0case 2: offset =3D 0; break; /* IA */
> + =A0 =A0 =A0 =A0 =A0 =A0case 1: offset =3D 0; break; /* IA */
> + =A0 =A0 =A0 =A0 =A0 =A0case 2: offset =3D -8; break; /* DB */
> =A0 =A0 =A0 =A0 =A0 =A0case 3: offset =3D 4; break; /* IB */
> =A0 =A0 =A0 =A0 =A0 =A0default: abort();
> =A0 =A0 =A0 =A0 =A0 =A0}
> @@ -5774,8 +5775,8 @@
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Base writeback. =A0*/
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0switch (i) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 0: offset =3D -8; break;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 1: offset =3D -4; break;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 2: offset =3D 4; break;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 1: offset =3D 4; break;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 2: offset =3D -4; break;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 3: offset =3D 0; break;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0default: abort();
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> @@ -5786,6 +5787,7 @@
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0dead_tmp(addr);
> =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0gen_rfe(s, tmp, tmp2);
> + =A0 =A0 =A0 =A0 =A0 =A0return;
> =A0 =A0 =A0 =A0} else if ((insn & 0x0e000000) =3D=3D 0x0a000000) {
> =A0 =A0 =A0 =A0 =A0 =A0/* branch link and change to thumb (blx <offset>) =
*/
> =A0 =A0 =A0 =A0 =A0 =A0int32_t offset;
>
>
>
>

Can this patch be applied please? The code as it is now is wrong to
the point of being unusable, it references registers that are not
filled with any values...

Best regards,
Filip Navara