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

2015-10-02 Thread Chris Metcalf 

On 10/1/2015 10:26 PM, Richard Henderson wrote:

On 10/02/2015 11:31 AM, Chris Metcalf wrote:


It disables interrupts from being delivered.  This means asynchronous
interrupts get deferred until ICS is set back to zero, and synchronous
interrupts (page fault, etc) cause a double-fault instead. ICS is automatically
set on entry to interrupt handlers, so the handler has time to acquire any
information about the interrupt from SPRs, and it is expected that ICS is
cleared as soon as possible.  ICS can also be used before returning from
interrupts if you need to do something like adjust the interrupt mask prior to
returning.


Which is all very well and good for supervisor mode... but what's it good for 
in user mode?


In user-space it is primarily useful for userspace drivers that are receiving 
device interrupts.  You can also configure to receive performance counter 
interrupts in userspace.  The bottom line is that is indeed a rarely-used 
feature.


I was about to quote you from 2012 (https://lkml.org/lkml/2012/3/30/994):


In general we want to avoid ever touching memory while within an
interrupt critical section, since the page fault path goes through
a different path from the hypervisor when in an interrupt critical
section, and we carefully decided with tilegx that we didn't need
to support this path in the kernel.


Which implies that tilegx userland does nothing at all with ICS, and is in fact 
unsupported?


This refers to the kernel's own use of the interrupt critical section, i.e. 
having that SPR set while running at PL2.  Each protection level (PL) has its 
own ICS.  So the text you quoted is orthogonal to userspace use of ICS.

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

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

2015-10-01 Thread gang . chen . 5i5j 
From: Chen Gang 

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". 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

Until now, EX_CONTEXT_0_0 and EX_CONTEXT_0_1 are only used in mtspr, so
just skip them, at present. "jrp lr" in __longjmp is for historical
reasons, and might get removed in the future.

After this patch, busybox sh can run correctly.

Signed-off-by: Chen Gang 
---
 target-tilegx/translate.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
index 421766b..b7bb4f3 100644
--- a/target-tilegx/translate.c
+++ b/target-tilegx/translate.c
@@ -563,8 +563,14 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
 break;
 case OE_RR_X0(FSINGLE_PACK1):
 case OE_RR_Y0(FSINGLE_PACK1):
-case OE_RR_X1(IRET):
 return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
+case OE_RR_X1(IRET):
+if (srca) {
+return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
+}
+srca = TILEGX_R_LR;
+mnemonic = "iret";
+goto do_jr;
 case OE_RR_X1(LD1S):
 memop = MO_SB;
 mnemonic = "ld1s"; /* prefetch_l1_fault */
@@ -1823,6 +1829,8 @@ static const TileSPR *find_spr(unsigned spr)
   offsetof(CPUTLGState, spregs[TILEGX_SPR_CRITICAL_SEC]), 0, 0)
 D(SIM_CONTROL,
   offsetof(CPUTLGState, spregs[TILEGX_SPR_SIM_CONTROL]), 0, 0)
+D(EX_CONTEXT_0_0, -1, 0, 0) /* Skip it */
+D(EX_CONTEXT_0_1, -1, 0, 0) /* Skip it */
 }
 
 #undef D
@@ -1836,9 +1844,11 @@ static TileExcp gen_mtspr_x1(DisasContext *dc, unsigned 
spr, unsigned srca)
 const TileSPR *def = find_spr(spr);
 TCGv tsrca;
 
-if (def == NULL) {
+if (!def) {
 qemu_log_mask(CPU_LOG_TB_IN_ASM, "mtspr spr[%u], %s", spr, 
reg_names[srca]);
 return TILEGX_EXCP_OPCODE_UNKNOWN;
+} else if (def->offset == -1) {
+goto tail;
 }
 
 tsrca = load_gr(dc, srca);
@@ -1847,6 +1857,8 @@ static TileExcp gen_mtspr_x1(DisasContext *dc, unsigned 
spr, unsigned srca)
 } else {
 tcg_gen_st_tl(tsrca, cpu_env, def->offset);
 }
+
+tail:
 qemu_log_mask(CPU_LOG_TB_IN_ASM, "mtspr %s, %s", def->name, 
reg_names[srca]);
 return TILEGX_EXCP_NONE;
 }
@@ -1856,7 +1868,7 @@ static TileExcp gen_mfspr_x1(DisasContext *dc, unsigned 
dest, unsigned spr)
 const TileSPR *def = find_spr(spr);
 TCGv tdest;
 
-if (def == NULL) {
+if (!def || def->offset == -1) {
 qemu_log_mask(CPU_LOG_TB_IN_ASM, "mtspr %s, spr[%u]", reg_names[dest], 
spr);
 return TILEGX_EXCP_OPCODE_UNKNOWN;
 }
-- 
1.9.3

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

2015-10-01 Thread Richard Henderson 

On 10/01/2015 10:37 PM, gang.chen.5...@gmail.com wrote:

  {
   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

Until now, EX_CONTEXT_0_0 and EX_CONTEXT_0_1 are only used in mtspr, so
just skip them, at present. "jrp lr" in __longjmp is for historical
reasons, and might get removed in the future.


So, really, iret is supposed to branch to EX_CONTEXT_0_0, and (presumably) 
validate the privilege level in EX_CONTEXT_0_1 continues to be user-mode.



+case OE_RR_X1(IRET):
+if (srca) {
+return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
+}
+srca = TILEGX_R_LR;
+mnemonic = "iret";
+goto do_jr;


which means this is wrong, but just happens to work for __longjmp.

It appears that the entire point of this iret path is to atomically branch and 
set INTERRUPT_CRITICAL_SECTION at the same time.  So, this isn't complete.


What INTERRUPT_CRITICAL_SECTION is supposed to *do* at user mode, I don't know.


r~

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

2015-10-01 Thread Chen Gang 
On 10/2/15 08:36, Richard Henderson wrote:
> On 10/01/2015 10:37 PM, gang.chen.5...@gmail.com wrote:
>> {
>> 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
>>
>> Until now, EX_CONTEXT_0_0 and EX_CONTEXT_0_1 are only used in mtspr, so
>> just skip them, at present. "jrp lr" in __longjmp is for historical
>> reasons, and might get removed in the future.
>
> So, really, iret is supposed to branch to EX_CONTEXT_0_0, and (presumably) 
> validate the privilege level in EX_CONTEXT_0_1 continues to be user-mode.
>

Oh, really.

>> + case OE_RR_X1(IRET):
>> + if (srca) {
>> + return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
>> + }
>> + srca = TILEGX_R_LR;
>> + mnemonic = "iret";
>> + goto do_jr;
>
> which means this is wrong, but just happens to work for __longjmp.
>
> It appears that the entire point of this iret path is to atomically branch 
> and set INTERRUPT_CRITICAL_SECTION at the same time. So, this isn't complete.
>

OK, thanks.


> What INTERRUPT_CRITICAL_SECTION is supposed to *do* at user mode, I don't 
> know.
>

Welcome any other members' ideas, suggestions and completions.


Thanks.
--
Chen Gang (陈刚)

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

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

2015-10-01 Thread Chris Metcalf 

On 10/1/2015 8:36 PM, Richard Henderson wrote:

On 10/01/2015 10:37 PM, gang.chen.5...@gmail.com wrote:

  {
   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

Until now, EX_CONTEXT_0_0 and EX_CONTEXT_0_1 are only used in mtspr, so
just skip them, at present. "jrp lr" in __longjmp is for historical
reasons, and might get removed in the future.


So, really, iret is supposed to branch to EX_CONTEXT_0_0, and (presumably) 
validate the privilege level in EX_CONTEXT_0_1 continues to be user-mode.


Yes, I gave the same feedback earlier today.  EX_CONTEXT_0_1 should be either 0 
or 1 to set INTERRUPT_CRITICAL_SECTION appropriately, and raise GPV for any 
other value.  (Obviously it's more complex if you're really emulating system 
software, but for now that's out of scope, I think.)




+case OE_RR_X1(IRET):
+if (srca) {
+return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
+}
+srca = TILEGX_R_LR;
+mnemonic = "iret";
+goto do_jr;


which means this is wrong, but just happens to work for __longjmp.

It appears that the entire point of this iret path is to atomically branch and 
set INTERRUPT_CRITICAL_SECTION at the same time.  So, this isn't complete.

What INTERRUPT_CRITICAL_SECTION is supposed to *do* at user mode, I don't know.


It disables interrupts from being delivered.  This means asynchronous 
interrupts get deferred until ICS is set back to zero, and synchronous 
interrupts (page fault, etc) cause a double-fault instead.  ICS is 
automatically set on entry to interrupt handlers, so the handler has time to 
acquire any information about the interrupt from SPRs, and it is expected that 
ICS is cleared as soon as possible.  ICS can also be used before returning from 
interrupts if you need to do something like adjust the interrupt mask prior to 
returning.

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

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

2015-10-01 Thread Chen Gang 

OK, thanks. I shall try to send patch v4 for it within 2 days.

On 10/2/15 09:31, Chris Metcalf wrote:
> On 10/1/2015 8:36 PM, Richard Henderson wrote:
>> On 10/01/2015 10:37 PM, gang.chen.5...@gmail.com wrote:
>>>   {
>>>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
>>>
>>> Until now, EX_CONTEXT_0_0 and EX_CONTEXT_0_1 are only used in mtspr, so
>>> just skip them, at present. "jrp lr" in __longjmp is for historical
>>> reasons, and might get removed in the future.
>>
>> So, really, iret is supposed to branch to EX_CONTEXT_0_0, and (presumably) 
>> validate the privilege level in EX_CONTEXT_0_1 continues to be user-mode.
> 
> Yes, I gave the same feedback earlier today.  EX_CONTEXT_0_1 should be either 
> 0 or 1 to set INTERRUPT_CRITICAL_SECTION appropriately, and raise GPV for any 
> other value.  (Obviously it's more complex if you're really emulating system 
> software, but for now that's out of scope, I think.)
> 
>>
>>> +case OE_RR_X1(IRET):
>>> +if (srca) {
>>> +return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
>>> +}
>>> +srca = TILEGX_R_LR;
>>> +mnemonic = "iret";
>>> +goto do_jr;
>>
>> which means this is wrong, but just happens to work for __longjmp.
>>
>> It appears that the entire point of this iret path is to atomically branch 
>> and set INTERRUPT_CRITICAL_SECTION at the same time.  So, this isn't 
>> complete.
>>
>> What INTERRUPT_CRITICAL_SECTION is supposed to *do* at user mode, I don't 
>> know.
> 
> It disables interrupts from being delivered.  This means asynchronous 
> interrupts get deferred until ICS is set back to zero, and synchronous 
> interrupts (page fault, etc) cause a double-fault instead.  ICS is 
> automatically set on entry to interrupt handlers, so the handler has time to 
> acquire any information about the interrupt from SPRs, and it is expected 
> that ICS is cleared as soon as possible.  ICS can also be used before 
> returning from interrupts if you need to do something like adjust the 
> interrupt mask prior to returning.
> 

-- 
Chen Gang (陈刚)

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

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

2015-10-01 Thread Richard Henderson 

On 10/02/2015 11:31 AM, Chris Metcalf wrote:


It disables interrupts from being delivered.  This means asynchronous
interrupts get deferred until ICS is set back to zero, and synchronous
interrupts (page fault, etc) cause a double-fault instead. ICS is automatically
set on entry to interrupt handlers, so the handler has time to acquire any
information about the interrupt from SPRs, and it is expected that ICS is
cleared as soon as possible.  ICS can also be used before returning from
interrupts if you need to do something like adjust the interrupt mask prior to
returning.


Which is all very well and good for supervisor mode... but what's it good for 
in user mode?


I was about to quote you from 2012 (https://lkml.org/lkml/2012/3/30/994):


In general we want to avoid ever touching memory while within an
interrupt critical section, since the page fault path goes through
a different path from the hypervisor when in an interrupt critical
section, and we carefully decided with tilegx that we didn't need
to support this path in the kernel.


Which implies that tilegx userland does nothing at all with ICS, and is in fact 
unsupported?



r~