From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752689AbcBKCtN (ORCPT ); Wed, 10 Feb 2016 21:49:13 -0500 Received: from mail-ob0-f176.google.com ([209.85.214.176]:36022 "EHLO mail-ob0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750748AbcBKCtK convert rfc822-to-8bit (ORCPT ); Wed, 10 Feb 2016 21:49:10 -0500 MIME-Version: 1.0 In-Reply-To: <20160210145116.GF23914@pd.tnic> References: <8a62b23ad686888cee01da134c91409e22064db9.1454096309.git.luto@kernel.org> <20160210120827.GA11832@pd.tnic> <20160210145116.GF23914@pd.tnic> From: Andy Lutomirski Date: Wed, 10 Feb 2016 18:48:50 -0800 Message-ID: Subject: Re: [PATCH -v1.1] x86/mm: Fix INVPCID asm constraint To: Borislav Petkov Cc: Michael Matz , Ingo Molnar , Thomas Gleixner , Oleg Nesterov , Brian Gerst , Luis Rodriguez , Dave Hansen , Andrew Morton , Denys Vlasenko , Peter Zijlstra , Linus Torvalds , "linux-kernel@vger.kernel.org" , Toshi Kani , Andrew Lutomirski , Andrey Ryabinin , "H. Peter Anvin" , "linux-tip-commits@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Feb 10, 2016 at 6:51 AM, Borislav Petkov wrote: > On Wed, Feb 10, 2016 at 02:48:02PM +0100, Michael Matz wrote: >> Hi, >> >> On Wed, 10 Feb 2016, Borislav Petkov wrote: >> >> > --- a/arch/x86/include/asm/tlbflush.h >> > +++ b/arch/x86/include/asm/tlbflush.h >> > @@ -23,7 +23,7 @@ static inline void __invpcid(unsigned long pcid, unsigned long addr, >> > * invpcid (%rcx), %rax in long mode. >> > */ >> > asm volatile (".byte 0x66, 0x0f, 0x38, 0x82, 0x01" >> > - : : "m" (desc), "a" (type), "c" (desc) : "memory"); >> > + : : "m" (*desc), "a" (type), "c" (desc) : "memory"); >> >> That still doesn't do what you want. Arrays in C are funny. *desc is >> exactly equivalent to desc[0], _not_ to the whole array, > > Doh! > >> indeed there's no C syntax to name an lvalue of array type in normal >> expressions. You need to jump through hoops for this: >> >> "m" (*(struct {unsigned long x[2];} *)desc) > > Aha! That's why we wrapped the array in clwb() in a struct too, btw: > > static inline void clwb(volatile void *__p) > { > volatile struct { char x[64]; } *p = __p; > ... > >> It'd probably be easier to simply declare the descriptor as a struct, >> rather than an array, then the original syntax would have been mostly >> correct: >> >> struct {u64 d[2];} desc = { pcid, addr }; >> asm ... "m" (desc), "c" (&desc) > > Sounds better. Done. How does that below look like? > > Thanks Micha! > > --- > From: Borislav Petkov > Date: Wed, 10 Feb 2016 12:53:48 +0100 > Subject: [PATCH -v1.1] x86/mm: Fix INVPCID asm constraint > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > So we want to specify the dependency on both @pcid and @addr so that the > compiler doesn't reorder accesses to them *before* the TLB flush. But > for that to work, we need to express this properly in the inline asm and > deref the whole desc array, not the pointer to it. See clwb() for an > example. > > This fixes the build error on 32-bit: > > arch/x86/include/asm/tlbflush.h: In function ‘__invpcid’: > arch/x86/include/asm/tlbflush.h:26:18: error: memory input 0 is not directly addressable > Acked-by: Andy Lutomirski