From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934676AbbIVU1w (ORCPT ); Tue, 22 Sep 2015 16:27:52 -0400 Received: from www.linutronix.de ([62.245.132.108]:43034 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759339AbbIVU1u (ORCPT ); Tue, 22 Sep 2015 16:27:50 -0400 Date: Tue, 22 Sep 2015 22:27:11 +0200 (CEST) From: Thomas Gleixner To: Dave Hansen cc: x86@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 10/26] x86, pkeys: notify userspace about protection key faults In-Reply-To: <5601B82F.6070601@sr71.net> Message-ID: References: <20150916174903.E112E464@viggo.jf.intel.com> <20150916174906.51062FBC@viggo.jf.intel.com> <5601B82F.6070601@sr71.net> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 22 Sep 2015, Dave Hansen wrote: > On 09/22/2015 01:03 PM, Thomas Gleixner wrote: > > On Wed, 16 Sep 2015, Dave Hansen wrote: > >> > >> +static inline u16 vma_pkey(struct vm_area_struct *vma) > >> +{ > >> + u16 pkey = 0; > >> +#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS > >> + unsigned long f = vma->vm_flags; > >> + pkey |= (!!(f & VM_HIGH_ARCH_0)) << 0; > >> + pkey |= (!!(f & VM_HIGH_ARCH_1)) << 1; > >> + pkey |= (!!(f & VM_HIGH_ARCH_2)) << 2; > >> + pkey |= (!!(f & VM_HIGH_ARCH_3)) << 3; > > > > Eew. What's wrong with: > > > > pkey = (vma->vm_flags & VM_PKEY_MASK) >> VM_PKEY_SHIFT; > > I didn't do that only because we don't have any other need for > VM_PKEY_MASK or VM_PKEY_SHIFT. We could do: > > #define VM_PKEY_MASK (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2...) > > static inline u16 vma_pkey(struct vm_area_struct *vma) > { > int vm_pkey_shift = __ffs(VM_PKEY_MASK) > return (vma->vm_flags & VM_PKEY_MASK) >> vm_pkey_shift; > } > > That's probably the same number of lines of code in the end. The > compiler _probably_ ends up doing the same thing either way. > > >> +static u16 fetch_pkey(unsigned long address, struct task_struct *tsk) > > > > So here we get a u16 and assign it to si_pkey > > > >> + if (boot_cpu_has(X86_FEATURE_OSPKE) && si_code == SEGV_PKUERR) > >> + info.si_pkey = fetch_pkey(address, tsk); > > > > which is int. > > > >> + int _pkey; /* FIXME: protection key value?? > > > > Inconsistent at least. > > So I defined all the kernel-internal types as u16 since I *know* the > size of the hardware. > > The user-exposed ones should probably be a bit more generic. I did just > realize that this is an int and my proposed syscall is a long. That I > definitely need to make consistent. > > Does anybody care whether it's an int or a long? long is frowned upon due to 32/64bit. Even if that key stuff is only available on 64bit for now .... Thanks, tglx