lkml.org 
[lkml]   [2016]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH -v1.1] x86/mm: Fix INVPCID asm constraint
On Wed, Feb 10, 2016 at 6:51 AM, Borislav Petkov <bp@alien8.de> 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 <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
>

Acked-by: Andy Lutomirski <luto@kernel.org>

\
 
 \ /
  Last update: 2016-02-11 04:21    [W:1.271 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site