qemu-devel
[Top][All Lists]
Advanced

[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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]