2022-05-11 14:44:43

by Sumit Garg

[permalink] [raw]
Subject: [PATCH v3 0/2] arm64: Fix pending single-step debugging issues

This patch-set reworks pending fixes from Wei's series [1] to make
single-step debugging via kgdb/kdb on arm64 work as expected. There was
a prior discussion on ML [2] regarding if we should keep the interrupts
enabled during single-stepping. So patch #1 follows suggestion from Will
[3] to not disable interrupts during single stepping but rather skip
single stepping within interrupt handler.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/CAD=FV=Voyfq3Qz0T3RY+aYWYJ0utdH=P_AweB=13rcV8GDBeyQ@mail.gmail.com/
[3] https://lore.kernel.org/all/20200626095551.GA9312@willie-the-truck/

Changes in v3:
- Reword commit descriptions as per Daniel's suggestions.

Changes in v2:
- Replace patch #1 to rather follow Will's suggestion.

Sumit Garg (2):
arm64: entry: Skip single stepping into interrupt handlers
arm64: kgdb: Set PSTATE.SS to 1 to re-enable single-step

arch/arm64/include/asm/debug-monitors.h | 1 +
arch/arm64/kernel/debug-monitors.c | 5 +++++
arch/arm64/kernel/entry-common.c | 18 +++++++++++++++++-
arch/arm64/kernel/kgdb.c | 2 ++
4 files changed, 25 insertions(+), 1 deletion(-)

--
2.25.1



2022-07-01 22:32:10

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] arm64: Fix pending single-step debugging issues

Hi,

On Tue, May 10, 2022 at 11:05 PM Sumit Garg <[email protected]> wrote:
>
> This patch-set reworks pending fixes from Wei's series [1] to make
> single-step debugging via kgdb/kdb on arm64 work as expected. There was
> a prior discussion on ML [2] regarding if we should keep the interrupts
> enabled during single-stepping. So patch #1 follows suggestion from Will
> [3] to not disable interrupts during single stepping but rather skip
> single stepping within interrupt handler.
>
> [1] https://lore.kernel.org/all/[email protected]/
> [2] https://lore.kernel.org/all/CAD=FV=Voyfq3Qz0T3RY+aYWYJ0utdH=P_AweB=13rcV8GDBeyQ@mail.gmail.com/
> [3] https://lore.kernel.org/all/20200626095551.GA9312@willie-the-truck/
>
> Changes in v3:
> - Reword commit descriptions as per Daniel's suggestions.
>
> Changes in v2:
> - Replace patch #1 to rather follow Will's suggestion.
>
> Sumit Garg (2):
> arm64: entry: Skip single stepping into interrupt handlers
> arm64: kgdb: Set PSTATE.SS to 1 to re-enable single-step
>
> arch/arm64/include/asm/debug-monitors.h | 1 +
> arch/arm64/kernel/debug-monitors.c | 5 +++++
> arch/arm64/kernel/entry-common.c | 18 +++++++++++++++++-
> arch/arm64/kernel/kgdb.c | 2 ++
> 4 files changed, 25 insertions(+), 1 deletion(-)

Sorry it took so long for me to respond. I kept dreaming that I'd find
the time to really dig deep into this to understand it fully and I'm
finally giving up on it. I'm going to hope that Will and/or Catalin
knows this area of the code well and can give it a good review. If not
then I'll strive harder to make the time...

In any case, I poked around with this a bunch and it definitely
improved the stepping behavior a whole lot. I still got one case where
gdb hit an assertion while I was stepping, but I could believe that
was a problem with gdb? I couldn't reproduce it. Thus I can at least
give:

Tested-by: Douglas Anderson <[email protected]>

I'll also note that I _think_ I remember that with Wei's series that
the gdb function "call" started working. I tried that here and it
didn't seem so happy. To keep things simple, I created a dummy
function in my kernel that looked like:

void doug_test(void)
{
pr_info("testing, 1 2 3\n");
}

I broke into the debugger by echoing "g" to /proc/sysrq-trigger and
then tried "call doug_test()". I guess my printout actually printed
but it wasn't so happy after that. Seems like it somehow ended up
returning to a bogus address after the call which then caused a crash.

testing, 1 2 3
BUG: sleeping function called from invalid context at
arch/arm64/mm/fault.c:593
in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 3393, name: bash
preempt_count: 0, expected: 0
RCU nest depth: 1, expected: 0
CPU: 6 PID: 3393 Comm: bash Not tainted 5.19.0-rc4+ #3
dbec0bdb8582e447bccdcf2e70d7fe04477b1aac
Hardware name: Google Herobrine (rev1+) (DT)
Call trace:
dump_backtrace+0xf0/0x110
show_stack+0x24/0x70
dump_stack_lvl+0x64/0x7c
dump_stack+0x18/0x38
__might_resched+0x144/0x154
__might_sleep+0x54/0x84
do_page_fault+0x1d4/0x42c
do_mem_abort+0x4c/0xb0
el1_abort+0x3c/0x5c
el1h_64_sync_handler+0x4c/0xc4
el1h_64_sync+0x64/0x68
0xffffffc008000000
__handle_sysrq+0x15c/0x184
write_sysrq_trigger+0x94/0x128
proc_reg_write+0xbc/0xec
vfs_write+0xf0/0x2c8
ksys_write+0x84/0xf0
__arm64_sys_write+0x28/0x34
invoke_syscall+0x4c/0x120
el0_svc_common+0x94/0xfc
do_el0_svc+0x38/0xc0
el0_svc+0x2c/0x7c
el0t_64_sync_handler+0x48/0x114
el0t_64_sync+0x18c/0x190
Unable to handle kernel execute from non-executable memory at
virtual address ffffffc008000000
Mem abort info:
ESR = 0x000000008600000f
EC = 0x21: IABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x0f: level 3 permission fault
swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000082863000
[ffffffc008000000] pgd=100000027ffff003, p4d=100000027ffff003,
pud=100000027ffff003, pmd=100000027fffe003, pte=00680001001c3703
Internal error: Oops: 8600000f [#1] PREEMPT SMP

I'm not sure if that's a sign that something is missing with your patch or not.

-Doug

2022-07-08 17:04:42

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] arm64: Fix pending single-step debugging issues

On Fri, Jul 01, 2022 at 03:14:16PM -0700, Doug Anderson wrote:
> Hi,
>
> On Tue, May 10, 2022 at 11:05 PM Sumit Garg <[email protected]> wrote:
> >
> > This patch-set reworks pending fixes from Wei's series [1] to make
> > single-step debugging via kgdb/kdb on arm64 work as expected. There was
> > a prior discussion on ML [2] regarding if we should keep the interrupts
> > enabled during single-stepping. So patch #1 follows suggestion from Will
> > [3] to not disable interrupts during single stepping but rather skip
> > single stepping within interrupt handler.
> >
> > [1] https://lore.kernel.org/all/[email protected]/
> > [2] https://lore.kernel.org/all/CAD=FV=Voyfq3Qz0T3RY+aYWYJ0utdH=P_AweB=13rcV8GDBeyQ@mail.gmail.com/
> > [3] https://lore.kernel.org/all/20200626095551.GA9312@willie-the-truck/
> >
> > Changes in v3:
> > - Reword commit descriptions as per Daniel's suggestions.
> >
> > Changes in v2:
> > - Replace patch #1 to rather follow Will's suggestion.
> >
> > Sumit Garg (2):
> > arm64: entry: Skip single stepping into interrupt handlers
> > arm64: kgdb: Set PSTATE.SS to 1 to re-enable single-step
> >
> > arch/arm64/include/asm/debug-monitors.h | 1 +
> > arch/arm64/kernel/debug-monitors.c | 5 +++++
> > arch/arm64/kernel/entry-common.c | 18 +++++++++++++++++-
> > arch/arm64/kernel/kgdb.c | 2 ++
> > 4 files changed, 25 insertions(+), 1 deletion(-)
>
> Sorry it took so long for me to respond. I kept dreaming that I'd find
> the time to really dig deep into this to understand it fully and I'm
> finally giving up on it. I'm going to hope that Will and/or Catalin
> knows this area of the code well and can give it a good review. If not
> then I'll strive harder to make the time...

So the good news is that I spent a couple of days on this last week.

The bad news is that I'm nowhere done and about to disappear on holiday
for a week!

But anyway, I might be able to give this a review when I'm back. Failing
that, I wonder if enough of us have a little bit of time each that we
could hack on an agreed design together which covers the debug exception
behaviour beyond kgdb?

Will

2022-07-08 17:08:30

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] arm64: Fix pending single-step debugging issues

Hi,

On Fri, Jul 8, 2022 at 9:31 AM Will Deacon <[email protected]> wrote:
>
> On Fri, Jul 01, 2022 at 03:14:16PM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Tue, May 10, 2022 at 11:05 PM Sumit Garg <[email protected]> wrote:
> > >
> > > This patch-set reworks pending fixes from Wei's series [1] to make
> > > single-step debugging via kgdb/kdb on arm64 work as expected. There was
> > > a prior discussion on ML [2] regarding if we should keep the interrupts
> > > enabled during single-stepping. So patch #1 follows suggestion from Will
> > > [3] to not disable interrupts during single stepping but rather skip
> > > single stepping within interrupt handler.
> > >
> > > [1] https://lore.kernel.org/all/[email protected]/
> > > [2] https://lore.kernel.org/all/CAD=FV=Voyfq3Qz0T3RY+aYWYJ0utdH=P_AweB=13rcV8GDBeyQ@mail.gmail.com/
> > > [3] https://lore.kernel.org/all/20200626095551.GA9312@willie-the-truck/
> > >
> > > Changes in v3:
> > > - Reword commit descriptions as per Daniel's suggestions.
> > >
> > > Changes in v2:
> > > - Replace patch #1 to rather follow Will's suggestion.
> > >
> > > Sumit Garg (2):
> > > arm64: entry: Skip single stepping into interrupt handlers
> > > arm64: kgdb: Set PSTATE.SS to 1 to re-enable single-step
> > >
> > > arch/arm64/include/asm/debug-monitors.h | 1 +
> > > arch/arm64/kernel/debug-monitors.c | 5 +++++
> > > arch/arm64/kernel/entry-common.c | 18 +++++++++++++++++-
> > > arch/arm64/kernel/kgdb.c | 2 ++
> > > 4 files changed, 25 insertions(+), 1 deletion(-)
> >
> > Sorry it took so long for me to respond. I kept dreaming that I'd find
> > the time to really dig deep into this to understand it fully and I'm
> > finally giving up on it. I'm going to hope that Will and/or Catalin
> > knows this area of the code well and can give it a good review. If not
> > then I'll strive harder to make the time...
>
> So the good news is that I spent a couple of days on this last week.

Excellent, thanks!


> The bad news is that I'm nowhere done and about to disappear on holiday
> for a week!

No worries. It's been broken for so many years and isn't the most
urgent thing, but it's also one of those things that I do eventually
want to get fixed so I just want to make sure it doesn't get put off
indefinitely...


> But anyway, I might be able to give this a review when I'm back. Failing
> that, I wonder if enough of us have a little bit of time each that we
> could hack on an agreed design together which covers the debug exception
> behaviour beyond kgdb?

If it will unblock this then I can figure out how to make time for it.
Definitely my biggest utility would be in testing, but I'm also happy
to knock around ideas. ...and as per above if the way to move forward
is to block off a day or two to learn more about all the status bits
and how they interact in the kernel then I can do that too. Barring
unforeseen circumstances I'll also be in Dublin in September if that's
somehow useful. ;-)

-Doug

2022-07-11 12:59:54

by Sumit Garg

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] arm64: Fix pending single-step debugging issues

Hi Will,

On Fri, 8 Jul 2022 at 22:01, Will Deacon <[email protected]> wrote:
>
> On Fri, Jul 01, 2022 at 03:14:16PM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Tue, May 10, 2022 at 11:05 PM Sumit Garg <[email protected]> wrote:
> > >
> > > This patch-set reworks pending fixes from Wei's series [1] to make
> > > single-step debugging via kgdb/kdb on arm64 work as expected. There was
> > > a prior discussion on ML [2] regarding if we should keep the interrupts
> > > enabled during single-stepping. So patch #1 follows suggestion from Will
> > > [3] to not disable interrupts during single stepping but rather skip
> > > single stepping within interrupt handler.
> > >
> > > [1] https://lore.kernel.org/all/[email protected]/
> > > [2] https://lore.kernel.org/all/CAD=FV=Voyfq3Qz0T3RY+aYWYJ0utdH=P_AweB=13rcV8GDBeyQ@mail.gmail.com/
> > > [3] https://lore.kernel.org/all/20200626095551.GA9312@willie-the-truck/
> > >
> > > Changes in v3:
> > > - Reword commit descriptions as per Daniel's suggestions.
> > >
> > > Changes in v2:
> > > - Replace patch #1 to rather follow Will's suggestion.
> > >
> > > Sumit Garg (2):
> > > arm64: entry: Skip single stepping into interrupt handlers
> > > arm64: kgdb: Set PSTATE.SS to 1 to re-enable single-step
> > >
> > > arch/arm64/include/asm/debug-monitors.h | 1 +
> > > arch/arm64/kernel/debug-monitors.c | 5 +++++
> > > arch/arm64/kernel/entry-common.c | 18 +++++++++++++++++-
> > > arch/arm64/kernel/kgdb.c | 2 ++
> > > 4 files changed, 25 insertions(+), 1 deletion(-)
> >
> > Sorry it took so long for me to respond. I kept dreaming that I'd find
> > the time to really dig deep into this to understand it fully and I'm
> > finally giving up on it. I'm going to hope that Will and/or Catalin
> > knows this area of the code well and can give it a good review. If not
> > then I'll strive harder to make the time...
>
> So the good news is that I spent a couple of days on this last week.
>

Thanks for spending time to review this.

> The bad news is that I'm nowhere done and about to disappear on holiday
> for a week!
>
> But anyway, I might be able to give this a review when I'm back. Failing
> that, I wonder if enough of us have a little bit of time each that we
> could hack on an agreed design together which covers the debug exception
> behaviour beyond kgdb?

Sure I will be happy to contribute to improving overall debug
exception behaviour. I look forward to any further
comments/suggestions.

-Sumit

>
> Will

2022-07-11 13:18:58

by Sumit Garg

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] arm64: Fix pending single-step debugging issues

Hi Doug,

On Sat, 2 Jul 2022 at 03:44, Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Tue, May 10, 2022 at 11:05 PM Sumit Garg <[email protected]> wrote:
> >
> > This patch-set reworks pending fixes from Wei's series [1] to make
> > single-step debugging via kgdb/kdb on arm64 work as expected. There was
> > a prior discussion on ML [2] regarding if we should keep the interrupts
> > enabled during single-stepping. So patch #1 follows suggestion from Will
> > [3] to not disable interrupts during single stepping but rather skip
> > single stepping within interrupt handler.
> >
> > [1] https://lore.kernel.org/all/[email protected]/
> > [2] https://lore.kernel.org/all/CAD=FV=Voyfq3Qz0T3RY+aYWYJ0utdH=P_AweB=13rcV8GDBeyQ@mail.gmail.com/
> > [3] https://lore.kernel.org/all/20200626095551.GA9312@willie-the-truck/
> >
> > Changes in v3:
> > - Reword commit descriptions as per Daniel's suggestions.
> >
> > Changes in v2:
> > - Replace patch #1 to rather follow Will's suggestion.
> >
> > Sumit Garg (2):
> > arm64: entry: Skip single stepping into interrupt handlers
> > arm64: kgdb: Set PSTATE.SS to 1 to re-enable single-step
> >
> > arch/arm64/include/asm/debug-monitors.h | 1 +
> > arch/arm64/kernel/debug-monitors.c | 5 +++++
> > arch/arm64/kernel/entry-common.c | 18 +++++++++++++++++-
> > arch/arm64/kernel/kgdb.c | 2 ++
> > 4 files changed, 25 insertions(+), 1 deletion(-)
>
> Sorry it took so long for me to respond. I kept dreaming that I'd find
> the time to really dig deep into this to understand it fully and I'm
> finally giving up on it.

No worries and apologies on my part as well as I had to find some time
to reproduce the issue that you have reported below.

> I'm going to hope that Will and/or Catalin
> knows this area of the code well and can give it a good review. If not
> then I'll strive harder to make the time...
>
> In any case, I poked around with this a bunch and it definitely
> improved the stepping behavior a whole lot. I still got one case where
> gdb hit an assertion while I was stepping, but I could believe that
> was a problem with gdb? I couldn't reproduce it. Thus I can at least
> give:
>
> Tested-by: Douglas Anderson <[email protected]>
>

Thanks for the testing.

> I'll also note that I _think_ I remember that with Wei's series that
> the gdb function "call" started working. I tried that here and it
> didn't seem so happy. To keep things simple, I created a dummy
> function in my kernel that looked like:
>
> void doug_test(void)
> {
> pr_info("testing, 1 2 3\n");
> }
>
> I broke into the debugger by echoing "g" to /proc/sysrq-trigger and
> then tried "call doug_test()". I guess my printout actually printed
> but it wasn't so happy after that. Seems like it somehow ended up
> returning to a bogus address after the call which then caused a crash.
>

I am able to reproduce this issue on my setup as well. But it doesn't
seem to be a regression caused by this patch-set over Wei's series. As
I could reproduce this issue with v1 [1] patch-set as well which was
just a forward port of pending patches from Wei's series to the latest
upstream.

Maybe it's a different regression caused by other changes? BTW, do you
remember the kernel version you tested with Wei's series applied?

[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/T/

-Sumit

> testing, 1 2 3
> BUG: sleeping function called from invalid context at
> arch/arm64/mm/fault.c:593
> in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 3393, name: bash
> preempt_count: 0, expected: 0
> RCU nest depth: 1, expected: 0
> CPU: 6 PID: 3393 Comm: bash Not tainted 5.19.0-rc4+ #3
> dbec0bdb8582e447bccdcf2e70d7fe04477b1aac
> Hardware name: Google Herobrine (rev1+) (DT)
> Call trace:
> dump_backtrace+0xf0/0x110
> show_stack+0x24/0x70
> dump_stack_lvl+0x64/0x7c
> dump_stack+0x18/0x38
> __might_resched+0x144/0x154
> __might_sleep+0x54/0x84
> do_page_fault+0x1d4/0x42c
> do_mem_abort+0x4c/0xb0
> el1_abort+0x3c/0x5c
> el1h_64_sync_handler+0x4c/0xc4
> el1h_64_sync+0x64/0x68
> 0xffffffc008000000
> __handle_sysrq+0x15c/0x184
> write_sysrq_trigger+0x94/0x128
> proc_reg_write+0xbc/0xec
> vfs_write+0xf0/0x2c8
> ksys_write+0x84/0xf0
> __arm64_sys_write+0x28/0x34
> invoke_syscall+0x4c/0x120
> el0_svc_common+0x94/0xfc
> do_el0_svc+0x38/0xc0
> el0_svc+0x2c/0x7c
> el0t_64_sync_handler+0x48/0x114
> el0t_64_sync+0x18c/0x190
> Unable to handle kernel execute from non-executable memory at
> virtual address ffffffc008000000
> Mem abort info:
> ESR = 0x000000008600000f
> EC = 0x21: IABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x0f: level 3 permission fault
> swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000082863000
> [ffffffc008000000] pgd=100000027ffff003, p4d=100000027ffff003,
> pud=100000027ffff003, pmd=100000027fffe003, pte=00680001001c3703
> Internal error: Oops: 8600000f [#1] PREEMPT SMP
>
> I'm not sure if that's a sign that something is missing with your patch or not.
>
> -Doug

2022-07-11 14:15:46

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] arm64: Fix pending single-step debugging issues

Hi,

On Mon, Jul 11, 2022 at 5:44 AM Sumit Garg <[email protected]> wrote:
>
> > I'll also note that I _think_ I remember that with Wei's series that
> > the gdb function "call" started working. I tried that here and it
> > didn't seem so happy. To keep things simple, I created a dummy
> > function in my kernel that looked like:
> >
> > void doug_test(void)
> > {
> > pr_info("testing, 1 2 3\n");
> > }
> >
> > I broke into the debugger by echoing "g" to /proc/sysrq-trigger and
> > then tried "call doug_test()". I guess my printout actually printed
> > but it wasn't so happy after that. Seems like it somehow ended up
> > returning to a bogus address after the call which then caused a crash.
> >
>
> I am able to reproduce this issue on my setup as well. But it doesn't
> seem to be a regression caused by this patch-set over Wei's series. As
> I could reproduce this issue with v1 [1] patch-set as well which was
> just a forward port of pending patches from Wei's series to the latest
> upstream.
>
> Maybe it's a different regression caused by other changes? BTW, do you
> remember the kernel version you tested with Wei's series applied?

Sorry, I don't remember! :( I can't even be 100% sure that I'm
remembering correctly that I tested it back in the day, so it's
possible that it simply never worked...

-Doug

2022-07-11 14:16:42

by Sumit Garg

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] arm64: Fix pending single-step debugging issues

On Mon, 11 Jul 2022 at 19:17, Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Mon, Jul 11, 2022 at 5:44 AM Sumit Garg <[email protected]> wrote:
> >
> > > I'll also note that I _think_ I remember that with Wei's series that
> > > the gdb function "call" started working. I tried that here and it
> > > didn't seem so happy. To keep things simple, I created a dummy
> > > function in my kernel that looked like:
> > >
> > > void doug_test(void)
> > > {
> > > pr_info("testing, 1 2 3\n");
> > > }
> > >
> > > I broke into the debugger by echoing "g" to /proc/sysrq-trigger and
> > > then tried "call doug_test()". I guess my printout actually printed
> > > but it wasn't so happy after that. Seems like it somehow ended up
> > > returning to a bogus address after the call which then caused a crash.
> > >
> >
> > I am able to reproduce this issue on my setup as well. But it doesn't
> > seem to be a regression caused by this patch-set over Wei's series. As
> > I could reproduce this issue with v1 [1] patch-set as well which was
> > just a forward port of pending patches from Wei's series to the latest
> > upstream.
> >
> > Maybe it's a different regression caused by other changes? BTW, do you
> > remember the kernel version you tested with Wei's series applied?
>
> Sorry, I don't remember! :( I can't even be 100% sure that I'm
> remembering correctly that I tested it back in the day, so it's
> possible that it simply never worked...

Okay, no worries. Let me see if I can come up with a separate fix for this.

-Sumit

>
> -Doug

2022-08-04 10:17:58

by Sumit Garg

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] arm64: Fix pending single-step debugging issues

On Mon, 11 Jul 2022 at 19:21, Sumit Garg <[email protected]> wrote:
>
> On Mon, 11 Jul 2022 at 19:17, Doug Anderson <[email protected]> wrote:
> >
> > Hi,
> >
> > On Mon, Jul 11, 2022 at 5:44 AM Sumit Garg <[email protected]> wrote:
> > >
> > > > I'll also note that I _think_ I remember that with Wei's series that
> > > > the gdb function "call" started working. I tried that here and it
> > > > didn't seem so happy. To keep things simple, I created a dummy
> > > > function in my kernel that looked like:
> > > >
> > > > void doug_test(void)
> > > > {
> > > > pr_info("testing, 1 2 3\n");
> > > > }
> > > >
> > > > I broke into the debugger by echoing "g" to /proc/sysrq-trigger and
> > > > then tried "call doug_test()". I guess my printout actually printed
> > > > but it wasn't so happy after that. Seems like it somehow ended up
> > > > returning to a bogus address after the call which then caused a crash.
> > > >
> > >
> > > I am able to reproduce this issue on my setup as well. But it doesn't
> > > seem to be a regression caused by this patch-set over Wei's series. As
> > > I could reproduce this issue with v1 [1] patch-set as well which was
> > > just a forward port of pending patches from Wei's series to the latest
> > > upstream.
> > >
> > > Maybe it's a different regression caused by other changes? BTW, do you
> > > remember the kernel version you tested with Wei's series applied?
> >
> > Sorry, I don't remember! :( I can't even be 100% sure that I'm
> > remembering correctly that I tested it back in the day, so it's
> > possible that it simply never worked...
>
> Okay, no worries. Let me see if I can come up with a separate fix for this.
>

After digging deep into GDB call function operations for aarch64, it
is certain that function calls simply never worked due to below
reasons:

1. On aarch64, GDB call function inserts a breakpoint at the
entrypoint of kernel (which is ffffffc008000000 from your dump) as
return address from function called. And since it refers to the
"_text" symbol which is marked non-executable as the actual text
section starts with the "_stext" symbol. I did a following hack that
makes it executable:

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 626ec32873c6..e39ad1a5f5d6 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -700,7 +700,7 @@ static bool arm64_early_this_cpu_has_bti(void)
static void __init map_kernel(pgd_t *pgdp)
{
static struct vm_struct vmlinux_text, vmlinux_rodata, vmlinux_inittext,
- vmlinux_initdata, vmlinux_data;
+ vmlinux_initdata, vmlinux_data, vmlinux_htext;

/*
* External debuggers may need to write directly to the text
@@ -721,6 +721,8 @@ static void __init map_kernel(pgd_t *pgdp)
* Only rodata will be remapped with different permissions later on,
* all other segments are allowed to use contiguous mappings.
*/
+ map_kernel_segment(pgdp, _text, _stext, text_prot, &vmlinux_htext, 0,
+ VM_NO_GUARD);
map_kernel_segment(pgdp, _stext, _etext, text_prot, &vmlinux_text, 0,
VM_NO_GUARD);
map_kernel_segment(pgdp, __start_rodata, __inittext_begin, PAGE_KERNEL,

2. For the GDB function "call" to work, GDB inserts a dummy stack
frame. But in case of kernel on aarch64, the stack used is common
among the exception handler and the kernel threads. So it's not
trivial to insert a dummy stack frame and requires rework of exception
entry code as it pushes pt_regs onto the stack.

-Sumit

>
> >
> > -Doug