Re: [PATCH v17 11/16] fprobe: Rewrite fprobe on function-graph tracer

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Wed, 16 Oct 2024 20:14:54 +0200
Sven Schnelle <svens@linux.ibm.com> wrote:

> Masami Hiramatsu (Google) <mhiramat@kernel.org> writes:
> 
> > On Wed, 16 Oct 2024 14:07:31 +0200
> > Sven Schnelle <svens@linux.ibm.com> wrote:
> >> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> writes:
> >> I think that still has the issue that the size is encoded in the
> >> leftmost fields of the pointer, which doesn't work on all
> >> architectures. I reported this already in v15
> >> (https://lore.kernel.org/all/yt9dmsjyx067.fsf@linux.ibm.com/)
> >
> > Oops, thanks for reporting. I should missed that.
> >
> >> I haven't yet fully understood why this logic is needed, but the
> >> WARN_ON_ONCE triggers on s390. I'm assuming this fails because fp always
> >> has the upper bits of the address set on x86 (and likely others). As an
> >> example, in my test setup, fp is 0x8feec218 on s390, while it is
> >> 0xffff888100add118 in x86-kvm.
> >
> > Ah, so s390 kernel/user memory layout is something like 4G/4G?
> > Hmm, this encode expects the leftmost 4bit is filled. For the
> > architecture which has 32bit address space, we may be possible to
> > use "unsigned long long" for 'val' on shadow stack (and use the
> > first 32bit for fp and another 32bit for size).
> >
> > Anyway, I need to redesign it depending on architecture.
> 
> Could you explain a bit more what redesign means? Thanks!

This "encoded" data is for recording the *fp (the address of fprobe)
and its data size into one value and storing it on the shadow stack.

On x86-64, the kernel objects are puts on the highest memory address,
thus the highest bits are always same. So it uses 4bits for recording
the data size. Most of other 64bit architecture are similar memory
layout, so we can use the highest bits. Note that the data size must
be a multiplier of u64 (== 8byte), so 4bits is enough since shadow
stack size is limited.

The s390 and other 32bit address space architectures need special
care for it. Thus I think we can use 2 slots (= 2 * u32) for saving
data in this case.

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>