[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architectu
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user |
Date: |
Tue, 2 Jun 2015 18:40:30 +0100 |
On 30 May 2015 at 22:10, Chen Gang <address@hidden> wrote:
> Add main working flow feature, system call processing feature, and elf64
> tilegx binary loading feature, based on Linux kernel tilegx 64-bit
> implementation.
>
> Signed-off-by: Chen Gang <address@hidden>
> ---
> include/elf.h | 2 +
> linux-user/elfload.c | 23 +++++
> linux-user/main.c | 236
> ++++++++++++++++++++++++++++++++++++++++++++++
> linux-user/syscall_defs.h | 14 ++-
> 4 files changed, 270 insertions(+), 5 deletions(-)
>
> diff --git a/include/elf.h b/include/elf.h
> index 4afd474..79859f0 100644
> --- a/include/elf.h
> +++ b/include/elf.h
> @@ -133,6 +133,8 @@ typedef int64_t Elf64_Sxword;
>
> #define EM_AARCH64 183
>
> +#define EM_TILEGX 191 /* TILE-Gx */
> +
> /* This is the info that is needed to parse the dynamic section of the file
> */
> #define DT_NULL 0
> #define DT_NEEDED 1
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 0ba9706..fbf9212 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -1189,6 +1189,29 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>
> #endif /* TARGET_S390X */
>
> +#ifdef TARGET_TILEGX
> +
> +/* 42 bits real used address, a half for user mode */
> +#define ELF_START_MMAP (0x00000020000000000ULL)
> +
> +#define elf_check_arch(x) ((x) == EM_TILEGX)
> +
> +#define ELF_CLASS ELFCLASS64
> +#define ELF_DATA ELFDATA2LSB
> +#define ELF_ARCH EM_TILEGX
> +
> +static inline void init_thread(struct target_pt_regs *regs,
> + struct image_info *infop)
> +{
> + regs->pc = infop->entry;
> + regs->sp = infop->start_stack;
> +
> +}
> +
> +#define ELF_EXEC_PAGESIZE 65536 /* TILE-Gx page size is 64KB */
> +
> +#endif /* TARGET_TILEGX */
> +
> #ifndef ELF_PLATFORM
> #define ELF_PLATFORM (NULL)
> #endif
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 3f32db0..8e7fe86 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -3416,6 +3416,231 @@ void cpu_loop(CPUS390XState *env)
>
> #endif /* TARGET_S390X */
>
> +#ifdef TARGET_TILEGX
> +
> +static uint64_t get_regval(CPUTLGState *env, uint8_t reg)
> +{
> + if (likely(reg < TILEGX_R_COUNT)) {
> + return env->regs[reg];
> + } else if (reg != TILEGX_R_ZERO) {
> + fprintf(stderr, "invalid register r%d for reading.\n", reg);
> + g_assert_not_reached();
You don't appear to be guaranteeing that the register value
is < TILEGX_R_COUNT anywhere: get_SrcA_X1() and friends
mask with 0x3f, but that only means you're guaranteed the
value is between 0 and 63, wherease TILEGX_R_COUNT is 56.
What does real hardware do if the encoded register value
is 56..63 ?
Also, if (something) {
g_assert_not_reached();
}
is an awkward way to write
g_assert(!something);
> + }
> + return 0;
> +}
> +
> +static void set_regval(CPUTLGState *env, uint8_t reg, uint64_t val)
> +{
> + if (likely(reg < TILEGX_R_COUNT)) {
> + env->regs[reg] = val;
> + } else if (reg != TILEGX_R_ZERO) {
> + fprintf(stderr, "invalid register r%d for writing.\n", reg);
> + g_assert_not_reached();
> + }
> +}
> +
> +/*
> + * Compare the 8-byte contents of the CmpValue SPR with the 8-byte value in
> + * memory at the address held in the first source register. If the values are
> + * not equal, then no memory operation is performed. If the values are equal,
> + * the 8-byte quantity from the second source register is written into memory
> + * at the address held in the first source register. In either case, the
> result
> + * of the instruc- tion is the value read from memory. The compare and write
> to
stray "- ".
> + * memory are atomic and thus can be used for synchronization purposes. This
> + * instruction only operates for addresses aligned to a 8-byte boundary.
> + * Unaligned memory access causes an Unaligned Data Reference interrupt.
> + *
> + * Functional Description (64-bit)
> + * uint64_t memVal = memoryReadDoubleWord (rf[SrcA]);
> + * rf[Dest] = memVal;
> + * if (memVal == SPR[CmpValueSPR])
> + * memoryWriteDoubleWord (rf[SrcA], rf[SrcB]);
> + *
> + * Functional Description (32-bit)
> + * uint64_t memVal = signExtend32 (memoryReadWord (rf[SrcA]));
> + * rf[Dest] = memVal;
> + * if (memVal == signExtend32 (SPR[CmpValueSPR]))
> + * memoryWriteWord (rf[SrcA], rf[SrcB]);
> + *
> + *
> + * For exch(4), will no cmp spr.
Not sure what this sentence means?
> + */
> +static void do_exch(CPUTLGState *env, int8_t quad, int8_t cmp)
quad and cmp are just booleans, right? Why int8_t not bool?
> +{
> + uint8_t rdst, rsrc, rsrcb;
> + target_ulong addr, tmp;
> + target_long val, sprval;
> + target_siginfo_t info;
> +
> + start_exclusive();
> +
> + rdst = (env->excparam >> 16) & 0xff;
> + rsrc = (env->excparam >> 8) & 0xff;
> + rsrcb = env->excparam & 0xff;
Consider extract32().
> +
> + addr = get_regval(env, rsrc);
> + if (quad ? get_user_s64(val, addr) : get_user_s32(val, addr)) {
> + goto do_sigsegv;
> + }
> + tmp = (target_ulong)val; /* rdst may be the same to rsrcb, so buffer it
> */
Why do this, when we could just use a different variable
rather than trashing val below?
> +
> + if (cmp) {
> + if (quad) {
> + sprval = (target_long)env->spregs[TILEGX_SPR_CMPEXCH];
Pointless cast.
> + } else {
> + sprval = (int32_t)(env->spregs[TILEGX_SPR_CMPEXCH] & 0xffffffff);
Clearer as
sprval = sextract64(env->spregs[TILEGX_SPR_CMPEXCH], 0, 32);
> + }
> + }
> +
> + if (!cmp || val == sprval) {
> + val = get_regval(env, rsrcb);
If you say "target_long srcbval = ..." you don't trash val.
> + if (quad ? put_user_u64(val, addr) : put_user_u32(val, addr)) {
> + goto do_sigsegv;
> + }
> + }
> +
> + set_regval(env, rdst, tmp);
> +
> + end_exclusive();
> + return;
> +
> +do_sigsegv:
> + end_exclusive();
> +
> + info.si_signo = TARGET_SIGSEGV;
> + info.si_errno = 0;
> + info.si_code = TARGET_SEGV_MAPERR;
> + info._sifields._sigfault._addr = addr;
> + queue_signal(env, TARGET_SIGSEGV, &info);
> +}
> +
> +static void do_fetch(CPUTLGState *env, int trapnr, int8_t quad)
> +{
> + uint8_t rdst, rsrc, rsrcb;
> + int8_t write = 1;
> + target_ulong addr;
> + target_long val, tmp;
> + target_siginfo_t info;
> +
> + start_exclusive();
> +
> + rdst = (env->excparam >> 16) & 0xff;
> + rsrc = (env->excparam >> 8) & 0xff;
> + rsrcb = env->excparam & 0xff;
> +
> + addr = get_regval(env, rsrc);
> + if (quad ? get_user_s64(val, addr) : get_user_s32(val, addr)) {
> + goto do_sigsegv;
> + }
> + tmp = val; /* rdst may be the same to rsrcb, so buffer it */
Again, unnecessary copy when you could just use a different
variable for the value in rsrcb.
> + val = get_regval(env, rsrcb);
> + switch (trapnr) {
> + case TILEGX_EXCP_OPCODE_FETCHADD:
> + case TILEGX_EXCP_OPCODE_FETCHADD4:
> + case TILEGX_EXCP_OPCODE_FETCHADDGEZ:
> + case TILEGX_EXCP_OPCODE_FETCHADDGEZ4:
> + val += tmp;
> + if (trapnr == TILEGX_EXCP_OPCODE_FETCHADDGEZ) {
> + if (val < 0) {
> + write = 0;
> + }
> + } else if (trapnr == TILEGX_EXCP_OPCODE_FETCHADDGEZ4) {
> + if ((int32_t)val < 0) {
> + write = 0;
> + }
> + }
Just give these their own case blocks rather than
writing an if() that's conditioned on the same variable
we're switching on. The only duplication you're saving
is a single line addition.
> + break;
> + case TILEGX_EXCP_OPCODE_FETCHAND:
> + case TILEGX_EXCP_OPCODE_FETCHAND4:
> + val &= tmp;
> + break;
> + case TILEGX_EXCP_OPCODE_FETCHOR:
> + case TILEGX_EXCP_OPCODE_FETCHOR4:
> + val |= tmp;
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +
> + if (write) {
> + if (quad ? put_user_u64(val, addr) : put_user_u32(val, addr)) {
> + goto do_sigsegv;
> + }
> + }
> +
> + set_regval(env, rdst, tmp);
> +
> + end_exclusive();
> + return;
> +
> +do_sigsegv:
> + end_exclusive();
> +
> + info.si_signo = TARGET_SIGSEGV;
> + info.si_errno = 0;
> + info.si_code = TARGET_SEGV_MAPERR;
> + info._sifields._sigfault._addr = addr;
> + queue_signal(env, TARGET_SIGSEGV, &info);
> +}
> +
> +void cpu_loop(CPUTLGState *env)
> +{
> + CPUState *cs = CPU(tilegx_env_get_cpu(env));
> + int trapnr;
> +
> + while (1) {
> + cpu_exec_start(cs);
> + trapnr = cpu_tilegx_exec(env);
> + cpu_exec_end(cs);
> + switch (trapnr) {
> + case TILEGX_EXCP_SYSCALL:
> + env->regs[TILEGX_R_RE] = do_syscall(env, env->regs[TILEGX_R_NR],
> + env->regs[0], env->regs[1],
> + env->regs[2], env->regs[3],
> + env->regs[4], env->regs[5],
> + env->regs[6], env->regs[7]);
> + env->regs[TILEGX_R_ERR] =
> TILEGX_IS_ERRNO(env->regs[TILEGX_R_RE]) ?
> + env->regs[TILEGX_R_RE] : 0;
> + break;
> + case TILEGX_EXCP_OPCODE_EXCH:
> + do_exch(env, 1, 0);
these should be true, false assuming you change the arg type to bool.
> + break;
> + case TILEGX_EXCP_OPCODE_EXCH4:
> + do_exch(env, 0, 0);
> + break;
> + case TILEGX_EXCP_OPCODE_CMPEXCH:
> + do_exch(env, 1, 1);
> + break;
> + case TILEGX_EXCP_OPCODE_CMPEXCH4:
> + do_exch(env, 0, 1);
> + break;
> + case TILEGX_EXCP_OPCODE_FETCHADD:
> + do_fetch(env, trapnr, 1);
> + break;
> + case TILEGX_EXCP_OPCODE_FETCHADDGEZ:
> + case TILEGX_EXCP_OPCODE_FETCHAND:
> + case TILEGX_EXCP_OPCODE_FETCHOR:
> + do_fetch(env, trapnr, 1);
> + exit(1);
???
> + break;
> + case TILEGX_EXCP_OPCODE_FETCHADD4:
> + case TILEGX_EXCP_OPCODE_FETCHADDGEZ4:
> + case TILEGX_EXCP_OPCODE_FETCHAND4:
> + case TILEGX_EXCP_OPCODE_FETCHOR4:
> + do_fetch(env, trapnr, 0);
> + exit(1);
Again.
> + break;
> + default:
> + fprintf(stderr, "trapnr is %d[0x%x].\n", trapnr, trapnr);
> + g_assert_not_reached();
> + }
> + process_pending_signals(env);
> + }
> +}
> +
> +#endif
> +
> THREAD CPUState *thread_cpu;
>
> void task_settid(TaskState *ts)
> @@ -4389,6 +4614,17 @@ int main(int argc, char **argv, char **envp)
> env->psw.mask = regs->psw.mask;
> env->psw.addr = regs->psw.addr;
> }
> +#elif defined(TARGET_TILEGX)
> + {
> + int i;
> + for (i = 0; i < TILEGX_R_COUNT; i++) {
> + env->regs[i] = regs->regs[i];
> + }
> + for (i = 0; i < TILEGX_SPR_COUNT; i++) {
> + env->spregs[i] = 0;
> + }
> + env->pc = regs->pc;
> + }
> #else
> #error unsupported target CPU
> #endif
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index edd5f3c..e6af073 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -64,8 +64,9 @@
> #endif
>
> #if defined(TARGET_I386) || defined(TARGET_ARM) || defined(TARGET_SH4) \
> - || defined(TARGET_M68K) || defined(TARGET_CRIS) ||
> defined(TARGET_UNICORE32) \
> - || defined(TARGET_S390X) || defined(TARGET_OPENRISC)
> + || defined(TARGET_M68K) || defined(TARGET_CRIS) \
> + || defined(TARGET_UNICORE32) || defined(TARGET_S390X) \
> + || defined(TARGET_OPENRISC) || defined(TARGET_TILEGX)
>
> #define TARGET_IOC_SIZEBITS 14
> #define TARGET_IOC_DIRBITS 2
> @@ -365,7 +366,8 @@ int do_sigaction(int sig, const struct target_sigaction
> *act,
> || defined(TARGET_PPC) || defined(TARGET_MIPS) || defined(TARGET_SH4) \
> || defined(TARGET_M68K) || defined(TARGET_ALPHA) || defined(TARGET_CRIS)
> \
> || defined(TARGET_MICROBLAZE) || defined(TARGET_UNICORE32) \
> - || defined(TARGET_S390X) || defined(TARGET_OPENRISC)
> + || defined(TARGET_S390X) || defined(TARGET_OPENRISC) \
> + || defined(TARGET_TILEGX)
>
> #if defined(TARGET_SPARC)
> #define TARGET_SA_NOCLDSTOP 8u
> @@ -1871,7 +1873,7 @@ struct target_stat {
> abi_ulong target_st_ctime_nsec;
> unsigned int __unused[2];
> };
> -#elif defined(TARGET_OPENRISC)
> +#elif defined(TARGET_OPENRISC) || defined(TARGET_TILEGX)
>
> /* These are the asm-generic versions of the stat and stat64 structures */
>
> @@ -2264,7 +2266,9 @@ struct target_flock {
> struct target_flock64 {
> short l_type;
> short l_whence;
> -#if defined(TARGET_PPC) || defined(TARGET_X86_64) || defined(TARGET_MIPS) ||
> defined(TARGET_SPARC) || defined(TARGET_HPPA) || defined (TARGET_MICROBLAZE)
> +#if defined(TARGET_PPC) || defined(TARGET_X86_64) || defined(TARGET_MIPS) \
> + || defined(TARGET_SPARC) || defined(TARGET_HPPA) \
> + || defined(TARGET_MICROBLAZE) || defined(TARGET_TILEGX)
> int __pad;
> #endif
> unsigned long long l_start;
> --
> 1.9.3
thanks
-- PMM
- Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user,
Peter Maydell <=
- Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user, Chen Gang, 2015/06/03
- Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user, Peter Maydell, 2015/06/03
- Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user, Chen Gang, 2015/06/03
- Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user, Chris Metcalf, 2015/06/03
- Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user, Peter Maydell, 2015/06/03
- Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user, Chris Metcalf, 2015/06/03
- Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user, Chen Gang, 2015/06/04
- Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user, Peter Maydell, 2015/06/04
- Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user, Chen Gang, 2015/06/04
- Re: [Qemu-devel] [PATCH 02/10 v11] linux-user: Support tilegx architecture in linux-user, Richard Henderson, 2015/06/03