LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Hillf Danton <hillf.zj@alibaba-inc.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Vince Weaver <vince@deater.net>,
	Paul Mackerras <paulus@samba.org>,
	Kees Cook <keescook@chromium.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
Subject: Re: [PATCH v2 7/8] x86, perf: Only allow rdpmc if a perf_event is mapped
Date: Mon, 27 Oct 2014 21:27:56 -0700
Message-ID: <CALCETrUPMSPdf_AwhskirnKkED4NcbJ+Lxw+Fdq6ZTxUEUcVxw@mail.gmail.com> (raw)
In-Reply-To: <007e01cff264$bce6afc0$36b40f40$@alibaba-inc.com>

On Mon, Oct 27, 2014 at 9:07 PM, Hillf Danton <hillf.zj@alibaba-inc.com> wrote:
>> >> Subject: Re: [PATCH v2 7/8] x86, perf: Only allow rdpmc if a perf_event is mapped
>> >>
>> > CPU D                   CPU A
>> > switch_mm
>> > load_mm_cr4
>> >                         x86_pmu_event_unmapped
>> >
>> > I wonder if the X86_CR4_PCE set on CPU D is
>> > cleared by CPU A by broadcasting IPI.
>> >
>>
>> It should be okay.  The IPI does:
>>
>> +       if (current->mm)
>> +               load_mm_cr4(current->mm);
>>
>> which refers to the current task running on the targetted CPU, not to
>> the IPI sender's task.  So, if it happens after a context switch, it
>> will harmlessly reload the new task's cr4.
>>
> Right, but prev != next is checked in switch_mm.

If that happens and !cpumask_test_cpu(cpu, mm_cpumask(next)), then cr4
will be reloaded.  So, in the case you described, we should still be
okay.

It's worth checking whether a more complicated race could be a
problem.  I think it's okay.  Here's my argument.

In x86_pmu_event_mapped / x86_pmu_event_unmapped, for each cpu, either
that cpu is set in mm_cpumask or it's clear.  If it's set, then we'll
send the IPI and that cpu is guaranteed to be updated.  If it's clear,
then it must become set before any user code in this mm can be
executed.  There are no paths through switch_mm that set the bit in
mm_cpumask without reloading cr4, so we should be safe.

Is that convincing?

--Andy

>
> Hillf
>
>



-- 
Andy Lutomirski
AMA Capital Management, LLC

  reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-27  7:16 张静(长谷)
2014-10-27 15:45 ` Andy Lutomirski
2014-10-28  3:35   ` Hillf Danton
2014-10-28  3:57     ` Andy Lutomirski
2014-10-28  4:07       ` Hillf Danton
2014-10-28  4:27         ` Andy Lutomirski [this message]
  -- strict thread matches above, loose matches on Subject: below --
2014-10-24 22:58 [PATCH v2 0/8] CR4 handling improvements Andy Lutomirski
2014-10-24 22:58 ` [PATCH v2 7/8] x86, perf: Only allow rdpmc if a perf_event is mapped Andy Lutomirski
2014-10-31 17:54   ` Paolo Bonzini
2014-10-31 18:25     ` Andy Lutomirski

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=CALCETrUPMSPdf_AwhskirnKkED4NcbJ+Lxw+Fdq6ZTxUEUcVxw@mail.gmail.com \
    --to=luto@amacapital.net \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=aarcange@redhat.com \
    --cc=acme@kernel.org \
    --cc=hillf.zj@alibaba-inc.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=vince@deater.net \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lore.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git