Re: [Qemu-devel] [PATCH] tilegx: Support raise instruction

2015-09-24 Thread Chen Gang

On 9/24/15 02:34, Richard Henderson wrote:
> You forgot to cc qemu-devel.
> 

Oh, we can not find it in qemu mail archive list. But I really cc
qemu-devel.

When I send the next patches, I'll notice about it, if still "can not cc
qemu-devel", I shall try to send patches from my hotmail client.


> This patch needs to be split.
> 

OK.

> On 09/22/2015 03:38 PM, gang.chen.5...@gmail.com wrote:
>>  
>> +static int sigdata_code(uint64_t sigdata)
>> +{
>> +return (sigdata & 0x3ff) >> 6;
>> +}
>> +
>> +static int sigdata_no(uint64_t sigdata)
>> +{
>> +return sigdata & 0x3f;
>> +}
>> +
>> +static void do_raise(CPUTLGState *env)
>> +{
>> +target_siginfo_t info;
>> +
>> +info.si_signo = sigdata_no(env->sigdata);
>> +info.si_errno = 0;
>> +info.si_code = sigdata_code(env->sigdata);
>> +info._sifields._sigfault._addr = env->pc;
>> +queue_signal(env, info.si_signo, &info);
>> +}
> 
> 
> This should look much more like the linux kernel code, where instead of 
> passing
> "sigdata", we load the instruction that faulted and parse out the data.  You
> should only need TILEGX_EXCP_OPCODE_ILL.
>

OK, thank, I will try, next.

>> 
>> +env->spregs[TILEGX_SPR_EX_CONTEXT_1] = 
>> TILEGX_PL_ICS_EX1(TILEGX_USER_PL, 1);
>> +env->regs[TILEGX_R_SP] = (unsigned long) frame;
>> +env->regs[TILEGX_R_LR] = restorer;
>> +env->regs[0] = (unsigned long) sig;
>> +env->regs[1] = (unsigned long) &frame->info;
>> +env->regs[2] = (unsigned long) &frame->uc;
>> +/* regs->flags |= PT_FLAGS_CALLER_SAVES; FIXME: we can skip it? */
>> +
>> +unlock_user_struct(frame, frame_addr, 1);
>> +return;
>> +
>> +give_sigsegv:
>> +if (sig == TARGET_SIGSEGV) {
>> +ka->_sa_handler = TARGET_SIG_DFL;
>> +}
>> +force_sig(TARGET_SIGSEGV /* , current */);
>> +}
>> +
>> +/* kernel: arch/tile/kernel/signal.c */
>> +long do_rt_sigreturn(CPUArchState *env)
>> +{
>> +fprintf(stderr, "do_rt_sigreturn: not implemented\n");
>> +return -TARGET_ENOSYS;
>> +}
>> +
> 
> The introduction of signal handling needs to be it's own patch.
> 

OK, thanks.

> I'm also disapointed that you didn't fill in rt_sigreturn; there's no point in
> not doing them together.
> 

This patch is only for raise insn, so I only implement the related code
for raise.

But, OK, I will let them together in one patch, next.

>>  
>>  #include "exec/cpu-defs.h"
>> +#include "spr_def_64.h"
>>  
>> +#define TILEGX_EX1_PL(ex1) \
>> +  (((ex1) >> SPR_EX_CONTEXT_1_1__PL_SHIFT) & SPR_EX_CONTEXT_1_1__PL_RMASK)
>> +#define TILEGX_EX1_ICS(ex1) \
>> +  (((ex1) >> SPR_EX_CONTEXT_1_1__ICS_SHIFT) & SPR_EX_CONTEXT_1_1__ICS_RMASK)
>> +#define TILEGX_PL_ICS_EX1(pl, ics) \
>> +  (((pl) << SPR_EX_CONTEXT_1_1__PL_SHIFT) | \
>> +   ((ics) << SPR_EX_CONTEXT_1_1__ICS_SHIFT))
>> +
>> +#define TILEGX_USER_PL 0
> 
> What's this for?  It appears to be something unrelated and not used.
>

The previous setup_rt_frame() will use some of them.

But it is OK for me to move them to the related file. 
 
>>  
>>  case OE(ADDLI_OPCODE_X0, 0, X0):
>>  case OE(ADDLI_OPCODE_X1, 0, X1):
>> -tcg_gen_addi_tl(tdest, tsrca, imm);
>> +if ((srca == TILEGX_R_ZERO) && (dest == TILEGX_R_ZERO)) {
>> +t0 = tcg_const_tl(imm & 0x);
>> +tcg_gen_st_tl(t0, cpu_env, offsetof(CPUTLGState, sigdata));
>> +tcg_temp_free(t0);
>> +} else {
>> +tcg_gen_addi_tl(tdest, tsrca, imm);
>> +}
>>  mnemonic = "addli";
>>  break;
>>  case OE(ADDXLI_OPCODE_X0, 0, X0):
>>
> 
> Certainly you should not be complicating addli like this.
> 

OK, I'll try.


Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] tilegx: Support raise instruction

2015-09-23 Thread Richard Henderson
You forgot to cc qemu-devel.

This patch needs to be split.

On 09/22/2015 03:38 PM, gang.chen.5...@gmail.com wrote:
> From: Chen Gang 
> 
> It passes the related test.
> 
> Signed-off-by: Chen Gang 
> ---
>  linux-user/main.c |  25 +
>  linux-user/signal.c   | 129 
> +-
>  target-tilegx/cpu.h   |  14 +
>  target-tilegx/translate.c |  15 +-
>  4 files changed, 180 insertions(+), 3 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 782037d..55a2670 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -3414,6 +3414,27 @@ void cpu_loop(CPUS390XState *env)
>  
>  #ifdef TARGET_TILEGX
>  
> +static int sigdata_code(uint64_t sigdata)
> +{
> +return (sigdata & 0x3ff) >> 6;
> +}
> +
> +static int sigdata_no(uint64_t sigdata)
> +{
> +return sigdata & 0x3f;
> +}
> +
> +static void do_raise(CPUTLGState *env)
> +{
> +target_siginfo_t info;
> +
> +info.si_signo = sigdata_no(env->sigdata);
> +info.si_errno = 0;
> +info.si_code = sigdata_code(env->sigdata);
> +info._sifields._sigfault._addr = env->pc;
> +queue_signal(env, info.si_signo, &info);
> +}


This should look much more like the linux kernel code, where instead of passing
"sigdata", we load the instruction that faulted and parse out the data.  You
should only need TILEGX_EXCP_OPCODE_ILL.

> +
>  static void gen_sigsegv_maperr(CPUTLGState *env, target_ulong addr)
>  {
>  target_siginfo_t info;
> @@ -3622,10 +3643,14 @@ void cpu_loop(CPUTLGState *env)
>  case TILEGX_EXCP_OPCODE_FETCHOR4:
>  do_fetch(env, trapnr, false);
>  break;
> +case TILEGX_EXCP_OPCODE_ILL:
>  case TILEGX_EXCP_REG_IDN_ACCESS:
>  case TILEGX_EXCP_REG_UDN_ACCESS:
>  gen_sigill_reg(env);
>  break;
> +case TILEGX_EXCP_OPCODE_RAISE:
> +do_raise(env);
> +break;
>  case TILEGX_EXCP_SEGV:
>  gen_sigsegv_maperr(env, env->excaddr);
>  break;
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 502efd9..29144e5 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -5543,6 +5543,132 @@ long do_rt_sigreturn(CPUAlphaState *env)
>  force_sig(TARGET_SIGSEGV);
>  }
>  
> +#elif defined(TARGET_TILEGX)
> +
> +struct target_sigcontext {
> +union {
> +/* General-purpose registers.  */
> +abi_ulong gregs[56];
> +struct {
> +abi_ulong __gregs[53];
> +abi_ulong tp;/* Aliases gregs[TREG_TP].  */
> +abi_ulong sp;/* Aliases gregs[TREG_SP].  */
> +abi_ulong lr;/* Aliases gregs[TREG_LR].  */
> +};
> +};
> +abi_ulong pc;/* Program counter.  */
> +abi_ulong ics;   /* In Interrupt Critical Section?  */
> +abi_ulong faultnum;  /* Fault number.  */
> +abi_ulong pad[5];
> +};
> +
> +struct target_ucontext {
> +abi_ulong tuc_flags;
> +abi_ulong tuc_link;
> +target_stack_t tuc_stack;
> +struct target_sigcontext tuc_mcontext;
> +target_sigset_t tuc_sigmask;   /* mask last for extensibility */
> +};
> +
> +struct target_rt_sigframe {
> +unsigned char save_area[16]; /* caller save area */
> +struct target_siginfo info;
> +struct target_ucontext uc;
> +};
> +
> +static void setup_sigcontext(struct target_sigcontext *sc,
> + CPUArchState *env, int signo)
> +{
> +int i;
> +
> +for (i = 0; i < TILEGX_R_COUNT; ++i) {
> +__put_user(env->regs[i], &sc->gregs[i]);
> +}
> +
> +__put_user(env->pc, &sc->pc);
> +__put_user(0, &sc->ics);
> +__put_user(signo, &sc->faultnum);
> +}
> +
> +static abi_ulong get_sigframe(struct target_sigaction *ka, CPUArchState *env,
> +  size_t frame_size)
> +{
> +unsigned long sp = env->regs[TILEGX_R_SP];
> +
> +if (on_sig_stack(sp) && !likely(on_sig_stack(sp - frame_size))) {
> +return -1UL;
> +}
> +
> +if ((ka->sa_flags & SA_ONSTACK) && !sas_ss_flags(sp)) {
> +sp = target_sigaltstack_used.ss_sp + target_sigaltstack_used.ss_size;
> +}
> +
> +sp -= frame_size;
> +sp &= -16UL;
> +return sp;
> +}
> +
> +static void setup_rt_frame(int sig, struct target_sigaction *ka,
> +   target_siginfo_t *info,
> +   target_sigset_t *set, CPUArchState *env)
> +{
> +abi_ulong frame_addr;
> +struct target_rt_sigframe *frame;
> +unsigned long restorer;
> +
> +frame_addr = get_sigframe(ka, env, sizeof(*frame));
> +if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
> +goto give_sigsegv;
> +}
> +
> +/* Always write at least the signal number for the stack backtracer. */
> +if (ka->sa_flags & TARGET_SA_SIGINFO) {
> +/* At sigreturn time, restore the callee-save registers too. */
> +tswap_siginfo(&frame->info,