Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1681868 > unrolled thread

[RFC v5 00/38] powerpc: Memory Protection Keys

Back to article view | Back to linux.kernel

Contents

  [RFC v5 00/38] powerpc: Memory Protection Keys Ram Pai <linuxram@us.ibm.com> - 2017-07-05 23:40 +0200
    Re: [RFC v5 00/38] powerpc: Memory Protection Keys Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-07-10 07:50 +0200
      Re: [RFC v5 00/38] powerpc: Memory Protection Keys Ram Pai <linuxram@us.ibm.com> - 2017-07-10 08:10 +0200
        Re: [RFC v5 00/38] powerpc: Memory Protection Keys Ram Pai <linuxram@us.ibm.com> - 2017-07-10 19:20 +0200
    Re: [RFC v5 00/38] powerpc: Memory Protection Keys Michal Hocko <mhocko@kernel.org> - 2017-07-11 17:00 +0200
      Re: [RFC v5 00/38] powerpc: Memory Protection Keys Ram Pai <linuxram@us.ibm.com> - 2017-07-11 21:40 +0200
        Re: [RFC v5 00/38] powerpc: Memory Protection Keys Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-07-12 00:10 +0200
        Re: [RFC v5 00/38] powerpc: Memory Protection Keys Michal Hocko <mhocko@kernel.org> - 2017-07-12 09:30 +0200
          Re: [RFC v5 00/38] powerpc: Memory Protection Keys Michal Hocko <mhocko@kernel.org> - 2017-07-12 09:50 +0200
          Re: [RFC v5 00/38] powerpc: Memory Protection Keys Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-07-13 01:00 +0200
            Re: [RFC v5 00/38] powerpc: Memory Protection Keys Michal Hocko <mhocko@kernel.org> - 2017-07-13 08:30 +0200
    Re: [RFC v5 36/38] selftest: PowerPC specific test updates to memory  protection keys Dave Hansen <dave.hansen@intel.com> - 2017-07-11 19:40 +0200
      Re: [RFC v5 36/38] selftest: PowerPC specific test updates to memory  protection keys Ram Pai <linuxram@us.ibm.com> - 2017-07-13 00:00 +0200

#1681868 — [RFC v5 00/38] powerpc: Memory Protection Keys

Memory protection keys enable applications to protect its
address space from inadvertent access or corruption from
itself.

The overall idea:

 A process allocates a   key  and associates it with
 an  address  range  within    its   address   space.
 The process  then  can  dynamically  set read/write 
 permissions on  the   key   without  involving  the 
 kernel. Any  code that  violates   the  permissions
 of  the address space; as defined by its associated
 key, will receive a segmentation fault.

This patch series enables the feature on PPC64 HPTE
platform.

ISA3.0 section 5.7.13 describes the detailed specifications.


Testing:
	This patch series has passed all the protection key
	tests available in  the selftests directory.
	The tests are updated to work on both x86 and powerpc.

version v5:
	(1) reverted back to the old design -- store the 
	    key in the pte, instead of bypassing it.
	    The v4 design slowed down the hash page path.
	(2) detects key violation when kernel is told to 
		access user pages.
	(3) further refined the patches into smaller consumable
		units
	(4) page faults handlers captures the faulting key 
	    from the pte instead of the vma. This closes a
	    race between where the key update in the vma and
	    a key fault caused cause by the key programmed
	    in the pte.
	(5) a key created with access-denied should
	    also set it up to deny write. Fixed it.
	(6) protection-key number is displayed in smaps
		the x86 way.
version v4:
	(1) patches no more depend on the pte bits to program
		the hpte -- comment by Balbir
	(2) documentation updates
	(3) fixed a bug in the selftest.
	(4) unlike x86, powerpc lets signal handler change key
	    permission bits; the change will persist across
	    signal handler boundaries. Earlier we allowed
	    the signal handler to modify a field in the siginfo
	    structure which would than be used by the kernel
	    to program the key protection register (AMR)
       		-- resolves a issue raised by Ben.
    		"Calls to sys_swapcontext with a made-up context
	        will end up with a crap AMR if done by code who
	       	didn't know about that register".
	(5) these changes enable protection keys on 4k-page 
		kernel aswell.

version v3:
	(1) split the patches into smaller consumable
		patches.
	(2) added the ability to disable execute permission
		on a key at creation.
	(3) rename  calc_pte_to_hpte_pkey_bits() to
	    pte_to_hpte_pkey_bits() -- suggested by Anshuman
	(4) some code optimization and clarity in
		do_page_fault()  
	(5) A bug fix while invalidating a hpte slot in 
		__hash_page_4K() -- noticed by Aneesh
	

version v2:
	(1) documentation and selftest added
 	(2) fixed a bug in 4k hpte backed 64k pte where page
	    invalidation was not done correctly, and 
	    initialization of second-part-of-the-pte was not
	    done correctly if the pte was not yet Hashed
	    with a hpte.  Reported by Aneesh.
	(3) Fixed ABI breakage caused in siginfo structure.
		Reported by Anshuman.
	

version v1: Initial version

Ram Pai (38):
  powerpc: Free up four 64K PTE bits in 4K backed HPTE pages
  powerpc: Free up four 64K PTE bits in 64K backed HPTE pages
  powerpc: introduce pte_set_hash_slot() helper
  powerpc: introduce pte_get_hash_gslot() helper
  powerpc: capture the PTE format changes in the dump pte report
  powerpc: use helper functions in __hash_page_64K() for 64K PTE
  powerpc: use helper functions in __hash_page_huge() for 64K PTE
  powerpc: use helper functions in __hash_page_4K() for 64K PTE
  powerpc: use helper functions in __hash_page_4K() for 4K PTE
  powerpc: use helper functions in flush_hash_page()
  mm: introduce an additional vma bit for powerpc pkey
  mm: ability to disable execute permission on a key at creation
  x86: disallow pkey creation with PKEY_DISABLE_EXECUTE
  powerpc: initial plumbing for key management
  powerpc: helper function to read,write AMR,IAMR,UAMOR registers
  powerpc: implementation for arch_set_user_pkey_access()
  powerpc: sys_pkey_alloc() and sys_pkey_free() system calls
  powerpc: store and restore the pkey state across context switches
  powerpc: introduce execute-only pkey
  powerpc: ability to associate pkey to a vma
  powerpc: implementation for arch_override_mprotect_pkey()
  powerpc: map vma key-protection bits to pte key bits.
  powerpc: sys_pkey_mprotect() system call
  powerpc: Program HPTE key protection bits
  powerpc: helper to validate key-access permissions of a pte
  powerpc: check key protection for user page access
  powerpc: Macro the mask used for checking DSI exception
  powerpc: implementation for arch_vma_access_permitted()
  powerpc: Handle exceptions caused by pkey violation
  powerpc: capture AMR register content on pkey violation
  powerpc: introduce get_pte_pkey() helper
  powerpc: capture the violated protection key on fault
  powerpc: Deliver SEGV signal on pkey violation
  procfs: display the protection-key number associated with a vma
  selftest: Move protecton key selftest to arch neutral directory
  selftest: PowerPC specific test updates to memory protection keys
  Documentation: Move protecton key documentation to arch neutral
    directory
  Documentation: PowerPC specific updates to memory protection keys

 Documentation/vm/protection-keys.txt          |  130 +++
 Documentation/x86/protection-keys.txt         |   85 --
 arch/powerpc/Kconfig                          |   16 +
 arch/powerpc/include/asm/book3s/64/hash-4k.h  |   20 +
 arch/powerpc/include/asm/book3s/64/hash-64k.h |   60 +-
 arch/powerpc/include/asm/book3s/64/hash.h     |    7 +-
 arch/powerpc/include/asm/book3s/64/mmu-hash.h |   10 +
 arch/powerpc/include/asm/book3s/64/mmu.h      |   10 +
 arch/powerpc/include/asm/book3s/64/pgtable.h  |   96 ++-
 arch/powerpc/include/asm/mman.h               |   16 +-
 arch/powerpc/include/asm/mmu_context.h        |    5 +
 arch/powerpc/include/asm/paca.h               |    4 +
 arch/powerpc/include/asm/pkeys.h              |  159 +++
 arch/powerpc/include/asm/processor.h          |    5 +
 arch/powerpc/include/asm/reg.h                |    7 +-
 arch/powerpc/include/asm/systbl.h             |    3 +
 arch/powerpc/include/asm/unistd.h             |    6 +-
 arch/powerpc/include/uapi/asm/ptrace.h        |    3 +-
 arch/powerpc/include/uapi/asm/unistd.h        |    3 +
 arch/powerpc/kernel/asm-offsets.c             |    6 +
 arch/powerpc/kernel/exceptions-64s.S          |    2 +-
 arch/powerpc/kernel/process.c                 |   18 +
 arch/powerpc/kernel/setup_64.c                |    8 +
 arch/powerpc/kernel/signal_32.c               |    5 +
 arch/powerpc/kernel/signal_64.c               |    4 +
 arch/powerpc/kernel/traps.c                   |   14 +
 arch/powerpc/mm/Makefile                      |    1 +
 arch/powerpc/mm/dump_linuxpagetables.c        |    3 +-
 arch/powerpc/mm/fault.c                       |   26 +
 arch/powerpc/mm/hash64_4k.c                   |   14 +-
 arch/powerpc/mm/hash64_64k.c                  |  124 ++-
 arch/powerpc/mm/hash_utils_64.c               |   68 +-
 arch/powerpc/mm/hugetlbpage-hash64.c          |   16 +-
 arch/powerpc/mm/mmu_context_book3s64.c        |    5 +
 arch/powerpc/mm/pkeys.c                       |  243 ++++
 arch/x86/kernel/fpu/xstate.c                  |    3 +
 fs/proc/task_mmu.c                            |    6 +-
 include/linux/mm.h                            |   18 +-
 include/uapi/asm-generic/mman-common.h        |    4 +-
 tools/testing/selftests/vm/Makefile           |    1 +
 tools/testing/selftests/vm/pkey-helpers.h     |  365 ++++++
 tools/testing/selftests/vm/protection_keys.c  | 1488 +++++++++++++++++++++++++
 tools/testing/selftests/x86/Makefile          |    2 +-
 tools/testing/selftests/x86/pkey-helpers.h    |  219 ----
 tools/testing/selftests/x86/protection_keys.c | 1395 -----------------------
 45 files changed, 2872 insertions(+), 1831 deletions(-)
 create mode 100644 Documentation/vm/protection-keys.txt
 delete mode 100644 Documentation/x86/protection-keys.txt
 create mode 100644 arch/powerpc/include/asm/pkeys.h
 create mode 100644 arch/powerpc/mm/pkeys.c
 create mode 100644 tools/testing/selftests/vm/pkey-helpers.h
 create mode 100644 tools/testing/selftests/vm/protection_keys.c
 delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h
 delete mode 100644 tools/testing/selftests/x86/protection_keys.c

[toc] | [next] | [standalone]

#1683965

On 07/06/2017 02:51 AM, Ram Pai wrote:
> Memory protection keys enable applications to protect its
> address space from inadvertent access or corruption from
> itself.
> 
> The overall idea:
> 
>  A process allocates a   key  and associates it with
>  an  address  range  within    its   address   space.
>  The process  then  can  dynamically  set read/write 
>  permissions on  the   key   without  involving  the 
>  kernel. Any  code that  violates   the  permissions
>  of  the address space; as defined by its associated
>  key, will receive a segmentation fault.
> 
> This patch series enables the feature on PPC64 HPTE
> platform.
> 
> ISA3.0 section 5.7.13 describes the detailed specifications.
> 
> 
> Testing:
> 	This patch series has passed all the protection key
> 	tests available in  the selftests directory.
> 	The tests are updated to work on both x86 and powerpc.
> 
> version v5:
> 	(1) reverted back to the old design -- store the 
> 	    key in the pte, instead of bypassing it.
> 	    The v4 design slowed down the hash page path.
> 	(2) detects key violation when kernel is told to 
> 		access user pages.
> 	(3) further refined the patches into smaller consumable
> 		units
> 	(4) page faults handlers captures the faulting key 
> 	    from the pte instead of the vma. This closes a
> 	    race between where the key update in the vma and
> 	    a key fault caused cause by the key programmed
> 	    in the pte.
> 	(5) a key created with access-denied should
> 	    also set it up to deny write. Fixed it.
> 	(6) protection-key number is displayed in smaps
> 		the x86 way.

Hello Ram,

This patch series has now grown a lot. Do you have this
hosted some where for us to pull and test it out ? BTW
do you have data points to show the difference in
performance between this version and the last one where
we skipped the bits from PTE and directly programmed the
HPTE entries looking into VMA bits.

- Anshuman

[toc] | [prev] | [next] | [standalone]

#1683974

On Mon, Jul 10, 2017 at 11:13:23AM +0530, Anshuman Khandual wrote:
> On 07/06/2017 02:51 AM, Ram Pai wrote:
> > Memory protection keys enable applications to protect its
> > address space from inadvertent access or corruption from
> > itself.
> > 
> > The overall idea:
> > 
> >  A process allocates a   key  and associates it with
> >  an  address  range  within    its   address   space.
> >  The process  then  can  dynamically  set read/write 
> >  permissions on  the   key   without  involving  the 
> >  kernel. Any  code that  violates   the  permissions
> >  of  the address space; as defined by its associated
> >  key, will receive a segmentation fault.
> > 
> > This patch series enables the feature on PPC64 HPTE
> > platform.
> > 
> > ISA3.0 section 5.7.13 describes the detailed specifications.
> > 
> > 
> > Testing:
> > 	This patch series has passed all the protection key
> > 	tests available in  the selftests directory.
> > 	The tests are updated to work on both x86 and powerpc.
> > 
> > version v5:
> > 	(1) reverted back to the old design -- store the 
> > 	    key in the pte, instead of bypassing it.
> > 	    The v4 design slowed down the hash page path.
> > 	(2) detects key violation when kernel is told to 
> > 		access user pages.
> > 	(3) further refined the patches into smaller consumable
> > 		units
> > 	(4) page faults handlers captures the faulting key 
> > 	    from the pte instead of the vma. This closes a
> > 	    race between where the key update in the vma and
> > 	    a key fault caused cause by the key programmed
> > 	    in the pte.
> > 	(5) a key created with access-denied should
> > 	    also set it up to deny write. Fixed it.
> > 	(6) protection-key number is displayed in smaps
> > 		the x86 way.
> 
> Hello Ram,
> 
> This patch series has now grown a lot. Do you have this
> hosted some where for us to pull and test it out ? BTW

https://github.com/rampai/memorykeys.git
branch memkey.v5.3

> do you have data points to show the difference in
> performance between this version and the last one where
> we skipped the bits from PTE and directly programmed the
> HPTE entries looking into VMA bits.

No. I dont. I am hoping you can help me out with this.
RP

[toc] | [prev] | [next] | [standalone]

#1684434

On Sun, Jul 09, 2017 at 11:05:44PM -0700, Ram Pai wrote:
> On Mon, Jul 10, 2017 at 11:13:23AM +0530, Anshuman Khandual wrote:
> > On 07/06/2017 02:51 AM, Ram Pai wrote:
.....
> 
> > do you have data points to show the difference in
> > performance between this version and the last one where
> > we skipped the bits from PTE and directly programmed the
> > HPTE entries looking into VMA bits.
> 
> No. I dont. I am hoping you can help me out with this.
Anshuman,
	The last version where we skipped the PTE bits is guaranteed
	to be bad/horrible. For one it has a bug, since it accesses
	the vma without a lock. And even if we did take a lock, it
	will slow down the page-hash path un-acceptably. So there is
	no point measuring the performance of that design.

	I think the number we want to measure is -- the performance 
	with the current design and comparing that to the performance
	without memkey feature. We want to find if there is
	any degradation by adding this feature.

RP

[toc] | [prev] | [next] | [standalone]

#1685110

On Wed 05-07-17 14:21:37, Ram Pai wrote:
> Memory protection keys enable applications to protect its
> address space from inadvertent access or corruption from
> itself.
> 
> The overall idea:
> 
>  A process allocates a   key  and associates it with
>  an  address  range  within    its   address   space.
>  The process  then  can  dynamically  set read/write 
>  permissions on  the   key   without  involving  the 
>  kernel. Any  code that  violates   the  permissions
>  of  the address space; as defined by its associated
>  key, will receive a segmentation fault.
> 
> This patch series enables the feature on PPC64 HPTE
> platform.
> 
> ISA3.0 section 5.7.13 describes the detailed specifications.

Could you describe the highlevel design of this feature in the cover
letter. I have tried to get some idea from the patchset but it was
really far from trivial. Patches are not very well split up (many
helpers are added without their users etc..). 

> 
> Testing:
> 	This patch series has passed all the protection key
> 	tests available in  the selftests directory.
> 	The tests are updated to work on both x86 and powerpc.
> 
> version v5:
> 	(1) reverted back to the old design -- store the 
> 	    key in the pte, instead of bypassing it.
> 	    The v4 design slowed down the hash page path.

This surprised me a lot but I couldn't find the respective code. Why do
you need to store anything in the pte? My understanding of PKEYs is that
the setup and teardown should be very cheap and so no page tables have
to updated. Or do I just misunderstand what you wrote here?
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1685317

On Tue, Jul 11, 2017 at 04:52:46PM +0200, Michal Hocko wrote:
> On Wed 05-07-17 14:21:37, Ram Pai wrote:
> > Memory protection keys enable applications to protect its
> > address space from inadvertent access or corruption from
> > itself.
> > 
> > The overall idea:
> > 
> >  A process allocates a   key  and associates it with
> >  an  address  range  within    its   address   space.
> >  The process  then  can  dynamically  set read/write 
> >  permissions on  the   key   without  involving  the 
> >  kernel. Any  code that  violates   the  permissions
> >  of  the address space; as defined by its associated
> >  key, will receive a segmentation fault.
> > 
> > This patch series enables the feature on PPC64 HPTE
> > platform.
> > 
> > ISA3.0 section 5.7.13 describes the detailed specifications.
> 
> Could you describe the highlevel design of this feature in the cover
> letter.

Yes it can be hard to understand without the big picture.  I will
provide the high level design and the rationale behind the patch split
towards the end.  Also I will have it in the cover letter for my next
revision of the patchset.


> I have tried to get some idea from the patchset but it was
> really far from trivial. Patches are not very well split up (many
> helpers are added without their users etc..). 

I see your point. Earlier, I had the patches split such a way that the
users of the helpers were in the same patch as that of the helper.
But then comments from others lead to the current split.

> 
> > 
> > Testing:
> > 	This patch series has passed all the protection key
> > 	tests available in  the selftests directory.
> > 	The tests are updated to work on both x86 and powerpc.
> > 
> > version v5:
> > 	(1) reverted back to the old design -- store the 
> > 	    key in the pte, instead of bypassing it.
> > 	    The v4 design slowed down the hash page path.
> 
> This surprised me a lot but I couldn't find the respective code. Why do
> you need to store anything in the pte? My understanding of PKEYs is that
> the setup and teardown should be very cheap and so no page tables have
> to updated. Or do I just misunderstand what you wrote here?

Ideally the MMU looks at the PTE for keys, in order to enforce
protection. This is the case with x86 and is the case with power9 Radix
page table. Hence the keys have to be programmed into the PTE.

However with HPT on power, these keys do not necessarily have to be
programmed into the PTE. We could bypass the Linux Page Table Entry(PTE)
and instead just program them into the Hash Page Table(HPTE), since
the MMU does not refer the PTE but refers the HPTE. The last version
of the page attempted to do that.   It worked as follows:

a) when a address range is requested to be associated with a key; by the
   application through key_mprotect() system call, the kernel
   stores that key in the vmas corresponding to that address
   range.

b) Whenever there is a hash page fault for that address, the fault
   handler reads the key from the VMA and programs the key into the
   HPTE. __hash_page() is the function that does that.

c) Once the hpte is programmed, the MMU can sense key violations and
   generate key-faults.

The problem is with step (b).  This step is really a very critical
path which is performance sensitive. We dont want to add any delays.
However if we want to access the key from the vma, we will have to
hold the vma semaphore, and that is a big NO-NO. As a result, this
design had to be dropped.



I reverted back to the old design i.e the design in v4 version. In this
version we do the following:

a) when a address range is requested to be associated with a key; by the
   application through key_mprotect() system call, the kernel
   stores that key in the vmas corresponding to that address
   range. Also the kernel programs the key into Linux PTE coresponding to all the
   pages associated with the address range.

b) Whenever there is a hash page fault for that address, the fault
   handler reads the key from the Linux PTE and programs the key into 
   the HPTE.

c) Once the HPTE is programmed, the MMU can sense key violations and
   generate key-faults.


Since step (b) in this case has easy access to the Linux PTE, and hence
to the key, it is fast to access it and program the HPTE. Thus we avoid
taking any performance hit on this critical path.

Hope this explains the rationale,


As promised here is the high level design:

(1) When a application associates a key with a address range,
    program the key in the Linux PTE.
    
(2) Program the key into HPTE, when a HPTE is allocated to back
    the Linux PTE.
    
(3) And finally when the MMU detects a key violation due to invalid
    user access, invoke the registered signal handler and provide it
    with the key number that got violated and the state of the key 
    register (AMR) at the time it faulted.


In order to accomplish (1) we need to free up 5 bits in the Linux PTE to
store the key. This is accomplished by patches 

powerpc: Free up four 64K PTE bits in 4K backed HPTE
powerpc: Free up four 64K PTE bits in 64K backed HPTE pages



The above two patches modify the way the HPTE slots are stored
in the PTE different various configurations. The details are abstracted
out into two helper functions introduced by the following two
patches.

powerpc: introduce pte_set_hash_slot() helper
powerpc: introduce pte_get_hash_gslot() helper


Now we go and modify all the code that can benefit by the
above abstraction. The following 5 patches handle that.

 powerpc: use helper functions in __hash_page_64K() for 64K PTE
 powerpc: use helper functions in __hash_page_huge() for 64K PTE
 powerpc: use helper functions in __hash_page_4K() for 64K PTE
 powerpc: use helper functions in __hash_page_4K() for 4K PTE
 powerpc: use helper functions in flush_hash_page()


Since we have modified the PTE format, it has to be correctly reflected
in the dump report provided through debugfs. the following patch does
it.
 powerpc: capture the PTE format changes in the dump pte report


Till now we have done nothing much other then prepared ourselves to
accomadate memory key bits in the PTE. The next set of patches do
the actual work.

The VMA stores the key value.  The x86 implementation needed
just 4bits in the VMA flags, since they support only 16keys.  But
PowerPC supports 32 keys, so we need one more bit. The following patch
does that.
 mm: introduce an additional vma bit for powerpc pkey


Also x86 does not allow one to create a key with execute-denied permission.
PowerPC can handle that. So we add the ability to
support such a feature if the arch can handle it. The following two
patch help towards that.
 mm: ability to disable execute permission on a key at creation
 x86: disallow pkey creation with PKEY_DISABLE_EXECUTE


We than introduce the ability to house-keep the protection keys. There
are 32 keys. We need to track; which keys are available, which keys are
allocated and which keys are reserved. All that is handled in the
following patch

powerpc: initial plumbing for key management


Before we introduce the pkey_alloc() and pkey_free() system calls, we 
need to implement infrastructure that can allocate and free the keys,
and can program the hardware registers correspondingly.
So the following patches enable that.

 powerpc: helper function to read,write AMR,IAMR,UAMOR registers
 powerpc: implementation for arch_set_user_pkey_access()
 powerpc: sys_pkey_alloc() and sys_pkey_free() system calls


The key state has to be stored and restored across context switches, since
each task has its own key state. the next patch helps towards that.

 powerpc: store and restore the pkey state across context switches


x86 implementation introduced the concept of execute-only key where a
key can be set aside with execute-only permissions and the kernel can
use the key to associate with address-spaces that are execute only. We
facilitate that requirement through the next patch

 powerpc: introduce execute-only pkey



At this point we are ready to support the key_mprotect() system call.
the following four patches accomplish that. These patches togather
handle programming the key into the pte bits. All the hard work
done to release some pte bits; in the initially patches,
are finally bearing fruits here.

  powerpc: ability to associate pkey to a vma
  powerpc: implementation for arch_override_mprotect_pkey()
  powerpc: map vma key-protection bits to pte key bits.
  powerpc: sys_pkey_mprotect() system call


Given that the PTE holds the key bits, we can copy them
bit into the HPTE, because that is where they should land eventually
for any key-faults to trigger. The following patch accomplishes that.

   powerpc: Program HPTE key protection bits


Side stepping a bit. We also need the ability for the kernel to validate
key violation when accessing user pages. things like copy_*_user().
So the following patches help towards that.

 powerpc: check key protection for user page access
 powerpc: helper to validate key-access permissions of a pte


Ok. back to the main theme. The key is programmed into the HPTE. 
the MMU is able to detect key violations and generate key faults. But
then the kernel has to be cognizant of the key faults or else it will
drop them. So the next few patches help towards that.

 powerpc: Handle exceptions caused by pkey violation
 powerpc: implementation for arch_vma_access_permitted()
 powerpc: Macro the mask used for checking DSI exception


Everything is in place now, just the final peice of informing user space
on key violation is missing. So the next set of patches accomplish that.

  powerpc: capture AMR register content on pkey violation
  powerpc: introduce get_pte_pkey() helper
  powerpc: capture the violated protection key on fault
  powerpc: Deliver SEGV signal on pkey violation


One missing piece. We need the ability to tell -- which key is associated
with each VMA my looking at the smaps. the following patch helps towards it.
  procfs: display the protection-key number associated with a vma


Well everything accomplished...but how do we know if everything is in place
and works as expected? The next set of patches modify the selftest, by first
moving them into arch-independent directory and then abstracting out
the arch-depended pieces, and finally adding some additional tests
to make it even more robust.

 selftest: Move protecton key selftest to arch neutral directory
 selftest: PowerPC specific test updates to memory protection keys


Nothing is complete without Documentation. and that is what the final
two patches accomplish. Again they move the documentation into
arch independent directory and explains the differences between
x86 and powerpc.

  Documentation: Move protecton key documentation to arch neutral directory
  Documentation: PowerPC specific updates to memory protection keys


Hope the above explanation helps.

NOTE: key support for power9 radix is not supported yet, but the above
design will make it easy to support it as and when the hardware is ready
to handle it.

Thanks for your valuable comments.
RP

> -- 
> Michal Hocko
> SUSE Labs

-- 
Ram Pai

[toc] | [prev] | [next] | [standalone]

#1685372

On Tue, 2017-07-11 at 12:32 -0700, Ram Pai wrote:
> Ideally the MMU looks at the PTE for keys, in order to enforce
> protection. This is the case with x86 and is the case with power9 Radix
> page table. Hence the keys have to be programmed into the PTE.

POWER9 radix doesn't currently support keys.

Cheers,
Ben.

[toc] | [prev] | [next] | [standalone]

#1685564

On Tue 11-07-17 12:32:57, Ram Pai wrote:
> On Tue, Jul 11, 2017 at 04:52:46PM +0200, Michal Hocko wrote:
> > On Wed 05-07-17 14:21:37, Ram Pai wrote:
> > > Memory protection keys enable applications to protect its
> > > address space from inadvertent access or corruption from
> > > itself.
> > > 
> > > The overall idea:
> > > 
> > >  A process allocates a   key  and associates it with
> > >  an  address  range  within    its   address   space.
> > >  The process  then  can  dynamically  set read/write 
> > >  permissions on  the   key   without  involving  the 
> > >  kernel. Any  code that  violates   the  permissions
> > >  of  the address space; as defined by its associated
> > >  key, will receive a segmentation fault.
> > > 
> > > This patch series enables the feature on PPC64 HPTE
> > > platform.
> > > 
> > > ISA3.0 section 5.7.13 describes the detailed specifications.
> > 
> > Could you describe the highlevel design of this feature in the cover
> > letter.
> 
> Yes it can be hard to understand without the big picture.  I will
> provide the high level design and the rationale behind the patch split
> towards the end.  Also I will have it in the cover letter for my next
> revision of the patchset.

Thanks!
 
> > I have tried to get some idea from the patchset but it was
> > really far from trivial. Patches are not very well split up (many
> > helpers are added without their users etc..). 
> 
> I see your point. Earlier, I had the patches split such a way that the
> users of the helpers were in the same patch as that of the helper.
> But then comments from others lead to the current split.

It is not my call here, obviously. I cannot review arch specific parts
due to lack of familiarity but it is a general good practice to include
helpers along with their users to make the usage clear. Also, as much as
I like small patches because they are easier to review, having very many
of them can lead to a harder review in the end because you easily lose
a higher level overview.

> > > Testing:
> > > 	This patch series has passed all the protection key
> > > 	tests available in  the selftests directory.
> > > 	The tests are updated to work on both x86 and powerpc.
> > > 
> > > version v5:
> > > 	(1) reverted back to the old design -- store the 
> > > 	    key in the pte, instead of bypassing it.
> > > 	    The v4 design slowed down the hash page path.
> > 
> > This surprised me a lot but I couldn't find the respective code. Why do
> > you need to store anything in the pte? My understanding of PKEYs is that
> > the setup and teardown should be very cheap and so no page tables have
> > to updated. Or do I just misunderstand what you wrote here?
> 
> Ideally the MMU looks at the PTE for keys, in order to enforce
> protection. This is the case with x86 and is the case with power9 Radix
> page table. Hence the keys have to be programmed into the PTE.

But x86 doesn't update ptes for PKEYs, that would be just too expensive.
You could use standard mprotect to do the same...
 
> However with HPT on power, these keys do not necessarily have to be
> programmed into the PTE. We could bypass the Linux Page Table Entry(PTE)
> and instead just program them into the Hash Page Table(HPTE), since
> the MMU does not refer the PTE but refers the HPTE. The last version
> of the page attempted to do that.   It worked as follows:
> 
> a) when a address range is requested to be associated with a key; by the
>    application through key_mprotect() system call, the kernel
>    stores that key in the vmas corresponding to that address
>    range.
> 
> b) Whenever there is a hash page fault for that address, the fault
>    handler reads the key from the VMA and programs the key into the
>    HPTE. __hash_page() is the function that does that.

What causes the fault here?

> c) Once the hpte is programmed, the MMU can sense key violations and
>    generate key-faults.
> 
> The problem is with step (b).  This step is really a very critical
> path which is performance sensitive. We dont want to add any delays.
> However if we want to access the key from the vma, we will have to
> hold the vma semaphore, and that is a big NO-NO. As a result, this
> design had to be dropped.
> 
> 
> 
> I reverted back to the old design i.e the design in v4 version. In this
> version we do the following:
> 
> a) when a address range is requested to be associated with a key; by the
>    application through key_mprotect() system call, the kernel
>    stores that key in the vmas corresponding to that address
>    range. Also the kernel programs the key into Linux PTE coresponding to all the
>    pages associated with the address range.

OK, so how is this any different from the regular mprotect then?

> b) Whenever there is a hash page fault for that address, the fault
>    handler reads the key from the Linux PTE and programs the key into 
>    the HPTE.
> 
> c) Once the HPTE is programmed, the MMU can sense key violations and
>    generate key-faults.
> 
> 
> Since step (b) in this case has easy access to the Linux PTE, and hence
> to the key, it is fast to access it and program the HPTE. Thus we avoid
> taking any performance hit on this critical path.
> 
> Hope this explains the rationale,
> 
> 
> As promised here is the high level design:

I will read through that later
[...]
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1685575

On Wed 12-07-17 09:23:37, Michal Hocko wrote:
> On Tue 11-07-17 12:32:57, Ram Pai wrote:
[...]
> > Ideally the MMU looks at the PTE for keys, in order to enforce
> > protection. This is the case with x86 and is the case with power9 Radix
> > page table. Hence the keys have to be programmed into the PTE.
> 
> But x86 doesn't update ptes for PKEYs, that would be just too expensive.
> You could use standard mprotect to do the same...

OK, this seems to be a misunderstanding and confusion on my end.
do_mprotect_pkey does mprotect_fixup even for the pkey path which is
quite surprising to me. I guess my misunderstanding comes from
Documentation/x86/protection-keys.txt
"
Memory Protection Keys provides a mechanism for enforcing page-based
protections, but without requiring modification of the page tables
when an application changes protection domains.  It works by
dedicating 4 previously ignored bits in each page table entry to a
"protection key", giving 16 possible keys.
"

So please disregard my previous comments about page tables and sorry
about the confusion.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1686104

On Wed, 2017-07-12 at 09:23 +0200, Michal Hocko wrote:
> 
> > 
> > Ideally the MMU looks at the PTE for keys, in order to enforce
> > protection. This is the case with x86 and is the case with power9 Radix
> > page table. Hence the keys have to be programmed into the PTE.
> 
> But x86 doesn't update ptes for PKEYs, that would be just too expensive.
> You could use standard mprotect to do the same...

What do you mean ? x86 ends up in mprotect_fixup -> change_protection()
which will update the PTEs just the same as we do.

Changing the key for a page is a form mprotect. Changing the access
permissions for keys is different, for us it's a special register
(AMR).

I don't understand why you think we are doing any differently than x86
here.

> > However with HPT on power, these keys do not necessarily have to be
> > programmed into the PTE. We could bypass the Linux Page Table Entry(PTE)
> > and instead just program them into the Hash Page Table(HPTE), since
> > the MMU does not refer the PTE but refers the HPTE. The last version
> > of the page attempted to do that.   It worked as follows:
> > 
> > a) when a address range is requested to be associated with a key; by the
> >    application through key_mprotect() system call, the kernel
> >    stores that key in the vmas corresponding to that address
> >    range.
> > 
> > b) Whenever there is a hash page fault for that address, the fault
> >    handler reads the key from the VMA and programs the key into the
> >    HPTE. __hash_page() is the function that does that.
> 
> What causes the fault here?

The hardware. With the hash MMU, the HW walks a hash table which is
effectively a large in-memory TLB extension. When a page isn't found
there, a  "hash fault" is generated allowing Linux to populate that
hash table with the content of the corresponding PTE. 

> > c) Once the hpte is programmed, the MMU can sense key violations and
> >    generate key-faults.
> > 
> > The problem is with step (b).  This step is really a very critical
> > path which is performance sensitive. We dont want to add any delays.
> > However if we want to access the key from the vma, we will have to
> > hold the vma semaphore, and that is a big NO-NO. As a result, this
> > design had to be dropped.
> > 
> > 
> > 
> > I reverted back to the old design i.e the design in v4 version. In this
> > version we do the following:
> > 
> > a) when a address range is requested to be associated with a key; by the
> >    application through key_mprotect() system call, the kernel
> >    stores that key in the vmas corresponding to that address
> >    range. Also the kernel programs the key into Linux PTE coresponding to all the
> >    pages associated with the address range.
> 
> OK, so how is this any different from the regular mprotect then?

It takes the key argument. This is nothing new. This was done for x86
already, we are just re-using the infrastructure. Look at
do_mprotect_pkey() in mm/mprotect.c today. It's all the same code,
pkey_mprotect() is just mprotect with an added key argument.

> > b) Whenever there is a hash page fault for that address, the fault
> >    handler reads the key from the Linux PTE and programs the key into 
> >    the HPTE.
> > 
> > c) Once the HPTE is programmed, the MMU can sense key violations and
> >    generate key-faults.
> > 
> > 
> > Since step (b) in this case has easy access to the Linux PTE, and hence
> > to the key, it is fast to access it and program the HPTE. Thus we avoid
> > taking any performance hit on this critical path.
> > 
> > Hope this explains the rationale,
> > 
> > 
> > As promised here is the high level design:
> 
> I will read through that later
> [...]

[toc] | [prev] | [next] | [standalone]

#1686279

On Thu 13-07-17 08:53:52, Benjamin Herrenschmidt wrote:
> On Wed, 2017-07-12 at 09:23 +0200, Michal Hocko wrote:
> > 
> > > 
> > > Ideally the MMU looks at the PTE for keys, in order to enforce
> > > protection. This is the case with x86 and is the case with power9 Radix
> > > page table. Hence the keys have to be programmed into the PTE.
> > 
> > But x86 doesn't update ptes for PKEYs, that would be just too expensive.
> > You could use standard mprotect to do the same...
> 
> What do you mean ? x86 ends up in mprotect_fixup -> change_protection()
> which will update the PTEs just the same as we do.
> 
> Changing the key for a page is a form mprotect. Changing the access
> permissions for keys is different, for us it's a special register
> (AMR).
> 
> I don't understand why you think we are doing any differently than x86
> here.

That was a misunderstanding on my side as explained in other reply.

> > > However with HPT on power, these keys do not necessarily have to be
> > > programmed into the PTE. We could bypass the Linux Page Table Entry(PTE)
> > > and instead just program them into the Hash Page Table(HPTE), since
> > > the MMU does not refer the PTE but refers the HPTE. The last version
> > > of the page attempted to do that.   It worked as follows:
> > > 
> > > a) when a address range is requested to be associated with a key; by the
> > >    application through key_mprotect() system call, the kernel
> > >    stores that key in the vmas corresponding to that address
> > >    range.
> > > 
> > > b) Whenever there is a hash page fault for that address, the fault
> > >    handler reads the key from the VMA and programs the key into the
> > >    HPTE. __hash_page() is the function that does that.
> > 
> > What causes the fault here?
> 
> The hardware. With the hash MMU, the HW walks a hash table which is
> effectively a large in-memory TLB extension. When a page isn't found
> there, a  "hash fault" is generated allowing Linux to populate that
> hash table with the content of the corresponding PTE. 

Thanks for the clarification
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1685245 — Re: [RFC v5 36/38] selftest: PowerPC specific test updates to memory protection keys

On 07/05/2017 02:22 PM, Ram Pai wrote:
> Abstracted out the arch specific code into the header file, and
> added powerpc specific changes.
> 
> a) added 4k-backed hpte, memory allocator, powerpc specific.
> b) added three test case where the key is associated after the page is
> 	accessed/allocated/mapped.
> c) cleaned up the code to make checkpatch.pl happy

There's a *lot* of churn here.  If it breaks, I'm going to have a heck
of a time figuring out which hunk broke.  Is there any way to break this
up into a series of things that we have a chance at bisecting?

[toc] | [prev] | [next] | [standalone]

#1686067 — Re: [RFC v5 36/38] selftest: PowerPC specific test updates to memory protection keys

On Tue, Jul 11, 2017 at 10:33:09AM -0700, Dave Hansen wrote:
> On 07/05/2017 02:22 PM, Ram Pai wrote:
> > Abstracted out the arch specific code into the header file, and
> > added powerpc specific changes.
> > 
> > a) added 4k-backed hpte, memory allocator, powerpc specific.
> > b) added three test case where the key is associated after the page is
> > 	accessed/allocated/mapped.
> > c) cleaned up the code to make checkpatch.pl happy
> 
> There's a *lot* of churn here.  If it breaks, I'm going to have a heck
> of a time figuring out which hunk broke.  Is there any way to break this
> up into a series of things that we have a chance at bisecting?

Just finished breaking down the changes into 20 gradual increments.
I have pushed it to my github tree at

https://github.com/rampai/memorykeys.git
branch is memkey.v6-rc3

See if it works for you. I am sure I would have broken something on
x86 since I dont have a x86 platform to test.

Let me know, Thanks,
RP

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web