From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f72.google.com (mail-pg0-f72.google.com [74.125.83.72]) by kanga.kvack.org (Postfix) with ESMTP id D1BB92802A5 for ; Fri, 10 Nov 2017 17:09:07 -0500 (EST) Received: by mail-pg0-f72.google.com with SMTP id k190so8813211pga.10 for ; Fri, 10 Nov 2017 14:09:07 -0800 (PST) Received: from mga07.intel.com (mga07.intel.com. [134.134.136.100]) by mx.google.com with ESMTPS id 100si9894202pld.634.2017.11.10.14.09.06 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 10 Nov 2017 14:09:06 -0800 (PST) Subject: Re: [PATCH 21/30] x86, mm: put mmu-to-h/w ASID translation in one place References: <20171110193058.BECA7D88@viggo.jf.intel.com> <20171110193146.5908BE13@viggo.jf.intel.com> From: Dave Hansen Message-ID: <101307eb-b924-69ef-13dd-05e63fbaf587@linux.intel.com> Date: Fri, 10 Nov 2017 14:09:06 -0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Andy Lutomirski Cc: "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , moritz.lipp@iaik.tugraz.at, Daniel Gruss , michael.schwarz@iaik.tugraz.at, richard.fellner@student.tugraz.at, Linus Torvalds , Kees Cook , Hugh Dickins , X86 ML On 11/10/2017 02:03 PM, Andy Lutomirski wrote: >> +static inline u16 kern_asid(u16 asid) >> +{ >> + VM_WARN_ON_ONCE(asid > MAX_ASID_AVAILABLE); >> + /* >> + * If PCID is on, ASID-aware code paths put the ASID+1 into the PCID >> + * bits. This serves two purposes. It prevents a nasty situation in >> + * which PCID-unaware code saves CR3, loads some other value (with PCID >> + * == 0), and then restores CR3, thus corrupting the TLB for ASID 0 if >> + * the saved ASID was nonzero. It also means that any bugs involving >> + * loading a PCID-enabled CR3 with CR4.PCIDE off will trigger >> + * deterministically. >> + */ >> + return asid + 1; >> +} > This seems really error-prone. Maybe we should have a pcid_t type and > make all the interfaces that want a h/w PCID take pcid_t. Yeah, totally agree. I actually had a nasty bug or two around this area because of this. I divided them among hw_asid_t and sw_asid_t. You can turn a sw_asid_t into a kernel hw_asid_t or a user hw_asid_t. But, it cause too much churn across the TLB flushing code so I shelved it for now. I'd love to come back nd fix this up properly though. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754215AbdKJWJI (ORCPT ); Fri, 10 Nov 2017 17:09:08 -0500 Received: from mga05.intel.com ([192.55.52.43]:21332 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751186AbdKJWJG (ORCPT ); Fri, 10 Nov 2017 17:09:06 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,376,1505804400"; d="scan'208";a="6335796" Subject: Re: [PATCH 21/30] x86, mm: put mmu-to-h/w ASID translation in one place To: Andy Lutomirski References: <20171110193058.BECA7D88@viggo.jf.intel.com> <20171110193146.5908BE13@viggo.jf.intel.com> Cc: "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , moritz.lipp@iaik.tugraz.at, Daniel Gruss , michael.schwarz@iaik.tugraz.at, richard.fellner@student.tugraz.at, Linus Torvalds , Kees Cook , Hugh Dickins , X86 ML From: Dave Hansen Message-ID: <101307eb-b924-69ef-13dd-05e63fbaf587@linux.intel.com> Date: Fri, 10 Nov 2017 14:09:06 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/10/2017 02:03 PM, Andy Lutomirski wrote: >> +static inline u16 kern_asid(u16 asid) >> +{ >> + VM_WARN_ON_ONCE(asid > MAX_ASID_AVAILABLE); >> + /* >> + * If PCID is on, ASID-aware code paths put the ASID+1 into the PCID >> + * bits. This serves two purposes. It prevents a nasty situation in >> + * which PCID-unaware code saves CR3, loads some other value (with PCID >> + * == 0), and then restores CR3, thus corrupting the TLB for ASID 0 if >> + * the saved ASID was nonzero. It also means that any bugs involving >> + * loading a PCID-enabled CR3 with CR4.PCIDE off will trigger >> + * deterministically. >> + */ >> + return asid + 1; >> +} > This seems really error-prone. Maybe we should have a pcid_t type and > make all the interfaces that want a h/w PCID take pcid_t. Yeah, totally agree. I actually had a nasty bug or two around this area because of this. I divided them among hw_asid_t and sw_asid_t. You can turn a sw_asid_t into a kernel hw_asid_t or a user hw_asid_t. But, it cause too much churn across the TLB flushing code so I shelved it for now. I'd love to come back nd fix this up properly though.