All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Dmitry Safonov <dsafonov@virtuozzo.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Shuah Khan <shuahkh@osg.samsung.com>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Borislav Petkov <bp@alien8.de>,
	khorenko@virtuozzo.com, X86 ML <x86@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	xemul@virtuozzo.com, linux-kselftest@vger.kernel.org,
	Cyrill Gorcunov <gorcunov@openvz.org>,
	Dmitry Safonov <0x7f454c46@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH 1/2] x86/arch_prctl: add ARCH_SET_{COMPAT,NATIVE} to change compatible mode
Date: Wed, 20 Apr 2016 21:05:07 +0200	[thread overview]
Message-ID: <20160420190507.GF3408@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <CALCETrUxiT5Y9GMf0pRFXoh2wMACMdGbbBeT2zoy38idS-fC5g@mail.gmail.com>

On Wed, Apr 20, 2016 at 08:40:23AM -0700, Andy Lutomirski wrote:
> Do LBR, PEBS, and similar report user regs or do they merely want to
> know the instruction format?  If the latter, I could whip up a tiny
> function to do just that (like perf_get_regs_user but just for ABI --
> it would be simpler).

Just the instruction format, nothing else.

> >> Peter, I got lost in the code that calls this.  Are regs coming from
> >> the overflow interrupt's regs, current_pt_regs(), or
> >> perf_get_regs_user?
> >
> > So get_perf_callchain() will get regs from:
> >
> >  - interrupt/NMI regs
> >  - perf_arch_fetch_caller_regs()
> >
> > And when user && !user_mode(), we'll use:
> >
> >  - task_pt_regs() (which arguably should maybe be perf_get_regs_user())
> 
> Could you point me to this bit of the code?

kernel/events/callchain.c:198

> > to call perf_callchain_user(), which then, ands up calling
> > perf_callchain_user32() which is expected to NO-OP for 64bit userspace.
> >
> >> If it's the perf_get_regs_user, then this should be okay, but passing
> >> in the ABI field directly would be even nicer.  If they're coming from
> >> the overflow interrupt's regs or current_pt_regs(), could we change
> >> that?
> >>
> >> It might also be nice to make sure that we call perf_get_regs_user
> >> exactly once per overflow interrupt -- i.e. we could push it into the
> >> main code rather than the regs sampling code.
> >
> > The risk there is that we might not need the user regs at all to handle
> > the overflow thingy, so doing it unconditionally would be unwanted.
> 
> One call to perf_get_user_regs per interrupt shouldn't be too bad --
> certainly much better then one per PEBS record.  One call to get user
> ABI per overflow would be even less bad, but at that point, folding it
> in to the PEBS code wouldn't be so bad either.

Right; although note that the whole fixup_ip() thing requires a single
record per interrupt (for we need the LBR state for each record in order
to rewind).

Also, HSW+ PEBS doesn't do the fixup anymore.

> If I'm understanding this right (a big, big if), if we get a PEBS
> overflow while running in user mode, we'll dump out the user regs (and
> call perf_get_regs_user) and all the PEBS entries (subject to
> exclude_kernel and with all the decoding magic).  So, in that case, we
> call perf_get_user_regs.

We only dump user regs if PERF_SAMPLE_REGS_USER, and in case we hit
userspace userspace with the interrupt we use the interrupt regs; see
perf_sample_regs_user().

> If we get a PEBS overflow while running in kernel mode, we'll report
> the kernel regs (if !exclude_kernel) and report the PEBS data as well.
> If any of those records are in user mode, then, ideally, we'd invoke
> perf_get_regs_user or similar *once* to get the ABI.  Although, if we
> can get the user ABI efficiently enough, then maybe we don't care if
> we call it once per PEBS record.

Right, if we interrupt kernel mode, we'll call perf_get_regs_user() if
PERF_SAMPLE_REGS_USER (| PERF_SAMPLE_STACK_USER).

The problem here is that the overflow stuff is designed for a single
'event' per interrupt, so passing it data for multiple events is
somewhat icky.

  reply	other threads:[~2016-04-20 19:05 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-06 16:29 [PATCH 0/2] x86: add arch_prctl to switch between native/compat modes Dmitry Safonov
2016-04-06 16:29 ` [PATCH 1/2] x86/arch_prctl: add ARCH_SET_{COMPAT,NATIVE} to change compatible mode Dmitry Safonov
2016-04-06 18:04   ` Andy Lutomirski
2016-04-06 18:49     ` Andy Lutomirski
2016-04-07 12:11     ` Dmitry Safonov
2016-04-07 12:21       ` Cyrill Gorcunov
2016-04-07 12:35         ` Dmitry Safonov
2016-04-07 14:39       ` Andy Lutomirski
2016-04-07 15:18         ` Dmitry Safonov
2016-04-08 13:50         ` Dmitry Safonov
2016-04-08 15:56           ` Andy Lutomirski
2016-04-08 16:18             ` Dmitry Safonov
2016-04-08 20:44               ` Andy Lutomirski
2016-04-09  8:06                 ` Dmitry Safonov
2016-04-13 16:55                 ` Dmitry Safonov
2016-04-14 18:27                   ` Andy Lutomirski
2016-04-20 11:04                     ` Peter Zijlstra
2016-04-20 15:40                       ` Andy Lutomirski
2016-04-20 19:05                         ` Peter Zijlstra [this message]
2016-04-21 19:39                           ` Andy Lutomirski
2016-04-21 20:12                             ` Peter Zijlstra
2016-04-21 23:27                               ` Andy Lutomirski
2016-04-21 23:46                                 ` Andy Lutomirski
2016-04-25 15:16                                 ` Peter Zijlstra
2016-04-25 16:50                                   ` Andy Lutomirski
2016-04-06 16:29 ` [PATCH 2/2] x86/tools/testing: add test for ARCH_SET_COMPAT Dmitry Safonov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160420190507.GF3408@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=0x7f454c46@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=dsafonov@virtuozzo.com \
    --cc=gorcunov@openvz.org \
    --cc=hpa@zytor.com \
    --cc=khorenko@virtuozzo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mingo@redhat.com \
    --cc=shuahkh@osg.samsung.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=xemul@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.