From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:48581)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cmetcalf@ezchip.com>) id 1ZjUsG-0006Jy-W3
	for qemu-devel@nongnu.org; Tue, 06 Oct 2015 12:13:58 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <cmetcalf@ezchip.com>) id 1ZjUsE-0005Bj-6v
	for qemu-devel@nongnu.org; Tue, 06 Oct 2015 12:13:56 -0400
Received: from mail-db3on0053.outbound.protection.outlook.com
	([157.55.234.53]:56652
	helo=emea01-db3-obe.outbound.protection.outlook.com)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cmetcalf@ezchip.com>) id 1ZjUsD-0005B9-VF
	for qemu-devel@nongnu.org; Tue, 06 Oct 2015 12:13:54 -0400
References: <COL130-W94A2EEB4179B2EC3C42AAB9370@phx.gbl>
From: Chris Metcalf <cmetcalf@ezchip.com>
Message-ID: <5613F32E.4070301@ezchip.com>
Date: Tue, 6 Oct 2015 12:13:34 -0400
MIME-Version: 1.0
In-Reply-To: <COL130-W94A2EEB4179B2EC3C42AAB9370@phx.gbl>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v5] target-tilegx: Support iret instruction
 and related special registers
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/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: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Chen Gang <xili_gchen_5257@hotmail.com>, "rth@twiddle.net" <rth@twiddle.net>, Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel <qemu-devel@nongnu.org>

Comments just on the commit message:

On 10/06/2015 10:55 AM, Chen Gang wrote:
>  From fa0950e403bbb98989117f632215ae0e698457d7 Mon Sep 17 00:00:00 2001
> From: Chen Gang <gang.chen.5i5j@gmail.com>
> Date: Sun, 4 Oct 2015 17:41:14 +0800
> Subject: [PATCH v5] target-tilegx: Support iret instruction and related special registers
>
> Acording to the __longjmp tilegx libc implementation, and reference from
> tilegx ISA document, and suggested by tilegx architecture member, we can
> treat iret instruction as "jrp lr".

You need to update this comment to reflect the actual code in the
commit.  You aren't treating iret as jrp lr anymore.

>   The related code is below:
>
>    ENTRY (__longjmp)
>           FEEDBACK_ENTER(__longjmp)
>
>    #define RESTORE(r) { LD r, r0 ; ADDI_PTR r0, r0, REGSIZE }
>           FOR_EACH_CALLEE_SAVED_REG(RESTORE)
>
>           {
>            LD r2, r0       ; retrieve ICS bit from jmp_buf
>            movei r3, 1
>            CMPEQI r0, r1, 0
>           }
>
>           {
>            mtspr INTERRUPT_CRITICAL_SECTION, r3
>            shli r2, r2, SPR_EX_CONTEXT_0_1__ICS_SHIFT
>           }
>
>           {
>            mtspr EX_CONTEXT_0_0, lr
>            ori r2, r2, RETURN_PL
>           }
>
>           {
>            or r0, r1, r0
>            mtspr EX_CONTEXT_0_1, r2
>           }
>
>           iret
>
>           jrp lr

I don't think this code snippet is helpful to the commit message.

> EX_CONTEXT_0_0 is used for jumping address, and EX_CONTEXT_0_1 is for
> INTERRUPT_CRITICAL_SECTION, which should only be 0 or 1 in user mode, or
> it will cause target SEGV (and the patch doesn't implement system mode).

You correctly modified the change to raise SIGILL, so you should
also update the commit message the same way.

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com