On 25 November 2013 19:52, Will Deacon will.deacon@arm.com wrote:
On Mon, Nov 18, 2013 at 12:06:57PM +0000, vijay.kilari@gmail.com wrote:
From: Vijaya Kumar K Vijaya.Kumar@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
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)
+{
+}
Erm...
- `? 1 : 0' doesn't do anything
- TIF_FOREIGN_FPSTATE doesn't seem to appear in my source tree (3.13-rc1)
- 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