[{"id":1789103,"web_url":"http://patchwork.ozlabs.org/comment/1789103/","msgid":"<20171018144914.6252f52a@firefly.ozlabs.ibm.com>","date":"2017-10-18T03:49:14","subject":"Re: [PATCH 10/25] powerpc: store and restore the pkey state across\n\tcontext switches","submitter":{"id":9347,"url":"http://patchwork.ozlabs.org/api/people/9347/","name":"Balbir Singh","email":"bsingharora@gmail.com"},"content":"On Fri, 8 Sep 2017 15:44:58 -0700\nRam Pai wrote:\n\n> Store and restore the AMR, IAMR and UAMOR register state of the task\n> before scheduling out and after scheduling in, respectively.\n> \n> Signed-off-by: Ram Pai \n> ---\n> arch/powerpc/include/asm/pkeys.h | 4 +++\n> arch/powerpc/include/asm/processor.h | 5 ++++\n> arch/powerpc/kernel/process.c | 10 ++++++++\n> arch/powerpc/mm/pkeys.c | 39 ++++++++++++++++++++++++++++++++++\n> 4 files changed, 58 insertions(+), 0 deletions(-)\n> \n> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h\n> index 7fd48a4..78c5362 100644\n> --- a/arch/powerpc/include/asm/pkeys.h\n> +++ b/arch/powerpc/include/asm/pkeys.h\n> @@ -143,5 +143,9 @@ static inline void pkey_mm_init(struct mm_struct *mm)\n> \tmm_pkey_allocation_map(mm) = initial_allocation_mask;\n> }\n> \n> +extern void thread_pkey_regs_save(struct thread_struct *thread);\n> +extern void thread_pkey_regs_restore(struct thread_struct *new_thread,\n> +\t\t\tstruct thread_struct *old_thread);\n> +extern void thread_pkey_regs_init(struct thread_struct *thread);\n> extern void pkey_initialize(void);\n> #endif /*_ASM_PPC64_PKEYS_H */\n> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h\n> index fab7ff8..de9d9ba 100644\n> --- a/arch/powerpc/include/asm/processor.h\n> +++ b/arch/powerpc/include/asm/processor.h\n> @@ -309,6 +309,11 @@ struct thread_struct {\n> \tstruct thread_vr_state ckvr_state; /* Checkpointed VR state */\n> \tunsigned long\tckvrsave; /* Checkpointed VRSAVE */\n> #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */\n> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> +\tunsigned long\tamr;\n> +\tunsigned long\tiamr;\n> +\tunsigned long\tuamor;\n> +#endif\n> #ifdef CONFIG_KVM_BOOK3S_32_HANDLER\n> \tvoid*\t\tkvm_shadow_vcpu; /* KVM internal data */\n> #endif /* CONFIG_KVM_BOOK3S_32_HANDLER */\n> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c\n> index a0c74bb..ba80002 100644\n> --- a/arch/powerpc/kernel/process.c\n> +++ b/arch/powerpc/kernel/process.c\n> @@ -42,6 +42,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -1085,6 +1086,9 @@ static inline void save_sprs(struct thread_struct *t)\n> \t\tt->tar = mfspr(SPRN_TAR);\n> \t}\n> #endif\n> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> +\tthread_pkey_regs_save(t);\n> +#endif\n\nJust define two variants of thread_pkey_regs_save() based on\nCONFIG_PPC64_MEMORY_PROTECTION_KEYS and remove the #ifdefs from process.c\nDitto for the lines below\n\n> }\n> \n> static inline void restore_sprs(struct thread_struct *old_thread,\n> @@ -1120,6 +1124,9 @@ static inline void restore_sprs(struct thread_struct *old_thread,\n> \t\t\tmtspr(SPRN_TAR, new_thread->tar);\n> \t}\n> #endif\n> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> +\tthread_pkey_regs_restore(new_thread, old_thread);\n> +#endif\n> }\n> \n> #ifdef CONFIG_PPC_BOOK3S_64\n> @@ -1705,6 +1712,9 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)\n> \tcurrent->thread.tm_tfiar = 0;\n> \tcurrent->thread.load_tm = 0;\n> #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */\n> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> +\tthread_pkey_regs_init(¤t->thread);\n> +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */\n> }\n> EXPORT_SYMBOL(start_thread);\n> \n> diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c\n> index 2282864..7cd1be4 100644\n> --- a/arch/powerpc/mm/pkeys.c\n> +++ b/arch/powerpc/mm/pkeys.c\n> @@ -149,3 +149,42 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,\n> \tinit_amr(pkey, new_amr_bits);\n> \treturn 0;\n> }\n> +\n> +void thread_pkey_regs_save(struct thread_struct *thread)\n> +{\n> +\tif (!pkey_inited)\n> +\t\treturn;\n> +\n> +\t/* @TODO skip saving any registers if the thread\n> +\t * has not used any keys yet.\n> +\t */\n\nComment style is broken\n\n> +\n> +\tthread->amr = read_amr();\n> +\tthread->iamr = read_iamr();\n> +\tthread->uamor = read_uamor();\n> +}\n> +\n> +void thread_pkey_regs_restore(struct thread_struct *new_thread,\n> +\t\t\tstruct thread_struct *old_thread)\n> +{\n> +\tif (!pkey_inited)\n> +\t\treturn;\n> +\n> +\t/* @TODO just reset uamor to zero if the new_thread\n> +\t * has not used any keys yet.\n> +\t */\n\nComment style is broken.\n\n> +\n> +\tif (old_thread->amr != new_thread->amr)\n> +\t\twrite_amr(new_thread->amr);\n> +\tif (old_thread->iamr != new_thread->iamr)\n> +\t\twrite_iamr(new_thread->iamr);\n> +\tif (old_thread->uamor != new_thread->uamor)\n> +\t\twrite_uamor(new_thread->uamor);\n\nIs this order correct? Ideally, You want to write the uamor first\nbut since we are in supervisor state, I think we can get away\nwith this order. Do we want to expose the uamor to user space\nfor it to modify the AMR directly?\n\n> +}\n> +\n> +void thread_pkey_regs_init(struct thread_struct *thread)\n> +{\n> +\twrite_amr(0x0ul);\n> +\twrite_iamr(0x0ul);\n> +\twrite_uamor(0x0ul);\n\nThis is not correct, reserved keys should not be set to 0's\n\nBalbir Singh.","headers":{"Return-Path":"","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yGylQ67Y0z9t38\n\tfor ;\n\tWed, 18 Oct 2017 14:50:58 +1100 (AEDT)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3yGylQ4m8QzDrRN\n\tfor ;\n\tWed, 18 Oct 2017 14:50:58 +1100 (AEDT)","from mail-pf0-x243.google.com (mail-pf0-x243.google.com\n\t[IPv6:2607:f8b0:400e:c00::243])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3yGyjf0jv0zDrG0\n\tfor ;\n\tWed, 18 Oct 2017 14:49:25 +1100 (AEDT)","by mail-pf0-x243.google.com with SMTP id 17so2922693pfn.12\n\tfor ;\n\tTue, 17 Oct 2017 20:49:25 -0700 (PDT)","from firefly.ozlabs.ibm.com ([122.99.82.10])\n\tby smtp.gmail.com with ESMTPSA id\n\ti11sm24220101pgc.88.2017.10.17.20.49.19\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 17 Oct 2017 20:49:23 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"ALzuEQiL\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"ALzuEQiL\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c00::243; helo=mail-pf0-x243.google.com;\n\tenvelope-from=bsingharora@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"ALzuEQiL\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=9M6C4CyEqwnkEe1aEdJH01IzmCe/0b1MpStDlg9svqo=;\n\tb=ALzuEQiL5J/xTSdSXKkUMcCsAWg6JlkVlgD7DGBjDNG9NqnItfMO2BTi7cTAqeWgGk\n\taLfxeozQdns0VtJDcdKuV+w+bLAwddiBXFSARMVVJV3bBlnZtTw32132HsdfIr9C81DO\n\tz7/qtGcCEuHH0KUcRgKiQSup0oxGVlv3EU6SpxTyuhARrznAUEVVs/qmG/isVu1rrJw4\n\t3Cxy570tkesklRo+i5ODUnkR0TeoUgLjcFr1TM1k/wp9LG9g4nYulVCTwazqNYWkr0Iy\n\tbTGPU1jhPRK1Ro3/7T5uvFtfmw9eHa9w9iW4uVvsk2Ir6BCZYSuqVkqElwbTo45VE9Ey\n\t6aQQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=9M6C4CyEqwnkEe1aEdJH01IzmCe/0b1MpStDlg9svqo=;\n\tb=H54Kw6GFilfyxsmudQY7Asw1iscU/zlVGx6MWRwSNsDG6FOn47KMMrhCPpPwpEtmvY\n\tvOSk1RGKYrxObVvYoBV2axmEsk2N3ElYadjwIrIJ0cTUcDk7gmeZWs9gBPWIhsyDAjSg\n\t8N5VSirN0Ge9jDGnMjirEemoFlRVLbJ74BDyEendFnfqbMHo+cbdINQEelZKMzdTdZz8\n\ttWDBSh0rjvUUR4c9rpYUIoRzpxqIkP4eZ/i+LxySPldkDq1JmFBW+noyfXL6D3U+PLLV\n\t2I6tqNUwXho48UwyhB6bvTmzGLvRkvo1Hw6PBR+lSKPcapa9TUIe+UBX7njP8vrO9064\n\tnfYQ==","X-Gm-Message-State":"AMCzsaVLzeBIQuChFzvPo1d6Egaraagoed2pq12xEco7q+nyzaX6SeTj\n\tactVGO4MiWEEm6co8qrVTeYoKwZf","X-Google-Smtp-Source":"AOwi7QAdfWVEGcHlz2Gya7Aa1bx2lV5Td/xoq6Ai7cV01y3L5f2A1T6lm0r39h1d+ARsgGRaDWr+4Q==","X-Received":"by 10.98.8.74 with SMTP id c71mr13517824pfd.101.1508298563583;\n\tTue, 17 Oct 2017 20:49:23 -0700 (PDT)","Date":"Wed, 18 Oct 2017 14:49:14 +1100","From":"Balbir Singh ","To":"Ram Pai ","Subject":"Re: [PATCH 10/25] powerpc: store and restore the pkey state across\n\tcontext switches","Message-ID":"<20171018144914.6252f52a@firefly.ozlabs.ibm.com>","In-Reply-To":"<1504910713-7094-19-git-send-email-linuxram@us.ibm.com>","References":"<1504910713-7094-1-git-send-email-linuxram@us.ibm.com>\n\t<1504910713-7094-19-git-send-email-linuxram@us.ibm.com>","X-Mailer":"Claws Mail 3.15.1-dirty (GTK+ 2.24.31; x86_64-redhat-linux-gnu)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List\n\t","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"ebiederm@xmission.com, mhocko@kernel.org, paulus@samba.org,\n\taneesh.kumar@linux.vnet.ibm.com, bauerman@linux.vnet.ibm.com,\n\tlinuxppc-dev@lists.ozlabs.org, khandual@linux.vnet.ibm.com","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}},{"id":1789962,"web_url":"http://patchwork.ozlabs.org/comment/1789962/","msgid":"<20171018204705.GF5617@ram.oc3035372033.ibm.com>","date":"2017-10-18T20:47:05","subject":"Re: [PATCH 10/25] powerpc: store and restore the pkey state across\n\tcontext switches","submitter":{"id":2667,"url":"http://patchwork.ozlabs.org/api/people/2667/","name":"Ram Pai","email":"linuxram@us.ibm.com"},"content":"On Wed, Oct 18, 2017 at 02:49:14PM +1100, Balbir Singh wrote:\n> On Fri, 8 Sep 2017 15:44:58 -0700\n> Ram Pai wrote:\n> \n> > Store and restore the AMR, IAMR and UAMOR register state of the task\n> > before scheduling out and after scheduling in, respectively.\n> > \n> > Signed-off-by: Ram Pai \n> > ---\n> > arch/powerpc/include/asm/pkeys.h | 4 +++\n> > arch/powerpc/include/asm/processor.h | 5 ++++\n> > arch/powerpc/kernel/process.c | 10 ++++++++\n> > arch/powerpc/mm/pkeys.c | 39 ++++++++++++++++++++++++++++++++++\n> > 4 files changed, 58 insertions(+), 0 deletions(-)\n> > \n> > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h\n> > index 7fd48a4..78c5362 100644\n> > --- a/arch/powerpc/include/asm/pkeys.h\n> > +++ b/arch/powerpc/include/asm/pkeys.h\n> > @@ -143,5 +143,9 @@ static inline void pkey_mm_init(struct mm_struct *mm)\n> > \tmm_pkey_allocation_map(mm) = initial_allocation_mask;\n> > }\n> > \n> > +extern void thread_pkey_regs_save(struct thread_struct *thread);\n> > +extern void thread_pkey_regs_restore(struct thread_struct *new_thread,\n> > +\t\t\tstruct thread_struct *old_thread);\n> > +extern void thread_pkey_regs_init(struct thread_struct *thread);\n> > extern void pkey_initialize(void);\n> > #endif /*_ASM_PPC64_PKEYS_H */\n> > diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h\n> > index fab7ff8..de9d9ba 100644\n> > --- a/arch/powerpc/include/asm/processor.h\n> > +++ b/arch/powerpc/include/asm/processor.h\n> > @@ -309,6 +309,11 @@ struct thread_struct {\n> > \tstruct thread_vr_state ckvr_state; /* Checkpointed VR state */\n> > \tunsigned long\tckvrsave; /* Checkpointed VRSAVE */\n> > #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */\n> > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > +\tunsigned long\tamr;\n> > +\tunsigned long\tiamr;\n> > +\tunsigned long\tuamor;\n> > +#endif\n> > #ifdef CONFIG_KVM_BOOK3S_32_HANDLER\n> > \tvoid*\t\tkvm_shadow_vcpu; /* KVM internal data */\n> > #endif /* CONFIG_KVM_BOOK3S_32_HANDLER */\n> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c\n> > index a0c74bb..ba80002 100644\n> > --- a/arch/powerpc/kernel/process.c\n> > +++ b/arch/powerpc/kernel/process.c\n> > @@ -42,6 +42,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \n> > #include \n> > @@ -1085,6 +1086,9 @@ static inline void save_sprs(struct thread_struct *t)\n> > \t\tt->tar = mfspr(SPRN_TAR);\n> > \t}\n> > #endif\n> > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > +\tthread_pkey_regs_save(t);\n> > +#endif\n> \n> Just define two variants of thread_pkey_regs_save() based on\n> CONFIG_PPC64_MEMORY_PROTECTION_KEYS and remove the #ifdefs from process.c\n> Ditto for the lines below\n\nok.\n\n> \n> > }\n> > \n> > static inline void restore_sprs(struct thread_struct *old_thread,\n> > @@ -1120,6 +1124,9 @@ static inline void restore_sprs(struct thread_struct *old_thread,\n> > \t\t\tmtspr(SPRN_TAR, new_thread->tar);\n> > \t}\n> > #endif\n> > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > +\tthread_pkey_regs_restore(new_thread, old_thread);\n> > +#endif\n\nok.\n\n> > }\n> > \n> > #ifdef CONFIG_PPC_BOOK3S_64\n> > @@ -1705,6 +1712,9 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)\n> > \tcurrent->thread.tm_tfiar = 0;\n> > \tcurrent->thread.load_tm = 0;\n> > #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */\n> > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > +\tthread_pkey_regs_init(¤t->thread);\n> > +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */\n> > }\n> > EXPORT_SYMBOL(start_thread);\n> > \n> > diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c\n> > index 2282864..7cd1be4 100644\n> > --- a/arch/powerpc/mm/pkeys.c\n> > +++ b/arch/powerpc/mm/pkeys.c\n> > @@ -149,3 +149,42 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,\n> > \tinit_amr(pkey, new_amr_bits);\n> > \treturn 0;\n> > }\n> > +\n> > +void thread_pkey_regs_save(struct thread_struct *thread)\n> > +{\n> > +\tif (!pkey_inited)\n> > +\t\treturn;\n> > +\n> > +\t/* @TODO skip saving any registers if the thread\n> > +\t * has not used any keys yet.\n> > +\t */\n> \n> Comment style is broken\n\nok. this time i will fix them. It misses by radar screen because\ncheckpatch.pl does not complain. \n\n> \n> > +\n> > +\tthread->amr = read_amr();\n> > +\tthread->iamr = read_iamr();\n> > +\tthread->uamor = read_uamor();\n> > +}\n> > +\n> > +void thread_pkey_regs_restore(struct thread_struct *new_thread,\n> > +\t\t\tstruct thread_struct *old_thread)\n> > +{\n> > +\tif (!pkey_inited)\n> > +\t\treturn;\n> > +\n> > +\t/* @TODO just reset uamor to zero if the new_thread\n> > +\t * has not used any keys yet.\n> > +\t */\n> \n> Comment style is broken.\n> \n> > +\n> > +\tif (old_thread->amr != new_thread->amr)\n> > +\t\twrite_amr(new_thread->amr);\n> > +\tif (old_thread->iamr != new_thread->iamr)\n> > +\t\twrite_iamr(new_thread->iamr);\n> > +\tif (old_thread->uamor != new_thread->uamor)\n> > +\t\twrite_uamor(new_thread->uamor);\n> \n> Is this order correct? Ideally, You want to write the uamor first\n> but since we are in supervisor state, I think we can get away\n> with this order. \n\nwe could be in hypervisor state too, as is the case when we run\na powernv kernel.\n\nBut..does it matter in which order they are written? if\nthe thread is in the kernel, it cannot execute any instructions\nin userspace. So it wont see a intermediate state. right?\nor am i getting this wrong?\n\n> Do we want to expose the uamor to user space\n> for it to modify the AMR directly?\n\nsorry I did not understand the comment. UAMOR cannot\nbe accessed from usespace. and there are no system calls\ncurrently to help userspace to program the UAMOR on its\nbehalf.\n\n> \n> > +}\n> > +\n> > +void thread_pkey_regs_init(struct thread_struct *thread)\n> > +{\n> > +\twrite_amr(0x0ul);\n> > +\twrite_iamr(0x0ul);\n> > +\twrite_uamor(0x0ul);\n> \n> This is not correct, reserved keys should not be set to 0's\n\nok. makes sense. best to not touch reserved key bits here.\n\n> \n> Balbir Singh.","headers":{"Return-Path":"","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yHPKD1pgdz9t6K\n\tfor ;\n\tThu, 19 Oct 2017 07:48:16 +1100 (AEDT)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3yHPKD0ylvzDqmS\n\tfor ;\n\tThu, 19 Oct 2017 07:48:16 +1100 (AEDT)","from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com\n\t[148.163.156.1])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3yHPJ41G10zDqBn\n\tfor ;\n\tThu, 19 Oct 2017 07:47:15 +1100 (AEDT)","from pps.filterd (m0098399.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv9IKkSwp103463\n\tfor ; Wed, 18 Oct 2017 16:47:13 -0400","from e16.ny.us.ibm.com (e16.ny.us.ibm.com [129.33.205.206])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2dpax6bgs2-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Wed, 18 Oct 2017 16:47:13 -0400","from localhost\n\tby e16.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tWed, 18 Oct 2017 16:47:12 -0400","from b01cxnp22035.gho.pok.ibm.com (9.57.198.25)\n\tby e16.ny.us.ibm.com (146.89.104.203) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tWed, 18 Oct 2017 16:47:09 -0400","from b01ledav004.gho.pok.ibm.com (b01ledav004.gho.pok.ibm.com\n\t[9.57.199.109])\n\tby b01cxnp22035.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP\n\tid v9IKl8uh50266116; Wed, 18 Oct 2017 20:47:08 GMT","from b01ledav004.gho.pok.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 51A18112040;\n\tWed, 18 Oct 2017 16:46:41 -0400 (EDT)","from ram.oc3035372033.ibm.com (unknown [9.85.176.245])\n\tby b01ledav004.gho.pok.ibm.com (Postfix) with ESMTPS id 12285112034; \n\tWed, 18 Oct 2017 16:46:39 -0400 (EDT)"],"Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=us.ibm.com\n\t(client-ip=148.163.156.1; helo=mx0a-001b2d01.pphosted.com;\n\tenvelope-from=linuxram@us.ibm.com; receiver=)","Date":"Wed, 18 Oct 2017 13:47:05 -0700","From":"Ram Pai ","To":"Balbir Singh ","Subject":"Re: [PATCH 10/25] powerpc: store and restore the pkey state across\n\tcontext switches","References":"<1504910713-7094-1-git-send-email-linuxram@us.ibm.com>\n\t<1504910713-7094-19-git-send-email-linuxram@us.ibm.com>\n\t<20171018144914.6252f52a@firefly.ozlabs.ibm.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20171018144914.6252f52a@firefly.ozlabs.ibm.com>","User-Agent":"Mutt/1.5.20 (2009-12-10)","X-TM-AS-GCONF":"00","x-cbid":"17101820-0024-0000-0000-000002E4AAEC","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007915; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000237; SDB=6.00933058; UDB=6.00469930;\n\tIPR=6.00713348; \n\tBA=6.00005648; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017596;\n\tXFM=3.00000015; UTC=2017-10-18 20:47:11","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17101820-0025-0000-0000-000045C5A142","Message-Id":"<20171018204705.GF5617@ram.oc3035372033.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-10-18_08:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1710180287","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List\n\t","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"Ram Pai ","Cc":"ebiederm@xmission.com, mhocko@kernel.org, paulus@samba.org,\n\taneesh.kumar@linux.vnet.ibm.com, bauerman@linux.vnet.ibm.com,\n\tlinuxppc-dev@lists.ozlabs.org, khandual@linux.vnet.ibm.com","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}},{"id":1790030,"web_url":"http://patchwork.ozlabs.org/comment/1790030/","msgid":"<20171019100038.57ecebc2@MiWiFi-R3-srv>","date":"2017-10-18T23:00:38","subject":"Re: [PATCH 10/25] powerpc: store and restore the pkey state across\n\tcontext switches","submitter":{"id":9347,"url":"http://patchwork.ozlabs.org/api/people/9347/","name":"Balbir Singh","email":"bsingharora@gmail.com"},"content":"On Wed, 18 Oct 2017 13:47:05 -0700\nRam Pai wrote:\n\n> On Wed, Oct 18, 2017 at 02:49:14PM +1100, Balbir Singh wrote:\n> > On Fri, 8 Sep 2017 15:44:58 -0700\n> > Ram Pai wrote:\n> > \n> > > Store and restore the AMR, IAMR and UAMOR register state of the task\n> > > before scheduling out and after scheduling in, respectively.\n> > > \n> > > Signed-off-by: Ram Pai \n> > > ---\n> > > arch/powerpc/include/asm/pkeys.h | 4 +++\n> > > arch/powerpc/include/asm/processor.h | 5 ++++\n> > > arch/powerpc/kernel/process.c | 10 ++++++++\n> > > arch/powerpc/mm/pkeys.c | 39 ++++++++++++++++++++++++++++++++++\n> > > 4 files changed, 58 insertions(+), 0 deletions(-)\n> > > \n> > > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h\n> > > index 7fd48a4..78c5362 100644\n> > > --- a/arch/powerpc/include/asm/pkeys.h\n> > > +++ b/arch/powerpc/include/asm/pkeys.h\n> > > @@ -143,5 +143,9 @@ static inline void pkey_mm_init(struct mm_struct *mm)\n> > > \tmm_pkey_allocation_map(mm) = initial_allocation_mask;\n> > > }\n> > > \n> > > +extern void thread_pkey_regs_save(struct thread_struct *thread);\n> > > +extern void thread_pkey_regs_restore(struct thread_struct *new_thread,\n> > > +\t\t\tstruct thread_struct *old_thread);\n> > > +extern void thread_pkey_regs_init(struct thread_struct *thread);\n> > > extern void pkey_initialize(void);\n> > > #endif /*_ASM_PPC64_PKEYS_H */\n> > > diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h\n> > > index fab7ff8..de9d9ba 100644\n> > > --- a/arch/powerpc/include/asm/processor.h\n> > > +++ b/arch/powerpc/include/asm/processor.h\n> > > @@ -309,6 +309,11 @@ struct thread_struct {\n> > > \tstruct thread_vr_state ckvr_state; /* Checkpointed VR state */\n> > > \tunsigned long\tckvrsave; /* Checkpointed VRSAVE */\n> > > #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */\n> > > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > > +\tunsigned long\tamr;\n> > > +\tunsigned long\tiamr;\n> > > +\tunsigned long\tuamor;\n> > > +#endif\n> > > #ifdef CONFIG_KVM_BOOK3S_32_HANDLER\n> > > \tvoid*\t\tkvm_shadow_vcpu; /* KVM internal data */\n> > > #endif /* CONFIG_KVM_BOOK3S_32_HANDLER */\n> > > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c\n> > > index a0c74bb..ba80002 100644\n> > > --- a/arch/powerpc/kernel/process.c\n> > > +++ b/arch/powerpc/kernel/process.c\n> > > @@ -42,6 +42,7 @@\n> > > #include \n> > > #include \n> > > #include \n> > > +#include \n> > > \n> > > #include \n> > > #include \n> > > @@ -1085,6 +1086,9 @@ static inline void save_sprs(struct thread_struct *t)\n> > > \t\tt->tar = mfspr(SPRN_TAR);\n> > > \t}\n> > > #endif\n> > > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > > +\tthread_pkey_regs_save(t);\n> > > +#endif \n> > \n> > Just define two variants of thread_pkey_regs_save() based on\n> > CONFIG_PPC64_MEMORY_PROTECTION_KEYS and remove the #ifdefs from process.c\n> > Ditto for the lines below \n> \n> ok.\n> \n> > \n> > > }\n> > > \n> > > static inline void restore_sprs(struct thread_struct *old_thread,\n> > > @@ -1120,6 +1124,9 @@ static inline void restore_sprs(struct thread_struct *old_thread,\n> > > \t\t\tmtspr(SPRN_TAR, new_thread->tar);\n> > > \t}\n> > > #endif\n> > > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > > +\tthread_pkey_regs_restore(new_thread, old_thread);\n> > > +#endif \n> \n> ok.\n> \n> > > }\n> > > \n> > > #ifdef CONFIG_PPC_BOOK3S_64\n> > > @@ -1705,6 +1712,9 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)\n> > > \tcurrent->thread.tm_tfiar = 0;\n> > > \tcurrent->thread.load_tm = 0;\n> > > #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */\n> > > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > > +\tthread_pkey_regs_init(¤t->thread);\n> > > +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */\n> > > }\n> > > EXPORT_SYMBOL(start_thread);\n> > > \n> > > diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c\n> > > index 2282864..7cd1be4 100644\n> > > --- a/arch/powerpc/mm/pkeys.c\n> > > +++ b/arch/powerpc/mm/pkeys.c\n> > > @@ -149,3 +149,42 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,\n> > > \tinit_amr(pkey, new_amr_bits);\n> > > \treturn 0;\n> > > }\n> > > +\n> > > +void thread_pkey_regs_save(struct thread_struct *thread)\n> > > +{\n> > > +\tif (!pkey_inited)\n> > > +\t\treturn;\n> > > +\n> > > +\t/* @TODO skip saving any registers if the thread\n> > > +\t * has not used any keys yet.\n> > > +\t */ \n> > \n> > Comment style is broken \n> \n> ok. this time i will fix them. It misses by radar screen because\n> checkpatch.pl does not complain. \n>\n\nYep, there is an lkml thread on this style of coding. It's\nbest avoided.\n\n> > \n> > > +\n> > > +\tthread->amr = read_amr();\n> > > +\tthread->iamr = read_iamr();\n> > > +\tthread->uamor = read_uamor();\n> > > +}\n> > > +\n> > > +void thread_pkey_regs_restore(struct thread_struct *new_thread,\n> > > +\t\t\tstruct thread_struct *old_thread)\n> > > +{\n> > > +\tif (!pkey_inited)\n> > > +\t\treturn;\n> > > +\n> > > +\t/* @TODO just reset uamor to zero if the new_thread\n> > > +\t * has not used any keys yet.\n> > > +\t */ \n> > \n> > Comment style is broken.\n> > \n> > > +\n> > > +\tif (old_thread->amr != new_thread->amr)\n> > > +\t\twrite_amr(new_thread->amr);\n> > > +\tif (old_thread->iamr != new_thread->iamr)\n> > > +\t\twrite_iamr(new_thread->iamr);\n> > > +\tif (old_thread->uamor != new_thread->uamor)\n> > > +\t\twrite_uamor(new_thread->uamor); \n> > \n> > Is this order correct? Ideally, You want to write the uamor first\n> > but since we are in supervisor state, I think we can get away\n> > with this order. \n> \n> we could be in hypervisor state too, as is the case when we run\n> a powernv kernel.\n> \n> But..does it matter in which order they are written? if\n> the thread is in the kernel, it cannot execute any instructions\n> in userspace. So it wont see a intermediate state. right?\n> or am i getting this wrong?\n\nYou are right, since uamor + amor control what can and\ncannot be set, there is a subtle dependency, but it does\nnot apply to the kernel doing the context switch. AMR has\ntwo SPR values, 13 and 29. I presume we are using SPR #29\nhere?\n\n> \n> > Do we want to expose the uamor to user space\n> > for it to modify the AMR directly? \n> \n> sorry I did not understand the comment. UAMOR cannot\n> be accessed from usespace. and there are no system calls\n> currently to help userspace to program the UAMOR on its\n> behalf.\n> \n\nI was just wondering how two threads can share a key if\nthey decide to. They would need uamor to give them permissions\nto the same set of keys and then reuse the key via\npkey_mprotect(.., pkey). I am missing the bit about how\nuamor's across these threads would be synchronized.\n\n\n> > \n> > > +}\n> > > +\n> > > +void thread_pkey_regs_init(struct thread_struct *thread)\n> > > +{\n> > > +\twrite_amr(0x0ul);\n> > > +\twrite_iamr(0x0ul);\n> > > +\twrite_uamor(0x0ul); \n> > \n> > This is not correct, reserved keys should not be set to 0's \n> \n> ok. makes sense. best to not touch reserved key bits here.\n\nAlso this implies that at init time, the thread has access to all\nkeys, but it can't modify any of the keys in the AMR/IAMR.\n\n> \n> > \n> > Balbir Singh. \n>","headers":{"Return-Path":"","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yHSJf4WR1z9t4P\n\tfor ;\n\tThu, 19 Oct 2017 10:02:58 +1100 (AEDT)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3yHSJf2cZtzDqG3\n\tfor ;\n\tThu, 19 Oct 2017 10:02:58 +1100 (AEDT)","from mail-pf0-x241.google.com (mail-pf0-x241.google.com\n\t[IPv6:2607:f8b0:400e:c00::241])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3yHSGF4nrszDq5x\n\tfor ;\n\tThu, 19 Oct 2017 10:00:52 +1100 (AEDT)","by mail-pf0-x241.google.com with SMTP id 17so5045460pfn.12\n\tfor ;\n\tWed, 18 Oct 2017 16:00:52 -0700 (PDT)","from MiWiFi-R3-srv ([122.99.82.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tu8sm20487032pgp.17.2017.10.18.16.00.45\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 18 Oct 2017 16:00:49 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"YGMdvccf\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"YGMdvccf\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c00::241; helo=mail-pf0-x241.google.com;\n\tenvelope-from=bsingharora@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"YGMdvccf\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=/5xVCLoF4ndZI/kxR96dUdpADMhyXk5Jx7ZkwBUS0xU=;\n\tb=YGMdvccfvfh+pmAFYGoCgTqHBxCYLk/osFw35FK9rii+LbM6hhuxWzPup81uW2VVBP\n\tjtcnzImyqg7mqfxl2KmJqMCJUnY+/XJtHYalhqE1SrE1TM8Oqjf4vKEui9Qxj/ZBl+4G\n\t0EhcSp9k8EtCPdK6jCg3F9N5NhoGqILri1XrXkaL7i4oebbOXLaVvb0LAGsd76Bgahgp\n\tedLy3isiqsc+ovb9jxslTDPHEAapjqbkwiK6EyM76nFCyxvZOzS/PqJwl+ZPCUPep+k2\n\tGDEunNFM/8w3/914ojxB9pmF6XbL8Cw0NF//FcrbyYiIX3G0rrGAkKEA8o5G8XKXiimR\n\tPTcQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=/5xVCLoF4ndZI/kxR96dUdpADMhyXk5Jx7ZkwBUS0xU=;\n\tb=LFC02RJ5mQ77j7baMVGdKTWD7sRoAwMM4KVti2lNT6dXGdLBDOcz6wPlajAfvK6dki\n\tBLHkC4hRnW7/Pi9x3zTOXpw2LvxHGC07RAQB4juP0Rl8vcnl693kdQeTa71jMs1BoFLY\n\t4IG9wE4msFYVCfGG2orJwBqxz1fCT/CZhrCrfxvA9Ftk3j8KJ4ehgqrIiJ5GJkrPiT14\n\tk0ksyQeH2+woa3i3zyePl00SCZzqRn4hvbq0qtnn/XrFAWF0eriCHN1pk6iMFM/j/AtU\n\t8Uq6A9woCwvOXBGPLnrxRLnV+y3kPP2vl5xMcCp9EZP7MHUWAM3RjM/V+U+SV8mOPJwO\n\tRQ0g==","X-Gm-Message-State":"AMCzsaUpObTylomDEfytw2eWgMoi9dkQAPdoYzynz4gePSNAQxYsV/A0\n\t+j1JgDf8NYxABJykulbSWBs=","X-Google-Smtp-Source":"ABhQp+Rs4U1L7k+XdIPbq8JcLvX6hPSG73NMONCE2czG72rwRDYRye5KTD6E0oVeruIFMjnZTm/rmw==","X-Received":"by 10.84.245.147 with SMTP id j19mr479873pll.138.1508367649884; \n\tWed, 18 Oct 2017 16:00:49 -0700 (PDT)","Date":"Thu, 19 Oct 2017 10:00:38 +1100","From":"Balbir Singh ","To":"Ram Pai ","Subject":"Re: [PATCH 10/25] powerpc: store and restore the pkey state across\n\tcontext switches","Message-ID":"<20171019100038.57ecebc2@MiWiFi-R3-srv>","In-Reply-To":"<20171018204705.GF5617@ram.oc3035372033.ibm.com>","References":"<1504910713-7094-1-git-send-email-linuxram@us.ibm.com>\n\t<1504910713-7094-19-git-send-email-linuxram@us.ibm.com>\n\t<20171018144914.6252f52a@firefly.ozlabs.ibm.com>\n\t<20171018204705.GF5617@ram.oc3035372033.ibm.com>","X-Mailer":"Claws Mail 3.15.1-dirty (GTK+ 2.24.31; x86_64-redhat-linux-gnu)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List\n\t","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"ebiederm@xmission.com, mhocko@kernel.org, paulus@samba.org,\n\taneesh.kumar@linux.vnet.ibm.com, bauerman@linux.vnet.ibm.com,\n\tlinuxppc-dev@lists.ozlabs.org, khandual@linux.vnet.ibm.com","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}},{"id":1790047,"web_url":"http://patchwork.ozlabs.org/comment/1790047/","msgid":"<20171019005203.GQ5617@ram.oc3035372033.ibm.com>","date":"2017-10-19T00:52:03","subject":"Re: [PATCH 10/25] powerpc: store and restore the pkey state across\n\tcontext switches","submitter":{"id":2667,"url":"http://patchwork.ozlabs.org/api/people/2667/","name":"Ram Pai","email":"linuxram@us.ibm.com"},"content":"On Thu, Oct 19, 2017 at 10:00:38AM +1100, Balbir Singh wrote:\n> On Wed, 18 Oct 2017 13:47:05 -0700\n> Ram Pai wrote:\n> \n> > On Wed, Oct 18, 2017 at 02:49:14PM +1100, Balbir Singh wrote:\n> > > On Fri, 8 Sep 2017 15:44:58 -0700\n> > > Ram Pai wrote:\n> > > \n> > > > Store and restore the AMR, IAMR and UAMOR register state of the task\n> > > > before scheduling out and after scheduling in, respectively.\n> > > > \n> > > > Signed-off-by: Ram Pai \n> > > > ---\n> > > > arch/powerpc/include/asm/pkeys.h | 4 +++\n> > > > arch/powerpc/include/asm/processor.h | 5 ++++\n> > > > arch/powerpc/kernel/process.c | 10 ++++++++\n> > > > arch/powerpc/mm/pkeys.c | 39 ++++++++++++++++++++++++++++++++++\n> > > > 4 files changed, 58 insertions(+), 0 deletions(-)\n> > > > \n> > > > diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h\n> > > > index 7fd48a4..78c5362 100644\n> > > > --- a/arch/powerpc/include/asm/pkeys.h\n> > > > +++ b/arch/powerpc/include/asm/pkeys.h\n> > > > @@ -143,5 +143,9 @@ static inline void pkey_mm_init(struct mm_struct *mm)\n> > > > \tmm_pkey_allocation_map(mm) = initial_allocation_mask;\n> > > > }\n> > > > \n> > > > +extern void thread_pkey_regs_save(struct thread_struct *thread);\n> > > > +extern void thread_pkey_regs_restore(struct thread_struct *new_thread,\n> > > > +\t\t\tstruct thread_struct *old_thread);\n> > > > +extern void thread_pkey_regs_init(struct thread_struct *thread);\n> > > > extern void pkey_initialize(void);\n> > > > #endif /*_ASM_PPC64_PKEYS_H */\n> > > > diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h\n> > > > index fab7ff8..de9d9ba 100644\n> > > > --- a/arch/powerpc/include/asm/processor.h\n> > > > +++ b/arch/powerpc/include/asm/processor.h\n> > > > @@ -309,6 +309,11 @@ struct thread_struct {\n> > > > \tstruct thread_vr_state ckvr_state; /* Checkpointed VR state */\n> > > > \tunsigned long\tckvrsave; /* Checkpointed VRSAVE */\n> > > > #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */\n> > > > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > > > +\tunsigned long\tamr;\n> > > > +\tunsigned long\tiamr;\n> > > > +\tunsigned long\tuamor;\n> > > > +#endif\n> > > > #ifdef CONFIG_KVM_BOOK3S_32_HANDLER\n> > > > \tvoid*\t\tkvm_shadow_vcpu; /* KVM internal data */\n> > > > #endif /* CONFIG_KVM_BOOK3S_32_HANDLER */\n> > > > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c\n> > > > index a0c74bb..ba80002 100644\n> > > > --- a/arch/powerpc/kernel/process.c\n> > > > +++ b/arch/powerpc/kernel/process.c\n> > > > @@ -42,6 +42,7 @@\n> > > > #include \n> > > > #include \n> > > > #include \n> > > > +#include \n> > > > \n> > > > #include \n> > > > #include \n> > > > @@ -1085,6 +1086,9 @@ static inline void save_sprs(struct thread_struct *t)\n> > > > \t\tt->tar = mfspr(SPRN_TAR);\n> > > > \t}\n> > > > #endif\n> > > > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > > > +\tthread_pkey_regs_save(t);\n> > > > +#endif \n> > > \n> > > Just define two variants of thread_pkey_regs_save() based on\n> > > CONFIG_PPC64_MEMORY_PROTECTION_KEYS and remove the #ifdefs from process.c\n> > > Ditto for the lines below \n> > \n> > ok.\n> > \n> > > \n> > > > }\n> > > > \n> > > > static inline void restore_sprs(struct thread_struct *old_thread,\n> > > > @@ -1120,6 +1124,9 @@ static inline void restore_sprs(struct thread_struct *old_thread,\n> > > > \t\t\tmtspr(SPRN_TAR, new_thread->tar);\n> > > > \t}\n> > > > #endif\n> > > > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > > > +\tthread_pkey_regs_restore(new_thread, old_thread);\n> > > > +#endif \n> > \n> > ok.\n> > \n> > > > }\n> > > > \n> > > > #ifdef CONFIG_PPC_BOOK3S_64\n> > > > @@ -1705,6 +1712,9 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)\n> > > > \tcurrent->thread.tm_tfiar = 0;\n> > > > \tcurrent->thread.load_tm = 0;\n> > > > #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */\n> > > > +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS\n> > > > +\tthread_pkey_regs_init(¤t->thread);\n> > > > +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */\n> > > > }\n> > > > EXPORT_SYMBOL(start_thread);\n> > > > \n> > > > diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c\n> > > > index 2282864..7cd1be4 100644\n> > > > --- a/arch/powerpc/mm/pkeys.c\n> > > > +++ b/arch/powerpc/mm/pkeys.c\n> > > > @@ -149,3 +149,42 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,\n> > > > \tinit_amr(pkey, new_amr_bits);\n> > > > \treturn 0;\n> > > > }\n> > > > +\n> > > > +void thread_pkey_regs_save(struct thread_struct *thread)\n> > > > +{\n> > > > +\tif (!pkey_inited)\n> > > > +\t\treturn;\n> > > > +\n> > > > +\t/* @TODO skip saving any registers if the thread\n> > > > +\t * has not used any keys yet.\n> > > > +\t */ \n> > > \n> > > Comment style is broken \n> > \n> > ok. this time i will fix them. It misses by radar screen because\n> > checkpatch.pl does not complain. \n> >\n> \n> Yep, there is an lkml thread on this style of coding. It's\n> best avoided.\n> \n> > > \n> > > > +\n> > > > +\tthread->amr = read_amr();\n> > > > +\tthread->iamr = read_iamr();\n> > > > +\tthread->uamor = read_uamor();\n> > > > +}\n> > > > +\n> > > > +void thread_pkey_regs_restore(struct thread_struct *new_thread,\n> > > > +\t\t\tstruct thread_struct *old_thread)\n> > > > +{\n> > > > +\tif (!pkey_inited)\n> > > > +\t\treturn;\n> > > > +\n> > > > +\t/* @TODO just reset uamor to zero if the new_thread\n> > > > +\t * has not used any keys yet.\n> > > > +\t */ \n> > > \n> > > Comment style is broken.\n> > > \n> > > > +\n> > > > +\tif (old_thread->amr != new_thread->amr)\n> > > > +\t\twrite_amr(new_thread->amr);\n> > > > +\tif (old_thread->iamr != new_thread->iamr)\n> > > > +\t\twrite_iamr(new_thread->iamr);\n> > > > +\tif (old_thread->uamor != new_thread->uamor)\n> > > > +\t\twrite_uamor(new_thread->uamor); \n> > > \n> > > Is this order correct? Ideally, You want to write the uamor first\n> > > but since we are in supervisor state, I think we can get away\n> > > with this order. \n> > \n> > we could be in hypervisor state too, as is the case when we run\n> > a powernv kernel.\n> > \n> > But..does it matter in which order they are written? if\n> > the thread is in the kernel, it cannot execute any instructions\n> > in userspace. So it wont see a intermediate state. right?\n> > or am i getting this wrong?\n> \n> You are right, since uamor + amor control what can and\n> cannot be set, there is a subtle dependency, but it does\n> not apply to the kernel doing the context switch. AMR has\n> two SPR values, 13 and 29. I presume we are using SPR #29\n> here?\n\nit is SPRN_AMR, which is 29 (0x1d)\n\n> \n> > \n> > > Do we want to expose the uamor to user space\n> > > for it to modify the AMR directly? \n> > \n> > sorry I did not understand the comment. UAMOR cannot\n> > be accessed from usespace. and there are no system calls\n> > currently to help userspace to program the UAMOR on its\n> > behalf.\n> > \n> \n> I was just wondering how two threads can share a key if\n> they decide to. They would need uamor to give them permissions\n> to the same set of keys and then reuse the key via\n> pkey_mprotect(.., pkey). I am missing the bit about how\n> uamor's across these threads would be synchronized.\n\nAs it stands now, two threads are discouraged to share the same key,\nsince we dont provide synchronization of keys across threads. A key\nallocated in one thread's context has no meaning in the context of\nanother thread. I think, it is constraining to the application, but\nthat is how the semantics were defined and i have implemented it that\nway for powerpc. Yes eventually; one day I am sure, as applications\nstart exploiting this feature they will demand more flexibility. But\nfor now that is what it is. So given the above semantics, there is\nno need currently to synchronize umor/amr/iamr across threads.\n\n> \n> \n> > > \n> > > > +}\n> > > > +\n> > > > +void thread_pkey_regs_init(struct thread_struct *thread)\n> > > > +{\n> > > > +\twrite_amr(0x0ul);\n> > > > +\twrite_iamr(0x0ul);\n> > > > +\twrite_uamor(0x0ul); \n> > > \n> > > This is not correct, reserved keys should not be set to 0's \n> > \n> > ok. makes sense. best to not touch reserved key bits here.\n> \n> Also this implies that at init time, the thread has access to all\n> keys, but it can't modify any of the keys in the AMR/IAMR.\n\nit shouldn't modify the bit corresponding to the reserved keys.\n\nThese patches dont touch AMOR. the hypervisor is entirely in control of\nthe AMOR.\n\nAMOR is the master controller for these keys. If a reserved key is\ndisabled in AMOR, any changes to the bits corresponding to the\nreserved-key in AMR or IAMR or UAMOR will have any effect. \n\nBut if AMOR has enabled a reserved-key, than we will cause bad things by\nchanging the bits in AMR/IAMR/UAMOR. So you are right. We better not\ntouch the bits corresponding to the reserved-keys.\n\nRP","headers":{"Return-Path":"","X-Original-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@ozlabs.org","linuxppc-dev@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yHVmD4w3Lz9t3n\n\tfor ;\n\tThu, 19 Oct 2017 11:53:32 +1100 (AEDT)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3yHVmD41QlzDqhh\n\tfor ;\n\tThu, 19 Oct 2017 11:53:32 +1100 (AEDT)","from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com\n\t[148.163.158.5])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3yHVkl3KKGzDqBH\n\tfor ;\n\tThu, 19 Oct 2017 11:52:13 +1100 (AEDT)","from pps.filterd (m0098417.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv9J0nbaT046507\n\tfor ; Wed, 18 Oct 2017 20:52:11 -0400","from e33.co.us.ibm.com (e33.co.us.ibm.com [32.97.110.151])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2dpbx58v13-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Wed, 18 Oct 2017 20:52:11 -0400","from localhost\n\tby e33.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tWed, 18 Oct 2017 18:52:10 -0600","from b03cxnp07029.gho.boulder.ibm.com (9.17.130.16)\n\tby e33.co.us.ibm.com (192.168.1.133) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tWed, 18 Oct 2017 18:52:08 -0600","from b03ledav004.gho.boulder.ibm.com\n\t(b03ledav004.gho.boulder.ibm.com [9.17.130.235])\n\tby b03cxnp07029.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with\n\tESMTP id v9J0q8e29568550; Wed, 18 Oct 2017 17:52:08 -0700","from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 211D178037;\n\tWed, 18 Oct 2017 18:52:08 -0600 (MDT)","from ram.oc3035372033.ibm.com (unknown [9.85.176.245])\n\tby b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTPS id\n\t3814478038; Wed, 18 Oct 2017 18:52:06 -0600 (MDT)"],"Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=us.ibm.com\n\t(client-ip=148.163.158.5; helo=mx0a-001b2d01.pphosted.com;\n\tenvelope-from=linuxram@us.ibm.com; receiver=)","Date":"Wed, 18 Oct 2017 17:52:03 -0700","From":"Ram Pai ","To":"Balbir Singh ","Subject":"Re: [PATCH 10/25] powerpc: store and restore the pkey state across\n\tcontext switches","References":"<1504910713-7094-1-git-send-email-linuxram@us.ibm.com>\n\t<1504910713-7094-19-git-send-email-linuxram@us.ibm.com>\n\t<20171018144914.6252f52a@firefly.ozlabs.ibm.com>\n\t<20171018204705.GF5617@ram.oc3035372033.ibm.com>\n\t<20171019100038.57ecebc2@MiWiFi-R3-srv>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20171019100038.57ecebc2@MiWiFi-R3-srv>","User-Agent":"Mutt/1.5.20 (2009-12-10)","X-TM-AS-GCONF":"00","x-cbid":"17101900-0008-0000-0000-000008BB7F0A","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007917; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000237; SDB=6.00933139; UDB=6.00469979;\n\tIPR=6.00713430; \n\tBA=6.00005649; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017599;\n\tXFM=3.00000015; UTC=2017-10-19 00:52:10","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17101900-0009-0000-0000-0000446B43B6","Message-Id":"<20171019005203.GQ5617@ram.oc3035372033.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-10-18_09:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1710190010","X-BeenThere":"linuxppc-dev@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List\n\t","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"Ram Pai ","Cc":"ebiederm@xmission.com, mhocko@kernel.org, paulus@samba.org,\n\taneesh.kumar@linux.vnet.ibm.com, bauerman@linux.vnet.ibm.com,\n\tlinuxppc-dev@lists.ozlabs.org, khandual@linux.vnet.ibm.com","Errors-To":"linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org","Sender":"\"Linuxppc-dev\"\n\t"}}]