[RFC v2 PATCH 1/1] ARM64: KGDB: Add FP/SIMD debug support
Ard Biesheuvel
ard.biesheuvel at linaro.org
Mon Nov 25 14:03:59 EST 2013
On 25 November 2013 19:52, Will Deacon <will.deacon at arm.com> wrote:
> 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?
>
I think Vijay did mention at some point that his patches depend on my
patch 'arm64: defer reloading a task's FPSIMD state to userland
resume' which I posted here
http://marc.info/?l=linux-arm-kernel&m=138418879110655&w=2
(with you on cc, IIRC) but which did not evoke any response on the list.
--
Ard.
>> 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