From: Ira Weiny <ira.weiny@intel.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Rik van Riel <riel@surriel.com>,
Dave Hansen <dave.hansen@intel.com>,
Dave Hansen <dave.hansen@linux.intel.com>, <x86@kernel.org>,
<linux-kernel@vger.kernel.org>, <kernel-team@fb.com>
Subject: Re: [RFC PATCH 2/5] entry: Add calls for save/restore auxiliary pt_regs
Date: Tue, 9 Aug 2022 15:33:05 -0700 [thread overview]
Message-ID: <YvLgoZyyZp5pkGkk@iweiny-desk3> (raw)
In-Reply-To: <YvLT6FipPdSkCW/6@zn.tnic>
On Tue, Aug 09, 2022 at 11:38:48PM +0200, Borislav Petkov wrote:
> On Tue, Aug 09, 2022 at 11:14:19PM +0200, Thomas Gleixner wrote:
> > Ira is right. If we want it for everything, then the generic code is the
> > right place.
>
> But what is "everything"? Currently, and AFAIU, it is for the PKS use
> case on x86 only.
>
> I'm not saying it should not be done this way eventually - all I'm
> saying is we should not design "preemptively" before it is really needed
> for other arches.
>
> Unless putting it in generic code makes it all simpler and cleaner to
> do, that is.
For the cpu use case we could limit the number of call sites. However, for PKS
the patch would have required changing x86 code in approximately 9 places for
the enter code.
$ git grep 'irqentry_enter(regs)' arch/x86 | wc -l
9
How about we drop patch 1 (I'll rework it to be less churn and submit it for
clean up separately because it will no longer be needed). Keep patch 3 as is.
Then combine 2 and 5 as below. The saving of the CPU can be lifted later if
needed.
Ira
commit 4c1d646888dd7471ae71a24109d587901a00f87d
Author: Ira Weiny <ira.weiny@intel.com>
Date: Mon Jan 10 15:06:07 2022 -0800
x86/entry: Store CPU info on exception entry
x86 has auxiliary pt_regs space available to store information on the
stack during exceptions. This information is easier to obtain and store
within C code.
The CPU information of a page fault is useful in determining where bad
CPUs are in a large data center.
Define aux_pt_regs_save_cpu() and set ARCH_HAS_PTREGS_AUXILIARY default
to yes.
Store the CPU on page fault entry and use it later.
Cc: Rik van Riel <riel@surriel.com>
Suggested-by: Borislav Petkov <bp@alien8.de>
Suggested-by: Dave Hansen <dave.hansen@intel.com>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
Changes from RFC:
New patch combining 2 and 5 from original series and modified.
Boris/Thomas - eliminate generic calls to save the cpu and call
only from exc_page_fault
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b35f6a472e09..707650a6ecb2 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1876,7 +1876,7 @@ config X86_INTEL_MEMORY_PROTECTION_KEYS
config ARCH_HAS_PTREGS_AUXILIARY
depends on X86_64
- bool
+ def_bool y
choice
prompt "TSX enable mode"
diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
index 5a9c85893459..b403b469996f 100644
--- a/arch/x86/include/asm/ptrace.h
+++ b/arch/x86/include/asm/ptrace.h
@@ -97,6 +97,7 @@ struct pt_regs {
* ARCH_HAS_PTREGS_AUXILIARY. Failure to do so will result in a build failure.
*/
struct pt_regs_auxiliary {
+ int cpu;
};
struct pt_regs_extended {
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 82cf23975aa1..b9b8344b69ad 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -768,9 +768,9 @@ static inline void
show_signal_msg(struct pt_regs *regs, unsigned long error_code,
unsigned long address, struct task_struct *tsk)
{
+ struct pt_regs_auxiliary *aux_pt_regs = &to_extended_pt_regs(regs)->aux;
const char *loglvl = task_pid_nr(tsk) > 1 ? KERN_INFO : KERN_EMERG;
- /* This is a racy snapshot, but it's better than nothing. */
- int cpu = raw_smp_processor_id();
+ int cpu = aux_pt_regs->cpu;
if (!unhandled_signal(tsk, SIGSEGV))
return;
@@ -1503,6 +1503,13 @@ handle_page_fault(struct pt_regs *regs, unsigned long error_code,
}
}
+static void aux_pt_regs_save_cpu(struct pt_regs *regs)
+{
+ struct pt_regs_auxiliary *aux_pt_regs = &to_extended_pt_regs(regs)->aux;
+
+ aux_pt_regs->cpu = raw_smp_processor_id();
+}
+
DEFINE_IDTENTRY_RAW_ERRORCODE(exc_page_fault)
{
unsigned long address = read_cr2();
@@ -1546,6 +1553,7 @@ DEFINE_IDTENTRY_RAW_ERRORCODE(exc_page_fault)
*/
state = irqentry_enter(regs);
+ aux_pt_regs_save_cpu(regs);
instrumentation_begin();
handle_page_fault(regs, error_code, address);
instrumentation_end();
next prev parent reply other threads:[~2022-08-09 22:33 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-05 17:30 [RFC PATCH 0/5] Print CPU at segfault time ira.weiny
2022-08-05 17:30 ` [RFC PATCH 1/5] entry: Pass pt_regs to irqentry_exit_cond_resched() ira.weiny
2022-08-05 18:33 ` Rik van Riel
2022-08-08 10:38 ` Borislav Petkov
2022-08-08 17:34 ` Ira Weiny
2022-08-08 17:38 ` Borislav Petkov
2022-08-08 17:43 ` Dave Hansen
2022-08-08 17:52 ` Borislav Petkov
2022-08-09 23:18 ` Thomas Gleixner
2022-08-10 7:25 ` Thomas Gleixner
2022-08-05 17:30 ` [RFC PATCH 2/5] entry: Add calls for save/restore auxiliary pt_regs ira.weiny
2022-08-05 18:34 ` Rik van Riel
2022-08-09 12:05 ` Borislav Petkov
2022-08-09 18:38 ` Ira Weiny
2022-08-09 18:49 ` Borislav Petkov
2022-08-09 21:14 ` Thomas Gleixner
2022-08-09 21:38 ` Borislav Petkov
2022-08-09 22:33 ` Ira Weiny [this message]
2022-08-09 21:49 ` Ira Weiny
2022-08-09 23:53 ` Thomas Gleixner
2022-08-05 17:30 ` [RFC PATCH 3/5] x86/entry: Add auxiliary pt_regs space ira.weiny
2022-08-05 18:45 ` Rik van Riel
2022-08-05 17:30 ` [RFC PATCH 4/5] x86,mm: print likely CPU at segfault time ira.weiny
2022-08-05 17:30 ` [RFC PATCH 5/5] x86/entry: Store CPU info on exception entry ira.weiny
2022-08-05 18:46 ` Rik van Riel
2022-08-05 18:47 ` Dave Hansen
2022-08-06 9:01 ` Ingo Molnar
2022-08-06 9:11 ` Borislav Petkov
2022-08-07 10:02 ` Ingo Molnar
2022-08-07 10:35 ` Borislav Petkov
2022-08-07 20:02 ` Ira Weiny
2022-08-08 11:03 ` Ingo Molnar
2022-08-08 12:01 ` Borislav Petkov
2022-08-09 20:06 ` Thomas Gleixner
2022-08-08 16:16 ` Dave Hansen
2022-08-08 17:24 ` Rik van Riel
2022-08-09 21:19 ` 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=YvLgoZyyZp5pkGkk@iweiny-desk3 \
--to=ira.weiny@intel.com \
--cc=bp@alien8.de \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=riel@surriel.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/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.