Subject: Re: [PATCH 0/8] Add ftrace direct call for arm64
To:     Xu Kuohai <xukuohai@huawei.com>,
        Florent Revest <revest@chromium.org>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-trace-kernel@vger.kernel.org, bpf@vger.kernel.org
Cc:     catalin.marinas@arm.com, will@kernel.org, rostedt@goodmis.org,
        mhiramat@kernel.org, mark.rutland@arm.com, ast@kernel.org,
        andrii@kernel.org, kpsingh@kernel.org, jolsa@kernel.org,
        xukuohai@huaweicloud.com
References: <20230201163420.1579014-1-revest@chromium.org>
 <85af713d-00fe-b113-1331-1a44480c016f@huawei.com>
From:   Daniel Borkmann <daniel@iogearbox.net>
Message-ID: <eacfbd23-da92-3572-7f57-3de425117c8a@iogearbox.net>
Date:   Thu, 2 Feb 2023 11:50:24 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <85af713d-00fe-b113-1331-1a44480c016f@huawei.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 2/2/23 9:36 AM, Xu Kuohai wrote:
> On 2/2/2023 12:34 AM, Florent Revest wrote:
>> This series adds ftrace direct call support to arm64.
>> This makes BPF tracing programs (fentry/fexit/fmod_ret/lsm) work on arm64.
>>
>> It is meant to apply on top of the arm64 tree which contains Mark Rutland's
>> series on CALL_OPS [1] under the for-next/ftrace tag.
>> > The first three patches consolidate the two existing ftrace APIs for registering
>> direct calls. They are split to make the reviewers lives easier but if it'd be a
>> preferred style, I'd be happy to squash them in the next revision.
>> Currently, there is both a _ftrace_direct and _ftrace_direct_multi API. Apart
>> from samples and selftests, there are no users of the _ftrace_direct API left
>> in-tree so this deletes it and renames the _ftrace_direct_multi API to
>> _ftrace_direct for simplicity.
>>
>> The main benefit of this refactoring is that, with the API that's left, an
>> ftrace_ops backing a direct call will only ever point to one direct call. We can
>> therefore store the direct called trampoline address in the ops (patch 4) and
>> look it up from the ftrace trampoline on arm64 (patch 7) in the case when the
>> destination would be out of reach of a BL instruction at the ftrace callsite.
>> (in this case, ftrace_caller acts as a lightweight intermediary trampoline)
>>
>> This series has been tested on both arm64 and x86_64 with:
>> 1- CONFIG_FTRACE_SELFTEST (cf: patch 6)
>> 2- samples/ftrace/*.ko (cf: patch 8)
>> 3- tools/testing/selftests/bpf/test_progs (both -t lsm and -t fentry_fexit)

Thanks a ton for working on this!

> so it's time to update DENYLIST.aarch64 to unblock tests that failed due to lack of direct call.

+1, with regards to logistics, if possible it might be nice to eventually gets
this into a feature branch on arm64 tree, then we could pull it too from there
for bpf-next and hash out the BPF CI bits for arm64 in the meantime.

>> This follows up on prior art by Xu Kuohai [2].
>> The implementation here is totally different but the fix for ftrace selftests
>> (patch 6) is a trivial rebase of a patch originally by Xu so I kept his
>> authorship and trailers untouched on that patch, I hope that's ok. >
> 
> that's ok for me, thanks.
> 
>> 1: https://lore.kernel.org/all/20230123134603.1064407-1-mark.rutland@arm.com/
>> 2: https://lore.kernel.org/bpf/20220913162732.163631-1-xukuohai@huaweicloud.com/
>>
>> Florent Revest (7):
>>    ftrace: Replace uses of _ftrace_direct APIs with _ftrace_direct_multi
>>    ftrace: Remove the legacy _ftrace_direct API
>>    ftrace: Rename _ftrace_direct_multi APIs to _ftrace_direct APIs
>>    ftrace: Store direct called addresses in their ops
>>    ftrace: Make DIRECT_CALLS work WITH_ARGS and !WITH_REGS
>>    arm64: ftrace: Add direct call support
>>    arm64: ftrace: Add direct called trampoline samples support
>>
>> Xu Kuohai (1):
>>    ftrace: Fix dead loop caused by direct call in ftrace selftest
>>
>>   arch/arm64/Kconfig                          |   4 +
>>   arch/arm64/include/asm/ftrace.h             |  24 ++
>>   arch/arm64/kernel/asm-offsets.c             |   6 +
>>   arch/arm64/kernel/entry-ftrace.S            |  70 +++-
>>   arch/arm64/kernel/ftrace.c                  |  36 +-
>>   include/linux/ftrace.h                      |  51 +--
>>   kernel/bpf/trampoline.c                     |  14 +-
>>   kernel/trace/Kconfig                        |   2 +-
>>   kernel/trace/ftrace.c                       | 433 +-------------------
>>   kernel/trace/trace_selftest.c               |  14 +-
>>   samples/Kconfig                             |   2 +-
>>   samples/ftrace/ftrace-direct-modify.c       |  41 +-
>>   samples/ftrace/ftrace-direct-multi-modify.c |  44 +-
>>   samples/ftrace/ftrace-direct-multi.c        |  28 +-
>>   samples/ftrace/ftrace-direct-too.c          |  35 +-
>>   samples/ftrace/ftrace-direct.c              |  33 +-
>>   16 files changed, 333 insertions(+), 504 deletions(-)
>>
>