From: Vijaya Kumar K Vijaya.Kumar@caviumnetworks.com
Based on ARM64 KGDB support patches, KGDB support for FPSIMD is added. Only debugging of FPSIMD is kernel context is supported.
This patch also requires Ard's patches where in kernel support for NEON is added. So CONFIG_KERNEL_MODE_NEON should be enabled. with this, FPSIMD registers can be viewed or set from gdb tool.
Unlike CPU registers, the FPSIMD registers are not saved on exception entry. With the known restriction that FPSIMD should not be touched in interrupt/exception context, in this patch the FPSIMD registers are directly read/written on gdb tool request
Here, the FPSIMD registers are read and restored for every FPSIMD register read and write by GDB tool. So this has impact on gdb tool response which is neglible. Other architectures like mips are also implemented similarly
Tested on ARM64 simulator
Vijaya Kumar K (1): ARM64: KGDB: Add FP/SIMD debug support
arch/arm64/kernel/kgdb.c | 98 ++++++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 38 deletions(-)
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
Signed-off-by: Vijaya Kumar K Vijaya.Kumar@caviumnetworks.com --- arch/arm64/kernel/kgdb.c | 98 ++++++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 38 deletions(-)
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index f10f2ba..07d3d00 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,18 @@ 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 + 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); +#endif + } return dbg_reg_def[regno].name; }
@@ -113,9 +128,16 @@ int dbg_set_reg(int regno, void *mem, struct pt_regs *regs) if (regno >= DBG_MAX_REG_NUM || regno < 0) return -EINVAL;
- if (dbg_reg_def[regno].offset != -1) + if (dbg_reg_def[regno].offset != -1 && regno < _GP_REGS) memcpy((void *)regs + dbg_reg_def[regno].offset, mem, - dbg_reg_def[regno].size); + dbg_reg_def[regno].size); +#ifdef CONFIG_KERNEL_MODE_NEON + else { + memcpy((void *)&kgdb_fpsimd_regs + dbg_reg_def[regno].offset, + mem, dbg_reg_def[regno].size); + fpsimd_load_state(&kgdb_fpsimd_regs); + } +#endif return 0; }
On Thu, Nov 07, 2013 at 12:38:41PM +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
Signed-off-by: Vijaya Kumar K Vijaya.Kumar@caviumnetworks.com
arch/arm64/kernel/kgdb.c | 98 ++++++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 38 deletions(-)
[...]
@@ -100,11 +108,18 @@ 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
fpsimd_save_state(&kgdb_fpsimd_regs);memcpy(mem, (void *)&kgdb_fpsimd_regs +dbg_reg_def[regno].offset, dbg_reg_def[regno].size);
Don't you only want to do this if you're inside a kernel_neon_{begin,end} section? Otherwise, you'll be copying out user registers. It really depends on what semantics you want.
Will
On Fri, Nov 8, 2013 at 8:36 PM, Will Deacon will.deacon@arm.com wrote:
On Thu, Nov 07, 2013 at 12:38:41PM +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
Signed-off-by: Vijaya Kumar K Vijaya.Kumar@caviumnetworks.com
arch/arm64/kernel/kgdb.c | 98 ++++++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 38 deletions(-)
[...]
@@ -100,11 +108,18 @@ 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
fpsimd_save_state(&kgdb_fpsimd_regs);memcpy(mem, (void *)&kgdb_fpsimd_regs +dbg_reg_def[regno].offset, dbg_reg_def[regno].size);Don't you only want to do this if you're inside a kernel_neon_{begin,end} section? Otherwise, you'll be copying out user registers. It really depends on what semantics you want.
Yes, we can do that. I couldn't find any api to know the state of the neon. I think of using a status variable and update in kernel_neon_begin/end?
Will
On 11 November 2013 09:47, Vijay Kilari vijay.kilari@gmail.com wrote:
On Fri, Nov 8, 2013 at 8:36 PM, Will Deacon will.deacon@arm.com wrote:
On Thu, Nov 07, 2013 at 12:38:41PM +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
Signed-off-by: Vijaya Kumar K Vijaya.Kumar@caviumnetworks.com
arch/arm64/kernel/kgdb.c | 98 ++++++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 38 deletions(-)
[...]
@@ -100,11 +108,18 @@ 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
fpsimd_save_state(&kgdb_fpsimd_regs);memcpy(mem, (void *)&kgdb_fpsimd_regs +dbg_reg_def[regno].offset, dbg_reg_def[regno].size);Don't you only want to do this if you're inside a kernel_neon_{begin,end} section? Otherwise, you'll be copying out user registers. It really depends on what semantics you want.
Yes, we can do that. I couldn't find any api to know the state of the neon. I think of using a status variable and update in kernel_neon_begin/end?
I have posted a patch here
http://marc.info/?l=linux-arm-kernel&m=138324498203157&w=2
that already keeps track of whether the current NEON register contents belong to the current userland or not. This is for a different purpose (lazy restore) but could potentially be reused here I suppose.
linaro-kernel@lists.linaro.org
-
Ard Biesheuvel -
Vijay Kilari -
vijay.kilari＠gmail.com
-
Will Deacon