Re: [PATCH RFC 1/6] perf/x86: Add perf text poke event

[Mike Leach <mike.leach@linaro.org>]

Hi Leo,

On Wed, 30 Oct 2019 at 14:20, Leo Yan <leo.yan@linaro.org> wrote:
>
> Hi Peter,
>
> [ + Mike Leach ]
>
> On Wed, Oct 30, 2019 at 01:46:59PM +0100, Peter Zijlstra wrote:
> > On Wed, Oct 30, 2019 at 06:47:47PM +0800, Leo Yan wrote:
> > > Hi Adrian,
> > >
> > > On Fri, Oct 25, 2019 at 03:59:55PM +0300, Adrian Hunter wrote:
> > > > Record changes to kernel text (i.e. self-modifying code) in order to
> > > > support tracers like Intel PT decoding through jump labels.
> > > >
> > > > A copy of the running kernel code is needed as a reference point
> > > > (e.g. from /proc/kcore). The text poke event records the old bytes
> > > > and the new bytes so that the event can be processed forwards or backwards.
> > > >
> > > > The text poke event has 'flags' to describe when the event is sent. At
> > > > present the only flag is PERF_TEXT_POKE_UPDATE which indicates the
> > > > point at which tools should update their reference kernel executable to
> > > > change the old bytes to the new bytes.
> > > >
> > > > Other architectures may wish to emit a pair of text poke events. One before
> > > > and one after a text poke. In that case, PERF_TEXT_POKE_UPDATE flag would
> > > > be set on only one of the pair.
> > >
> > > Thanks a lot for the patch set.
> > >
> > > Below is my understanding for implementation 'emit a pair of text poke
> > > events' as you mentioned:
> > >
> > > Since Arm64 instruction is fixed size, it doesn't need to rely on INT3
> > > liked mechanism to replace instructions and directly uses two operations
> > > to alter instruction (modify instruction and flush icache line).  So
> > > Arm64 has no chance to send perf event in the middle of altering
> > > instruction.
> >
> > Right.
> >
> > > Thus we can send pair of text poke events in the kernel:
> > >
> > >   perf_event_text_poke(PERF_TEXT_POKE_UPDATE_PREV)
> > >
> > >     Change instruction
> > >     Flush icache
> > >
> > >   perf_event_text_poke(PERF_TEXT_POKE_UPDATE_POST)
> > >
> > > In the userspace, if perf tool detects the instruction is changed
> > > from nop to branch,
> >
> > There is _far_ more text poking than just jump_label's NOP/JMP
> > transitions. Ftrace also does NOP/CALL, CALL/CALL, and the static_call
> > infrastructure that I posted here:
> >
> >   https://lkml.kernel.org/r/20191007082708.013939311@infradead.org
> >
> > Has: JMP/RET, JMP/JMP and if it has inline patching support: NOP/CALL,
> > CALL/CALL, patching.
>
> Thanks for the info.  I took a bit time to look your patch set and
> Steven's patch set for dynamic function, though don't completely
> understand, but get more sense for the context.
>
> > Anyway, the below argument doesn't care much, it works for NOP/JMP just
> > fine.
>
> We can support NOP/JMP case as the first step, but later should can
> extend to support other transitions.
>
> > > we need to update dso cache for the
> > > 'PERF_TEXT_POKE_UPDATE_PREV' event; if detect the instruction is
> > > changed from branch to nop, we need to update dso cache for
> > > 'PERF_TEXT_POKE_UPDATE_POST' event.  The main idea is to ensure the
> > > branch instructions can be safely contained in the dso file and any
> > > branch samples can read out correct branch instruction.
> > >
> > > Could you confirm this is the same with your understanding?  Or I miss
> > > anything?  I personally even think the pair events can be used for
> > > different arches (e.g. the solution can be reused on Arm64/x86, etc).
> >
> > So the problem we have with PT is that it is a bit-stream of
> > branch taken/not-taken decisions. In order to decode that we need to
> > have an accurate view of the unconditional code flow.
> >
> > Both NOP/JMP are unconditional and we need to exactly know which of the
> > two was encountered.
>
> If I understand correctly, PT decoder needs to read out instructions
> from dso and decide the instruction type (NOP or JMP), and finally
> generate the accurate code flow.
>
> So PT decoder relies on (cached) DSO for decoding.  As I know, this
> might be different from Arm CS, since Arm CS decoder is merely
> generate packets and it doesn't need to rely on DSO for decoding.
>
> I think my answer is not very convinced, in case I mislead, loop Mike
> to confirm for this.
>
The CoreSight decoder needs the same information. When a branch / call
instruction is traced an atom is generated in the trace to determine
if that call as taken or not.
During the decode process the decoder walks the code image to match up
atoms to call / branch instructions - which means for correct trace
decode we must have an accurate code image as executed, as otherwise
trace decode will go wrong.
Within perf, the OpenCSD decoder will call back into perf to ask for
the memory image for a given address - which perf will supply from the
list of .so / executable files it has in the .debug directory.

Self modifying code presents an issue in this respect.

Correlation of the modification information with the captured trace
becomes the primary concern.

Regards

Mike


> > With your scheme, I don't see how we can ever actually know that. When
> > we get the PRE event, all we really know is that we're going to change
> > a specific instruction into another. And at the POST event we know it
> > has been done. But in between these two events, we have no clue which of
> > the two instructions is live on which CPU (two CPUs might in fact have a
> > different live instruction at the same time).
> >
> > This means we _cannot_ unambiguously decode a taken/not-taken decision
> > stream.
> >
> > Does CS have this same problem, and how would the PRE/POST events help
> > with that?
>
> My purpose is to use PRE event and POST event to update cached DSO,
> thus perf tool can read out 'correct' instructions and fill them into
> instruction/branch samples.  On the other hand, as mentioned, I don't
> aware the instruction read out from DSO will be used for Arm CS decoder;
> Arm CS also reads the instructions from DSO, usually it's used to decide
> instruction size or decide the sample flags (flags for CALL/RETURN/hw
> int, etc...), but it isn't used for decoder.
>
> @Mike, please help confirm for this as well, from Arm CoreSight's
> decoder pespective.
>
> > So our (x86) horrible (variable instruction length induced) complexity
> > for text poking is actually in our advantage this one time. The
> > exception and exception-return paths allow us to unambiguously know what
> > happens around the time of poking.
>
> Thanks a lot for the info!
> Leo.



-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK