From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752898AbbC3O1f (ORCPT ); Mon, 30 Mar 2015 10:27:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54143 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750938AbbC3O1d (ORCPT ); Mon, 30 Mar 2015 10:27:33 -0400 Message-ID: <55195D3E.4060608@redhat.com> Date: Mon, 30 Mar 2015 16:27:10 +0200 From: Denys Vlasenko User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Andy Lutomirski CC: Borislav Petkov , X86 ML , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] x86/asm/entry/64: better check for canonical address References: <1427373731-13056-1-git-send-email-dvlasenk@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/26/2015 07:45 PM, Andy Lutomirski wrote: > On Thu, Mar 26, 2015 at 5:42 AM, Denys Vlasenko wrote: >> This change makes the check exact (no more false positives >> on kernel addresses). >> >> It isn't really important to be fully correct here - >> almost all addresses we'll ever see will be userspace ones, >> but OTOH it looks to be cheap enough: >> the new code uses two more ALU ops but preserves %rcx, >> allowing to not reload it from pt_regs->cx again. >> On disassembly level, the changes are: >> >> cmp %rcx,0x80(%rsp) -> mov 0x80(%rsp),%r11; cmp %rcx,%r11 >> shr $0x2f,%rcx -> shl $0x10,%rcx; sar $0x10,%rcx; cmp %rcx,%r11 >> mov 0x58(%rsp),%rcx -> (eliminated) >> >> Signed-off-by: Denys Vlasenko >> CC: Borislav Petkov >> CC: x86@kernel.org >> CC: linux-kernel@vger.kernel.org >> --- >> >> Andy, I'd undecided myself on the merits of doing this. >> If you like it, feel free to take it in your tree. >> I trimmed CC list to not bother too many people with this trivial >> and quite possibly "useless churn"-class change. > > I suspect that the two added ALU ops are free for all practical > purposes, and the performance of this path isn't *that* critical. > > If anyone is running with vsyscall=native because they need the > performance, then this would be a big win. Otherwise I don't have a > real preference. Anyone else have any thoughts here? > > Let me just run through the math quickly to make sure I believe all the numbers: > > Canonical addresses either start with 17 zeros or 17 ones. > > In the old code, we checked that the top (64-47) = 17 bits were all > zero. We did this by shifting right by 47 bits and making sure that > nothing was left. > > In the new code, we're shifting left by (64 - 48) = 16 bits and then > signed shifting right by the same amount, this propagating the 17th > highest bit to all positions to its left. If we get the same value we > started with, then we're good to go. > > So it looks okay to me. So please take it into your tree :)