[RFC v2 PATCH 1/1] ARM64: KGDB: Add FP/SIMD debug support

Will Deacon will.deacon at arm.com
Mon Nov 25 13:52:48 EST 2013


On Mon, Nov 18, 2013 at 12:06:57PM +0000, vijay.kilari at gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar at caviumnetworks.com>
> 
> Add KGDB debug support for FP/SIMD processor.This support
> only debugging of FP/SIMD in kernel mode.
> With this patch one can view or set FP/SIMD registers in
> kernel context
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar at caviumnetworks.com>
> ---
>  arch/arm64/include/asm/fpsimd.h |    1 +
>  arch/arm64/kernel/fpsimd.c      |    5 ++
>  arch/arm64/kernel/kgdb.c        |  105 +++++++++++++++++++++++++--------------
>  3 files changed, 73 insertions(+), 38 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
> index 609bc44..5039d6d 100644
> --- a/arch/arm64/include/asm/fpsimd.h
> +++ b/arch/arm64/include/asm/fpsimd.h
> @@ -60,6 +60,7 @@ extern void fpsimd_load_state(struct fpsimd_state *state);
>  extern void fpsimd_thread_switch(struct task_struct *next);
>  extern void fpsimd_flush_thread(void);
>  extern void fpsimd_reload_fpstate(void);
> +extern int  fpsimd_thread_state(void);
>  
>  #endif
>  
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 5b13c17..27604c1 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -124,6 +124,11 @@ void fpsimd_flush_thread(void)
>  	set_thread_flag(TIF_FOREIGN_FPSTATE);
>  }
>  
> +int fpsimd_thread_state(void)
> +{
> +	return (current->mm && test_thread_flag(TIF_FOREIGN_FPSTATE)) ? 1 : 0;
> +}

Erm...

  1. `? 1 : 0' doesn't do anything
  2. TIF_FOREIGN_FPSTATE doesn't seem to appear in my source tree (3.13-rc1)
  3. Does it make sense for threads without an mm to have TIF_FOREIGN_FPSTATE set?

>  void fpsimd_reload_fpstate(void)
>  {
>  	preempt_disable();
> diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
> index f10f2ba..5e34852 100644
> --- a/arch/arm64/kernel/kgdb.c
> +++ b/arch/arm64/kernel/kgdb.c
> @@ -23,6 +23,14 @@
>  #include <linux/kdebug.h>
>  #include <linux/kgdb.h>
>  #include <asm/traps.h>
> +#include <asm/fpsimd.h>
> +
> +#ifdef CONFIG_KERNEL_MODE_NEON
> +/*
> + * Structure to hold FP/SIMD register contents
> + */
> +struct fpsimd_state kgdb_fpsimd_regs;
> +#endif
>  
>  struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
>  	{ "x0", 8, offsetof(struct pt_regs, regs[0])},
> @@ -59,40 +67,40 @@ struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
>  	{ "sp", 8, offsetof(struct pt_regs, sp)},
>  	{ "pc", 8, offsetof(struct pt_regs, pc)},
>  	{ "pstate", 4, offsetof(struct pt_regs, pstate)},
> -	{ "v0", 16, -1 },
> -	{ "v1", 16, -1 },
> -	{ "v2", 16, -1 },
> -	{ "v3", 16, -1 },
> -	{ "v4", 16, -1 },
> -	{ "v5", 16, -1 },
> -	{ "v6", 16, -1 },
> -	{ "v7", 16, -1 },
> -	{ "v8", 16, -1 },
> -	{ "v9", 16, -1 },
> -	{ "v10", 16, -1 },
> -	{ "v11", 16, -1 },
> -	{ "v12", 16, -1 },
> -	{ "v13", 16, -1 },
> -	{ "v14", 16, -1 },
> -	{ "v15", 16, -1 },
> -	{ "v16", 16, -1 },
> -	{ "v17", 16, -1 },
> -	{ "v18", 16, -1 },
> -	{ "v19", 16, -1 },
> -	{ "v20", 16, -1 },
> -	{ "v21", 16, -1 },
> -	{ "v22", 16, -1 },
> -	{ "v23", 16, -1 },
> -	{ "v24", 16, -1 },
> -	{ "v25", 16, -1 },
> -	{ "v26", 16, -1 },
> -	{ "v27", 16, -1 },
> -	{ "v28", 16, -1 },
> -	{ "v29", 16, -1 },
> -	{ "v30", 16, -1 },
> -	{ "v31", 16, -1 },
> -	{ "fpsr", 4, -1 },
> -	{ "fpcr", 4, -1 },
> +	{ "v0", 16, offsetof(struct fpsimd_state, vregs[0])},
> +	{ "v1", 16, offsetof(struct fpsimd_state, vregs[1])},
> +	{ "v2", 16, offsetof(struct fpsimd_state, vregs[2])},
> +	{ "v3", 16, offsetof(struct fpsimd_state, vregs[3])},
> +	{ "v4", 16, offsetof(struct fpsimd_state, vregs[4])},
> +	{ "v5", 16, offsetof(struct fpsimd_state, vregs[5])},
> +	{ "v6", 16, offsetof(struct fpsimd_state, vregs[6])},
> +	{ "v7", 16, offsetof(struct fpsimd_state, vregs[7])},
> +	{ "v8", 16, offsetof(struct fpsimd_state, vregs[8])},
> +	{ "v9", 16, offsetof(struct fpsimd_state, vregs[9])},
> +	{ "v10", 16, offsetof(struct fpsimd_state, vregs[10])},
> +	{ "v11", 16, offsetof(struct fpsimd_state, vregs[11])},
> +	{ "v12", 16, offsetof(struct fpsimd_state, vregs[12])},
> +	{ "v13", 16, offsetof(struct fpsimd_state, vregs[13])},
> +	{ "v14", 16, offsetof(struct fpsimd_state, vregs[14])},
> +	{ "v15", 16, offsetof(struct fpsimd_state, vregs[15])},
> +	{ "v16", 16, offsetof(struct fpsimd_state, vregs[16])},
> +	{ "v17", 16, offsetof(struct fpsimd_state, vregs[17])},
> +	{ "v18", 16, offsetof(struct fpsimd_state, vregs[18])},
> +	{ "v19", 16, offsetof(struct fpsimd_state, vregs[19])},
> +	{ "v20", 16, offsetof(struct fpsimd_state, vregs[20])},
> +	{ "v21", 16, offsetof(struct fpsimd_state, vregs[21])},
> +	{ "v22", 16, offsetof(struct fpsimd_state, vregs[22])},
> +	{ "v23", 16, offsetof(struct fpsimd_state, vregs[23])},
> +	{ "v24", 16, offsetof(struct fpsimd_state, vregs[24])},
> +	{ "v25", 16, offsetof(struct fpsimd_state, vregs[25])},
> +	{ "v26", 16, offsetof(struct fpsimd_state, vregs[26])},
> +	{ "v27", 16, offsetof(struct fpsimd_state, vregs[27])},
> +	{ "v28", 16, offsetof(struct fpsimd_state, vregs[28])},
> +	{ "v29", 16, offsetof(struct fpsimd_state, vregs[29])},
> +	{ "v30", 16, offsetof(struct fpsimd_state, vregs[30])},
> +	{ "v31", 16, offsetof(struct fpsimd_state, vregs[31])},
> +	{ "fpsr", 4, offsetof(struct fpsimd_state, fpsr)},
> +	{ "fpcr", 4, offsetof(struct fpsimd_state, fpcr)},
>  };
>  
>  char *dbg_get_reg(int regno, void *mem, struct pt_regs *regs)
> @@ -100,11 +108,22 @@ char *dbg_get_reg(int regno, void *mem, struct pt_regs *regs)
>  	if (regno >= DBG_MAX_REG_NUM || regno < 0)
>  		return NULL;
>  
> -	if (dbg_reg_def[regno].offset != -1)
> +	if (dbg_reg_def[regno].offset != -1 && regno < _GP_REGS)
>  		memcpy(mem, (void *)regs + dbg_reg_def[regno].offset,
>  		       dbg_reg_def[regno].size);
> -	else
> +	else {
> +#ifdef CONFIG_KERNEL_MODE_NEON
> +		if (fpsimd_thread_state()) {
> +			fpsimd_save_state(&kgdb_fpsimd_regs);
> +			memcpy(mem, (void *)&kgdb_fpsimd_regs +
> +					dbg_reg_def[regno].offset,
> +					dbg_reg_def[regno].size);
> +		} else
> +			memset(mem, 0, dbg_reg_def[regno].size);
> +#else
>  		memset(mem, 0, dbg_reg_def[regno].size);

Can you use IS_ENABLED to tidy this up?

Will



More information about the linux-arm-kernel mailing list