Re: [RFC 7/7] Add S390 support for linux-kernel target

[Luis Machado <lgustavo@codesourcery.com>]

On 01/12/2017 05:32 AM, Philipp Rudo wrote:
> Implement functions for the linux-kernel target hooks.
>
> gdb/ChangeLog:
>
> 	* s390-linux-tdep.h: Define macros for address translation.
> 	* s390-linux-tdep.c: Include gdbthread.h and lk-low.h.
> 	(s390_gregmap_lk): New array.
> 	(s390_gregset_lk): New type.
> 	(s390_lk_get_registers, s390_lk_get_percpu_offset)
> 	(s390_lk_map_running_task_to_cpu, s390_lk_is_kvaddr)
> 	(s390_lk_read_table_entry, s390_lk_vtop, s390_lk_init_private): New
> 	function.
> 	(s390_gdbarch_init): Adjust.
> ---
>  gdb/s390-linux-tdep.c | 270 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/s390-linux-tdep.h |  62 ++++++++++++
>  2 files changed, 332 insertions(+)
>
> diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
> index 054c6d5..f436a74 100644
> --- a/gdb/s390-linux-tdep.c
> +++ b/gdb/s390-linux-tdep.c

I wonder if we should put the new lk-specific code in its own file, like 
s390-lk-tdep.c or something. It would make thing a bit more clear and 
would pollute the current s390-linux-tdep.c file?

> @@ -29,6 +29,7 @@
>  #include "target.h"
>  #include "gdbcore.h"
>  #include "gdbcmd.h"
> +#include "gdbthread.h"
>  #include "objfiles.h"
>  #include "floatformat.h"
>  #include "regcache.h"
> @@ -49,6 +50,8 @@
>  #include "auxv.h"
>  #include "xml-syscall.h"
>
> +#include "lk-low.h"
> +
>  #include "stap-probe.h"
>  #include "ax.h"
>  #include "ax-gdb.h"
> @@ -783,6 +786,14 @@ static const struct regcache_map_entry s390_gregmap[] =
>      { 0 }
>    };
>
> +static const struct regcache_map_entry s390_gregmap_lk[] =
> +  {
> +    { 10, S390_R6_REGNUM }, /* r0-r5 volatile */
> +    { -2, REGCACHE_MAP_SKIP, 8 },
> +    { 1, S390_PSWA_REGNUM }, /* Use r14 for PSWA.  */
> +    { 0 }
> +  };
> +
>  static const struct regcache_map_entry s390_regmap_cr [] =
>    {
>      { 16, S390_CR0_REGNUM, 8 },
> @@ -891,6 +902,12 @@ const struct regset s390_gregset = {
>    regcache_collect_regset
>  };
>
> +const struct regset s390_gregset_lk = {
> +  s390_gregmap_lk,
> +  regcache_supply_regset,
> +  regcache_collect_regset
> +};
> +
>  const struct regset s390_cr_regset = {
>    s390_regmap_cr,
>    regcache_supply_regset,
> @@ -1080,6 +1097,256 @@ s390_core_read_description (struct gdbarch *gdbarch,
>      }
>  }
>
> +/* Funktion for Linux kernel target get_registers hook.  Supplies gprs for

"Function..."

> +   task TASK to REGCACHE.  Uses r14 (back jump address) as current pswa.  */
> +
> +void
> +s390_lk_get_registers (CORE_ADDR task, struct target_ops *target,
> +		       struct regcache *regcache, int regnum)
> +{
> +  const struct regset *regset;
> +  CORE_ADDR ksp, gprs, pswa;
> +  gdb_byte *buf;
> +  size_t size;
> +  struct cleanup *old_chain;
> +
> +  regset = &s390_gregset_lk;
> +
> +  ksp = lk_read_addr (task + LK_OFFSET (task_struct, thread)
> +		      + LK_OFFSET (thread_struct, ksp));
> +  gprs = ksp + LK_OFFSET (stack_frame, gprs);
> +  size = FIELD_SIZE (LK_FIELD (stack_frame, gprs));
> +

Do we have a reasonable MAX for this size? If so, we may try to have a 
static array instead of allocating and deallocating in such a short 
time. Just a suggestion.

> +  buf = XCNEWVEC (gdb_byte, size);
> +  old_chain = make_cleanup (xfree, buf);
> +  read_memory (gprs, buf, size);
> +  regset->supply_regset (regset, regcache, -1, buf, size);
> +  do_cleanups (old_chain);
> +}
> +
> +/* Function for Linux kernel target get_percpu_offset hook. Returns the

Two spaces after period.

> +   percpu_offset from lowcore for cpu CPU.  */
> +
> +CORE_ADDR
> +s390_lk_get_percpu_offset (int cpu)

Is cpu ever negative? If not, make it unsigned?

> +{
> +  CORE_ADDR lowcore_ptr, lowcore;
> +  int ptr_len = lk_builtin_type_size (unsigned_long);
> +
> +  lowcore_ptr = LK_ADDR (lowcore_ptr) + (ptr_len * cpu);
> +  lowcore = lk_read_addr (lowcore_ptr);
> +
> +  return lk_read_addr (lowcore + LK_OFFSET (lowcore, percpu_offset));
> +}
> +
> +/* Function for Linux kernel target map_running_task_to_cpu hook.  */
> +
> +int
> +s390_lk_map_running_task_to_cpu (struct thread_info *ti)
> +{
> +  struct regcache *regcache;
> +  enum register_status reg_status;
> +  CORE_ADDR lowcore;
> +  int cpu;
> +
> +  regcache = get_thread_regcache (ti->ptid);
> +  reg_status = regcache_raw_read_unsigned (regcache, S390_PREFIX_REGNUM,
> +					   (ULONGEST *) &lowcore);
> +  if (reg_status != REG_VALID)
> +    error (_("Could not find prefix register for thread with pid %d, lwp %li."),
> +	   ti->ptid.pid, ti->ptid.lwp);
> +
> +  cpu = lk_read_int (lowcore + LK_OFFSET (lowcore, cpu_nr));
> +
> +  return cpu;
> +}
> +
> +/* Funktion for Linux kernel target is_kvaddr hook.  */

"Function..."

> +
> +int
> +s390_lk_is_kvaddr (CORE_ADDR addr)
> +{
> +  return addr >= LK_ADDR (high_memory);
> +}
> +
> +/* Read table entry from TABLE at offset OFFSET.  Helper for s390_lk_vtop.  */
> +
> +static inline ULONGEST
> +s390_lk_read_table_entry (CORE_ADDR table, ULONGEST offset)
> +{
> +  return lk_read_ulong (table + offset * lk_builtin_type_size (unsigned_long));
> +}
> +
> +/* Function for Linux kernel target vtop hook.  Assume 64 bit addresses.  */
> +
> +CORE_ADDR
> +s390_lk_vtop (CORE_ADDR table, CORE_ADDR vaddr)
> +{
> +  ULONGEST entry, offset;
> +  CORE_ADDR paddr;
> +  int table_type;
> +
> +  /* Read first entry in table to get its type.  As the Table-Type bits are
> +     the same in every table assume Region1-Table.  */
> +  entry = s390_lk_read_table_entry (table, 0);
> +  table_type = (entry & S390_LK_RFTE_TT) >> 2;
> +
> +  switch (table_type)
> +    {
> +      case S390_LK_DAT_TT_REGION1:
> +	{
> +	  offset = (vaddr & S390_LK_VADDR_RFX) >> 53;
> +	  entry = s390_lk_read_table_entry (table, offset);
> +
> +	  /* Do sanity checks.  */
> +	  if (!entry)
> +	    warning (_("Trying to translate address %#lx with empty region-first-table entry."),
> +		   vaddr);

Use paddress (...) to print the hex address of vaddr or phex (...). More 
occurrences of this throughout the patch.

> +	  else if ((entry & S390_LK_RFTE_TT) >> 2 != S390_LK_DAT_TT_REGION1)
> +	    warning (_("Trying to translate address %#lx with corrupt table type in region-first-table entry."),
> +		     vaddr);
> +	  else if (entry & S390_LK_RFTE_I)
> +	    warning (_("Translating address %#lx with invalid bit set at region-first-table entry."),
> +		     vaddr);
> +
> +	  table = entry & S390_LK_RFTE_O;
> +	}
> +	/* fall through */
> +      case S390_LK_DAT_TT_REGION2:
> +	{
> +	  offset = (vaddr & S390_LK_VADDR_RSX) >> 42;
> +	  entry = s390_lk_read_table_entry (table, offset);
> +
> +	  /* Do sanity checks.  */
> +	  if (!entry)
> +	    warning (_("Trying to translate address %#lx with empty region-second-table entry."),
> +		   vaddr);
> +	  else if ((entry & S390_LK_RSTE_TT) >> 2 != S390_LK_DAT_TT_REGION2)
> +	    warning (_("Trying to translate address %#lx with corrupt table type in region-second-table entry."),
> +		     vaddr);
> +	  else if (entry & S390_LK_RSTE_I)
> +	    warning (_("Translating address %#lx with invalid bit set at region-second-table entry."),
> +		     vaddr);
> +
> +	  table = entry & S390_LK_RSTE_O;
> +	}
> +	/* fall through */
> +      case S390_LK_DAT_TT_REGION3:
> +	{
> +	  offset = (vaddr & S390_LK_VADDR_RTX) >> 31;
> +	  entry = s390_lk_read_table_entry (table, offset);
> +
> +	  /* Do sanity checks.  */
> +	  if (!entry)
> +	    warning (_("Trying to translate address %#lx with empty region-third-table entry."),
> +		   vaddr);
> +	  else if ((entry & S390_LK_RTTE_TT) >> 2 != S390_LK_DAT_TT_REGION3)
> +	    warning (_("Trying to translate address %#lx with corrupt table type in region-third-table entry."),
> +		     vaddr);
> +	  else if (entry & S390_LK_RTTE_I)
> +	    warning (_("Translating address %#lx with invalid bit set at region-third-table entry."),
> +		     vaddr);
> +
> +	  /* Check for huge page.  */
> +	  if (entry & S390_LK_RTTE_FC)
> +	    {
> +	      paddr = ((entry & S390_LK_RTTE_RFAA)
> +		       + (vaddr & ~S390_LK_RTTE_RFAA));
> +	      return paddr;
> +	    }
> +
> +	  table = entry & S390_LK_RTTE_O;
> +	}
> +	/* fall through */
> +      case S390_LK_DAT_TT_SEGMENT:
> +	{
> +	  offset = (vaddr & S390_LK_VADDR_SX) >> 20;
> +	  entry = s390_lk_read_table_entry (table, offset);
> +
> +	  /* Do sanity checks.  */
> +	  if (!entry)
> +	    warning (_("Trying to translate address %#lx with empty segment-table entry."),
> +		   vaddr);
> +	  else if ((entry & S390_LK_STE_TT) >> 2 != S390_LK_DAT_TT_SEGMENT)
> +	    warning (_("Trying to translate address %#lx with corrupt table type in segment-table entry."),
> +		     vaddr);
> +	  else if (entry & S390_LK_STE_I)
> +	    warning (_("Translating address %#lx with invalid bit set at segment-table entry."),
> +		     vaddr);
> +
> +	  /* Check for large page.  */

Is large different from huge?

> +	  if (entry & S390_LK_STE_FC)
> +	    {
> +	      paddr = ((entry & S390_LK_STE_SFAA)
> +		       + (vaddr & ~S390_LK_STE_SFAA));
> +	      return paddr;
> +	    }
> +
> +	  table = entry & S390_LK_STE_O;
> +	  break;
> +	}
> +    } /* switch (table_type) */
> +
> +  offset = (vaddr & S390_LK_VADDR_PX) >> 12;
> +  entry = s390_lk_read_table_entry (table, offset);
> +
> +  /* Do sanity checks.  */
> +  if (!entry)
> +    warning (_("Trying to translate address %#lx with empty page-table entry."),
> +	     vaddr);
> +  else if (entry & S390_LK_PTE_I)
> +    warning (_("Translating address %#lx with invalid bit set at page-table entry."),
> +	     vaddr);
> +
> +  paddr = ((entry & S390_LK_PTE_PFAA) + (vaddr & ~S390_LK_PTE_PFAA));
> +
> +  return paddr;
> +}
> +
> +/* Function for Linux kernel target get_module_text_offset hook.  */
> +
> +CORE_ADDR
> +s390_lk_get_module_text_offset (CORE_ADDR mod)
> +{
> +  CORE_ADDR offset, mod_arch;
> +
> +  mod_arch = mod + LK_OFFSET (module, arch);
> +  offset = lk_read_ulong (mod_arch + LK_OFFSET (mod_arch_specific, got_size));
> +  offset += lk_read_ulong (mod_arch + LK_OFFSET (mod_arch_specific, plt_size));
> +
> +  return offset;
> +}
> +
> +/* Initialize s390 dependent private data for linux kernel target.  */

Newline between comment and function declaration.

> +void
> +s390_lk_init_private (struct gdbarch *gdbarch)
> +{
> +  LK_DECLARE_FIELD (stack_frame, gprs);
> +
> +  LK_DECLARE_FIELD (thread_struct, ksp);
> +
> +  LK_DECLARE_STRUCT_ALIAS (_lowcore, lowcore); /* linux -4.4 */
> +  LK_DECLARE_STRUCT_ALIAS (lowcore, lowcore); /* linux 4.5+ */
> +  if (LK_STRUCT (lowcore) == NULL)
> +    error (_("Could not find struct lowcore. Abort."));

Two spaces after period. Also, "Aborting" instead of "Abort"?

> +  LK_DECLARE_FIELD (lowcore, percpu_offset);
> +  LK_DECLARE_FIELD (lowcore, current_pid);
> +  LK_DECLARE_FIELD (lowcore, cpu_nr);
> +
> +  LK_DECLARE_FIELD (mod_arch_specific, got_size);
> +  LK_DECLARE_FIELD (mod_arch_specific, plt_size);
> +
> +  LK_DECLARE_ADDR (lowcore_ptr);
> +  LK_DECLARE_ADDR (high_memory);
> +
> +  LK_HOOK->get_registers = s390_lk_get_registers;
> +  LK_HOOK->is_kvaddr = s390_lk_is_kvaddr;
> +  LK_HOOK->vtop = s390_lk_vtop;
> +  LK_HOOK->get_percpu_offset = s390_lk_get_percpu_offset;
> +  LK_HOOK->map_running_task_to_cpu = s390_lk_map_running_task_to_cpu;
> +  LK_HOOK->get_module_text_offset = s390_lk_get_module_text_offset;
> +}
> +
>
>  /* Decoding S/390 instructions.  */
>
> @@ -8220,6 +8487,9 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    set_gdbarch_process_record (gdbarch, s390_process_record);
>    set_gdbarch_process_record_signal (gdbarch, s390_linux_record_signal);
>
> +  /* Support linux kernel debugging.  */
> +  set_gdbarch_lk_init_private (gdbarch, s390_lk_init_private);
> +
>    s390_init_linux_record_tdep (&s390_linux_record_tdep, ABI_LINUX_S390);
>    s390_init_linux_record_tdep (&s390x_linux_record_tdep, ABI_LINUX_ZSERIES);
>
> diff --git a/gdb/s390-linux-tdep.h b/gdb/s390-linux-tdep.h
> index fdb3c3d..8a16b0b 100644
> --- a/gdb/s390-linux-tdep.h
> +++ b/gdb/s390-linux-tdep.h
> @@ -238,4 +238,66 @@ extern struct target_desc *tdesc_s390x_te_linux64;
>  extern struct target_desc *tdesc_s390x_vx_linux64;
>  extern struct target_desc *tdesc_s390x_tevx_linux64;
>
> +/* Definitions for address translation.  */
> +/* DAT Table types.  */
> +#define S390_LK_DAT_TT_REGION1  3
> +#define S390_LK_DAT_TT_REGION2  2
> +#define S390_LK_DAT_TT_REGION3  1
> +#define S390_LK_DAT_TT_SEGMENT  0
> +
> +/* Region-First-Table */
> +#define S390_LK_RFTE_TL  0x3ULL		/* Table-Length */
> +#define S390_LK_RFTE_TT  0xcULL		/* Table-Type */
> +#define S390_LK_RFTE_I   0x20ULL	/* Region-Invalid Bit */
> +#define S390_LK_RFTE_TF  0xc0ULL	/* Table Offset */
> +#define S390_LK_RFTE_P   0x200ULL	/* DAT-Protection Bit */
> +#define S390_LK_RFTE_O  ~0xfffULL	/* Region-Second-Table Origin */
> +
> +/* Region-Second-Table flags.  */
> +#define S390_LK_RSTE_TL  0x3ULL		/* Table-Length */
> +#define S390_LK_RSTE_TT  0xcULL		/* Table-Type */
> +#define S390_LK_RSTE_I   0x20ULL	/* Region-Invalid Bit */
> +#define S390_LK_RSTE_TF  0xc0ULL	/* Table Offset */
> +#define S390_LK_RSTE_P   0x200ULL	/* DAT-Protection Bit */
> +#define S390_LK_RSTE_O  ~0xfffULL	/* Region-Third-Table Origin */
> +
> +/* Region-Third-Table flags.  */
> +#define S390_LK_RTTE_TL    0x3ULL	  /* Table-Length */
> +#define S390_LK_RTTE_TT    0xcULL	  /* Table-Type */
> +#define S390_LK_RTTE_CR    0x10ULL	  /* Common-Region Bit */
> +#define S390_LK_RTTE_I     0x20ULL	  /* Region-Invalid Bit */
> +#define S390_LK_RTTE_TF    0xc0ULL	  /* Table Offset */
> +#define S390_LK_RTTE_P     0x200ULL	  /* DAT-Protection Bit */
> +#define S390_LK_RTTE_FC    0x400ULL	  /* Format-Control Bit */
> +#define S390_LK_RTTE_F     0x800ULL	  /* Fetch-Protection Bit */
> +#define S390_LK_RTTE_ACC   0xf000ULL	  /* Access-Control Bits */
> +#define S390_LK_RTTE_AV    0x10000ULL	  /* ACCF-Validy Control */

"Validy" or "Validity"?

> +#define S390_LK_RTTE_O    ~0xfffULL	  /* Segment-Table Origin */
> +#define S390_LK_RTTE_RFAA ~0x7fffffffULL  /* Region-Frame Absolute Address */
> +
> +/* Segment-Table flags.  */
> +#define S390_LK_STE_TT    0xcULL	/* Table-Type */
> +#define S390_LK_STE_I     0x20ULL	/* Segment-Invalid Bit */
> +#define S390_LK_STE_TF    0xc0ULL	/* Table Offset */
> +#define S390_LK_STE_P     0x200ULL	/* DAT-Protection Bit */
> +#define S390_LK_STE_FC    0x400ULL	/* Format-Control Bit */
> +#define S390_LK_STE_F     0x800ULL	/* Fetch-Protection Bit */
> +#define S390_LK_STE_ACC   0xf000ULL	/* Access-Control Bits */
> +#define S390_LK_STE_AV    0x10000ULL	/* ACCF-Validy Control */
> +#define S390_LK_STE_O    ~0x7ffULL	/* Page-Table Origin */
> +#define S390_LK_STE_SFAA ~0xfffffULL	/* Segment-Frame Absolute Address */
> +
> +/* Page-Table flags.  */
> +#define S390_LK_PTE_P     0x200ULL	/* DAT-Protection Bit */
> +#define S390_LK_PTE_I     0x400ULL	/* Page-Invalid Bit */
> +#define S390_LK_PTE_PFAA ~0xfffULL	/* Page-Frame Absolute Address */
> +
> +/* Virtual Address Fields.  */
> +#define S390_LK_VADDR_RFX 0xffe0000000000000ULL
> +#define S390_LK_VADDR_RSX 0x001ffc0000000000ULL
> +#define S390_LK_VADDR_RTX 0x000003ff80000000ULL
> +#define S390_LK_VADDR_SX  0x000000007ff00000ULL
> +#define S390_LK_VADDR_PX  0x00000000000ff000ULL
> +#define S390_LK_VADDR_BX  0x0000000000000fffULL
> +
>  #endif
>