RFC: FPA support removal from gdb and its impact on kgdb

[Russell King (Oracle)]

On Wed, Oct 05, 2022 at 04:03:04PM +0100, Luis Machado wrote:
> Hi,
> 
> Following the removal of Arm FPA support from GCC in ~2012, I'm pursuing the same for gdb [1].
> 
> We should be able to remove mostly everything from gdb, but there is a small portion of code that
> deals with targets (like kgdb) that don't expose their register sets through XML. This code in gdb
> attempts to detect the register set based on the size of the register buffer ('g' remote packet).
> 
> The problem is that CPSR was historically hardcoded to register 25 to make way for the FPA registers in the middle.
> 
> From arch/arm/kernel/kgdb.c:
> 
> struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] =
> {
>         { "r0", 4, offsetof(struct pt_regs, ARM_r0)},
>         { "r1", 4, offsetof(struct pt_regs, ARM_r1)},
>         { "r2", 4, offsetof(struct pt_regs, ARM_r2)},
>         { "r3", 4, offsetof(struct pt_regs, ARM_r3)},
>         { "r4", 4, offsetof(struct pt_regs, ARM_r4)},
>         { "r5", 4, offsetof(struct pt_regs, ARM_r5)},
>         { "r6", 4, offsetof(struct pt_regs, ARM_r6)},
>         { "r7", 4, offsetof(struct pt_regs, ARM_r7)},
>         { "r8", 4, offsetof(struct pt_regs, ARM_r8)},
>         { "r9", 4, offsetof(struct pt_regs, ARM_r9)},
>         { "r10", 4, offsetof(struct pt_regs, ARM_r10)},
>         { "fp", 4, offsetof(struct pt_regs, ARM_fp)},
>         { "ip", 4, offsetof(struct pt_regs, ARM_ip)},
>         { "sp", 4, offsetof(struct pt_regs, ARM_sp)},
>         { "lr", 4, offsetof(struct pt_regs, ARM_lr)},
>         { "pc", 4, offsetof(struct pt_regs, ARM_pc)},
>         { "f0", 12, -1 },
>         { "f1", 12, -1 },
>         { "f2", 12, -1 },
>         { "f3", 12, -1 },
>         { "f4", 12, -1 },
>         { "f5", 12, -1 },
>         { "f6", 12, -1 },
>         { "f7", 12, -1 },
>         { "fps", 4, -1 },
>         { "cpsr", 4, offsetof(struct pt_regs, ARM_cpsr)},
> };
> 
> Changing gdb to use CPSR as register 16 (not 25) would potentially break Linux's kgdb (and also
> *BSD's kgdb). Unless these -1 offsets get handled in a special way and the f<n> registers never
> make their way to the register buffer in the 'g'/'G' packets.

Looking at the code in the file you mention above, specifically
dbg_get_reg() which is immediately below the table you quoted above,
we see that an attempt to get the FPA registers (with an offset of
-1) will result in a value of all-zeros returned.

If we look further into the gdbstub that the kernel uses (in
kernel/debug/gdbstub.c) we find:

void pt_regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs)
{
        int i;
        int idx = 0;
        char *ptr = (char *)gdb_regs;

        for (i = 0; i < DBG_MAX_REG_NUM; i++) {
                dbg_get_reg(i, ptr + idx, regs);
                idx += dbg_reg_def[i].size;
        }
}

So, all registers are fetched to a block of memory (defined by the first
argument to this function). This is called from gdb_get_regs_helper()
and places the register values in a static-global gdb_regs array.

And then we have:

/* Handle the 'g' get registers request */
static void gdb_cmd_getregs(struct kgdb_state *ks)
{
        gdb_get_regs_helper(ks);
        kgdb_mem2hex((char *)gdb_regs, remcom_out_buffer, NUMREGBYTES);
}

So, it looks to me like the stub returns the registers as a block of
hex, and removing the FPA registers _will_ break the stub.

Given this, and this is a fundamental interface between two different
pieces of software, I fail to see how you can even consider removing
support for this - if you remove support from gdb, then later gdb
will be unable to debug existing kernels.

In kernel-land, we have a rule: don't break userspace. I think there
should also be a rule for userspace: don't break the kernel!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!