linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Liang, Kan" <kan.liang@linux.intel.com>
To: Dave Hansen <dave.hansen@intel.com>,
	peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
	namhyung@kernel.org, tglx@linutronix.de,
	dave.hansen@linux.intel.com, irogers@google.com,
	adrian.hunter@intel.com, jolsa@kernel.org,
	alexander.shishkin@linux.intel.com, linux-kernel@vger.kernel.org
Cc: dapeng1.mi@linux.intel.com, ak@linux.intel.com, zide.chen@intel.com
Subject: Re: [RFC PATCH 03/12] x86/fpu/xstate: Add xsaves_nmi
Date: Fri, 13 Jun 2025 10:54:43 -0400	[thread overview]
Message-ID: <03cfdc36-a7b9-4098-bc21-cb612907add2@linux.intel.com> (raw)
In-Reply-To: <c7bfcaf2-1669-4d6f-afb2-9bbcadfcde9e@intel.com>



On 2025-06-13 10:39 a.m., Dave Hansen wrote:
> On 6/13/25 06:49, kan.liang@linux.intel.com wrote:
>> + * This function can only be invoked in an NMI. It returns the *ACTUAL*
>> + * register contents when the NMI hit.
> 
> Yes, but why is this important and what are the implications?
> 
> It's important because all of the other mechanisms that deal with xstate
> are _trying_ to get something coherent. They're trying to, for instance,
> poke at the PKRU register for userspace and we need to ensure that the
> PKRU value that's being targeted is for the right task and is actually
> in memory (if that's what we're after).
> 
> This interface is totally *in*coherent. There's no telling what was in
> the registers when the NMI hit. That seems crazy compared to all the
> other FPU code in the kernel. But it's actually OK for perf because
> there's a separate hardware mechanism that saves XSAVE-managed state off
> to memory. That mechanism also writes whatever was in the registers when
> the NMI hit. It's also completely incoherent.
> 
> That's really the only reason this insanity is OK. perf can _already_
> handle XSAVE "snapshots" from random code running. This just provides
> another XSAVE data source at a random time.
> 
> Could we get some of that ^ into the changelog and function comment, please?

Sure. Thanks for the details. I will add it in both the changelog and
function comments.

> 
> One other thing...
> 
> XSAVES uses the modified optimization. That means if you did something
> like this:
> 
> NMI=>
> 	xsaves_nmi();
> <=IRET
> ... run a little bit in the kernel
> NMI=> // another NMI
> 	xsaves_nmi();
> <=IRET
> 
> The second XSAVES might not actually write anything to the buffer
> because the registers didn't change (they weren't modified). Is that OK?

Yes. The per-cpu buffer in perf is only used by this XSAVES. No one will
clear it or modify it between the two xsaves_nmi().

Thanks,
Kan


  reply	other threads:[~2025-06-13 14:54 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-13 13:49 [RFC PATCH 00/12] Support vector and more extended registers in perf kan.liang
2025-06-13 13:49 ` [RFC PATCH 01/12] perf/x86: Use x86_perf_regs in the x86 nmi handler kan.liang
2025-06-13 13:49 ` [RFC PATCH 02/12] perf/x86: Setup the regs data kan.liang
2025-06-13 13:49 ` [RFC PATCH 03/12] x86/fpu/xstate: Add xsaves_nmi kan.liang
2025-06-13 14:39   ` Dave Hansen
2025-06-13 14:54     ` Liang, Kan [this message]
2025-06-13 15:19       ` Dave Hansen
2025-06-13 13:49 ` [RFC PATCH 04/12] perf: Move has_extended_regs() to header file kan.liang
2025-06-13 13:49 ` [RFC PATCH 05/12] perf/x86: Support XMM register for non-PEBS and REGS_USER kan.liang
2025-06-13 15:15   ` Dave Hansen
2025-06-13 17:51     ` Liang, Kan
2025-06-13 15:34   ` Dave Hansen
2025-06-13 18:14     ` Liang, Kan
2025-06-13 13:49 ` [RFC PATCH 06/12] perf: Support extension of sample_regs kan.liang
2025-06-17  8:00   ` Mi, Dapeng
2025-06-17  8:14   ` Peter Zijlstra
2025-06-17  9:49     ` Mi, Dapeng
2025-06-17 10:28       ` Peter Zijlstra
2025-06-17 12:14         ` Mi, Dapeng
2025-06-17 13:33           ` Peter Zijlstra
2025-06-17 14:06             ` Peter Zijlstra
2025-06-17 14:24               ` Mark Rutland
2025-06-17 14:44                 ` Peter Zijlstra
2025-06-17 14:55                   ` Mark Rutland
2025-06-17 19:00                     ` Mark Brown
2025-06-17 20:32                     ` Liang, Kan
2025-06-18  9:35                       ` Peter Zijlstra
2025-06-18 10:10                         ` Liang, Kan
2025-06-18 13:30                           ` Peter Zijlstra
2025-06-18 13:52                             ` Liang, Kan
2025-06-18 14:30                               ` Dave Hansen
2025-06-18 14:47                                 ` Dave Hansen
2025-06-18 15:24                                   ` Liang, Kan
2025-06-18 14:45                               ` Peter Zijlstra
2025-06-18 15:22                                 ` Liang, Kan
2025-06-13 13:49 ` [RFC PATCH 07/12] perf/x86: Add YMMH in extended regs kan.liang
2025-06-13 15:48   ` Dave Hansen
2025-06-13 13:49 ` [RFC PATCH 08/12] perf/x86: Add APX " kan.liang
2025-06-13 16:02   ` Dave Hansen
2025-06-13 17:17     ` Liang, Kan
2025-06-17  8:19   ` Peter Zijlstra
2025-06-13 13:49 ` [RFC PATCH 09/12] perf/x86: Add OPMASK " kan.liang
2025-06-13 13:49 ` [RFC PATCH 10/12] perf/x86: Add ZMM " kan.liang
2025-06-13 13:49 ` [RFC PATCH 11/12] perf/x86: Add SSP " kan.liang
2025-06-13 13:49 ` [RFC PATCH 12/12] perf/x86/intel: Support extended registers kan.liang
2025-06-17  7:50 ` [RFC PATCH 00/12] Support vector and more extended registers in perf Mi, Dapeng
2025-06-17  8:24 ` Peter Zijlstra
2025-06-17 13:52   ` Liang, Kan
2025-06-17 14:29     ` Peter Zijlstra
2025-06-17 15:23       ` Liang, Kan
2025-06-17 17:34         ` Peter Zijlstra
2025-06-18  0:57         ` Mi, Dapeng
2025-06-18 10:47           ` Liang, Kan
2025-06-18 12:28             ` Mi, Dapeng
2025-06-18 13:15               ` Liang, Kan
2025-06-19  0:41                 ` Mi, Dapeng
2025-06-19 11:11                   ` Liang, Kan
2025-06-19 12:26                     ` Mi, Dapeng
2025-06-19 13:38                     ` Peter Zijlstra
2025-06-19 14:27                       ` Liang, Kan

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=03cfdc36-a7b9-4098-bc21-cb612907add2@linux.intel.com \
    --to=kan.liang@linux.intel.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=dapeng1.mi@linux.intel.com \
    --cc=dave.hansen@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=zide.chen@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).