Re: bpf: RFC for platform specific BPF helper addition

[Tero Kristo <tero.kristo@linux.intel.com>]

On 25/02/2023 02:01, Alexei Starovoitov wrote:
> On Fri, Feb 24, 2023 at 3:49 AM Tero Kristo <tero.kristo@linux.intel.com> wrote:
>>
>> On 23/02/2023 19:46, Alexei Starovoitov wrote:
>>> On Thu, Feb 23, 2023 at 5:23 AM Tero Kristo <tero.kristo@linux.intel.com> wrote:
>>>> Hi,
>>>>
>>>> Some background first; on x86 platforms there is a free running TSC
>>>> counter which can be used to generate extremely accurate profiling time
>>>> stamps. Currently this can be used by BPF programs via hooking into perf
>>>> subsystem and reading the value there; however this reduces the accuracy
>>>> due to latency + jitter involved with long execution chain, and also the
>>>> timebase gets converted into relative from the start of the execution of
>>>> the program, instead of getting an absolute system level value.
>>> Are you talking about rdtsc or some other counter?
>>> Does it need an arch specific setup?
>> Yes, this is rdtsc. TSC is setup automatically by the arch, but
>> exporting it to BPF takes a few lines of arch specific code (I did use
>> register_btf_kfunc_id_set() during init, under arch/x86/kernel/tsc.c.)
>>>> Now, I do have a pretty trivial patch (under internal review atm. at
>>>> Intel) that adds an x86 platform specific bpf helper that can directly
>>>> read this timestamp counter without relying to perf subsystem hooks.
>>>>
>>>> Do people have any feedback / insights on this list about addition of
>>>> such platform specific BPF helper, basically thumbs up/down for adding
>>>> such a thing? Currently I don't think there are any platform specific
>>>> helpers in the kernel.
>>> Right. That's one of the reasons we don't add new helpers anymore.
>>> Please use kfunc instead. You can add it to:
>>> arch/x86/net/bpf_jit_comp.c
>>> like:
>>> __bpf_kfunc u64 bpf_read_rdtsc(void)
>>> { asm ("...
>>> or to arch specific kernel module.
>>>
>>> Make sure to add selftests when you submit a patch.
Regarding this I got a follow up question, where would you recommend to 
put selftests for such functionality? Any of the BPF selftests appear to 
be generic currently.
>> Ok, I can take a look at the selftest side if things nudge forward,
>> however there is some internal pressure to ditch the whole idea of
>> bpf_rdtsc() due to potential of side channel attacks by using BPF, and
>> exploiting the accurate timer in the process. Any thoughts on that side?
>> Using BPF requires root access nowadays so it is sort of on-par to
>> out-of-tree kernel modules.
> Can you elaborate on that security concern?
> User space can do rdtsc, so not clear how doing the same in bpf prog
> loaded by root makes any difference.
> Unpriv bpf is pretty much non-existent.
> bpf subsystem went root only long ago.

I am working on this internally with our security team atm., and the 
initial assessment is that the concern may not be valid due to the 
points you mention also.

-Tero