Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1331248

[PATCH -v1.1] x86/mm: Fix INVPCID asm constraint

Show all headers | View raw

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 <bp@suse.de>
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

which gcc4.7 caught but 5.x didn't. Which is strange. :-\

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
Cc: Michael Matz <matz@suse.de>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: linux-mm@kvack.org
---
 arch/x86/include/asm/tlbflush.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 6f9e27aa2aaf..c24b4224d439 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -11,7 +11,7 @@
 static inline void __invpcid(unsigned long pcid, unsigned long addr,
 			     unsigned long type)
 {
-	u64 desc[2] = { pcid, addr };
+	struct { u64 d[2]; } desc = { { pcid, addr } };
 
 	/*
 	 * The memory clobber is because the whole point is to invalidate
@@ -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");
 }
 
 #define INVPCID_TYPE_INDIV_ADDR		0
-- 
2.3.5

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[tip:x86/mm] x86/mm: Add INVPCID helpers tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-02-09 17:10 +0100
  Re: [tip:x86/mm] x86/mm: Add INVPCID helpers Borislav Petkov <bp@alien8.de> - 2016-02-10 13:10 +0100
    Re: [tip:x86/mm] x86/mm: Add INVPCID helpers Michael Matz <matz@suse.de> - 2016-02-10 14:50 +0100
      [PATCH -v1.1] x86/mm: Fix INVPCID asm constraint Borislav Petkov <bp@alien8.de> - 2016-02-10 16:00 +0100
        Re: [PATCH -v1.1] x86/mm: Fix INVPCID asm constraint Andy Lutomirski <luto@amacapital.net> - 2016-02-11 03:50 +0100

csiph-web