[PATCH v6 03/28] powerpc/xmon: Move breakpoints to text section

[Christophe Leroy]

Le 28/04/2020 à 07:30, Jordan Niethe a écrit :
> On Tue, Apr 28, 2020 at 3:20 PM Christophe Leroy
> <christophe.leroy at c-s.fr> wrote:
>>
>>
>>
>> Le 28/04/2020 à 03:57, Jordan Niethe a écrit :
>>> The instructions for xmon's breakpoint are stored bpt_table[] which is in
>>> the data section. This is problematic as the data section may be marked
>>> as no execute. Move bpt_table[] to the text section.
>>>
>>> Signed-off-by: Jordan Niethe <jniethe5 at gmail.com>
>>> ---
>>> v6: - New to series. Was part of the previous patch.
>>>       - Make BPT_SIZE available in assembly
>>> ---
>>>    arch/powerpc/kernel/asm-offsets.c |  8 ++++++++
>>>    arch/powerpc/xmon/Makefile        |  2 +-
>>>    arch/powerpc/xmon/xmon.c          |  6 +-----
>>>    arch/powerpc/xmon/xmon_bpts.S     |  9 +++++++++
>>>    arch/powerpc/xmon/xmon_bpts.h     | 14 ++++++++++++++
>>>    5 files changed, 33 insertions(+), 6 deletions(-)
>>>    create mode 100644 arch/powerpc/xmon/xmon_bpts.S
>>>    create mode 100644 arch/powerpc/xmon/xmon_bpts.h
>>>
>>> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
>>> index c25e562f1cd9..2401f415f423 100644
>>> --- a/arch/powerpc/kernel/asm-offsets.c
>>> +++ b/arch/powerpc/kernel/asm-offsets.c
>>> @@ -70,6 +70,10 @@
>>>    #include <asm/fixmap.h>
>>>    #endif
>>>
>>> +#ifdef CONFIG_XMON
>>> +#include "../xmon/xmon_bpts.h"
>>> +#endif
>>> +
>>>    #define STACK_PT_REGS_OFFSET(sym, val)      \
>>>        DEFINE(sym, STACK_FRAME_OVERHEAD + offsetof(struct pt_regs, val))
>>>
>>> @@ -783,5 +787,9 @@ int main(void)
>>>        DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
>>>    #endif
>>>
>>> +#ifdef CONFIG_XMON
>>> +     DEFINE(BPT_SIZE, BPT_SIZE);
>>> +#endif
>>> +
>>>        return 0;
>>>    }
>>> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
>>> index c3842dbeb1b7..515a13ea6f28 100644
>>> --- a/arch/powerpc/xmon/Makefile
>>> +++ b/arch/powerpc/xmon/Makefile
>>> @@ -21,7 +21,7 @@ endif
>>>
>>>    ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>>>
>>> -obj-y                        += xmon.o nonstdio.o spr_access.o
>>> +obj-y                        += xmon.o nonstdio.o spr_access.o xmon_bpts.o
>>>
>>>    ifdef CONFIG_XMON_DISASSEMBLY
>>>    obj-y                       += ppc-dis.o ppc-opc.o
>>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>> index a064392df1b8..f7ce3ea8694c 100644
>>> --- a/arch/powerpc/xmon/xmon.c
>>> +++ b/arch/powerpc/xmon/xmon.c
>>> @@ -62,6 +62,7 @@
>>>
>>>    #include "nonstdio.h"
>>>    #include "dis-asm.h"
>>> +#include "xmon_bpts.h"
>>>
>>>    #ifdef CONFIG_SMP
>>>    static cpumask_t cpus_in_xmon = CPU_MASK_NONE;
>>> @@ -108,7 +109,6 @@ struct bpt {
>>>    #define BP_TRAP             2
>>>    #define BP_DABR             4
>>>
>>> -#define NBPTS        256
>>>    static struct bpt bpts[NBPTS];
>>>    static struct bpt dabr;
>>>    static struct bpt *iabr;
>>> @@ -116,10 +116,6 @@ static unsigned bpinstr = 0x7fe00008;    /* trap */
>>>
>>>    #define BP_NUM(bp)  ((bp) - bpts + 1)
>>>
>>> -#define BPT_SIZE       (sizeof(unsigned int) * 2)
>>> -#define BPT_WORDS      (BPT_SIZE / sizeof(unsigned int))
>>> -static unsigned int bpt_table[NBPTS * BPT_WORDS];
>>> -
>>>    /* Prototypes */
>>>    static int cmds(struct pt_regs *);
>>>    static int mread(unsigned long, void *, int);
>>> diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
>>> new file mode 100644
>>> index 000000000000..f3ad0ab50854
>>> --- /dev/null
>>> +++ b/arch/powerpc/xmon/xmon_bpts.S
>>> @@ -0,0 +1,9 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +#include <asm/ppc_asm.h>
>>> +#include <asm/asm-compat.h>
>>> +#include <asm/asm-offsets.h>
>>> +#include "xmon_bpts.h"
>>> +
>>> +.global bpt_table
>>> +bpt_table:
>>> +     .space NBPTS * BPT_SIZE
>>
>> No alignment required ? Standard alignment (probably 4 bytes ?) is
>> acceptable ?
> No, I'll add a .balign    4 to be sure.

I think it is aligned to 4 by default. My question was to know whether 4 
is enough.
I see BPT_SIZE is 8, should the alignment be at least 8 ?

>>
>>
>>> diff --git a/arch/powerpc/xmon/xmon_bpts.h b/arch/powerpc/xmon/xmon_bpts.h
>>> new file mode 100644
>>> index 000000000000..b7e94375db86
>>> --- /dev/null
>>> +++ b/arch/powerpc/xmon/xmon_bpts.h
>>> @@ -0,0 +1,14 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +#ifndef XMON_BPTS_H
>>> +#define XMON_BPTS_H
>>> +
>>> +#define NBPTS        256
>>> +#ifndef __ASSEMBLY__
>>> +#define BPT_SIZE     (sizeof(unsigned int) * 2)
>>> +#define BPT_WORDS    (BPT_SIZE / sizeof(unsigned int))
>>> +
>>> +extern unsigned int bpt_table[NBPTS * BPT_WORDS];
>>> +
>>> +#endif /* __ASSEMBLY__ */
>>> +
>>> +#endif /* XMON_BPTS_H */
>>>
>>
>> Christophe