linux-kernel - Re: [RFC 2/3] perf/x86: Control RDPMC access from .enable() hook

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 12 Aug 2021 09:50:39 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Rob Herring <robh@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Mark Rutland <mark.rutland@....com>,
        Will Deacon <will@...nel.org>,
        Kan Liang <kan.liang@...ux.intel.com>
Cc:     linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        linux-perf-users@...r.kernel.org
Subject: Re: [RFC 2/3] perf/x86: Control RDPMC access from .enable() hook

On 7/28/21 4:02 PM, Rob Herring wrote:
> Rather than controlling RDPMC access behind the scenes from switch_mm(),
> move RDPMC access controls to the PMU .enable() hook. The .enable() hook
> is called whenever the perf CPU or task context changes which is when
> the RDPMC access may need to change.
> 
> This is the first step in moving the RDPMC state tracking out of the mm
> context to the perf context.

Is this series supposed to be a user-visible change or not?  I'm confused.

If you intend to have an entire mm have access to RDPMC if an event is
mapped, then surely access needs to be context switched for the whole
mm.  If you intend to only have the thread to which the event is bound
have access, then the only reason I see to use IPIs is to revoke access
on munmap from the wrong thread.  But even that latter case could be
handled with a more targeted approach, not a broadcast to all of mm_cpumask.

Can you clarify what the overall intent is and what this particular
patch is trying to do?

> 
>  	if (atomic_dec_and_test(&mm->context.perf_rdpmc_allowed))
> -		on_each_cpu_mask(mm_cpumask(mm), cr4_update_pce, NULL, 1);
> +		on_each_cpu_mask(mm_cpumask(mm), x86_pmu_set_user_access_ipi, NULL, 1);

Here you do something for the whole mm.

> -		on_each_cpu(cr4_update_pce, NULL, 1);
> +		on_each_cpu(x86_pmu_set_user_access_ipi, NULL, 1);

Here too.

>  void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>  			struct task_struct *tsk)
>  {
> @@ -581,10 +556,8 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>  	this_cpu_write(cpu_tlbstate.loaded_mm, next);
>  	this_cpu_write(cpu_tlbstate.loaded_mm_asid, new_asid);
> 
> -	if (next != real_prev) {
> -		cr4_update_pce_mm(next);
> +	if (next != real_prev)
>  		switch_ldt(real_prev, next);
> -	}
>  }

But if you remove this, then what handles context switching?

> 
>  /*
> --
> 2.27.0
>