[PATCH V2 2/2] Add backend support for C-SKY

Mark Wielaard mark@klomp.org
Mon Apr 15 11:09:00 GMT 2019


On Wed, 2019-04-10 at 15:51 +0800, Mao Han wrote:
> C-SKY V2 ABI manual:
> https://github.com/c-sky/csky-doc/blob/master/C-SKY_V2_CPU_Applications_Binary_Interface_Standards_Manual.pdf
> C-SKY architecture user guide:
> https://github.com/c-sky/csky-doc/blob/master/CSKY%20Architecture%20user_guide.pdf
> 
> Signed-off-by: Mao Han <han_mao@c-sky.com>

Thanks for adding those references.

> diff --git a/backends/ChangeLog b/backends/ChangeLog
> index 0c61a0b..0c3193e 100644
> --- a/backends/ChangeLog
> +++ b/backends/ChangeLog
> @@ -1,3 +1,16 @@
> +2019-04-01 Mao Han <han_mao@c-sky.com>
> +
> +	* backends/Makefile.am: Add C-SKY.
> +	* backends/csky_cfi.c: New file.
> +	* backends/csky_corenote.c: Likewise.
> +	* backends/csky_init.c: Likewise.
> +	* backends/csky_initreg.c: Likewise.
> +	* backends/csky_regs.c: Likewise.
> +	* backends/csky_reloc.def: Likewise.
> +	* backends/csky_symbol.c: Likewise.
> +	* libebl/eblopenbackend.c: Add C-SKY.
> +	* src/elflint.c: Likewise.

We have ChangeLog files per directory.
So you don't need to prefix with backends/
And the last two entries should go into their own (libebl and src)
ChangeLog files.

> diff --git a/backends/Makefile.am b/backends/Makefile.am
> index 2126a2e..155db8a 100644
> --- a/backends/Makefile.am
> +++ b/backends/Makefile.am
> @@ -33,12 +33,13 @@ AM_CPPFLAGS += -I$(top_srcdir)/libebl -I$(top_srcdir)/libasm \
>  
>  
>  modules = i386 sh x86_64 ia64 alpha arm aarch64 sparc ppc ppc64 s390 \
> -	  tilegx m68k bpf riscv
> +	  tilegx m68k bpf riscv csky
>  libebl_pic = libebl_i386_pic.a libebl_sh_pic.a libebl_x86_64_pic.a    \
>  	     libebl_ia64_pic.a libebl_alpha_pic.a libebl_arm_pic.a    \
>  	     libebl_aarch64_pic.a libebl_sparc_pic.a libebl_ppc_pic.a \
>  	     libebl_ppc64_pic.a libebl_s390_pic.a libebl_tilegx_pic.a \
> -	     libebl_m68k_pic.a libebl_bpf_pic.a libebl_riscv_pic.a
> +	     libebl_m68k_pic.a libebl_bpf_pic.a libebl_riscv_pic.a    \
> +	     libebl_csky_pic.a
>  noinst_LIBRARIES = $(libebl_pic)
>  noinst_DATA = $(libebl_pic:_pic.a=.so)
>  
> @@ -136,6 +137,10 @@ riscv_SRCS = riscv_init.c riscv_symbol.c riscv_cfi.c riscv_regs.c \
>  libebl_riscv_pic_a_SOURCES = $(riscv_SRCS)
>  am_libebl_riscv_pic_a_OBJECTS = $(riscv_SRCS:.c=.os)
>  
> +csky_SRCS = csky_init.c csky_symbol.c csky_cfi.c csky_regs.c \
> +            csky_initreg.c csky_corenote.c
> +libebl_csky_pic_a_SOURCES = $(csky_SRCS)
> +am_libebl_csky_pic_a_OBJECTS = $(csky_SRCS:.c=.os)
>  
>  libebl_%.so libebl_%.map: libebl_%_pic.a $(libelf) $(libdw) $(libeu)
>  	@rm -f $(@:.so=.map)

Looks correct.

> diff --git a/backends/csky_cfi.c b/backends/csky_cfi.c
> new file mode 100644
> index 0000000..6895c9e
> --- /dev/null
> +++ b/backends/csky_cfi.c
> +#include <dwarf.h>
> +
> +#define BACKEND csky_
> +#include "libebl_CPU.h"
> +
> +
> +int
> +csky_abi_cfi (Ebl *ebl __attribute__ ((unused)), Dwarf_CIE *abi_info)
> +{
> +  static const uint8_t abi_cfi[] =
> +    {
> +      DW_CFA_def_cfa, ULEB128_7 (14), ULEB128_7 (0),
> +      DW_CFA_val_offset, ULEB128_7 (14), ULEB128_7 (0),
> +
> +#define SV(n) DW_CFA_same_value, ULEB128_7 (n)
> +      SV(4), SV (5), SV (6), SV (7), SV (8), SV (9), 
> +      SV(10), SV (11), SV (15), SV (16), SV (17)
> +#undef SV
> +    };
> +
> +  abi_info->initial_instructions = abi_cfi;
> +  abi_info->initial_instructions_end = &abi_cfi[sizeof abi_cfi];
> +  abi_info->data_alignment_factor = -4;
> +
> +  abi_info->return_address_register = 15; /* lr.  */
> +
> +  return 0;
> +}

OK, that seems to match the calling convention, CFA is defined as sp,
prev SP comes from the cfa address, the link register has the default
return address.

> diff --git a/backends/csky_corenote.c b/backends/csky_corenote.c
> new file mode 100644
> index 0000000..c32b386
> --- /dev/null
> +++ b/backends/csky_corenote.c
> +#include <elf.h>
> +#include <inttypes.h>
> +#include <stddef.h>
> +#include <stdio.h>
> +#include <sys/time.h>
> +
> +#define BACKEND	csky_
> +#include "libebl_CPU.h"
> +
> +#define	ULONG			uint32_t
> +#define PID_T			int32_t
> +#define	UID_T			uint32_t
> +#define	GID_T			uint32_t
> +#define ALIGN_ULONG		4
> +#define ALIGN_PID_T		4
> +#define ALIGN_UID_T		4
> +#define ALIGN_GID_T		4
> +#define TYPE_ULONG		ELF_T_WORD
> +#define TYPE_PID_T		ELF_T_SWORD
> +#define TYPE_UID_T		ELF_T_WORD
> +#define TYPE_GID_T		ELF_T_WORD
> +
> +static const Ebl_Register_Location prstatus_regs[] =
> +  {
> +    { .offset = 0, .regno = 0, .count = 38, .bits = 32 } /* r0..r31 */
> +  };
> +#define PRSTATUS_REGS_SIZE	(38 * 4)
> +
> +#include "linux-core-note.c"

OK, that is a very simple 32bit architecture/register setup.

> diff --git a/backends/csky_init.c b/backends/csky_init.c
> new file mode 100644
> index 0000000..2ae3481
> --- /dev/null
> +++ b/backends/csky_init.c
> @@ -0,0 +1,65 @@
> +/* Initialization of C-SKY specific backend library.
> +
> +#define BACKEND		csky_
> +#define RELOC_PREFIX	R_CKCORE_
> +#include "libebl_CPU.h"
> +
> +/* This defines the common reloc hooks based on csky_reloc.def.  */
> +#include "common-reloc.c"
> +
> +const char *
> +csky_init (Elf *elf __attribute__ ((unused)),
> +	   GElf_Half machine __attribute__ ((unused)),
> +	   Ebl *eh,
> +	   size_t ehlen)
> +{
> +  /* Check whether the Elf_BH object has a sufficent size.  */
> +  if (ehlen < sizeof (Ebl))
> +    return NULL;
> +
> +  /* We handle it.  */
> +  eh->name = "C-SKY";
> +  csky_init_reloc (eh);
> +  HOOK (eh, reloc_simple_type);
> +  HOOK (eh, register_info);
> +  HOOK (eh, abi_cfi);
> +  HOOK (eh, core_note);
> +  HOOK (eh, set_initial_registers_tid);
> +  HOOK (eh, machine_flag_check);
> +  HOOK (eh, section_type_name);
> +
> +  /* gcc/config/ #define DWARF_FRAME_REGISTERS.  */
> +  eh->frame_nregs = 71;
> +
> +  return MODVERSION;
> +}

The hooks are setup correctly.

The 71 matches what can/is being restored through other hooks. But
seems to not match the c-SKY V2 CPU ABI DWARF register mappings. Also
maybe it should be 70, because you are using link as return register
and don't explicitly need to store the pc (see below).

> diff --git a/backends/csky_initreg.c b/backends/csky_initreg.c
> new file mode 100644
> index 0000000..6563b8a
> --- /dev/null
> +++ b/backends/csky_initreg.c
> @@ -0,0 +1,86 @@
> +/* Fetch live process registers from TID. C-SKY version.
> +
> +#ifdef HAVE_CONFIG_H
> +# include <config.h>
> +#endif
> +
> +#include "system.h"
> +#include <assert.h>
> +#if defined __CSKY__ && defined __linux__
> +# include <sys/uio.h>
> +# include <sys/procfs.h>
> +# include <sys/ptrace.h>
> +#endif
> +
> +#define BACKEND csky_
> +#include "libebl_CPU.h"
> +
> +bool
> +csky_set_initial_registers_tid (pid_t tid __attribute__ ((unused)),
> +                                ebl_tid_registers_t *setfunc __attribute__ ((unused)),
> +                                void *arg __attribute__ ((unused)))
> +{
> +#if !defined __CSKY__ || !defined __linux__
> +  return false;
> +#else /* __CSKY__ */
> +  struct pt_regs user_regs;
> +  struct iovec iovec;
> +  iovec.iov_base = &user_regs;
> +  iovec.iov_len = sizeof (user_regs);
> +  if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iovec) != 0)
> +    return false;
> +
> +  Dwarf_Word dwarf_regs[71];
> +
> +  /* lr.  */
> +  dwarf_regs[15] = user_regs.lr;
> +  /* sp.  */
> +  dwarf_regs[14] = user_regs.usp;
> +  /* r0 ~ r13.  */
> +  dwarf_regs[0] = user_regs.a0;
> +  dwarf_regs[1] = user_regs.a1;
> +  dwarf_regs[2] = user_regs.a2;
> +  dwarf_regs[3] = user_regs.a3;
> +  for (int i = 4; i < 14; i++)
> +    dwarf_regs[i] = user_regs.regs[i - 4];
> +  /* r ~ r13.  */
> +  for (int i = 16; i < 31; i++)
> +    dwarf_regs[i] = user_regs.exregs[i - 16];
> +  /* tls.  */
> +  dwarf_regs[31] = user_regs.tls;
> +  /* hi.  */
> +  dwarf_regs[34] = user_regs.rhi;
> +  /* lo.  */
> +  dwarf_regs[35] = user_regs.rlo;
> +  /* pc.  */
> +  dwarf_regs[70] = user_regs.pc;
> +
> +  return setfunc (0, 71, dwarf_regs, arg);
> +#endif
> +}

I think you should use setfunc (-1, -1, &regs[70], arg) to set the pc
explicitly and then setfunc (0, 36, dwarf_regs, arg) to set the rest.
But you don't seem to actually need the full 71 dwarf_regs array. Again
I am confused about the actual DWARF register number mapping.

> diff --git a/backends/csky_regs.c b/backends/csky_regs.c
> new file mode 100644
> index 0000000..0b6706e
> --- /dev/null
> +++ b/backends/csky_regs.c
> @@ -0,0 +1,122 @@
> +/* Register names and numbers for C-SKY DWARF.
> +#include <string.h>
> +#include <dwarf.h>
> +
> +#define BACKEND csky_
> +#include "libebl_CPU.h"
> +
> +ssize_t
> +csky_register_info (Ebl *ebl  __attribute__ ((unused)),
> +		    int regno, char *name, size_t namelen,
> +		    const char **prefix, const char **setname,
> +		    int *bits, int *type)
> +{
> +  if (name == NULL)
> +    return 71;
> +
> +  *prefix = "";
> +  *bits = 38;
> +  *type = DW_ATE_signed;
> +  *setname = "integer";
> +
> +  switch (regno)
> +    {
> +    case 0:
> +      stpcpy (name, "tls");
> +      namelen = 3;
> +      break;
> +
> +    case 1:
> +      stpcpy (name, "lr");
> +      namelen = 2;
> +      break;
> +
> +    case 2:
> +      stpcpy (name, "pc");
> +      namelen = 2;
> +      break;
> +
> +    case 3:
> +      stpcpy (name, "sr");
> +      namelen = 2;
> +      break;
> +
> +    case 4:
> +      stpcpy (name, "usp");
> +      namelen = 2;
> +      break;
> +
> +    case 5 ... 14:
> +      name[0] = 'r';
> +      name[1] = regno - 5 + '0';
> +      namelen = 2;
> +      break;
> +
> +    case 15 ... 24:
> +      name[0] = 'r';
> +      name[1] = '1';
> +      name[2] = regno - 15 + '0';
> +      namelen = 3;
> +      break;
> +
> +    case 25 ... 34:
> +      name[0] = 'r';
> +      name[1] = '2';
> +      name[2] = regno - 25 + '0';
> +      namelen = 3;
> +      break;
> +
> +    case 35:
> +      stpcpy (name, "r31");
> +      namelen = 3;
> +      break;
> +
> +    case 36:
> +      stpcpy (name, "hi");
> +      namelen = 2;
> +      break;
> +
> +    case 37: 
> +      stpcpy (name, "lo");
> +      namelen = 2;
> +      break;
> +
> +    default:
> +      *setname = NULL;
> +      return 0;
> +    }
> +
> +  name[namelen++] = '\0';
> +  return namelen;
> +}

This confuses me too. So there are 71 registers? But you only handle
0..38? The names don't seem to match the usage in other places, but I
am assuming DWARF register numbers match native register numbering.
Which might not be how things actually work?

> diff --git a/backends/csky_reloc.def b/backends/csky_reloc.def
> new file mode 100644
> index 0000000..1108f0c
> --- /dev/null
> +++ b/backends/csky_reloc.def
> @@ -0,0 +1,86 @@
> +/* List the relocation types for csky.  -*- C -*-
> 
> +/*	    NAME,		REL|EXEC|DYN	*/
> +
> +
> +RELOC_TYPE (NONE,		REL|EXEC|DYN)
> +RELOC_TYPE (ADDR32,		REL|EXEC|DYN)
> +RELOC_TYPE (PCRELIMM8BY4,	REL)
> +RELOC_TYPE (PCRELIMM11BY2,	REL)
> +RELOC_TYPE (PCREL32,		REL|DYN)
> +RELOC_TYPE (PCRELJSR_IMM11BY2,	REL)
> +RELOC_TYPE (RELATIVE,		EXEC|DYN)
> +RELOC_TYPE (COPY,		EXEC|DYN)
> +RELOC_TYPE (GLOB_DAT,		EXEC|DYN)
> +RELOC_TYPE (JUMP_SLOT,		EXEC|DYN)
> +RELOC_TYPE (GOTOFF,		REL)
> +RELOC_TYPE (GOTPC,		REL)
> +RELOC_TYPE (GOT32,		REL)
> +RELOC_TYPE (PLT32,		REL)
> +RELOC_TYPE (ADDRGOT,		REL)
> +RELOC_TYPE (ADDRPLT,		REL)
> +RELOC_TYPE (PCREL_IMM26BY2,	REL)
> +RELOC_TYPE (PCREL_IMM16BY2,	REL)
> +RELOC_TYPE (PCREL_IMM16BY4,	REL)
> +RELOC_TYPE (PCREL_IMM10BY2,	REL)
> +RELOC_TYPE (PCREL_IMM10BY4,	REL)
> +RELOC_TYPE (ADDR_HI16,		REL|DYN)
> +RELOC_TYPE (ADDR_LO16,		REL|DYN)
> +RELOC_TYPE (GOTPC_HI16,		REL)
> +RELOC_TYPE (GOTPC_LO16,		REL)
> +RELOC_TYPE (GOTOFF_HI16,	REL)
> +RELOC_TYPE (GOTOFF_LO16,	REL)
> +RELOC_TYPE (GOT12,		REL)
> +RELOC_TYPE (GOT_HI16,		REL)
> +RELOC_TYPE (GOT_LO16,		REL)
> +RELOC_TYPE (PLT12,		REL)
> +RELOC_TYPE (PLT_HI16,		REL)
> +RELOC_TYPE (PLT_LO16,		REL)
> +RELOC_TYPE (ADDRGOT_HI16,	REL)
> +RELOC_TYPE (ADDRGOT_LO16,	REL)
> +RELOC_TYPE (ADDRPLT_HI16,	REL)
> +RELOC_TYPE (ADDRPLT_LO16,	REL)
> +RELOC_TYPE (PCREL_JSR_IMM26BY2,	REL|DYN)
> +RELOC_TYPE (TOFFSET_LO16,	REL)
> +RELOC_TYPE (DOFFSET_LO16,	REL)
> +RELOC_TYPE (PCREL_IMM18BY2,	REL)
> +RELOC_TYPE (DOFFSET_IMM18,	REL)
> +RELOC_TYPE (DOFFSET_IMM18BY2,	REL)
> +RELOC_TYPE (DOFFSET_IMM18BY4,	REL)
> +RELOC_TYPE (GOT_IMM18BY4,	REL)
> +RELOC_TYPE (PLT_IMM18BY4,	REL)
> +RELOC_TYPE (PCREL_IMM7BY4,	REL)
> +RELOC_TYPE (TLS_LE32,		REL)
> +RELOC_TYPE (TLS_IE32,		REL)
> +RELOC_TYPE (TLS_GD32,		REL)
> +RELOC_TYPE (TLS_LDM32,		REL)
> +RELOC_TYPE (TLS_LDO32,		REL)
> +RELOC_TYPE (TLS_DTPMOD32,	EXEC|DYN)
> +RELOC_TYPE (TLS_DTPOFF32,	EXEC|DYN)
> +RELOC_TYPE (TLS_TPOFF32,	EXEC|DYN)

OK, not double checked against ABI doc, but looks reasonable.

> diff --git a/backends/csky_symbol.c b/backends/csky_symbol.c
> new file mode 100644
> index 0000000..96235cb
> --- /dev/null
> +++ b/backends/csky_symbol.c
> @@ -0,0 +1,77 @@
> +/* C-SKY specific symbolic name handling.
> +
> +#include <assert.h>
> +#include <elf.h>
> +#include <stddef.h>
> +#include <string.h>
> +
> +#define BACKEND csky_
> +#include "libebl_CPU.h"
> +
> +/* Check for the simple reloc types.  */
> +Elf_Type
> +csky_reloc_simple_type (Ebl *ebl __attribute__ ((unused)), int type,
> +			int *addsub __attribute__ ((unused)))
> +{
> +  switch (type)
> +    {
> +    case R_CKCORE_ADDR32:
> +      return ELF_T_WORD;
> +    default:
> +      return ELF_T_NUM;
> +    }
> +}

Yes, R_CKCORE_ADDR32 is probably all that is ever needed for inter
DWARF relocations.

> +bool
> +csky_machine_flag_check (GElf_Word flags)
> +{
> +  switch (flags & EF_CSKY_ABIMASK)
> +    {
> +    case EF_CSKY_ABIV1:
> +    case EF_CSKY_ABIV2:
> +      return true;
> +    default:
> +      return false;
> +    }
> +}

OK.
Does the current backend handle both?

> +const char *
> +csky_section_type_name (int type,
> +                       char *buf __attribute__ ((unused)),
> +                       size_t len __attribute__ ((unused)))
> +{
> +  if (type == SHT_CSKY_ATTRIBUTES)
> +    return "CSKY_ATTRIBUTES";
> +
> +  return NULL;
> +}

OK.
I couldn't find any description of this section.
Is it like SHT_ARM_ATTRIBUTES/SHT_GNU_ATTRIBUTES?
Then you might also want to handle it like that in src/readelf.c
(print_attributes).

> diff --git a/libebl/eblopenbackend.c b/libebl/eblopenbackend.c
> index d54b720..e229dbd 100644
> --- a/libebl/eblopenbackend.c
> +++ b/libebl/eblopenbackend.c
> @@ -135,6 +135,7 @@ static const struct
>    { "bpf", "elf_bpf", "bpf", 3, EM_BPF, 0, 0 },
>    { "riscv", "elf_riscv", "riscv", 5, EM_RISCV, ELFCLASS64, ELFDATA2LSB },
>    { "riscv", "elf_riscv", "riscv", 5, EM_RISCV, ELFCLASS32, ELFDATA2LSB },
> +  { "csky", "elf_csky", "csky", 4, EM_CSKY, ELFCLASS32, ELFDATA2LSB },
>  };
>  #define nmachines (sizeof (machines) / sizeof (machines[0]))

Looks correct.

> diff --git a/src/elflint.c b/src/elflint.c
> index 810c8bd..edb466d 100644
> --- a/src/elflint.c
> +++ b/src/elflint.c
> @@ -330,7 +330,7 @@ static const int valid_e_machine[] =
>      EM_CRIS, EM_JAVELIN, EM_FIREPATH, EM_ZSP, EM_MMIX, EM_HUANY, EM_PRISM,
>      EM_AVR, EM_FR30, EM_D10V, EM_D30V, EM_V850, EM_M32R, EM_MN10300,
>      EM_MN10200, EM_PJ, EM_OPENRISC, EM_ARC_A5, EM_XTENSA, EM_ALPHA,
> -    EM_TILEGX, EM_TILEPRO, EM_AARCH64, EM_BPF, EM_RISCV
> +    EM_TILEGX, EM_TILEPRO, EM_AARCH64, EM_BPF, EM_RISCV, EM_CSKY
>    };
>  #define nvalid_e_machine \
>    (sizeof (valid_e_machine) / sizeof (valid_e_machine[0]))

Likewise.

Thanks,

Mark



More information about the Elfutils-devel mailing list