Messages in this thread Patch in this message | | | Date | Tue, 9 Aug 2022 15:33:05 -0700 | From | Ira Weiny <> | Subject | Re: [RFC PATCH 2/5] entry: Add calls for save/restore auxiliary pt_regs |
| |
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();
| |