Re: [Qemu-devel] [PATCH 01/12 v9] linux-user: tilegx: Firstly add architecture related features

[Peter Maydell]

On 27 March 2015 at 10:48, Chen Gang <address@hidden> wrote:
> They are based on Linux kernel tilegx architecture for 64 bit binary,
> also based on tilegx ABI reference document.
>
> Signed-off-by: Chen Gang <address@hidden>
> ---
>  linux-user/tilegx/syscall.h    |  80 ++++++++++++
>  linux-user/tilegx/syscall_nr.h | 278 ++++++++++++++++++++++++++++++++++++++++
>  linux-user/tilegx/termbits.h   | 285 
> +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 643 insertions(+)
>  create mode 100644 linux-user/tilegx/syscall.h
>  create mode 100644 linux-user/tilegx/syscall_nr.h
>  create mode 100644 linux-user/tilegx/termbits.h
>
> diff --git a/linux-user/tilegx/syscall.h b/linux-user/tilegx/syscall.h
> new file mode 100644
> index 0000000..561e158
> --- /dev/null
> +++ b/linux-user/tilegx/syscall.h
> @@ -0,0 +1,80 @@
> +#ifndef TILEGX_SYSCALLS_H
> +#define TILEGX_SYSCALLS_H
> +
> +#define UNAME_MACHINE "tilegx"
> +#define UNAME_MINIMUM_RELEASE "3.19"
> +
> +/* We use tilegx to keep things similar to the kernel sources.  */

This is true but a slightly odd place to say so.

> +typedef uint64_t tilegx_reg_t;
> +
> +struct target_pt_regs {
> +
> +    /* Can be as parameters */
> +    tilegx_reg_t r0;  /* Also for return value, both function and system 
> call */
> +    tilegx_reg_t r1;
> +    tilegx_reg_t r2;
> +    tilegx_reg_t r3;
> +    tilegx_reg_t r4;
> +    tilegx_reg_t r5;
> +    tilegx_reg_t r6;
> +    tilegx_reg_t r7;
> +    tilegx_reg_t r8;
> +    tilegx_reg_t r9;
> +
> +    /* Normal using, caller saved */
> +    tilegx_reg_t r10;  /* Also for system call */
> +    tilegx_reg_t r11;
> +    tilegx_reg_t r12;
> +    tilegx_reg_t r13;
> +    tilegx_reg_t r14;
> +    tilegx_reg_t r15;
> +    tilegx_reg_t r16;
> +    tilegx_reg_t r17;
> +    tilegx_reg_t r18;
> +    tilegx_reg_t r19;
> +    tilegx_reg_t r20;
> +    tilegx_reg_t r21;
> +    tilegx_reg_t r22;
> +    tilegx_reg_t r23;
> +    tilegx_reg_t r24;
> +    tilegx_reg_t r25;
> +    tilegx_reg_t r26;
> +    tilegx_reg_t r27;
> +    tilegx_reg_t r28;
> +    tilegx_reg_t r29;
> +
> +    /* Normal using, callee saved */
> +    tilegx_reg_t r30;
> +    tilegx_reg_t r31;
> +    tilegx_reg_t r32;
> +    tilegx_reg_t r33;
> +    tilegx_reg_t r34;
> +    tilegx_reg_t r35;
> +    tilegx_reg_t r36;
> +    tilegx_reg_t r37;
> +    tilegx_reg_t r38;
> +    tilegx_reg_t r39;
> +    tilegx_reg_t r40;
> +    tilegx_reg_t r41;
> +    tilegx_reg_t r42;
> +    tilegx_reg_t r43;
> +    tilegx_reg_t r44;
> +    tilegx_reg_t r45;
> +    tilegx_reg_t r46;
> +    tilegx_reg_t r47;
> +    tilegx_reg_t r48;
> +    tilegx_reg_t r49;
> +    tilegx_reg_t r50;
> +    tilegx_reg_t r51;
> +
> +    /* Control using */
> +    tilegx_reg_t r52;    /* optional frame pointer */

Why aren't we using an array, the way the kernel does?

> +    tilegx_reg_t tp;     /* thread-local data */
> +    tilegx_reg_t sp;     /* stack pointer */
> +    tilegx_reg_t lr;     /* lr pointer */

This is missing a bunch of stuff from the kernel uapi
pt_regs type, which is bad because this struct is part
of the user-facing ABI (it gets used in signal handling).

> diff --git a/linux-user/tilegx/termbits.h b/linux-user/tilegx/termbits.h
> new file mode 100644
> index 0000000..c11ce3e
> --- /dev/null
> +++ b/linux-user/tilegx/termbits.h

> +#define TARGET_TIOCNOTTY                0x5422
> +#define TARGET_TIOCSETD                 0x5423
> +#define TARGET_TIOCGETD                 0x5424
> +#define TARGET_TCSBRKP                  0x5425
> +#define TARGET_TIOCSBRK                 0x5427
> +#define TARGET_TIOCCBRK                 0x5428
> +#define TARGET_TIOCGSID                 0x5429
> +#define TARGET_TCGETS2                  _IOR('T', 0x2A, struct termios2)

You probably mean TARGET_IOR/TARGET_IOW here and below.


> +#define TARGET_TCSETS2                  _IOW('T', 0x2B, struct termios2)
> +#define TARGET_TCSETSW2                 _IOW('T', 0x2C, struct termios2)
> +#define TARGET_TCSETSF2                 _IOW('T', 0x2D, struct termios2)
> +#define TARGET_TIOCGRS485               0x542E
> +#define TARGET_TIOCSRS485               0x542F
> +#define TARGET_TIOCGPTN                 _IOR('T', 0x30, unsigned int)
> +#define TARGET_TIOCSPTLCK               _IOW('T', 0x31, int)
> +#define TARGET_TIOCGDEV                 _IOR('T', 0x32, unsigned int)
> +#define TARGET_TCGETX                   0x5432
> +#define TARGET_TCSETX                   0x5433
> +#define TARGET_TCSETXF                  0x5434
> +#define TARGET_TCSETXW                  0x5435
> +#define TARGET_TIOCSIG                  _IOW('T', 0x36, int)
> +#define TARGET_TIOCVHANGUP              0x5437
> +#define TARGET_TIOCGPKT                 _IOR('T', 0x38, int)
> +#define TARGET_TIOCGPTLCK               _IOR('T', 0x39, int)
> +#define TARGET_TIOCGEXCL                _IOR('T', 0x40, int)

thanks
-- PMM