lkml.org 
[lkml]   [2019]   [Dec]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.4 163/434] perf tests: Disable bp_signal testing for arm64
    Date
    From: Leo Yan <leo.yan@linaro.org>

    [ Upstream commit 6a5f3d94cb69a185b921cb92c39888dc31009acb ]

    As there are several discussions for enabling perf breakpoint signal
    testing on arm64 platform: arm64 needs to rely on single-step to execute
    the breakpointed instruction and then reinstall the breakpoint exception
    handler. But if we hook the breakpoint with a signal, the signal
    handler will do the stepping rather than the breakpointed instruction,
    this causes infinite loops as below:

    Kernel space | Userspace
    ---------------------------------|--------------------------------
    | __test_function() -> hit
    | breakpoint
    breakpoint_handler() |
    `-> user_enable_single_step() |
    do_signal() |
    | sig_handler() -> Step one
    | instruction and
    | trap to kernel
    single_step_handler() |
    `-> reinstall_suspended_bps() |
    | __test_function() -> hit
    | breakpoint again and
    | repeat up flow infinitely

    As Will Deacon mentioned [1]: "that we require the overflow handler to
    do the stepping on arm/arm64, which is relied upon by GDB/ptrace. The
    hw_breakpoint code is a complete disaster so my preference would be to
    rip out the perf part and just implement something directly in ptrace,
    but it's a pretty horrible job". Though Will commented this on arm
    architecture, but the comment also can apply on arm64 architecture.

    For complete information, I searched online and found a few years back,
    Wang Nan sent one patch 'arm64: Store breakpoint single step state into
    pstate' [2]; the patch tried to resolve this issue by avoiding single
    stepping in signal handler and defer to enable the signal stepping when
    return to __test_function(). The fixing was not merged due to the
    concern for missing to handle different usage cases.

    Based on the info, the most feasible way is to skip Perf breakpoint
    signal testing for arm64 and this could avoid the duplicate
    investigation efforts when people see the failure. This patch skips
    this case on arm64 platform, which is same with arm architecture.

    [1] https://lkml.org/lkml/2018/11/15/205
    [2] https://lkml.org/lkml/2015/12/23/477

    Signed-off-by: Leo Yan <leo.yan@linaro.org>
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
    Cc: Brajeswar Ghosh <brajeswar.linux@gmail.com>
    Cc: Florian Fainelli <f.fainelli@gmail.com>
    Cc: Jiri Olsa <jolsa@redhat.com>
    Cc: Mark Rutland <mark.rutland@arm.com>
    Cc: Michael Petlan <mpetlan@redhat.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Song Liu <songliubraving@fb.com>
    Cc: Souptick Joarder <jrdr.linux@gmail.com>
    Cc: Will Deacon <will@kernel.org>
    Link: http://lore.kernel.org/lkml/20191018085531.6348-3-leo.yan@linaro.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    tools/perf/tests/bp_signal.c | 15 ++++++---------
    1 file changed, 6 insertions(+), 9 deletions(-)

    diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c
    index c1c2c13de254..166f411568a5 100644
    --- a/tools/perf/tests/bp_signal.c
    +++ b/tools/perf/tests/bp_signal.c
    @@ -49,14 +49,6 @@ asm (
    "__test_function:\n"
    "incq (%rdi)\n"
    "ret\n");
    -#elif defined (__aarch64__)
    -extern void __test_function(volatile long *ptr);
    -asm (
    - ".globl __test_function\n"
    - "__test_function:\n"
    - "str x30, [x0]\n"
    - "ret\n");
    -
    #else
    static void __test_function(volatile long *ptr)
    {
    @@ -302,10 +294,15 @@ bool test__bp_signal_is_supported(void)
    * stepping into the SIGIO handler and getting stuck on the
    * breakpointed instruction.
    *
    + * Since arm64 has the same issue with arm for the single-step
    + * handling, this case also gets suck on the breakpointed
    + * instruction.
    + *
    * Just disable the test for these architectures until these
    * issues are resolved.
    */
    -#if defined(__powerpc__) || defined(__s390x__) || defined(__arm__)
    +#if defined(__powerpc__) || defined(__s390x__) || defined(__arm__) || \
    + defined(__aarch64__)
    return false;
    #else
    return true;
    --
    2.20.1


    \
     
     \ /
      Last update: 2019-12-29 19:08    [W:5.264 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site