Re: [RFCv2 05/10] x86/mm: Provide untagged_addr() helper

From: Thomas Gleixner <tglx@linutronix.de>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	x86@kernel.org, Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Alexander Potapenko <glider@google.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	"H . J . Lu" <hjl.tools@gmail.com>,
	Andi Kleen <ak@linux.intel.com>,
	Rick Edgecombe <rick.p.edgecombe@intel.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [RFCv2 05/10] x86/mm: Provide untagged_addr() helper
Date: Thu, 12 May 2022 17:16:11 +0200	[thread overview]
Message-ID: <87sfpevl1g.ffs@tglx> (raw)
In-Reply-To: <Yn0YdPNG/Q3lf+4G@hirez.programming.kicks-ass.net>

On Thu, May 12 2022 at 16:23, Peter Zijlstra wrote:
> On Thu, May 12, 2022 at 03:06:38PM +0200, Thomas Gleixner wrote:
>
>> #define untagged_addr(addr)	({			\
>> 	u64 __addr = (__force u64)(addr);		\
>> 							\
>> 	__addr &= current->thread.lam_untag_mask;	\
>> 	(__force __typeof__(addr))__addr;		\
>> })
>> 
>> No conditionals, fast _and_ correct. Setting this untag mask up once
>> when LAM is enabled is not rocket science.
>
> But that goes wrong if someone ever wants to untag a kernel address and
> not use the result for access_ok().
>
> I'd feel better about something like:
>
> 	s64 __addr = (addr);
> 	s64 __sign = __addr;
>
> 	__sign >>= 63;
> 	__sign &= lam_untag_mask;

that needs to be

 	__sign &= ~lam_untag_mask;

> 	__addr &= lam_untag_mask;
> 	__addr |= __sign;
>
> 	__addr;
>
> Which simply extends bit 63 downwards -- although possibly there's an
> easier way to do that, this is pretty gross.

For the price of a conditional:

    __addr &= lam_untag_mask;
    if (__addr & BIT(63))
        __addr |= ~lam_untag_mask;

Now you have the choice between gross and ugly.

Thanks,

        tglx