[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5] target-tilegx: Support iret instruction and
From: |
Chris Metcalf |
Subject: |
Re: [Qemu-devel] [PATCH v5] target-tilegx: Support iret instruction and related special registers |
Date: |
Tue, 6 Oct 2015 12:13:34 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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 <address@hidden>
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