MIME-Version: 1.0
References: <20221219102452.2860088-1-sumit.garg@linaro.org>
 <CAFA6WYOucB3JdVATbydxYuAb0Q5P7ff_JMb=-BgiF+uip8pdpg@mail.gmail.com>
 <Y9AdunpawWzWKIXN@aspen.lan> <d6a92e81-0f67-68ce-744d-149bbcdc06d9@arm.com>
In-Reply-To: <d6a92e81-0f67-68ce-744d-149bbcdc06d9@arm.com>
From:   Sumit Garg <sumit.garg@linaro.org>
Date:   Fri, 27 Jan 2023 11:45:57 +0530
Message-ID: <CAFA6WYN=no8NEWYmjxZRk2p5eEnCVyu+5Dw1DNxKaPijXcrByQ@mail.gmail.com>
Subject: Re: [PATCH v5 0/2] arm64: Fix pending single-step debugging issues
To:     Luis Machado <luis.machado@arm.com>
Cc:     Daniel Thompson <daniel.thompson@linaro.org>, will@kernel.org,
        catalin.marinas@arm.com, liwei391@huawei.com, mhiramat@kernel.org,
        maz@kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, mark.rutland@arm.com,
        dianders@chromium.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Luis,

On Wed, 25 Jan 2023 at 14:48, Luis Machado <luis.machado@arm.com> wrote:
>
> Hi,
>
> Is this expected to change single-stepping operation in usespace for debuggers (gdb/lldb)?

No it won't affect user-space debuggers as we are only touching the
interrupt path in EL1 mode.

-Sumit

> If so, it would be nice to at least
> test it a little to make sure it works.
>
> On 1/24/23 18:04, Daniel Thompson wrote:
> > On Thu, Jan 12, 2023 at 02:52:49PM +0530, Sumit Garg wrote:
> >> Hi Will, Catalin,
> >>
> >> On Mon, 19 Dec 2022 at 15:55, Sumit Garg <sumit.garg@linaro.org> 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/20200509214159.19680-1-liwei391@huawei.com/
> >>> [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 v5:
> >>> - Incorporated misc. comments from Mark.
> >>>
> >>
> >> Since patch #1 has already been reviewed/acked by Mark and the
> >> complete patchset has been tested by Doug, would it be fine for you to
> >> pick up this patchset? It fixes a real single stepping problem for
> >> kgdb on arm64.
> >
> > Sorry to be quiet for so long.
> >
> > Testing this patch set has proven to be a little difficult.
> >
> > It certainly fixes the single step tests in the kgdbtest suite.
> > That's a good start.
> >
> > Unfortunately when testing using qemu/KVM (hosted on NXP
> > 2k/Solidrun Honeycomb) the patch set is resulting in instability
> > running the built-in self tests (specifically this one:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/misc/kgdbts.c#n74 ). Running this test using the kgdbtest harness
> > results in the test failing roughly a third of the time.
> >
> > The error reported is that the trap handler tried to unlock a spinlock
> > that isn't currently locked. To be honest I suspect this is a generic
> > problem that the new feature happens to tickle (this test has
> > historically been unreliable on x86 too... and x86 is noteworthy for
> > being the only other platform I test using KVM rather than pure qemu).
> > Of course the only way to prove that would be to find and fix the
> > problem in the trap handler (which probably involves rewriting it) and I
> > haven't managed to do that yet.
> >
> > In short, I think the debugger is more useful with this patchset than
> > without so, although it is caveated by the above, I'd call this:
> >
> > Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> > Tested-by: Daniel Thompson <daniel.thompson@linaro.org>
> >
> >
> > Daniel.
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>