Re: [Qemu-devel] [PATCH v5] target-tilegx: Support iret instruction and related special registers

[Chen Gang]

Oh, sorry. I will send patch v6 for it.

Thanks.
--
Chen Gang

Open, share, and attitude like air, water, and life which God blessed


----------------------------------------
> Subject: Re: [PATCH v5] target-tilegx: Support iret instruction and related 
> special registers
> To: address@hidden; address@hidden; address@hidden
> CC: address@hidden
> From: address@hidden
> Date: Tue, 6 Oct 2015 12:13:34 -0400
>
> 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
>