Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 9 Aug 2022 15:33:05 -0700
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
Message-ID: <YvLgoZyyZp5pkGkk@iweiny-desk3>
References: <20220805173009.3128098-1-ira.weiny@intel.com>
 <20220805173009.3128098-3-ira.weiny@intel.com>
 <YvJNe2rzXfcogFFX@zn.tnic>
 <YvKpi/CVHko50PNQ@iweiny-desk3>
 <YvKsS3QuOu4JVzZU@zn.tnic>
 <87lerxqfv8.ffs@tglx>
 <YvLT6FipPdSkCW/6@zn.tnic>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <YvLT6FipPdSkCW/6@zn.tnic>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?UgYjkS2VVfXNevG7jWhTlnT3lhy56WBmzKFCWg9w+cruGVAvlWrnd3gP/bgt?=
 =?us-ascii?Q?CSIMWRLZq79eLxmKoixJb6hy3CS3rxEW3YSafXIYb/glIwEMeCEL5EIJatDv?=
 =?us-ascii?Q?ozCquNX/LBI2+jhfkeUJEPvk+so9aOTwJxHO/YBJi277ZFfGtnJFtYZpwhnt?=
 =?us-ascii?Q?gCSh1GGyGohJVbOKjzgFaDSoLquYHYOmAsQ+wD7KgAIM7qTo2TWvRQXWdNcM?=
 =?us-ascii?Q?Lm0HNX8buoUbYSdby8cGsY+XioyoiNz1FP3uIy2iBTYVPh+yULd5EO9P9mAt?=
 =?us-ascii?Q?S0JJBAhUxd0ZaI3/5tPC42FXwdB8haLg5ox6B59iB5pl6a2difEBzNiyRwQb?=
 =?us-ascii?Q?08Bv4fkGB8Hd1qqZIU5G1O2Xm7RHaUnNIsO5kp4+Um5S3rSzc1xbRvaWOV+W?=
 =?us-ascii?Q?FH6hb2h/fSMbo3Jg+KpE6kZxwvI33JMu+itkKK0a2k08f/SWm7FnDmpsjcEv?=
 =?us-ascii?Q?Ohu1WjkwEGtpnfoPRDWk2e95x/BHH7P/pV0pAdy+4HuO0e6TYstaZ2QNl44V?=
 =?us-ascii?Q?HHDxAlUSOOR9SNqtWZ8TzEoGX1hY9MislMd/A+a3TCk55/9FWxo6u7NF4bA5?=
 =?us-ascii?Q?VB0awR6Qz1cM0oy3FFcc2zGVSvdyPLZpSrSBaRTKOjDo4WDa9W7NLiur6Q2O?=
 =?us-ascii?Q?KjT92la9KTXAT9I6zcg3aQ8TD59Tw/lGF8iLkC12GTw+GCJLBrcePu2djBn/?=
 =?us-ascii?Q?8S0Qcb0Hy8rZ/PwbbIffZRMoHw4hwdIbH2R8lyz3ZrzVWjrjBbOvjbmQbiI8?=
 =?us-ascii?Q?bJoewKQLu41KBy0ZSehn56QPPhuiETXs3v2weunaPV9QVIMVyz7pZSb5vUwl?=
 =?us-ascii?Q?JIVnzv3PoqmFVjh8KPj3Fb4wqu84HwEOT083whLb5H/D5y2Y1kC/3j+oISSC?=
 =?us-ascii?Q?Go9RucSG6bpbdbA9mtxvb1QHJG+RQTGS9hdkIvxiwjmNt3BypOPKlFeItbtg?=
 =?us-ascii?Q?ya6RFMM9udtShN7KQdM+KVjgdVw8LciP6cpFjNdiDbQmQmJW1o8FEy1g16jG?=
 =?us-ascii?Q?48bSolhxIY4kwXTliJv8hwJ6ez8zMzKxE81Ku1W3z55DhZQ306iJLBZGS0mY?=
 =?us-ascii?Q?Y10Dkwh3T6P2okeaGDAdj8bVRy5SE5SOw8pWTaw69qjpkSz16TRr1cd+EXIZ?=
 =?us-ascii?Q?vQ/03R7k5y6HbNcGxxEYrJySuT4rr5d2r+GmSKj0qV63TtA1kHbXJSkJHA34?=
 =?us-ascii?Q?DA2v2erUfNa6PpJ38gC2iZgF9G3M2yv/zaSgK2JdUIUbDKH4cEIHlt96FSGf?=
 =?us-ascii?Q?6itXfq2G98uCrmQ6WwPJ8B33muW70pYiCWdZtYcQ/bS6cYRFCDKGgLbowiHP?=
 =?us-ascii?Q?+bquAt5IDCWITNAarCEJOkcVZX2ENKaKBPvgip5av7/yuDHbMnZPMcD3JRvg?=
 =?us-ascii?Q?+wur4rBIunhgVEXVzsL6dYDVI5xgHtrO7Hw10IeJcEmrmmsmhvtEqNrt8vAr?=
 =?us-ascii?Q?e2LwLWhALj3V0EHmmVwwsCKSv1bz4qk2anJnv0AFHC/ugVsXW8pfIXhF33yE?=
 =?us-ascii?Q?0Ek/ew8NyHva47i2bY59uOFmMbeGo3KolKzvRWSBRCIxOEmpko2luITwzHgU?=
 =?us-ascii?Q?e09YK7Ly/GG01u2ZhH6XaNDxQ0hUX/qMkuOCsB+iuI0RtUeSUrvAKAkXfPvH?=
 =?us-ascii?Q?KA=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2a58415f-4579-43ba-af58-08da7a572379
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6311.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Aug 2022 22:33:10.4298
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: xmk8YIXi0HLIPAuM1OEwDdmMfFUeuac+xQPt6GQ8x8gk3+rwGjSqK/kMJs+WKgFuNaA9ljTSoY8SzYBeODU0rA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB3853
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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();