Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932323Ab3DXXHb (ORCPT ); Wed, 24 Apr 2013 19:07:31 -0400 Received: from terminus.zytor.com ([198.137.202.10]:48382 "EHLO mail.zytor.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932204Ab3DXXH1 (ORCPT ); Wed, 24 Apr 2013 19:07:27 -0400 Message-ID: <5178657E.9020905@zytor.com> Date: Wed, 24 Apr 2013 16:06:38 -0700 From: "H. Peter Anvin" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Frederic Weisbecker CC: Oleg Nesterov , Ingo Molnar , Linus Torvalds , Cyrill Gorcunov , Peter Zijlstra , Thomas Gleixner , David Miller , "Theodore Ts'o" , Linux Kernel Mailing List , the arch/x86 maintainers , Network Development , "linux-ext4@vger.kernel.org" Subject: Re: [PATCH] x86: make DR*_RESERVED unsigned long References: <20130424072630.GB1780@gmail.com> <20130424170702.GA1867@redhat.com> In-Reply-To: X-Enigmail-Version: 1.5.1 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 Content-Length: 2070 Lines: 59 On 04/24/2013 03:48 PM, Frederic Weisbecker wrote: > 2013/4/24 Oleg Nesterov : >> DR6_RESERVED and DR_CONTROL_RESERVED are used to clear the set >> bits in the "unsigned long" data, make them long to ensure that >> "&~" doesn't clear the upper bits. >> >> do_debug() and ptrace_write_dr7() which use DR*_RESERVED look >> safe, but probably it makes sense to cleanup anyway. > > Agreed. The code looks safe, but the pattern is error prone. I'm all > for that cleanup. > Just something below: > >> >> Reported-by: Linus Torvalds >> Signed-off-by: Oleg Nesterov >> --- >> arch/x86/include/uapi/asm/debugreg.h | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/include/uapi/asm/debugreg.h b/arch/x86/include/uapi/asm/debugreg.h >> index 3c0874d..75d07dd 100644 >> --- a/arch/x86/include/uapi/asm/debugreg.h >> +++ b/arch/x86/include/uapi/asm/debugreg.h >> @@ -15,7 +15,7 @@ >> are either reserved or not of interest to us. */ >> >> /* Define reserved bits in DR6 which are always set to 1 */ >> -#define DR6_RESERVED (0xFFFF0FF0) >> +#define DR6_RESERVED (0xFFFF0FF0ul) > > You told in an earlier email that intel manual says upper 32 bits of > dr6 are reserved. > In this case don't we need to expand the mask in 64 bits like is done > for DR_CONTROL_RESERVED? > Arguably this would be a *good* use for ~ ... Instead of defining separate bitmasks for 32 and 64 bits have the reciprocal (non-reserved bits): #define DR6_RESERVED (~0x0000F00FUL) That does have the right value on both 32 and 64 bits. The leading zeroes aren't even really needed. Now, DR6 is a bit special in that a bunch of the reserved bits are hardwired to 1, not 0; I don't know offhand if that is true for bits [63:32]. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/