[PATCH] Add C-SKY architecture support

Guo Ren guoren at kernel.org
Sat Mar 23 00:27:24 UTC 2019


Hi Dmitry,

Thx for the review.

On Fri, Mar 22, 2019 at 03:47:23PM +0300, Dmitry V. Levin wrote:
> On Fri, Mar 22, 2019 at 07:43:46PM +0800, guoren at kernel.org wrote:
> > From: Guo Ren <ren_guo at c-sky.com>
> > 
> > This is port of C-SKY architecture for strace. There is a little
> > difference between abiv1 and abiv2, we use __CSKYABIV2__ from GCC-csky
> > to distinguish.
> 
> Thanks, see my comments below.
> 
> > TODO:
> >  - personality, mpers (multi personality support)
> 
> This would only worth doing if the kernel gets the support for it.
Currently csky kernel doesn't support audit in syscall trace implementation.
I'll give the strace and linux patches later.

> 
> > Signed-off-by: Guo Ren <ren_guo at c-sky.com>
> > Cc: Dmitry V. Levin <ldv at altlinux.org>
> > ---
> >  Makefile.am                   | 11 +++++++++++
> >  cacheflush.c                  |  4 ++--
> >  clone.c                       |  2 +-
> >  configure.ac                  |  4 ++++
> >  linux/csky/arch_defs_.h       |  6 ++++++
> >  linux/csky/arch_regs.c        | 11 +++++++++++
> >  linux/csky/get_error.c        | 18 ++++++++++++++++++
> >  linux/csky/get_scno.c         | 18 ++++++++++++++++++
> >  linux/csky/get_syscall_args.c | 19 +++++++++++++++++++
> >  linux/csky/ioctls_arch0.h     |  1 +
> >  linux/csky/ioctls_inc0.h      |  1 +
> >  linux/csky/raw_syscall.h      | 33 +++++++++++++++++++++++++++++++++
> >  linux/csky/set_error.c        | 20 ++++++++++++++++++++
> >  linux/csky/set_scno.c         | 17 +++++++++++++++++
> >  linux/csky/syscallent.h       |  4 ++++
> >  15 files changed, 166 insertions(+), 3 deletions(-)
> >  create mode 100644 linux/csky/arch_defs_.h
> >  create mode 100644 linux/csky/arch_regs.c
> >  create mode 100644 linux/csky/get_error.c
> >  create mode 100644 linux/csky/get_scno.c
> >  create mode 100644 linux/csky/get_syscall_args.c
> >  create mode 100644 linux/csky/ioctls_arch0.h
> >  create mode 100644 linux/csky/ioctls_inc0.h
> >  create mode 100644 linux/csky/raw_syscall.h
> >  create mode 100644 linux/csky/set_error.c
> >  create mode 100644 linux/csky/set_scno.c
> >  create mode 100644 linux/csky/syscallent.h
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 56987ae..f61978c 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -510,6 +510,17 @@ EXTRA_DIST =				\
> >  	linux/bfin/set_error.c		\
> >  	linux/bfin/set_scno.c		\
> >  	linux/bfin/syscallent.h		\
> > +	linux/csky/arch_defs_.h		\
> > +	linux/csky/arch_regs.c		\
> > +	linux/csky/get_error.c		\
> > +	linux/csky/get_scno.c		\
> > +	linux/csky/get_syscall_args.c	\
> > +	linux/csky/ioctls_arch0.h	\
> > +	linux/csky/ioctls_inc0.h	\
> > +	linux/csky/raw_syscall.h	\
> > +	linux/csky/set_error.c		\
> > +	linux/csky/set_scno.c		\
> > +	linux/csky/syscallent.h		\
> >  	linux/bfin/userent.h		\
> >  	linux/check_scno.c		\
> >  	linux/dummy.h			\
> 
> OK, but most likely you don't need linux/csky/arch_defs_.h
Ok.

...
> > +csky*)
> > +	arch=csky
> > +	AC_DEFINE([CSKY], 1, [Define for the RISC-V architecture])
> > +	;;
> >  hppa*|parisc*)
> >  	arch=hppa
> >  	AC_DEFINE([HPPA], 1, [Define for the HPPA architecture.])
> 
> Is it really RISC-V?
C-SKY :P

> 
> > diff --git a/linux/csky/arch_regs.c b/linux/csky/arch_regs.c
> > new file mode 100644
> > index 0000000..70f5beb
> > --- /dev/null
> > +++ b/linux/csky/arch_regs.c
> > @@ -0,0 +1,11 @@
> > +/*
> > + * Copyright (c) 2015-2018 The strace developers.
> 
> Are you sure it is correct and ends in 2018?
I just copy this from others, I'll delete this Copyright line OK?

> 
> > + * All rights reserved.
> > + *
> > + * SPDX-License-Identifier: LGPL-2.1-or-later
> > + */
> > +
> > +static struct pt_regs csky_regs;
> > +unsigned long *const csky_sp_ptr = &csky_regs.usp;
> > +# define ARCH_REGS_FOR_GETREGSET csky_regs
> > +#define ARCH_PC_REG csky_regs.pc
> 
> Please indent two consequent defines in the same way.
Ok

... 
> > + * All rights reserved.
> > + *
> > + * SPDX-License-Identifier: LGPL-2.1-or-later
> > + */
> > +
> > +/* Return -1 on error or 1 on success (never 0!). */
> > +static int
> > +arch_get_syscall_args(struct tcb *tcp)
> > +{
> > +	tcp->u_arg[0] = csky_regs.a0;
> 
> This does not match syscall_get_arguments() from
> arch/csky/include/asm/syscall.h where orig_a0 is used instead.
> 
> If this passes strace tests, then syscall_get_arguments() is most likely wrong.
Yes, you are right, it's my fault. regs->a0 could be replaced by return
value and we must use orig_a0.

Thank you very much.

> > diff --git a/linux/csky/ioctls_inc0.h b/linux/csky/ioctls_inc0.h
> > new file mode 100644
> > index 0000000..4aecf98
> > --- /dev/null
> > +++ b/linux/csky/ioctls_inc0.h
> > @@ -0,0 +1 @@
> > +#include "32/ioctls_inc.h"
> 
> Shouldn't this conditionalize on __CSKYABIV2__?
No, abiv1 and abiv2 are all 32-bit CPU and share the same ioctls'
definitions.

Best Regards
 Guo Ren


More information about the Strace-devel mailing list