From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752703AbdFTWp4 (ORCPT ); Tue, 20 Jun 2017 18:45:56 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:33884 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752428AbdFTWpy (ORCPT ); Tue, 20 Jun 2017 18:45:54 -0400 Date: Tue, 20 Jun 2017 15:45:44 -0700 From: Ram Pai To: Michael Ellerman Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, benh@kernel.crashing.org, paulus@samba.org, khandual@linux.vnet.ibm.com, aneesh.kumar@linux.vnet.ibm.com, bsingharora@gmail.com, dave.hansen@intel.com, hbabu@us.ibm.com Subject: Re: [RFC v2 03/12] powerpc: Implement sys_pkey_alloc and sys_pkey_free system call. Reply-To: Ram Pai References: <1497671564-20030-1-git-send-email-linuxram@us.ibm.com> <1497671564-20030-4-git-send-email-linuxram@us.ibm.com> <874lvcuq6e.fsf@concordia.ellerman.id.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <874lvcuq6e.fsf@concordia.ellerman.id.au> User-Agent: Mutt/1.5.20 (2009-12-10) X-TM-AS-GCONF: 00 x-cbid: 17062022-0016-0000-0000-00000701B856 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007263; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000214; SDB=6.00877639; UDB=6.00437219; IPR=6.00657778; BA=6.00005432; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00015904; XFM=3.00000015; UTC=2017-06-20 22:45:52 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17062022-0017-0000-0000-00003A3558FD Message-Id: <20170620224544.GE17588@ram.oc3035372033.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-06-20_11:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1706200392 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 19, 2017 at 10:18:01PM +1000, Michael Ellerman wrote: > Hi Ram, > > Ram Pai writes: > > Sys_pkey_alloc() allocates and returns available pkey > > Sys_pkey_free() frees up the pkey. > > > > Total 32 keys are supported on powerpc. However pkey 0,1 and 31 > > are reserved. So effectively we have 29 pkeys. > > > > Signed-off-by: Ram Pai > > --- > > include/linux/mm.h | 31 ++++--- > > include/uapi/asm-generic/mman-common.h | 2 +- > > Those changes need to be split out and acked by mm folks. > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index 7cb17c6..34ddac7 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -204,26 +204,35 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *, > > #define VM_MERGEABLE 0x80000000 /* KSM may merge identical pages */ > > > > #ifdef CONFIG_ARCH_USES_HIGH_VMA_FLAGS > > -#define VM_HIGH_ARCH_BIT_0 32 /* bit only usable on 64-bit architectures */ > > -#define VM_HIGH_ARCH_BIT_1 33 /* bit only usable on 64-bit architectures */ > > -#define VM_HIGH_ARCH_BIT_2 34 /* bit only usable on 64-bit architectures */ > > -#define VM_HIGH_ARCH_BIT_3 35 /* bit only usable on 64-bit architectures */ > > +#define VM_HIGH_ARCH_BIT_0 32 /* bit only usable on 64-bit arch */ > > +#define VM_HIGH_ARCH_BIT_1 33 /* bit only usable on 64-bit arch */ > > +#define VM_HIGH_ARCH_BIT_2 34 /* bit only usable on 64-bit arch */ > > +#define VM_HIGH_ARCH_BIT_3 35 /* bit only usable on 64-bit arch */ > > Please don't change the comments, it makes the diff harder to read. The lines were surpassing 80 columns. tried to compress the comments without loosing meaning. will restore. > > You're actually just adding this AFAICS: > > > +#define VM_HIGH_ARCH_BIT_4 36 /* bit only usable on 64-bit arch */ > > > #define VM_HIGH_ARCH_0 BIT(VM_HIGH_ARCH_BIT_0) > > #define VM_HIGH_ARCH_1 BIT(VM_HIGH_ARCH_BIT_1) > > #define VM_HIGH_ARCH_2 BIT(VM_HIGH_ARCH_BIT_2) > > #define VM_HIGH_ARCH_3 BIT(VM_HIGH_ARCH_BIT_3) > > +#define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4) > > #endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */ > > > > #if defined(CONFIG_X86) > ^ > > # define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */ > > -#if defined (CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS) > > -# define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0 > > -# define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 4-bit value */ > > -# define VM_PKEY_BIT1 VM_HIGH_ARCH_1 > > -# define VM_PKEY_BIT2 VM_HIGH_ARCH_2 > > -# define VM_PKEY_BIT3 VM_HIGH_ARCH_3 > > -#endif > > +#if defined(CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS) \ > > + || defined(CONFIG_PPC64_MEMORY_PROTECTION_KEYS) > > +#define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0 > > +#define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 5-bit value */ > ^ 4? > > +#define VM_PKEY_BIT1 VM_HIGH_ARCH_1 > > +#define VM_PKEY_BIT2 VM_HIGH_ARCH_2 > > +#define VM_PKEY_BIT3 VM_HIGH_ARCH_3 > > +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ > > That appears to be inside an #if defined(CONFIG_X86) ? > > > #elif defined(CONFIG_PPC) > ^ > Should be CONFIG_PPC64_MEMORY_PROTECTION_KEYS no? Its a little garbled. Will fix it. > > > +#define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 5-bit value */ > > +#define VM_PKEY_BIT1 VM_HIGH_ARCH_1 > > +#define VM_PKEY_BIT2 VM_HIGH_ARCH_2 > > +#define VM_PKEY_BIT3 VM_HIGH_ARCH_3 > > +#define VM_PKEY_BIT4 VM_HIGH_ARCH_4 /* intel does not use this bit */ > > + /* but reserved for future expansion */ > > But this hunk is for PPC ? > > Is it OK for the other arches & generic code to add another VM_PKEY_BIT4 ? No. it has to be PPC specific. > > Do you need to update show_smap_vma_flags() ? > > > # define VM_SAO VM_ARCH_1 /* Strong Access Ordering (powerpc) */ > > #elif defined(CONFIG_PARISC) > > # define VM_GROWSUP VM_ARCH_1 > > > diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h > > index 8c27db0..b13ecc6 100644 > > --- a/include/uapi/asm-generic/mman-common.h > > +++ b/include/uapi/asm-generic/mman-common.h > > @@ -76,5 +76,5 @@ > > #define PKEY_DISABLE_WRITE 0x2 > > #define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ > > PKEY_DISABLE_WRITE) > > - > > +#define PKEY_DISABLE_EXECUTE 0x4 > > How you can set that if it's not in PKEY_ACCESS_MASK? I was wondering how to handle this. x86 does not support this flag. However powerpc has the ability to enable/disable execute permission on a key. It cannot be done from userspace, but can be done through the sys_mprotect_pkey() sys call. Initially I was thinking of not enabling it in powerpc aswell, but than i think we should be not block the hardware feature from being used. I will make PKEY_DISABLE_EXECUTE as part of the PKEY_ACCESS_MASK, and have powerpc handle it. Also will x86 patch that return error if the flag is provided. makes sense? Thanks for your comments, RP > > See: > > SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val) > { > int pkey; > int ret; > > /* No flags supported yet. */ > if (flags) > return -EINVAL; > /* check for unsupported init values */ > if (init_val & ~PKEY_ACCESS_MASK) > return -EINVAL; > > > cheers -- Ram Pai