From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=39328 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PpI3B-0000qc-KP
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 05:22:30 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1PpI3A-0005up-Ag
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 05:22:29 -0500
Received: from mail-iy0-f173.google.com ([209.85.210.173]:37237)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1PpI3A-0005uk-57
	for qemu-devel@nongnu.org; Tue, 15 Feb 2011 05:22:28 -0500
Received: by iye19 with SMTP id 19so6295647iye.4
	for <qemu-devel@nongnu.org>; Tue, 15 Feb 2011 02:22:27 -0800 (PST)
MIME-Version: 1.0
In-Reply-To: <1297760349-35256-1-git-send-email-gingold@adacore.com>
References: <1297760349-35256-1-git-send-email-gingold@adacore.com>
Date: Tue, 15 Feb 2011 10:22:27 +0000
Message-ID: <AANLkTimU9=kNpw3aLHaFwGy4nXu7a4kFLQRVsP8St_H8@mail.gmail.com>
Subject: Re: [Qemu-devel] [PATCH] gdbstub/ppc: handle read and write of fpscr
From: Peter Maydell <peter.maydell@linaro.org>
Content-Type: text/plain; charset=UTF-8
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.nongnu.org/archive/html/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: Tristan Gingold <gingold@adacore.com>
Cc: qemu-devel@nongnu.org

On 15 February 2011 08:59, Tristan Gingold <gingold@adacore.com> wrote:
> @@ -770,7 +770,8 @@ static int cpu_gdb_write_register(CPUState *env, uint=
8_t *mem_buf, int n)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* fpscr */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (gdb_has_xml)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return 0;
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return 4;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0env->fpscr =3D ldtul_p(mem_buf=
);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return sizeof(target_ulong);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> =C2=A0 =C2=A0 }
> =C2=A0 =C2=A0 return 0;

Not a PPC expert, but this doesn't look right; for instance if you change
the rounding mode by fiddling with the FPSCR in the debugger this
won't update the softfloat rounding mode settings. (that is, it lets the
visible state in env->fpscr get out of sync with the hidden state of the
model). Also we probably shouldn't be letting the debugger change
reserved fpscr bits.

(Side note: linux-user/signal.c:restore_user_regs() appears to have
a similar fpscr-related bug.)

-- PMM