From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY, USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B43D8C2D0E4 for ; Thu, 19 Nov 2020 12:06:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 43C0E246F9 for ; Thu, 19 Nov 2020 12:06:47 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=oracle.com header.i=@oracle.com header.b="O1CRShF/" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727184AbgKSMGg (ORCPT ); Thu, 19 Nov 2020 07:06:36 -0500 Received: from aserp2130.oracle.com ([141.146.126.79]:57896 "EHLO aserp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726644AbgKSMGf (ORCPT ); Thu, 19 Nov 2020 07:06:35 -0500 Received: from pps.filterd (aserp2130.oracle.com [127.0.0.1]) by aserp2130.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 0AJC0Swt095992; Thu, 19 Nov 2020 12:06:05 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : from : to : cc : references : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=corp-2020-01-29; bh=qEx9t7tQaIGsPorhAyQjWXsRJWZTv6KV0Zpr9SxOTcg=; b=O1CRShF/T+ymsaVuC6253zCPcnGXCyRAH5+u4IkMtSDcjINfMzvg+bP06A5PU45bt6tu iyJCDN0EVbGQ0x1qN4MwB9A4u2/U6IcmJPSNusFeKX+AvLKydZcKfVCNjqyfO085wZ8T AXXQXm0JR2PTTufuqW+RdHtBG+BFeB29revK2kmldl2oyCnULMi5BeeE2hF/MMTwID0y t2wQhOB6Ivz45WUJWMnB/5tQ+cYpXHXNRATRoMhwxnUorGrZF5J+Y7h5HdvaHF1ObaMj m7bvSJfOIYSz7SUN9J0NK7c8pJv69KYQYT68cWASczd6dHpvr5TCRaPBGNipxAuhtp0v vA== Received: from userp3030.oracle.com (userp3030.oracle.com [156.151.31.80]) by aserp2130.oracle.com with ESMTP id 34t4rb54tq-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 19 Nov 2020 12:06:05 +0000 Received: from pps.filterd (userp3030.oracle.com [127.0.0.1]) by userp3030.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 0AJC0ewN085635; Thu, 19 Nov 2020 12:04:04 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userp3030.oracle.com with ESMTP id 34ts5yv844-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 19 Nov 2020 12:04:04 +0000 Received: from abhmp0002.oracle.com (abhmp0002.oracle.com [141.146.116.8]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id 0AJC3wVn028390; Thu, 19 Nov 2020 12:03:58 GMT Received: from linux.home (/10.175.56.254) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 19 Nov 2020 04:03:58 -0800 Subject: Re: [RFC][PATCH v2 12/21] x86/pti: Use PTI stack instead of trampoline stack From: Alexandre Chartre To: Andy Lutomirski Cc: Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , X86 ML , Dave Hansen , Peter Zijlstra , LKML , Tom Lendacky , Joerg Roedel , Konrad Rzeszutek Wilk , jan.setjeeilers@oracle.com, Junaid Shahid , oweisse@google.com, Mike Rapoport , Alexander Graf , mgross@linux.intel.com, kuzuno@gmail.com References: <20201116144757.1920077-1-alexandre.chartre@oracle.com> <20201116144757.1920077-13-alexandre.chartre@oracle.com> <6f513efb-cde8-50f4-7872-13a18a10c4a6@oracle.com> <2f6a446a-e656-627c-27f2-8411f318448c@oracle.com> Message-ID: <88bab705-4b33-bda9-3ece-563234822095@oracle.com> Date: Thu, 19 Nov 2020 13:06:19 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <2f6a446a-e656-627c-27f2-8411f318448c@oracle.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9809 signatures=668682 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 spamscore=0 phishscore=0 suspectscore=0 mlxscore=0 malwarescore=0 bulkscore=0 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2011190091 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9809 signatures=668682 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 clxscore=1015 malwarescore=0 impostorscore=0 lowpriorityscore=0 priorityscore=1501 mlxlogscore=999 adultscore=0 phishscore=0 suspectscore=0 spamscore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2011190091 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/19/20 9:05 AM, Alexandre Chartre wrote: >>>>>>>>> >>>>>>>>> When entering the kernel from userland, use the per-task PTI stack >>>>>>>>> instead of the per-cpu trampoline stack. Like the trampoline stack, >>>>>>>>> the PTI stack is mapped both in the kernel and in the user page-table. >>>>>>>>> Using a per-task stack which is mapped into the kernel and the user >>>>>>>>> page-table instead of a per-cpu stack will allow executing more code >>>>>>>>> before switching to the kernel stack and to the kernel page-table. >>>>>>>> >>>>>>>> Why? >>>>>>> >>>>>>> When executing more code in the kernel, we are likely to reach a point >>>>>>> where we need to sleep while we are using the user page-table, so we need >>>>>>> to be using a per-thread stack. >>>>>>> >>>>>>>> I can't immediately evaluate how nasty the page table setup is because >>>>>>>> it's not in this patch. >>>>>>> >>>>>>> The page-table is the regular page-table as introduced by PTI. It is just >>>>>>> augmented with a few additional mapping which are in patch 11 (x86/pti: >>>>>>> Extend PTI user mappings). >>>>>>> >>>>>>>>     But AFAICS the only thing that this enables is sleeping with user pagetables. >>>>>>> >>>>>>> That's precisely the point, it allows to sleep with the user page-table. >>>>>>> >>>>>>>> Do we really need to do that? >>>>>>> >>>>>>> Actually, probably not with this particular patchset, because I do the page-table >>>>>>> switch at the very beginning and end of the C handler. I had some code where I >>>>>>> moved the page-table switch deeper in the kernel handler where you definitively >>>>>>> can sleep (for example, if you switch back to the user page-table before >>>>>>> exit_to_user_mode_prepare()). >>>>>>> >>>>>>> So a first step should probably be to not introduce the per-task PTI trampoline stack, >>>>>>> and stick with the existing trampoline stack. The per-task PTI trampoline stack can >>>>>>> be introduced later when the page-table switch is moved deeper in the C handler and >>>>>>> we can effectively sleep while using the user page-table. >>>>>> >>>>>> Seems reasonable. >>>>>> >>>>> >>>>> I finally remember why I have introduced a per-task PTI trampoline stack right now: >>>>> that's to be able to move the CR3 switch anywhere in the C handler. To do so, we need >>>>> a per-task stack to enter (and return) from the C handler as the handler can potentially >>>>> go to sleep. >>>>> >>>>> Without a per-task trampoline stack, we would be limited to call the switch CR3 functions >>>>> from the assembly entry code before and after calling the C function handler (also called >>>>> from assembly). >>>> >>>> The noinstr part of the C entry code won't sleep. >>>> >>> >>> But the noinstr part of the handler can sleep, and if it does we will need to >>> preserve the trampoline stack (even if we switch to the per-task kernel stack to >>> execute the noinstr part). >>> >>> Example: >>> >>> #define DEFINE_IDTENTRY(func)                                           \ >>> static __always_inline void __##func(struct pt_regs *regs);             \ >>>                                                                           \ >>> __visible noinstr void func(struct pt_regs *regs)                       \ >>> {                                                                       \ >>>           irqentry_state_t state;         -+                              \ >>>                                            |                              \ >>>           user_pagetable_escape(regs);     | use trampoline stack (1) >>>           state = irqentry_enter(regs);    |                              \ >>>           instrumentation_begin();        -+                              \ >>>           run_idt(__##func, regs);       |===| run __func() on kernel stack (this can sleep) >>>           instrumentation_end();          -+                              \ >>>           irqentry_exit(regs, state);      | use trampoline stack (2) >>>           user_pagetable_return(regs);    -+                              \ >>> } >>> >>> Between (1) and (2) we need to preserve and use the same trampoline stack >>> in case __func() went sleeping. >>> >> >> Why?  Right now, we have the percpu entry stack, and we do just fine >> if we enter on one percpu stack and exit from a different one. >> We would need to call from asm to C on the entry stack, return back to >> asm, and then switch stacks. >> > > That's the problem: I didn't want to return back to asm, so that the pagetable > switch can be done anywhere in the C handler. > > So yes, returning to asm to switch the stack is the solution if we want to avoid > having per-task trampoline stack. The drawback is that this forces to do the > page-table switch at the beginning and end of the handler; the pagetable switch > cannot be moved deeper down into the C handler. > > But that's probably a good first step (effectively just moving CR3 switch to C > without adding per-task trampoline stack). I will update the patches to do that, > and we can defer the per-task trampoline stack to later if there's an effective > need for it. > That might not be a good first step after all... Calling CR3 switch C functions from assembly introduces extra pt_regs copies between the trampoline stack and the kernel stack. Currently when entering syscall, we immediately switches CR3 and builds pt_regs directly on the kernel stack. On return, registers are restored from pt_regs from the kernel stack, the return frame is built on the trampoline stack and then we switch CR3. To call CR3 switch C functions on syscall entry, we need to switch to the trampoline stack, build pt_regs on the trampoline stack, call CR3 switch, switch to the kernel stack, copy pt_regs to the kernel stack. On return, we have to copy pt_regs back to the trampoline stack, call CR3 switch, restore registers. This is less of an impact for interrupt because we enter on the trampoline stack and the current code already builds pt_regs on the trampoline stack and copies it to the kernel stack (although this can certainly be avoided in the current code). I am not comfortable adding these extra steps in syscall and interrupt as the current code is fairly optimized. With a per-task trampoline stack, we don't have extra copy because we can build pt_regs directly on the trampoline stack and it will preserved even when switching to the kernel stack. On syscall/interrupt return, it also saves a copy of the iret frame from the kernel stack to the trampoline stack. alex.