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: xili_gchen_5...@hotmail.com; r...@twiddle.net; peter.mayd...@linaro.org > CC: qemu-devel@nongnu.org > From: cmetc...@ezchip.com > 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 <gang.chen.5...@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 >