RPAGE_RSV0..4 pte bits are currently used for hpte slot
tracking. We need these bits for memory-protection
keys. Luckily these four bits are relatively easier
to move among all the other candidate bits.
For 64K linux-ptes backed by 4k hptes, these bits
are used for tracking the validity of the slot value
stored in the second-part-of-the-pte. We device a new
mechanism for tracking the validity without using
those bits. The mechanism is explained in the patch.
For 64K linux-pte backed by 64K hptes, we simply move
the slot tracking bits to the second-part-of-the-pte.
The above mechanism is also used to free the bits for
hugetlb linux-ptes.
For 4k linux-pte, we have only 3 free bits available.
We swizzle around the bits and release RPAGE_RSV{2,3,4}
for memory protection keys.
Testing:
--------
has survived kernel compilation on multiple platforms
p8 powernv hash-mode, p9 powernv hash-mode, p7 powervm,
p8-powervm, p8-kvm-guest.
Has survived git-bisect on p8 power-nv with 64K page
and 4K page.
History:
-------
This patchset is a spin-off from the memkey patchset.
version v9:
(1) rearranged the patch order. First the helper
routines are defined followed by the
patches that make use of the helpers.
version v8:
(1) an additional patch added to free up
RSV{2,3,4} on 4K linux-pte.
version v7:
(1) GIX bit reset change moved to the second
patch -- noticed by Aneesh.
(2) Separated this patches from memkey patchset
(3) merged a bunch of patches, that used the
helper function, into one.
version v6:
(1) No changes related to pte.
version v5:
(1) No changes related to pte.
version v4:
(1) No changes related to pte.
version v3:
(1) split the patches into smaller consumable
patches.
(2) A bug fix while invalidating a hpte slot
in __hash_page_4K()
-- noticed by Aneesh
version v2:
(1) 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.
version v1: Initial version
Ram Pai (7):
powerpc: introduce pte_set_hash_slot() helper
powerpc: introduce pte_get_hash_gslot() helper
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: Swizzle around 4K PTE bits to free up bit 5 and bit 6
powerpc: use helper functions to get and set hash slots
powerpc: capture the PTE format changes in the dump pte report
arch/powerpc/include/asm/book3s/64/hash-4k.h | 21 ++++
arch/powerpc/include/asm/book3s/64/hash-64k.h | 61 ++++++++----
arch/powerpc/include/asm/book3s/64/hash.h | 8 +-
arch/powerpc/mm/dump_linuxpagetables.c | 3 +-
arch/powerpc/mm/hash64_4k.c | 14 +--
arch/powerpc/mm/hash64_64k.c | 131 +++++++++++++------------
arch/powerpc/mm/hash_utils_64.c | 35 +++++--
arch/powerpc/mm/hugetlbpage-hash64.c | 18 ++--
8 files changed, 171 insertions(+), 120 deletions(-)
Introduce pte_set_hash_slot().It sets the (H_PAGE_F_SECOND|H_PAGE_F_GIX)
bits at the appropriate location in the PTE of 4K PTE. For
64K PTE, it sets the bits in the second part of the PTE. Though
the implementation for the former just needs the slot parameter, it does
take some additional parameters to keep the prototype consistent.
This function will be handy as we work towards re-arranging the
bits in the later patches.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 15 +++++++++++++++
arch/powerpc/include/asm/book3s/64/hash-64k.h | 25 +++++++++++++++++++++++++
2 files changed, 40 insertions(+), 0 deletions(-)
@@ -74,6 +74,31 @@ static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index)return(pte_val(rpte.pte)>>H_PAGE_F_GIX_SHIFT)&0xf;}+/*+*Committhehashslotandreturnptebitsthatneedstobemodified.+*Thecallerisexpectedtomodifytheptebitsaccordinglyand+*committheptetomemory.+*/+staticinlineunsignedlongpte_set_hash_slot(pte_t*ptep,real_pte_trpte,+unsignedintsubpg_index,unsignedlongslot)+{+unsignedlong*hidxp=(unsignedlong*)(ptep+PTRS_PER_PTE);++rpte.hidx&=~(0xfUL<<(subpg_index<<2));+*hidxp=rpte.hidx|(slot<<(subpg_index<<2));+/*+*Committhehidxbitstomemorybeforereturning.+*Anyonereadingptemustensurehidxbitsare+*readonlyafterreadingtheptebyusingthe+*read-sidebarriersmp_rmb().__real_pte()can+*helpensurethat.+*/+smp_wmb();++/* no pte bits to be modified, return 0x0UL */+return0x0UL;+}+#define __rpte_to_pte(r) ((r).pte)externbool__rpte_sub_valid(real_pte_trpte,unsignedlongindex);/*
Introduce pte_get_hash_gslot()() which returns the slot number of the
HPTE in the global hash table.
This function will come in handy as we work towards re-arranging the
PTE bits in the later patches.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/hash.h | 3 +++
arch/powerpc/mm/hash_utils_64.c | 18 ++++++++++++++++++
2 files changed, 21 insertions(+), 0 deletions(-)
@@ -166,6 +166,9 @@ static inline int hash__pte_none(pte_t pte)return(pte_val(pte)&~H_PTE_NONE_MASK)==0;}+unsignedlongpte_get_hash_gslot(unsignedlongvpn,unsignedlongshift,+intssize,real_pte_trpte,unsignedintsubpg_index);+/* This low level function performs the actual PTE insertion*SettingthePTEdependsontheMMUtypeandotherfactors.It's*anhorriblemessthatI'mnotgoingtotrytocleanupnowbut
@@ -1591,6 +1591,24 @@ static inline void tm_flush_hash_page(int local)}#endif+/*+*returntheglobalhashslot,correspondingtothegiven+*pte,whichcontainsthehpte.+*/+unsignedlongpte_get_hash_gslot(unsignedlongvpn,unsignedlongshift,+intssize,real_pte_trpte,unsignedintsubpg_index)+{+unsignedlonghash,slot,hidx;++hash=hpt_hash(vpn,shift,ssize);+hidx=__rpte_to_hidx(rpte,subpg_index);+if(hidx&_PTEIDX_SECONDARY)+hash=~hash;+slot=(hash&htab_hash_mask)*HPTES_PER_GROUP;+slot+=hidx&_PTEIDX_GROUP_IX;+returnslot;+}+/* WARNING: This is called from hash_low_64.S, if you change this prototype,*donotforgettoupdatetheassemblycallsite!*/
Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6,
in the 4K backed HPTE pages.These bits continue to be used
for 64K backed HPTE pages in this patch, but will be freed
up in the next patch. The bit numbers are big-endian as
defined in the ISA3.0
The patch does the following change to the 4k htpe backed
64K PTE's format.
H_PAGE_BUSY moves from bit 3 to bit 9 (B bit in the figure
below)
V0 which occupied bit 4 is not used anymore.
V1 which occupied bit 5 is not used anymore.
V2 which occupied bit 6 is not used anymore.
V3 which occupied bit 7 is not used anymore.
Before the patch, the 4k backed 64k PTE format was as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x|B|V0|V1|V2|V3|x| | |x|x|................|x|x|x|x| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
|S|G|I|X|S |G |I |X |S|G|I|X|..................|S|G|I|X| <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
After the patch, the 4k backed 64k PTE format is as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x| | | | | |x|B| |x|x|................|.|.|.|.| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
|S|G|I|X|S |G |I |X |S|G|I|X|..................|S|G|I|X| <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
the four bits S,G,I,X (one quadruplet per 4k HPTE) that
cache the hash-bucket slot value, is initialized to
1,1,1,1 indicating -- an invalid slot. If a HPTE gets
cached in a 1111 slot(i.e 7th slot of secondary hash
bucket), it is released immediately. In other words,
even though 1111 is a valid slot value in the hash
bucket, we consider it invalid and release the slot and
the HPTE. This gives us the opportunity to determine
the validity of S,G,I,X bits based on its contents and
not on any of the bits V0,V1,V2 or V3 in the primary PTE
When we release a HPTE cached in the 1111 slot
we also release a legitimate slot in the primary
hash bucket and unmap its corresponding HPTE. This
is to ensure that we do get a HPTE cached in a slot
of the primary hash bucket, the next time we retry.
Though treating 1111 slot as invalid, reduces the
number of available slots in the hash bucket and may
have an effect on the performance, the probabilty of
hitting a 1111 slot is extermely low.
Compared to the current scheme, the above scheme
reduces the number of false hash table updates
significantly and has the added advantage of releasing
four valuable PTE bits for other purpose.
NOTE:even though bits 3, 4, 5, 6, 7 are not used when
the 64K PTE is backed by 4k HPTE, they continue to be
used if the PTE gets backed by 64k HPTE. The next
patch will decouple that aswell, and truely release the
bits.
This idea was jointly developed by Paul Mackerras,
Aneesh, Michael Ellermen and myself.
4K PTE format remains unchanged currently.
The patch does the following code changes
a) PTE flags are split between 64k and 4k header files.
b) __hash_page_4K() is reimplemented to reflect the
above logic.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 2 +
arch/powerpc/include/asm/book3s/64/hash-64k.h | 8 +--
arch/powerpc/include/asm/book3s/64/hash.h | 1 -
arch/powerpc/mm/hash64_64k.c | 106 +++++++++++++------------
arch/powerpc/mm/hash_utils_64.c | 4 +-
5 files changed, 63 insertions(+), 58 deletions(-)
@@ -50,12 +38,11 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,intssize,intsubpg_prot){real_pte_trpte;-unsignedlong*hidxp;unsignedlonghpte_group;unsignedintsubpg_index;-unsignedlongrflags,pa,hidx;+unsignedlongrflags,pa;unsignedlongold_pte,new_pte,subpg_pte;-unsignedlongvpn,hash,slot;+unsignedlongvpn,hash,slot,gslot;unsignedlongshift=mmu_psize_defs[MMU_PAGE_4K].shift;/*
@@ -126,18 +113,13 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,if(__rpte_sub_valid(rpte,subpg_index)){intret;-hash=hpt_hash(vpn,shift,ssize);-hidx=__rpte_to_hidx(rpte,subpg_index);-if(hidx&_PTEIDX_SECONDARY)-hash=~hash;-slot=(hash&htab_hash_mask)*HPTES_PER_GROUP;-slot+=hidx&_PTEIDX_GROUP_IX;+gslot=pte_get_hash_gslot(vpn,shift,ssize,rpte,+subpg_index);+ret=mmu_hash_ops.hpte_updatepp(gslot,rflags,vpn,+MMU_PAGE_4K,MMU_PAGE_4K,ssize,flags);-ret=mmu_hash_ops.hpte_updatepp(slot,rflags,vpn,-MMU_PAGE_4K,MMU_PAGE_4K,-ssize,flags);/*-*ifwefailedbecausetypicallytheHPTEwasn'treallyhere+*ifwefailedbecausetypicallytheHPTEwasn'treallyhere*wetryaninsertion.*/if(ret==-1)
@@ -148,6 +130,15 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,}htab_insert_hpte:++/*+*initializeallhidxentriestoinvalidvalue,+*thefirsttimethePTEisabouttoallocate+*a4Khpte+*/+if(!(old_pte&H_PAGE_COMBO))+rpte.hidx=~0x0UL;+/**handleH_PAGE_4K_PFNcase*/
@@ -172,15 +163,41 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,*Primaryisfull,trythesecondary*/if(unlikely(slot==-1)){+boolsoft_invalid;+hpte_group=((~hash&htab_hash_mask)*HPTES_PER_GROUP)&~0x7UL;slot=mmu_hash_ops.hpte_insert(hpte_group,vpn,pa,rflags,HPTE_V_SECONDARY,MMU_PAGE_4K,MMU_PAGE_4K,ssize);-if(slot==-1){-if(mftb()&0x1)++soft_invalid=hpte_soft_invalid(slot);+if(unlikely(soft_invalid)){+/*+*wegotavalidslotfromahardwarepointofview.+*butwecannotuseit,becauseweusethisspecial+*value;asdefinedbyhpte_soft_invalid(),+*totrackinvalidslots.Wecannotuseit.+*Soinvalidateit.+*/+gslot=slot&_PTEIDX_GROUP_IX;+mmu_hash_ops.hpte_invalidate(hpte_group+gslot,vpn,+MMU_PAGE_4K,MMU_PAGE_4K,+ssize,0);+}++if(unlikely(slot==-1||soft_invalid)){+/*+*forsoftinvalidslot,letsensurethatwe+*releaseaslotfromtheprimary,withthe+*hopethatwewillacquirethatslotnext+*timewetry.Thiswillensurethatwedonot+*getthesamesoft-invalidslot.+*/+if(soft_invalid||(mftb()&0x1))hpte_group=((hash&htab_hash_mask)*HPTES_PER_GROUP)&~0x7UL;+mmu_hash_ops.hpte_remove(hpte_group);/**FIXME!!Shouldbetrythegroupfromwhichweremoved?
@@ -198,21 +215,10 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,MMU_PAGE_4K,MMU_PAGE_4K,old_pte);return-1;}-/*-*Insertslotnumber&secondarybitinPTEsecondhalf,-*clearH_PAGE_BUSYandsetappropriateHPTEslotbit-*SincewehaveH_PAGE_BUSYsetonptep,wecanbesure-*nobodyisundatinghidx.-*/-hidxp=(unsignedlong*)(ptep+PTRS_PER_PTE);-rpte.hidx&=~(0xfUL<<(subpg_index<<2));-*hidxp=rpte.hidx|(slot<<(subpg_index<<2));-new_pte=mark_subptegroup_valid(new_pte,subpg_index);-new_pte|=H_PAGE_HASHPTE;-/*-*check__real_ptefordetailsonmatchingsmp_rmb()-*/-smp_wmb();++new_pte|=pte_set_hash_slot(ptep,rpte,subpg_index,slot);+new_pte|=H_PAGE_HASHPTE;+*ptep=__pte(new_pte&~H_PAGE_BUSY);return0;}
Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6
in the 64K backed HPTE pages. This along with the earlier
patch will entirely free up the four bits from 64K PTE.
The bit numbers are big-endian as defined in the ISA3.0
This patch does the following change to 64K PTE backed
by 64K HPTE.
H_PAGE_F_SECOND (S) which occupied bit 4 moves to the
second part of the pte to bit 60.
H_PAGE_F_GIX (G,I,X) which occupied bit 5, 6 and 7 also
moves to the second part of the pte to bit 61,
62, 63, 64 respectively
since bit 7 is now freed up, we move H_PAGE_BUSY (B) from
bit 9 to bit 7.
The second part of the PTE will hold
(H_PAGE_F_SECOND|H_PAGE_F_GIX) at bit 60,61,62,63.
NOTE: None of the bits in the secondary PTE were not used
by 64k-HPTE backed PTE.
Before the patch, the 64K HPTE backed 64k PTE format was
as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x| |S |G |I |X |x|B| |x|x|................|x|x|x|x| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
| | | | | | | | | | | | |..................| | | | | <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
After the patch, the 64k HPTE backed 64k PTE format is
as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x| | | | |B |x| | |x|x|................|.|.|.|.| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
| | | | | | | | | | | | |..................|S|G|I|X| <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
The above PTE changes is applicable to hugetlbpages aswell.
The patch does the following code changes:
a) moves the H_PAGE_F_SECOND and H_PAGE_F_GIX to 4k PTE
header since it is no more needed b the 64k PTEs.
b) abstracts out __real_pte() and __rpte_to_hidx() so the
caller need not know the bit location of the slot.
c) moves the slot bits to the secondary pte.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 3 ++
arch/powerpc/include/asm/book3s/64/hash-64k.h | 29 +++++++++++-------------
arch/powerpc/include/asm/book3s/64/hash.h | 3 --
arch/powerpc/mm/hash64_64k.c | 23 ++++++++-----------
arch/powerpc/mm/hugetlbpage-hash64.c | 18 ++++++---------
5 files changed, 33 insertions(+), 43 deletions(-)
@@ -12,7 +12,7 @@*/#define H_PAGE_COMBO _RPAGE_RPN0 /* this is a combo 4k page */#define H_PAGE_4K_PFN _RPAGE_RPN1 /* PFN is for a single 4k page */-#define H_PAGE_BUSY _RPAGE_RPN42 /* software: PTE & hash are busy */+#define H_PAGE_BUSY _RPAGE_RPN44 /* software: PTE & hash are busy *//**WeneedtodifferentiatebetweenexplicithugepageandTHPhuge
@@ -103,8 +103,8 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,*Onhashinsertfailureweuseoldptevalueandwedon't*wantslotinformationthereifwehaveainsertfailure.*/-old_pte&=~(H_PAGE_HASHPTE|H_PAGE_F_GIX|H_PAGE_F_SECOND);-new_pte&=~(H_PAGE_HASHPTE|H_PAGE_F_GIX|H_PAGE_F_SECOND);+old_pte&=~H_PAGE_HASHPTE;+new_pte&=~H_PAGE_HASHPTE;gotohtab_insert_hpte;}/*
@@ -227,6 +227,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access,unsignedlongvsid,pte_t*ptep,unsignedlongtrap,unsignedlongflags,intssize){+real_pte_trpte;unsignedlonghpte_group;unsignedlongrflags,pa;unsignedlongold_pte,new_pte;
@@ -263,6 +264,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access,}while(!pte_xchg(ptep,__pte(old_pte),__pte(new_pte)));rflags=htab_convert_pte_flags(new_pte);+rpte=__real_pte(__pte(old_pte),ptep);if(cpu_has_feature(CPU_FTR_NOEXECUTE)&&!cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
@@ -270,18 +272,13 @@ int __hash_page_64K(unsigned long ea, unsigned long access,vpn=hpt_vpn(ea,vsid,ssize);if(unlikely(old_pte&H_PAGE_HASHPTE)){+unsignedlonggslot;/**ThereMIGHTbeanHPTEforthispte*/-hash=hpt_hash(vpn,shift,ssize);-if(old_pte&H_PAGE_F_SECOND)-hash=~hash;-slot=(hash&htab_hash_mask)*HPTES_PER_GROUP;-slot+=(old_pte&H_PAGE_F_GIX)>>H_PAGE_F_GIX_SHIFT;--if(mmu_hash_ops.hpte_updatepp(slot,rflags,vpn,MMU_PAGE_64K,-MMU_PAGE_64K,ssize,-flags)==-1)+gslot=pte_get_hash_gslot(vpn,shift,ssize,rpte,0);+if(mmu_hash_ops.hpte_updatepp(gslot,rflags,vpn,MMU_PAGE_64K,+MMU_PAGE_64K,ssize,flags)==-1)old_pte&=~_PAGE_HPTEFLAGS;}
@@ -328,9 +325,9 @@ int __hash_page_64K(unsigned long ea, unsigned long access,MMU_PAGE_64K,MMU_PAGE_64K,old_pte);return-1;}+new_pte=(new_pte&~_PAGE_HPTEFLAGS)|H_PAGE_HASHPTE;-new_pte|=(slot<<H_PAGE_F_GIX_SHIFT)&-(H_PAGE_F_SECOND|H_PAGE_F_GIX);+new_pte|=pte_set_hash_slot(ptep,rpte,0,slot);}*ptep=__pte(new_pte&~H_PAGE_BUSY);return0;
@@ -22,6 +22,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,pte_t*ptep,unsignedlongtrap,unsignedlongflags,intssize,unsignedintshift,unsignedintmmu_psize){+real_pte_trpte;unsignedlongvpn;unsignedlongold_pte,new_pte;unsignedlongrflags,pa,sz;
@@ -61,6 +62,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,}while(!pte_xchg(ptep,__pte(old_pte),__pte(new_pte)));rflags=htab_convert_pte_flags(new_pte);+rpte=__real_pte(__pte(old_pte),ptep);sz=((1UL)<<shift);if(!cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
@@ -71,16 +73,11 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,/* Check if pte already has an hpte (case 2) */if(unlikely(old_pte&H_PAGE_HASHPTE)){/* There MIGHT be an HPTE for this pte */-unsignedlonghash,slot;+unsignedlonggslot;-hash=hpt_hash(vpn,shift,ssize);-if(old_pte&H_PAGE_F_SECOND)-hash=~hash;-slot=(hash&htab_hash_mask)*HPTES_PER_GROUP;-slot+=(old_pte&H_PAGE_F_GIX)>>H_PAGE_F_GIX_SHIFT;--if(mmu_hash_ops.hpte_updatepp(slot,rflags,vpn,mmu_psize,-mmu_psize,ssize,flags)==-1)+gslot=pte_get_hash_gslot(vpn,shift,ssize,rpte,0);+if(mmu_hash_ops.hpte_updatepp(gslot,rflags,vpn,mmu_psize,+mmu_psize,ssize,flags)==-1)old_pte&=~_PAGE_HPTEFLAGS;}
@@ -106,8 +103,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,return-1;}-new_pte|=(slot<<H_PAGE_F_GIX_SHIFT)&-(H_PAGE_F_SECOND|H_PAGE_F_GIX);+new_pte|=pte_set_hash_slot(ptep,rpte,0,slot);}/*
We need PTE bits 3 ,4, 5, 6 and 57 to support protection-keys,
because these are the bits we want to consolidate on across all
configuration to support protection keys.
Bit 3,4,5 and 6 are currently used on 4K-pte kernels. But bit 9
and 10 are available. Hence we use the two available bits and
free up bit 5 and 6. We will still not be able to free up bit 3
and 4. In the absence of any other free bits, we will have to
stay satisfied with what we have :-(. This means we will not
be able to support 32 protection keys, but only 8. The bit
numbers are big-endian as defined in the ISA3.0
This patch does the following change to 4K PTE.
H_PAGE_F_SECOND (S) which occupied bit 4 moves to bit 7.
H_PAGE_F_GIX (G,I,X) which occupied bit 5, 6 and 7 also moves
to bit 8,9, 10 respectively.
H_PAGE_HASHPTE (H) which occupied bit 8 moves to bit 4.
Before the patch, the 4k PTE format was as follows
0 1 2 3 4 5 6 7 8 9 10....................57.....63
: : : : : : : : : : : : :
v v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x|B|S |G |I |X |H| | |x|x|................| |x|x|x|
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
After the patch, the 4k PTE format is as follows
0 1 2 3 4 5 6 7 8 9 10....................57.....63
: : : : : : : : : : : : :
v v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x|B|H | | |S |G|I|X|x|x|................| |.|.|.|
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
The patch has no code changes; just swizzles around bits.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 7 ++++---
arch/powerpc/include/asm/book3s/64/hash-64k.h | 1 +
arch/powerpc/include/asm/book3s/64/hash.h | 1 -
3 files changed, 5 insertions(+), 4 deletions(-)
@@ -13,6 +13,7 @@#define H_PAGE_COMBO _RPAGE_RPN0 /* this is a combo 4k page */#define H_PAGE_4K_PFN _RPAGE_RPN1 /* PFN is for a single 4k page */#define H_PAGE_BUSY _RPAGE_RPN44 /* software: PTE & hash are busy */+#define H_PAGE_HASHPTE _RPAGE_RPN43 /* PTE has associated HPTE *//**WeneedtodifferentiatebetweenexplicithugepageandTHPhuge
@@ -20,6 +20,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,pte_t*ptep,unsignedlongtrap,unsignedlongflags,intssize,intsubpg_prot){+real_pte_trpte;unsignedlonghpte_group;unsignedlongrflags,pa;unsignedlongold_pte,new_pte;
@@ -54,6 +55,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,*needtoaddin0x1ifit'saread-onlyuserpage*/rflags=htab_convert_pte_flags(new_pte);+rpte=__real_pte(__pte(old_pte),ptep);if(cpu_has_feature(CPU_FTR_NOEXECUTE)&&!cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
@@ -64,13 +66,10 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,/**ThereMIGHTbeanHPTEforthispte*/-hash=hpt_hash(vpn,shift,ssize);-if(old_pte&H_PAGE_F_SECOND)-hash=~hash;-slot=(hash&htab_hash_mask)*HPTES_PER_GROUP;-slot+=(old_pte&H_PAGE_F_GIX)>>H_PAGE_F_GIX_SHIFT;+unsignedlonggslot=pte_get_hash_gslot(vpn,shift,+ssize,rpte,0);-if(mmu_hash_ops.hpte_updatepp(slot,rflags,vpn,MMU_PAGE_4K,+if(mmu_hash_ops.hpte_updatepp(gslot,rflags,vpn,MMU_PAGE_4K,MMU_PAGE_4K,ssize,flags)==-1)old_pte&=~_PAGE_HPTEFLAGS;}
@@ -118,8 +117,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,return-1;}new_pte=(new_pte&~_PAGE_HPTEFLAGS)|H_PAGE_HASHPTE;-new_pte|=(slot<<H_PAGE_F_GIX_SHIFT)&-(H_PAGE_F_SECOND|H_PAGE_F_GIX);+new_pte|=pte_set_hash_slot(ptep,rpte,0,slot);}*ptep=__pte(new_pte&~H_PAGE_BUSY);return0;
@@ -1617,23 +1617,18 @@ unsigned long pte_get_hash_gslot(unsigned long vpn, unsigned long shift,voidflush_hash_page(unsignedlongvpn,real_pte_tpte,intpsize,intssize,unsignedlongflags){-unsignedlonghash,index,shift,hidx,slot;+unsignedlongindex,shift,gslot;intlocal=flags&HPTE_LOCAL_UPDATE;DBG_LOW("flush_hash_page(vpn=%016lx)\n",vpn);pte_iterate_hashed_subpages(pte,psize,vpn,index,shift){-hash=hpt_hash(vpn,shift,ssize);-hidx=__rpte_to_hidx(pte,index);-if(hidx&_PTEIDX_SECONDARY)-hash=~hash;-slot=(hash&htab_hash_mask)*HPTES_PER_GROUP;-slot+=hidx&_PTEIDX_GROUP_IX;-DBG_LOW(" sub %ld: hash=%lx, hidx=%lx\n",index,slot,hidx);+gslot=pte_get_hash_gslot(vpn,shift,ssize,pte,index);+DBG_LOW(" sub %ld: gslot=%lx\n",index,gslot);/**Weusesamebasepagesizeandactualpsize,becausewedon't*usethesefunctionsforhugepage*/-mmu_hash_ops.hpte_invalidate(slot,vpn,psize,psize,+mmu_hash_ops.hpte_invalidate(gslot,vpn,psize,psize,ssize,local);}pte_iterate_hashed_end();
The H_PAGE_F_SECOND,H_PAGE_F_GIX are not in the 64K main-PTE.
capture these changes in the dump pte report.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/mm/dump_linuxpagetables.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
Memory protection keys enable applications to protect its
address space from inadvertent access from or corruption
by itself.
These patches along with the pte-bit freeing patch series
enables the protection key feature on powerpc; 4k and 64k
hashpage kernels. A subsequent patch series that changes
the generic and x86 code will expose memkey features
through sysfs and provide testcases and Documentation
updates.
Patches are based on powerpc -next branch. All patches
can be found at --
https://github.com/rampai/memorykeys.git memkey.v8
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.
Highlevel view of the design:
---------------------------
When an application associates a key with a address
address range, program the key in the Linux PTE.
When the MMU detects a page fault, allocate a hash
page and program the key into HPTE. And finally
when the MMU detects a key violation; due to
invalid application access, invoke the registered
signal handler and provide the violated key number.
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.
NOTE: All the selftest related patches will be part
of a separate patch series.
History:
-------
version v8:
(1) Contents of the AMR register withdrawn from
the siginfo structure. Applications can always
read the AMR register.
(2) AMR/IAMR/UAMOR are now available through
ptrace system call. -- thanks to Thiago
(3) code changes to handle legacy power cpus
that do not support execute-disable.
(4) incorporates many code improvement
suggestions.
version v7:
(1) refers to device tree property to enable
protection keys.
(2) adds 4K PTE support.
(3) fixes a couple of bugs noticed by Thiago
(4) decouples this patch series from arch-
independent code. This patch series can
now stand by itself, with one kludge
patch(2).
version v7:
(1) refers to device tree property to enable
protection keys.
(2) adds 4K PTE support.
(3) fixes a couple of bugs noticed by Thiago
(4) decouples this patch series from arch-
independent code. This patch series can
now stand by itself, with one kludge
patch(2).
version v6:
(1) selftest changes are broken down into 20
incremental patches.
(2) A separate key allocation mask that
includes PKEY_DISABLE_EXECUTE is
added for powerpc
(3) pkey feature is enabled for 64K HPT case
only. RPT and 4k HPT is disabled.
(4) Documentation is updated to better
capture the semantics.
(5) introduced arch_pkeys_enabled() to find
if an arch enables pkeys. Correspond-
ing change the logic that displays
key value in smaps.
(6) code rearranged in many places based on
comments from Dave Hansen, Balbir,
Anshuman.
(7) fixed one bug where a bogus key could be
associated successfully in
pkey_mprotect().
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 fault-
ing 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
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 (24):
powerpc: initial pkey plumbing
powerpc: define an additional vma bit for protection keys.
powerpc: track allocation status of all pkeys
powerpc: helper function to read,write AMR,IAMR,UAMOR registers
powerpc: helper functions to initialize AMR, IAMR and UAMOR registers
powerpc: cleaup AMR,iAMR when a key is allocated or freed
powerpc: implementation for arch_set_user_pkey_access()
powerpc: sys_pkey_alloc() and sys_pkey_free() system calls
powerpc: ability to create execute-disabled pkeys
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: implementation for arch_vma_access_permitted()
powerpc: Handle exceptions caused by pkey violation
powerpc: introduce get_pte_pkey() helper
powerpc: capture the violated protection key on fault
powerpc: Deliver SEGV signal on pkey violation
powerpc: Enable pkey subsystem
Thiago Jung Bauermann (1):
powerpc/ptrace: Add memory protection key regset
arch/powerpc/Kconfig | 16 +
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 | 74 +++++-
arch/powerpc/include/asm/cputable.h | 15 +-
arch/powerpc/include/asm/mman.h | 16 +-
arch/powerpc/include/asm/mmu_context.h | 21 ++
arch/powerpc/include/asm/paca.h | 3 +
arch/powerpc/include/asm/pkeys.h | 252 +++++++++++++++++
arch/powerpc/include/asm/processor.h | 5 +
arch/powerpc/include/asm/systbl.h | 3 +
arch/powerpc/include/asm/unistd.h | 6 +-
arch/powerpc/include/uapi/asm/elf.h | 1 +
arch/powerpc/include/uapi/asm/mman.h | 6 +
arch/powerpc/include/uapi/asm/unistd.h | 3 +
arch/powerpc/kernel/asm-offsets.c | 5 +
arch/powerpc/kernel/process.c | 10 +
arch/powerpc/kernel/prom.c | 19 ++
arch/powerpc/kernel/ptrace.c | 66 +++++
arch/powerpc/kernel/setup_64.c | 4 +
arch/powerpc/kernel/traps.c | 22 ++
arch/powerpc/mm/Makefile | 1 +
arch/powerpc/mm/fault.c | 46 +++-
arch/powerpc/mm/hash_utils_64.c | 26 ++
arch/powerpc/mm/mmu_context_book3s64.c | 2 +
arch/powerpc/mm/pkeys.c | 374 +++++++++++++++++++++++++
include/uapi/linux/elf.h | 1 +
27 files changed, 1000 insertions(+), 17 deletions(-)
create mode 100644 arch/powerpc/include/asm/pkeys.h
create mode 100644 arch/powerpc/mm/pkeys.c
@@ -0,0 +1,33 @@+/*+*PowerPCMemoryProtectionKeysmanagement+*Copyright(c)2015,IntelCorporation.+*Copyright(c)2017,IBMCorporation.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsandconditionsoftheGNUGeneralPublicLicense,+*version2,aspublishedbytheFreeSoftwareFoundation.+*+*Thisprogramisdistributedinthehopeitwillbeuseful,butWITHOUT+*ANYWARRANTY;withouteventheimpliedwarrantyofMERCHANTABILITYor+*FITNESSFORAPARTICULARPURPOSE.SeetheGNUGeneralPublicLicensefor+*moredetails.+*/+#include<linux/pkeys.h> /* PKEY_* */++boolpkey_inited;+boolpkey_execute_disable_support;++void__initpkey_initialize(void)+{+/* disable the pkey system till everything+*isinplace.Apatchfurtherdownthe+*linewillenableit.+*/+pkey_inited=false;++/*+*disableexecute_disablesupportfornow.+*Apatchfurtherdownwillenableit.+*/+pkey_execute_disable_support=false;+}
powerpc needs an additional vma bit to support 32 keys.
Till the additional vma bit lands in include/linux/mm.h
we have to define it in powerpc specific header file.
This is needed to get pkeys working on power.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/pkeys.h | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
Total 32 keys are available on power7 and above. However
pkey 0,1 are reserved. So effectively we have 30 pkeys.
On 4K kernels, we do not have 5 bits in the PTE to
represent all the keys; we only have 3bits.Two of those
keys are reserved; pkey 0 and pkey 1. So effectively we
have 6 pkeys.
This patch keeps track of reserved keys, allocated keys
and keys that are currently free.
Also it adds skeletal functions and macros, that the
architecture-independent code expects to be available.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 9 ++++
arch/powerpc/include/asm/mmu_context.h | 1 +
arch/powerpc/include/asm/pkeys.h | 72 ++++++++++++++++++++++++++++--
arch/powerpc/mm/mmu_context_book3s64.c | 2 +
arch/powerpc/mm/pkeys.c | 28 ++++++++++++
5 files changed, 108 insertions(+), 4 deletions(-)
@@ -3,6 +3,8 @@externboolpkey_inited;externboolpkey_execute_disable_support;+externintpkeys_total;/* total pkeys as per device tree */+externu32initial_allocation_mask;/* bits set for reserved keys *//**powerpcneedsanadditionalvmabittosupport32keys.
@@ -21,21 +23,76 @@#define VM_PKEY_BIT4 VM_HIGH_ARCH_4#endif-#define ARCH_VM_PKEY_FLAGS 0+#define arch_max_pkey() pkeys_total+#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \+VM_PKEY_BIT3|VM_PKEY_BIT4)++#define pkey_alloc_mask(pkey) (0x1 << pkey)++#define mm_pkey_allocation_map(mm) (mm->context.pkey_allocation_map)++#define mm_set_pkey_allocated(mm, pkey) { \+mm_pkey_allocation_map(mm)|=pkey_alloc_mask(pkey);\+}++#define mm_set_pkey_free(mm, pkey) { \+mm_pkey_allocation_map(mm)&=~pkey_alloc_mask(pkey);\+}++#define mm_set_pkey_is_allocated(mm, pkey) \+(mm_pkey_allocation_map(mm)&pkey_alloc_mask(pkey))++#define mm_set_pkey_is_reserved(mm, pkey) (initial_allocation_mask & \+pkey_alloc_mask(pkey))staticinlineboolmm_pkey_is_allocated(structmm_struct*mm,intpkey){-return(pkey==0);+/* a reserved key is never considered as 'explicitly allocated' */+return((pkey<arch_max_pkey())&&+!mm_set_pkey_is_reserved(mm,pkey)&&+mm_set_pkey_is_allocated(mm,pkey));}+/*+*Returnsapositive,5-bitkeyonsuccess,or-1onfailure.+*/staticinlineintmm_pkey_alloc(structmm_struct*mm){-return-1;+/*+*Note:thisistheoneandonlyplacewemakesure+*thatthepkeyisvalidasfarasthehardwareis+*concerned.Therestofthekerneltruststhat+*onlygood,validpkeyscomeoutofhere.+*/+u32all_pkeys_mask=(u32)(~(0x0));+intret;++if(!pkey_inited)+return-1;+/*+*Areweoutofpkeys?Wemusthandlethisspecially+*becauseffz()behaviorisundefinedifthereareno+*zeros.+*/+if(mm_pkey_allocation_map(mm)==all_pkeys_mask)+return-1;++ret=ffz((u32)mm_pkey_allocation_map(mm));+mm_set_pkey_allocated(mm,ret);+returnret;}staticinlineintmm_pkey_free(structmm_struct*mm,intpkey){-return-EINVAL;+if(!pkey_inited)+return-1;++if(!mm_pkey_is_allocated(mm,pkey))+return-EINVAL;++mm_set_pkey_free(mm,pkey);++return0;}/*
@@ -59,5 +116,12 @@ static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,return0;}+staticinlinevoidpkey_mm_init(structmm_struct*mm)+{+if(!pkey_inited)+return;+mm_pkey_allocation_map(mm)=initial_allocation_mask;+}+externvoidpkey_initialize(void);#endif /*_ASM_PPC64_PKEYS_H */
@@ -16,9 +16,13 @@boolpkey_inited;boolpkey_execute_disable_support;+intpkeys_total;/* total pkeys as per device tree */+u32initial_allocation_mask;/* bits set for reserved keys */void__initpkey_initialize(void){+intos_reserved,i;+/* disable the pkey system till everything*isinplace.Apatchfurtherdownthe*linewillenableit.
Implements helper functions to read and write the key related
registers; AMR, IAMR, UAMOR.
AMR register tracks the read,write permission of a key
IAMR register tracks the execute permission of a key
UAMOR register enables and disables a key
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 31 ++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)
Introduce helper functions that can initialize the bits in the AMR,
IAMR and UAMOR register; the bits that correspond to the given pkey.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/pkeys.h | 1 +
arch/powerpc/mm/pkeys.c | 46 ++++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 0 deletions(-)
cleanup the bits corresponding to a key in the AMR, and IAMR
register, when the key is newly allocated/activated or is freed.
We dont want some residual bits cause the hardware enforce
unintended behavior when the key is activated or freed.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
This patch provides the detailed implementation for
a user to allocate a key and enable it in the hardware.
It provides the plumbing, but it cannot be used till
the system call is implemented. The next patch will
do so.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/pkeys.h | 9 ++++++++-
arch/powerpc/mm/pkeys.c | 28 ++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 1 deletions(-)
@@ -105,3 +111,25 @@ void __arch_deactivate_pkey(int pkey){pkey_status_change(pkey,false);}++/*+*settheaccessrightinAMRIAMRandUAMORregister+*for@pkeytothatspecifiedin@init_val.+*/+int__arch_set_user_pkey_access(structtask_struct*tsk,intpkey,+unsignedlonginit_val)+{+u64new_amr_bits=0x0ul;++if(!is_pkey_enabled(pkey))+return-EINVAL;++/* Set the bits we need in AMR: */+if(init_val&PKEY_DISABLE_ACCESS)+new_amr_bits|=AMR_RD_BIT|AMR_WR_BIT;+elseif(init_val&PKEY_DISABLE_WRITE)+new_amr_bits|=AMR_WR_BIT;++init_amr(pkey,new_amr_bits);+return0;+}
Finally this patch provides the ability for a process to
allocate and free a protection key.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/systbl.h | 2 ++
arch/powerpc/include/asm/unistd.h | 4 +---
arch/powerpc/include/uapi/asm/unistd.h | 2 ++
3 files changed, 5 insertions(+), 3 deletions(-)
powerpc has hardware support to disable execute on a pkey.
This patch enables the ability to create execute-disabled
keys.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/uapi/asm/mman.h | 6 ++++++
arch/powerpc/mm/pkeys.c | 16 ++++++++++++++++
2 files changed, 22 insertions(+), 0 deletions(-)
@@ -24,6 +24,14 @@ void __init pkey_initialize(void){intos_reserved,i;+/*+*wedefinePKEY_DISABLE_EXECUTEinadditiontothearch-neutral+*genericdefinesforPKEY_DISABLE_ACCESSandPKEY_DISABLE_WRITE.+*Ensurethatthebitsadistinct.+*/+BUILD_BUG_ON(PKEY_DISABLE_EXECUTE&+(PKEY_DISABLE_ACCESS|PKEY_DISABLE_WRITE));+/* disable the pkey system till everything*isinplace.Apatchfurtherdownthe*linewillenableit.
@@ -120,10 +128,18 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,unsignedlonginit_val){u64new_amr_bits=0x0ul;+u64new_iamr_bits=0x0ul;if(!is_pkey_enabled(pkey))return-EINVAL;+if((init_val&PKEY_DISABLE_EXECUTE)){+if(!pkey_execute_disable_support)+return-EINVAL;+new_iamr_bits|=IAMR_EX_BIT;+}+init_iamr(pkey,new_iamr_bits);+/* Set the bits we need in AMR: */if(init_val&PKEY_DISABLE_ACCESS)new_amr_bits|=AMR_RD_BIT|AMR_WR_BIT;
Store and restore the AMR, IAMR and UAMOR register state of the task
before scheduling out and after scheduling in, respectively.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/pkeys.h | 4 +++
arch/powerpc/include/asm/processor.h | 5 ++++
arch/powerpc/kernel/process.c | 10 ++++++++
arch/powerpc/mm/pkeys.c | 39 ++++++++++++++++++++++++++++++++++
4 files changed, 58 insertions(+), 0 deletions(-)
@@ -149,3 +149,42 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,init_amr(pkey,new_amr_bits);return0;}++voidthread_pkey_regs_save(structthread_struct*thread)+{+if(!pkey_inited)+return;++/* @TODO skip saving any registers if the thread+*hasnotusedanykeysyet.+*/++thread->amr=read_amr();+thread->iamr=read_iamr();+thread->uamor=read_uamor();+}++voidthread_pkey_regs_restore(structthread_struct*new_thread,+structthread_struct*old_thread)+{+if(!pkey_inited)+return;++/* @TODO just reset uamor to zero if the new_thread+*hasnotusedanykeysyet.+*/++if(old_thread->amr!=new_thread->amr)+write_amr(new_thread->amr);+if(old_thread->iamr!=new_thread->iamr)+write_iamr(new_thread->iamr);+if(old_thread->uamor!=new_thread->uamor)+write_uamor(new_thread->uamor);+}++voidthread_pkey_regs_init(structthread_struct*thread)+{+write_amr(0x0ul);+write_iamr(0x0ul);+write_uamor(0x0ul);+}
This patch provides the implementation of execute-only pkey.
The architecture-independent layer expects the arch-dependent
layer, to support the ability to create and enable a special
key which has execute-only permission.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 1 +
arch/powerpc/include/asm/pkeys.h | 9 ++++-
arch/powerpc/mm/pkeys.c | 57 ++++++++++++++++++++++++++++++
3 files changed, 66 insertions(+), 1 deletions(-)
@@ -188,3 +188,60 @@ void thread_pkey_regs_init(struct thread_struct *thread)write_iamr(0x0ul);write_uamor(0x0ul);}++staticinlineboolpkey_allows_readwrite(intpkey)+{+intpkey_shift=pkeyshift(pkey);++if(!(read_uamor()&(0x3UL<<pkey_shift)))+returntrue;++return!(read_amr()&((AMR_RD_BIT|AMR_WR_BIT)<<pkey_shift));+}++int__execute_only_pkey(structmm_struct*mm)+{+boolneed_to_set_mm_pkey=false;+intexecute_only_pkey=mm->context.execute_only_pkey;+intret;++/* Do we need to assign a pkey for mm's execute-only maps? */+if(execute_only_pkey==-1){+/* Go allocate one to use, which might fail */+execute_only_pkey=mm_pkey_alloc(mm);+if(execute_only_pkey<0)+return-1;+need_to_set_mm_pkey=true;+}++/*+*Wedonotwanttogothroughtherelativelycostly+*dancetosetAMRifwedonotneedto.Checkit+*firstandassumethatiftheexecute-onlypkeyis+*readwrite-disabledthanwedonothavetosetit+*ourselves.+*/+if(!need_to_set_mm_pkey&&+!pkey_allows_readwrite(execute_only_pkey))+returnexecute_only_pkey;++/*+*SetupAMRsothatitdeniesaccessforeverything+*otherthanexecution.+*/+ret=__arch_set_user_pkey_access(current,execute_only_pkey,+(PKEY_DISABLE_ACCESS|PKEY_DISABLE_WRITE));+/*+*IftheAMR-setoperationfailedsomehow,justreturn+*0andeffectivelydisableexecute-onlysupport.+*/+if(ret){+mm_set_pkey_free(mm,execute_only_pkey);+return-1;+}++/* We got one, store it and use it from here on out */+if(need_to_set_mm_pkey)+mm->context.execute_only_pkey=execute_only_pkey;+returnexecute_only_pkey;+}
arch-independent code expects the arch to map
a pkey into the vma's protection bit setting.
The patch provides that ability.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/mman.h | 8 +++++++-
arch/powerpc/include/asm/pkeys.h | 18 ++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletions(-)
@@ -245,3 +245,50 @@ int __execute_only_pkey(struct mm_struct *mm)mm->context.execute_only_pkey=execute_only_pkey;returnexecute_only_pkey;}++staticinlineboolvma_is_pkey_exec_only(structvm_area_struct*vma)+{+/* Do this check first since the vm_flags should be hot */+if((vma->vm_flags&(VM_READ|VM_WRITE|VM_EXEC))!=VM_EXEC)+returnfalse;++return(vma_pkey(vma)==vma->vm_mm->context.execute_only_pkey);+}++/*+*Thisshouldonlybecalledfor*plain*mprotectcalls.+*/+int__arch_override_mprotect_pkey(structvm_area_struct*vma,intprot,+intpkey)+{+/*+*Isthisanmprotect_pkey()call?Ifso,never+*overridethevaluethatcamefromtheuser.+*/+if(pkey!=-1)+returnpkey;++/*+*Ifthecurrentlyassociatedpkeyisexecute-only,+*buttherequestedprotectionrequiresreadorwrite,+*moveitbacktothedefaultpkey.+*/+if(vma_is_pkey_exec_only(vma)&&+(prot&(PROT_READ|PROT_WRITE)))+return0;++/*+*therequestedprotectionisexecute-only.Hence+*letsuseaexecute-onlypkey.+*/+if(prot==PROT_EXEC){+pkey=execute_only_pkey(vma->vm_mm);+if(pkey>0)+returnpkey;+}++/*+*nothingtooverride.+*/+returnvma_pkey(vma);+}
map the key protection bits of the vma to the pkey bits in
the PTE.
The Pte bits used for pkey are 3,4,5,6 and 57. The first
four bits are the same four bits that were freed up initially
in this patch series. remember? :-) Without those four bits
this patch would'nt be possible.
BUT, On 4k kernel, bit 3, and 4 could not be freed up. remember?
Hence we have to be satisfied with 5,6 and 7.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 25 ++++++++++++++++++++++++-
arch/powerpc/include/asm/mman.h | 8 ++++++++
arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++
3 files changed, 44 insertions(+), 1 deletions(-)
Patch provides the ability for a process to
associate a pkey with a address range.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/systbl.h | 1 +
arch/powerpc/include/asm/unistd.h | 4 +---
arch/powerpc/include/uapi/asm/unistd.h | 1 +
3 files changed, 3 insertions(+), 3 deletions(-)
@@ -232,6 +232,7 @@ unsigned long htab_convert_pte_flags(unsigned long pteflags)*/rflags|=HPTE_R_M;+rflags|=pte_to_hpte_pkey_bits(pteflags);returnrflags;}
helper function that checks if the read/write/execute is allowed
on the pte.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 4 +++
arch/powerpc/include/asm/pkeys.h | 12 +++++++++++
arch/powerpc/mm/pkeys.c | 28 ++++++++++++++++++++++++++
3 files changed, 44 insertions(+), 0 deletions(-)
Make sure that the kernel does not access user pages without
checking their key-protection.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
This patch provides the implementation for
arch_vma_access_permitted(). Returns true if the
requested access is allowed by pkey associated with the
vma.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/mmu_context.h | 5 +++-
arch/powerpc/mm/pkeys.c | 43 ++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 1 deletions(-)
@@ -320,3 +320,46 @@ bool arch_pte_access_permitted(u64 pte, bool write, bool execute)returnpkey_access_permitted(pte_to_pkey_bits(pte),write,execute);}++/*+*Weonlywanttoenforceprotectionkeysonthecurrentprocess+*becauseweeffectivelyhavenoaccesstoAMR/IAMRforother+*processesoranywaytotell*which*AMR/IAMRinathreaded+*processwecoulduse.+*+*SodonotenforcethingsiftheVMAisnotfromthecurrent+*mm,orifweareinakernelthread.+*/+staticinlineboolvma_is_foreign(structvm_area_struct*vma)+{+if(!current->mm)+returntrue;+/*+*iftheVMAisfromanotherprocess,thenAMR/IAMRhasno+*relevanceandshouldnotbeenforced.+*/+if(current->mm!=vma->vm_mm)+returntrue;++returnfalse;+}++boolarch_vma_access_permitted(structvm_area_struct*vma,+boolwrite,boolexecute,boolforeign)+{+intpkey;++if(!pkey_inited)+returntrue;++/* allow access if the VMA is not one from this process */+if(foreign||vma_is_foreign(vma))+returntrue;++pkey=vma_pkey(vma);++if(!pkey)+returntrue;++returnpkey_access_permitted(pkey,write,execute);+}
Handle Data and Instruction exceptions caused by memory
protection-key.
The CPU will detect the key fault if the HPTE is already
programmed with the key.
However if the HPTE is not hashed, a key fault will not
be detected by the hardware. The software will detect
pkey violation in such a case.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/mm/fault.c | 37 ++++++++++++++++++++++++++++++++-----
1 files changed, 32 insertions(+), 5 deletions(-)
@@ -145,6 +145,23 @@ static noinline int bad_area(struct pt_regs *regs, unsigned long address)return__bad_area(regs,address,SEGV_MAPERR);}+staticintbad_page_fault_exception(structpt_regs*regs,unsignedlongaddress,+intsi_code)+{+intsig=SIGBUS;+intcode=BUS_OBJERR;++#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS+if(si_code&DSISR_KEYFAULT){+sig=SIGSEGV;+code=SEGV_PKUERR;+}+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */++_exception(sig,regs,code,address);+return0;+}+staticintdo_sigbus(structpt_regs*regs,unsignedlongaddress,unsignedintfault){
@@ -391,11 +408,9 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,return0;if(unlikely(page_fault_is_bad(error_code))){-if(is_user){-_exception(SIGBUS,regs,BUS_OBJERR,address);-return0;-}-returnSIGBUS;+if(!is_user)+returnSIGBUS;+returnbad_page_fault_exception(regs,address,error_code);}/* Additional sanity check(s) */
@@ -492,6 +507,18 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,if(unlikely(access_error(is_write,is_exec,vma)))returnbad_area(regs,address);+#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS+if(!arch_vma_access_permitted(vma,flags&FAULT_FLAG_WRITE,+is_exec,0))+return__bad_area(regs,address,SEGV_PKUERR);+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */+++/* handle_mm_fault() needs to know if its a instruction access+*fault.+*/+if(is_exec)+flags|=FAULT_FLAG_INSTRUCTION;/**Ifforanyreasonatallwecouldn'thandlethefault,*makesureweexitgracefullyratherthanendlesslyredo
get_pte_pkey() helper returns the pkey associated with
a address corresponding to a given mm_struct.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 5 +++++
arch/powerpc/mm/hash_utils_64.c | 24 ++++++++++++++++++++++++
2 files changed, 29 insertions(+), 0 deletions(-)
@@ -451,6 +451,11 @@ extern int hash_page(unsigned long ea, unsigned long access, unsigned long trap,int__hash_page_huge(unsignedlongea,unsignedlongaccess,unsignedlongvsid,pte_t*ptep,unsignedlongtrap,unsignedlongflags,intssize,unsignedintshift,unsignedintmmu_psize);++#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS+u16get_pte_pkey(structmm_struct*mm,unsignedlongaddress);+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */+#ifdef CONFIG_TRANSPARENT_HUGEPAGEexternint__hash_page_thp(unsignedlongea,unsignedlongaccess,unsignedlongvsid,pmd_t*pmdp,unsignedlongtrap,
Capture the protection key that got violated in paca.
This value will be later used to inform the signal
handler.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/paca.h | 3 +++
arch/powerpc/kernel/asm-offsets.c | 5 +++++
arch/powerpc/mm/fault.c | 11 ++++++++++-
3 files changed, 18 insertions(+), 1 deletions(-)
The value of the pkey, whose protection got violated,
is made available in si_pkey field of the siginfo structure.
Also keep the thread's pkey-register fields up2date.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/kernel/traps.c | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
From: Thiago Jung Bauermann <redacted>
The AMR/IAMR/UAMOR are part of the program context.
Allow it to be accessed via ptrace and through core files.
Signed-off-by: Ram Pai <redacted>
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/include/asm/pkeys.h | 5 +++
arch/powerpc/include/uapi/asm/elf.h | 1 +
arch/powerpc/kernel/ptrace.c | 66 +++++++++++++++++++++++++++++++++++
include/uapi/linux/elf.h | 1 +
4 files changed, 73 insertions(+), 0 deletions(-)
@@ -1775,6 +1776,61 @@ static int pmu_set(struct task_struct *target,returnret;}#endif++#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS+staticintpkey_active(structtask_struct*target,+conststructuser_regset*regset)+{+if(!arch_pkeys_enabled())+return-ENODEV;++returnregset->n;+}++staticintpkey_get(structtask_struct*target,+conststructuser_regset*regset,+unsignedintpos,unsignedintcount,+void*kbuf,void__user*ubuf)+{+BUILD_BUG_ON(TSO(amr)+sizeof(unsignedlong)!=TSO(iamr));+BUILD_BUG_ON(TSO(iamr)+sizeof(unsignedlong)!=TSO(uamor));++if(!arch_pkeys_enabled())+return-ENODEV;++returnuser_regset_copyout(&pos,&count,&kbuf,&ubuf,+&target->thread.amr,0,+ELF_NPKEY*sizeof(unsignedlong));+}++staticintpkey_set(structtask_struct*target,+conststructuser_regset*regset,+unsignedintpos,unsignedintcount,+constvoid*kbuf,constvoid__user*ubuf)+{+u64new_amr;+intret;++if(!arch_pkeys_enabled())+return-ENODEV;++/* Only the AMR can be set from userspace */+if(pos!=0||count!=sizeof(new_amr))+return-EINVAL;++ret=user_regset_copyin(&pos,&count,&kbuf,&ubuf,+&new_amr,0,sizeof(new_amr));+if(ret)+returnret;++/* UAMOR determines which bits of the AMR can be set from userspace. */+target->thread.amr=(new_amr&target->thread.uamor)|+(target->thread.amr&~target->thread.uamor);++return0;+}+#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */+/**Theseareournativeregsetflavors.*/
PAPR defines 'ibm,processor-storage-keys' property. It exports
two values.The first value indicates the number of data-access
keys and the second indicates the number of instruction-access
keys. Though this hints that keys can be either data access
or instruction access only,that is not the case in reality.Any
key can be of any kind. This patch adds all the keys and uses
that as the total number of keys available to us.
Non PAPR platforms do not define this property in the device
tree yet. Here, we hardcode CPUs that support pkey by
consulting PowerISA3.0
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/cputable.h | 15 ++++++++++-----
arch/powerpc/include/asm/mmu_context.h | 1 +
arch/powerpc/include/asm/pkeys.h | 21 +++++++++++++++++++++
arch/powerpc/kernel/prom.c | 19 +++++++++++++++++++
arch/powerpc/mm/pkeys.c | 19 ++++++++++++++-----
5 files changed, 65 insertions(+), 10 deletions(-)
@@ -1,9 +1,12 @@#ifndef _ASM_PPC64_PKEYS_H#define _ASM_PPC64_PKEYS_H+#include<asm/firmware.h>+externboolpkey_inited;externboolpkey_execute_disable_support;externintpkeys_total;/* total pkeys as per device tree */+externintpkey_total_execute;/* total execute pkeys as per device tree */externu32initial_allocation_mask;/* bits set for reserved keys *//*
@@ -391,6 +409,7 @@ static int __init early_init_dt_scan_cpus(unsigned long node,check_cpu_feature_properties(node);check_cpu_pa_features(node);+check_cpu_pkey_feature(node);}identical_pvr_fixup(node);
On Sat, Sep 9, 2017 at 8:44 AM, Ram Pai [off-list ref] wrote:
quoted hunk
Introduce pte_set_hash_slot().It sets the (H_PAGE_F_SECOND|H_PAGE_F_GIX)
bits at the appropriate location in the PTE of 4K PTE. For
64K PTE, it sets the bits in the second part of the PTE. Though
the implementation for the former just needs the slot parameter, it does
take some additional parameters to keep the prototype consistent.
This function will be handy as we work towards re-arranging the
bits in the later patches.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 15 +++++++++++++++
arch/powerpc/include/asm/book3s/64/hash-64k.h | 25 +++++++++++++++++++++++++
2 files changed, 40 insertions(+), 0 deletions(-)
On Sat, Sep 9, 2017 at 8:44 AM, Ram Pai [off-list ref] wrote:
quoted hunk
Introduce pte_get_hash_gslot()() which returns the slot number of the
HPTE in the global hash table.
This function will come in handy as we work towards re-arranging the
PTE bits in the later patches.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/hash.h | 3 +++
arch/powerpc/mm/hash_utils_64.c | 18 ++++++++++++++++++
2 files changed, 21 insertions(+), 0 deletions(-)
@@ -166,6 +166,9 @@ static inline int hash__pte_none(pte_t pte)return(pte_val(pte)&~H_PTE_NONE_MASK)==0;}+unsignedlongpte_get_hash_gslot(unsignedlongvpn,unsignedlongshift,+intssize,real_pte_trpte,unsignedintsubpg_index);+/* This low level function performs the actual PTE insertion*SettingthePTEdependsontheMMUtypeandotherfactors.It's*anhorriblemessthatI'mnotgoingtotrytocleanupnowbut
Does this work with native/guest page tables? I guess both.
The comment sounds trivial, could you please elaborate more.
Looking at the code, it seems like given a real pte, we use
the hash value and hidx to figure out the slot value in the global
slot information. This uses information in the software page
tables. Is that correct? Do we have to consider validity and
present state here or is that guaranteed?
+ */
+unsigned long pte_get_hash_gslot(unsigned long vpn, unsigned long shift,
+ int ssize, real_pte_t rpte, unsigned int subpg_index)
+{
+ unsigned long hash, slot, hidx;
+
+ hash = hpt_hash(vpn, shift, ssize);
+ hidx = __rpte_to_hidx(rpte, subpg_index);
+ if (hidx & _PTEIDX_SECONDARY)
+ hash = ~hash;
+ slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
+ slot += hidx & _PTEIDX_GROUP_IX;
+ return slot;
+}
+
/* WARNING: This is called from hash_low_64.S, if you change this prototype,
* do not forget to update the assembly call site !
*/
On Wed, Sep 13, 2017 at 07:32:57PM +1000, Balbir Singh wrote:
On Sat, Sep 9, 2017 at 8:44 AM, Ram Pai [off-list ref] wrote:
quoted
Introduce pte_get_hash_gslot()() which returns the slot number of the
HPTE in the global hash table.
This function will come in handy as we work towards re-arranging the
PTE bits in the later patches.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/hash.h | 3 +++
arch/powerpc/mm/hash_utils_64.c | 18 ++++++++++++++++++
2 files changed, 21 insertions(+), 0 deletions(-)
@@ -166,6 +166,9 @@ static inline int hash__pte_none(pte_t pte)return(pte_val(pte)&~H_PTE_NONE_MASK)==0;}+unsignedlongpte_get_hash_gslot(unsignedlongvpn,unsignedlongshift,+intssize,real_pte_trpte,unsignedintsubpg_index);+/* This low level function performs the actual PTE insertion*SettingthePTEdependsontheMMUtypeandotherfactors.It's*anhorriblemessthatI'mnotgoingtotrytocleanupnowbut
Does this work with native/guest page tables? I guess both.
Yes. it is supposed to work with native as well as guest page tables.
The code has been this way for ages. This patch encapsulate
the logic in a standalone function.
The comment sounds trivial, could you please elaborate more.
Looking at the code, it seems like given a real pte, we use
the hash value and hidx to figure out the slot value in the global
slot information. This uses information in the software page
tables. Is that correct?
Yes. This uses information passed to it by the caller, the information
is expected to be derived from linux page table.
Do we have to consider validity and
present state here or is that guaranteed?
This function's job is to do the math and return the global slot based
on the input. It will return the calculated value regardless of the validity of
its inputs.
Its the the callers' job to validate the pte and ensure that it is hashed,
before meaningfully using the return value of the this function.
quoted
+ */
+unsigned long pte_get_hash_gslot(unsigned long vpn, unsigned long shift,
+ int ssize, real_pte_t rpte, unsigned int subpg_index)
+{
+ unsigned long hash, slot, hidx;
+
+ hash = hpt_hash(vpn, shift, ssize);
+ hidx = __rpte_to_hidx(rpte, subpg_index);
+ if (hidx & _PTEIDX_SECONDARY)
+ hash = ~hash;
+ slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;
+ slot += hidx & _PTEIDX_GROUP_IX;
+ return slot;
+}
+
/* WARNING: This is called from hash_low_64.S, if you change this prototype,
* do not forget to update the assembly call site !
*/
On Fri, 8 Sep 2017 15:44:43 -0700
Ram Pai [off-list ref] wrote:
Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6,
in the 4K backed HPTE pages.These bits continue to be used
for 64K backed HPTE pages in this patch, but will be freed
up in the next patch. The bit numbers are big-endian as
defined in the ISA3.0
The patch does the following change to the 4k htpe backed
64K PTE's format.
H_PAGE_BUSY moves from bit 3 to bit 9 (B bit in the figure
below)
V0 which occupied bit 4 is not used anymore.
V1 which occupied bit 5 is not used anymore.
V2 which occupied bit 6 is not used anymore.
V3 which occupied bit 7 is not used anymore.
Before the patch, the 4k backed 64k PTE format was as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x|B|V0|V1|V2|V3|x| | |x|x|................|x|x|x|x| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
|S|G|I|X|S |G |I |X |S|G|I|X|..................|S|G|I|X| <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
After the patch, the 4k backed 64k PTE format is as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x| | | | | |x|B| |x|x|................|.|.|.|.| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
|S|G|I|X|S |G |I |X |S|G|I|X|..................|S|G|I|X| <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
the four bits S,G,I,X (one quadruplet per 4k HPTE) that
cache the hash-bucket slot value, is initialized to
1,1,1,1 indicating -- an invalid slot. If a HPTE gets
cached in a 1111 slot(i.e 7th slot of secondary hash
bucket), it is released immediately. In other words,
even though 1111 is a valid slot value in the hash
bucket, we consider it invalid and release the slot and
the HPTE. This gives us the opportunity to determine
the validity of S,G,I,X bits based on its contents and
not on any of the bits V0,V1,V2 or V3 in the primary PTE
When we release a HPTE cached in the 1111 slot
we also release a legitimate slot in the primary
hash bucket and unmap its corresponding HPTE. This
is to ensure that we do get a HPTE cached in a slot
of the primary hash bucket, the next time we retry.
Though treating 1111 slot as invalid, reduces the
number of available slots in the hash bucket and may
have an effect on the performance, the probabilty of
hitting a 1111 slot is extermely low.
Compared to the current scheme, the above scheme
reduces the number of false hash table updates
significantly and has the added advantage of releasing
four valuable PTE bits for other purpose.
NOTE:even though bits 3, 4, 5, 6, 7 are not used when
the 64K PTE is backed by 4k HPTE, they continue to be
used if the PTE gets backed by 64k HPTE. The next
patch will decouple that aswell, and truely release the
bits.
This idea was jointly developed by Paul Mackerras,
Aneesh, Michael Ellermen and myself.
On Fri, 8 Sep 2017 15:44:44 -0700
Ram Pai [off-list ref] wrote:
quoted hunk
Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6
in the 64K backed HPTE pages. This along with the earlier
patch will entirely free up the four bits from 64K PTE.
The bit numbers are big-endian as defined in the ISA3.0
This patch does the following change to 64K PTE backed
by 64K HPTE.
H_PAGE_F_SECOND (S) which occupied bit 4 moves to the
second part of the pte to bit 60.
H_PAGE_F_GIX (G,I,X) which occupied bit 5, 6 and 7 also
moves to the second part of the pte to bit 61,
62, 63, 64 respectively
since bit 7 is now freed up, we move H_PAGE_BUSY (B) from
bit 9 to bit 7.
The second part of the PTE will hold
(H_PAGE_F_SECOND|H_PAGE_F_GIX) at bit 60,61,62,63.
NOTE: None of the bits in the secondary PTE were not used
by 64k-HPTE backed PTE.
Before the patch, the 64K HPTE backed 64k PTE format was
as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x| |S |G |I |X |x|B| |x|x|................|x|x|x|x| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
| | | | | | | | | | | | |..................| | | | | <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
After the patch, the 64k HPTE backed 64k PTE format is
as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x| | | | |B |x| | |x|x|................|.|.|.|.| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
| | | | | | | | | | | | |..................|S|G|I|X| <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
The above PTE changes is applicable to hugetlbpages aswell.
The patch does the following code changes:
a) moves the H_PAGE_F_SECOND and H_PAGE_F_GIX to 4k PTE
header since it is no more needed b the 64k PTEs.
b) abstracts out __real_pte() and __rpte_to_hidx() so the
caller need not know the bit location of the slot.
c) moves the slot bits to the secondary pte.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 3 ++
arch/powerpc/include/asm/book3s/64/hash-64k.h | 29 +++++++++++-------------
arch/powerpc/include/asm/book3s/64/hash.h | 3 --
arch/powerpc/mm/hash64_64k.c | 23 ++++++++-----------
arch/powerpc/mm/hugetlbpage-hash64.c | 18 ++++++---------
5 files changed, 33 insertions(+), 43 deletions(-)
@@ -12,7 +12,7 @@*/#define H_PAGE_COMBO _RPAGE_RPN0 /* this is a combo 4k page */#define H_PAGE_4K_PFN _RPAGE_RPN1 /* PFN is for a single 4k page */-#define H_PAGE_BUSY _RPAGE_RPN42 /* software: PTE & hash are busy */+#define H_PAGE_BUSY _RPAGE_RPN44 /* software: PTE & hash are busy *//**WeneedtodifferentiatebetweenexplicithugepageandTHPhuge
@@ -103,8 +103,8 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,*Onhashinsertfailureweuseoldptevalueandwedon't*wantslotinformationthereifwehaveainsertfailure.*/-old_pte&=~(H_PAGE_HASHPTE|H_PAGE_F_GIX|H_PAGE_F_SECOND);-new_pte&=~(H_PAGE_HASHPTE|H_PAGE_F_GIX|H_PAGE_F_SECOND);+old_pte&=~H_PAGE_HASHPTE;+new_pte&=~H_PAGE_HASHPTE;
Shouldn't we set old/new_pte.slot = invalid? via rpte.hidx
quoted hunk
goto htab_insert_hpte;
}
/*
@@ -227,6 +227,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access, unsigned long vsid, pte_t *ptep, unsigned long trap, unsigned long flags, int ssize) {+ real_pte_t rpte; unsigned long hpte_group; unsigned long rflags, pa; unsigned long old_pte, new_pte;
@@ -263,6 +264,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access, } while (!pte_xchg(ptep, __pte(old_pte), __pte(new_pte))); rflags = htab_convert_pte_flags(new_pte);+ rpte = __real_pte(__pte(old_pte), ptep); if (cpu_has_feature(CPU_FTR_NOEXECUTE) && !cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
@@ -270,18 +272,13 @@ int __hash_page_64K(unsigned long ea, unsigned long access, vpn = hpt_vpn(ea, vsid, ssize); if (unlikely(old_pte & H_PAGE_HASHPTE)) {+ unsigned long gslot; /* * There MIGHT be an HPTE for this pte */- hash = hpt_hash(vpn, shift, ssize);- if (old_pte & H_PAGE_F_SECOND)- hash = ~hash;- slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;- slot += (old_pte & H_PAGE_F_GIX) >> H_PAGE_F_GIX_SHIFT;-- if (mmu_hash_ops.hpte_updatepp(slot, rflags, vpn, MMU_PAGE_64K,- MMU_PAGE_64K, ssize,- flags) == -1)+ gslot = pte_get_hash_gslot(vpn, shift, ssize, rpte, 0);+ if (mmu_hash_ops.hpte_updatepp(gslot, rflags, vpn, MMU_PAGE_64K,+ MMU_PAGE_64K, ssize, flags) == -1) old_pte &= ~_PAGE_HPTEFLAGS; }
@@ -22,6 +22,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,pte_t*ptep,unsignedlongtrap,unsignedlongflags,intssize,unsignedintshift,unsignedintmmu_psize){+real_pte_trpte;unsignedlongvpn;unsignedlongold_pte,new_pte;unsignedlongrflags,pa,sz;
@@ -61,6 +62,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,}while(!pte_xchg(ptep,__pte(old_pte),__pte(new_pte)));rflags=htab_convert_pte_flags(new_pte);+rpte=__real_pte(__pte(old_pte),ptep);sz=((1UL)<<shift);if(!cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
@@ -71,16 +73,11 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,/* Check if pte already has an hpte (case 2) */if(unlikely(old_pte&H_PAGE_HASHPTE)){/* There MIGHT be an HPTE for this pte */-unsignedlonghash,slot;+unsignedlonggslot;-hash=hpt_hash(vpn,shift,ssize);-if(old_pte&H_PAGE_F_SECOND)-hash=~hash;-slot=(hash&htab_hash_mask)*HPTES_PER_GROUP;-slot+=(old_pte&H_PAGE_F_GIX)>>H_PAGE_F_GIX_SHIFT;--if(mmu_hash_ops.hpte_updatepp(slot,rflags,vpn,mmu_psize,-mmu_psize,ssize,flags)==-1)+gslot=pte_get_hash_gslot(vpn,shift,ssize,rpte,0);+if(mmu_hash_ops.hpte_updatepp(gslot,rflags,vpn,mmu_psize,+mmu_psize,ssize,flags)==-1)old_pte&=~_PAGE_HPTEFLAGS;}
@@ -106,8 +103,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,return-1;}-new_pte|=(slot<<H_PAGE_F_GIX_SHIFT)&-(H_PAGE_F_SECOND|H_PAGE_F_GIX);+new_pte|=pte_set_hash_slot(ptep,rpte,0,slot);}/*
On Fri, 8 Sep 2017 15:44:45 -0700
Ram Pai [off-list ref] wrote:
We need PTE bits 3 ,4, 5, 6 and 57 to support protection-keys,
because these are the bits we want to consolidate on across all
configuration to support protection keys.
Bit 3,4,5 and 6 are currently used on 4K-pte kernels. But bit 9
and 10 are available. Hence we use the two available bits and
free up bit 5 and 6. We will still not be able to free up bit 3
and 4. In the absence of any other free bits, we will have to
stay satisfied with what we have :-(. This means we will not
be able to support 32 protection keys, but only 8. The bit
numbers are big-endian as defined in the ISA3.0
Any chance for 4k PTE's we can do slot searching for the PTE?
I guess thats add additional complexity
Balbir Singh.
On Fri, 8 Sep 2017 15:44:47 -0700
Ram Pai [off-list ref] wrote:
The H_PAGE_F_SECOND,H_PAGE_F_GIX are not in the 64K main-PTE.
capture these changes in the dump pte report.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
So we lose slot and secondary information for 64K PTE's with
this change?
Balbir
@@ -864,6 +864,22 @@ config SECCOMPIfunsure,sayY.OnlyembeddedshouldsayNhere.+configPPC64_MEMORY_PROTECTION_KEYS+prompt"PowerPC Memory Protection Keys"+def_booly+# Note: only available in 64-bit mode+depends onPPC64
This is not sufficient right, you need PPC_BOOK3S_64
for compile time at-least?
quoted hunk
+ select ARCH_USES_HIGH_VMA_FLAGS
+ select ARCH_HAS_PKEYS
+ ---help---
+ 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.
+
+ For details, see Documentation/vm/protection-keys.txt
+
+ If unsure, say y.
+
endmenu
config ISA_DMA_API
@@ -0,0 +1,33 @@+/*+*PowerPCMemoryProtectionKeysmanagement+*Copyright(c)2015,IntelCorporation.+*Copyright(c)2017,IBMCorporation.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsandconditionsoftheGNUGeneralPublicLicense,+*version2,aspublishedbytheFreeSoftwareFoundation.+*+*Thisprogramisdistributedinthehopeitwillbeuseful,butWITHOUT+*ANYWARRANTY;withouteventheimpliedwarrantyofMERCHANTABILITYor+*FITNESSFORAPARTICULARPURPOSE.SeetheGNUGeneralPublicLicensefor+*moredetails.+*/+#include<linux/pkeys.h> /* PKEY_* */++boolpkey_inited;+boolpkey_execute_disable_support;++void__initpkey_initialize(void)+{+/* disable the pkey system till everything+*isinplace.Apatchfurtherdownthe+*linewillenableit.+*/
Comment style is broken
+ pkey_inited = false;
+
+ /*
+ * disable execute_disable support for now.
+ * A patch further down will enable it.
+ */
+ pkey_execute_disable_support = false;
+}
On Fri, 8 Sep 2017 15:44:50 -0700
Ram Pai [off-list ref] wrote:
powerpc needs an additional vma bit to support 32 keys.
Till the additional vma bit lands in include/linux/mm.h
we have to define it in powerpc specific header file.
This is needed to get pkeys working on power.
Signed-off-by: Ram Pai <redacted>
---
"This" being an arch specific hack for the additional bit?
Balbir
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2017-09-14 08:11:53
On Thu, 2017-09-14 at 14:38 +1000, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:44:50 -0700
Ram Pai [off-list ref] wrote:
quoted
powerpc needs an additional vma bit to support 32 keys.
Till the additional vma bit lands in include/linux/mm.h
we have to define it in powerpc specific header file.
This is needed to get pkeys working on power.
Signed-off-by: Ram Pai <redacted>
---
"This" being an arch specific hack for the additional bit?
Arch VMA bits ? really ? I'd rather we limit ourselves to 16 keys first
then push for adding the extra bit to the generic code.
Ben.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2017-09-14 08:14:20
On Fri, 2017-09-08 at 15:44 -0700, Ram Pai wrote:
The second part of the PTE will hold
(H_PAGE_F_SECOND|H_PAGE_F_GIX) at bit 60,61,62,63.
NOTE: None of the bits in the secondary PTE were not used
by 64k-HPTE backed PTE.
Have you measured the performance impact of this ? The second part of
the PTE being in a different cache line there could be one...
Cheers,
Ben.
On Thu, Sep 14, 2017 at 02:38:07PM +1000, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:44:50 -0700
Ram Pai [off-list ref] wrote:
quoted
powerpc needs an additional vma bit to support 32 keys.
Till the additional vma bit lands in include/linux/mm.h
we have to define it in powerpc specific header file.
This is needed to get pkeys working on power.
Signed-off-by: Ram Pai <redacted>
---
"This" being an arch specific hack for the additional bit?
Yes. arch-specific hack. I am trying to get the arch specific
changes merged parallelly, along with these patches. Don't know
which one will merge first. Regardless of which patch-set
lands-in first; I have organized the code such that nothing
breaks.
RP
@@ -864,6 +864,22 @@ config SECCOMPIfunsure,sayY.OnlyembeddedshouldsayNhere.+configPPC64_MEMORY_PROTECTION_KEYS+prompt"PowerPC Memory Protection Keys"+def_booly+# Note: only available in 64-bit mode+depends onPPC64
This is not sufficient right, you need PPC_BOOK3S_64
for compile time at-least?
Ok. Not thought too deep about this. Thanks for the input.
quoted
+ select ARCH_USES_HIGH_VMA_FLAGS
+
.....
quoted
+void __init pkey_initialize(void)
+{
+ /* disable the pkey system till everything
+ * is in place. A patch further down the
+ * line will enable it.
+ */
Comment style is broken
checkpatch.pl does not complain. So is it really a broken
comment style, or is it checkpatch.pl needs to be fixed?
RP
On Thu, Sep 14, 2017 at 01:22:27PM +1000, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:44:47 -0700
Ram Pai [off-list ref] wrote:
quoted
The H_PAGE_F_SECOND,H_PAGE_F_GIX are not in the 64K main-PTE.
capture these changes in the dump pte report.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
So we lose slot and secondary information for 64K PTE's with
this change?
yes. It was anyway not there for 4k-backed-64k ptes. Now it wont
be there for any 64k ptes.
RP
On Thu, Sep 14, 2017 at 11:48:34AM +1000, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:44:45 -0700
Ram Pai [off-list ref] wrote:
quoted
We need PTE bits 3 ,4, 5, 6 and 57 to support protection-keys,
because these are the bits we want to consolidate on across all
configuration to support protection keys.
Bit 3,4,5 and 6 are currently used on 4K-pte kernels. But bit 9
and 10 are available. Hence we use the two available bits and
free up bit 5 and 6. We will still not be able to free up bit 3
and 4. In the absence of any other free bits, we will have to
stay satisfied with what we have :-(. This means we will not
be able to support 32 protection keys, but only 8. The bit
numbers are big-endian as defined in the ISA3.0
Any chance for 4k PTE's we can do slot searching for the PTE?
I guess thats add additional complexity
Aneesh, i think, is working on moving slot information out of the PTE.
If that happens, we will have leg-space to support more keys.
RP
On Thu, Sep 14, 2017 at 11:44:49AM +1000, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:44:44 -0700
Ram Pai [off-list ref] wrote:
quoted
Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6
in the 64K backed HPTE pages. This along with the earlier
patch will entirely free up the four bits from 64K PTE.
The bit numbers are big-endian as defined in the ISA3.0
This patch does the following change to 64K PTE backed
by 64K HPTE.
H_PAGE_F_SECOND (S) which occupied bit 4 moves to the
second part of the pte to bit 60.
H_PAGE_F_GIX (G,I,X) which occupied bit 5, 6 and 7 also
moves to the second part of the pte to bit 61,
62, 63, 64 respectively
since bit 7 is now freed up, we move H_PAGE_BUSY (B) from
bit 9 to bit 7.
The second part of the PTE will hold
(H_PAGE_F_SECOND|H_PAGE_F_GIX) at bit 60,61,62,63.
NOTE: None of the bits in the secondary PTE were not used
by 64k-HPTE backed PTE.
Before the patch, the 64K HPTE backed 64k PTE format was
as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x| |S |G |I |X |x|B| |x|x|................|x|x|x|x| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
| | | | | | | | | | | | |..................| | | | | <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
After the patch, the 64k HPTE backed 64k PTE format is
as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x| | | | |B |x| | |x|x|................|.|.|.|.| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
| | | | | | | | | | | | |..................|S|G|I|X| <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
The above PTE changes is applicable to hugetlbpages aswell.
The patch does the following code changes:
a) moves the H_PAGE_F_SECOND and H_PAGE_F_GIX to 4k PTE
header since it is no more needed b the 64k PTEs.
b) abstracts out __real_pte() and __rpte_to_hidx() so the
caller need not know the bit location of the slot.
c) moves the slot bits to the secondary pte.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 3 ++
arch/powerpc/include/asm/book3s/64/hash-64k.h | 29 +++++++++++-------------
arch/powerpc/include/asm/book3s/64/hash.h | 3 --
arch/powerpc/mm/hash64_64k.c | 23 ++++++++-----------
arch/powerpc/mm/hugetlbpage-hash64.c | 18 ++++++---------
5 files changed, 33 insertions(+), 43 deletions(-)
@@ -12,7 +12,7 @@*/#define H_PAGE_COMBO _RPAGE_RPN0 /* this is a combo 4k page */#define H_PAGE_4K_PFN _RPAGE_RPN1 /* PFN is for a single 4k page */-#define H_PAGE_BUSY _RPAGE_RPN42 /* software: PTE & hash are busy */+#define H_PAGE_BUSY _RPAGE_RPN44 /* software: PTE & hash are busy *//**WeneedtodifferentiatebetweenexplicithugepageandTHPhuge
@@ -103,8 +103,8 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,*Onhashinsertfailureweuseoldptevalueandwedon't*wantslotinformationthereifwehaveainsertfailure.*/-old_pte&=~(H_PAGE_HASHPTE|H_PAGE_F_GIX|H_PAGE_F_SECOND);-new_pte&=~(H_PAGE_HASHPTE|H_PAGE_F_GIX|H_PAGE_F_SECOND);+old_pte&=~H_PAGE_HASHPTE;+new_pte&=~H_PAGE_HASHPTE;
Shouldn't we set old/new_pte.slot = invalid? via rpte.hidx
by resetting the H_PAGE_HASHPTE flag, we are invalidating
slot information. Would that not be sufficient?
RP
quoted
goto htab_insert_hpte;
}
/*
@@ -227,6 +227,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access, unsigned long vsid, pte_t *ptep, unsigned long trap, unsigned long flags, int ssize) {+ real_pte_t rpte; unsigned long hpte_group; unsigned long rflags, pa; unsigned long old_pte, new_pte;
@@ -263,6 +264,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access, } while (!pte_xchg(ptep, __pte(old_pte), __pte(new_pte))); rflags = htab_convert_pte_flags(new_pte);+ rpte = __real_pte(__pte(old_pte), ptep); if (cpu_has_feature(CPU_FTR_NOEXECUTE) && !cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
@@ -270,18 +272,13 @@ int __hash_page_64K(unsigned long ea, unsigned long access, vpn = hpt_vpn(ea, vsid, ssize); if (unlikely(old_pte & H_PAGE_HASHPTE)) {+ unsigned long gslot; /* * There MIGHT be an HPTE for this pte */- hash = hpt_hash(vpn, shift, ssize);- if (old_pte & H_PAGE_F_SECOND)- hash = ~hash;- slot = (hash & htab_hash_mask) * HPTES_PER_GROUP;- slot += (old_pte & H_PAGE_F_GIX) >> H_PAGE_F_GIX_SHIFT;-- if (mmu_hash_ops.hpte_updatepp(slot, rflags, vpn, MMU_PAGE_64K,- MMU_PAGE_64K, ssize,- flags) == -1)+ gslot = pte_get_hash_gslot(vpn, shift, ssize, rpte, 0);+ if (mmu_hash_ops.hpte_updatepp(gslot, rflags, vpn, MMU_PAGE_64K,+ MMU_PAGE_64K, ssize, flags) == -1) old_pte &= ~_PAGE_HPTEFLAGS; }
@@ -22,6 +22,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,pte_t*ptep,unsignedlongtrap,unsignedlongflags,intssize,unsignedintshift,unsignedintmmu_psize){+real_pte_trpte;unsignedlongvpn;unsignedlongold_pte,new_pte;unsignedlongrflags,pa,sz;
@@ -61,6 +62,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,}while(!pte_xchg(ptep,__pte(old_pte),__pte(new_pte)));rflags=htab_convert_pte_flags(new_pte);+rpte=__real_pte(__pte(old_pte),ptep);sz=((1UL)<<shift);if(!cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
@@ -71,16 +73,11 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,/* Check if pte already has an hpte (case 2) */if(unlikely(old_pte&H_PAGE_HASHPTE)){/* There MIGHT be an HPTE for this pte */-unsignedlonghash,slot;+unsignedlonggslot;-hash=hpt_hash(vpn,shift,ssize);-if(old_pte&H_PAGE_F_SECOND)-hash=~hash;-slot=(hash&htab_hash_mask)*HPTES_PER_GROUP;-slot+=(old_pte&H_PAGE_F_GIX)>>H_PAGE_F_GIX_SHIFT;--if(mmu_hash_ops.hpte_updatepp(slot,rflags,vpn,mmu_psize,-mmu_psize,ssize,flags)==-1)+gslot=pte_get_hash_gslot(vpn,shift,ssize,rpte,0);+if(mmu_hash_ops.hpte_updatepp(gslot,rflags,vpn,mmu_psize,+mmu_psize,ssize,flags)==-1)old_pte&=~_PAGE_HPTEFLAGS;}
@@ -106,8 +103,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,return-1;}-new_pte|=(slot<<H_PAGE_F_GIX_SHIFT)&-(H_PAGE_F_SECOND|H_PAGE_F_GIX);+new_pte|=pte_set_hash_slot(ptep,rpte,0,slot);}/*
On Thu, Sep 14, 2017 at 10:54:08AM -0700, Ram Pai wrote:
On Thu, Sep 14, 2017 at 11:44:49AM +1000, Balbir Singh wrote:
quoted
On Fri, 8 Sep 2017 15:44:44 -0700
Ram Pai [off-list ref] wrote:
quoted
Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6
in the 64K backed HPTE pages. This along with the earlier
patch will entirely free up the four bits from 64K PTE.
The bit numbers are big-endian as defined in the ISA3.0
This patch does the following change to 64K PTE backed
by 64K HPTE.
H_PAGE_F_SECOND (S) which occupied bit 4 moves to the
second part of the pte to bit 60.
H_PAGE_F_GIX (G,I,X) which occupied bit 5, 6 and 7 also
moves to the second part of the pte to bit 61,
62, 63, 64 respectively
since bit 7 is now freed up, we move H_PAGE_BUSY (B) from
bit 9 to bit 7.
The second part of the PTE will hold
(H_PAGE_F_SECOND|H_PAGE_F_GIX) at bit 60,61,62,63.
NOTE: None of the bits in the secondary PTE were not used
by 64k-HPTE backed PTE.
Before the patch, the 64K HPTE backed 64k PTE format was
as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x| |S |G |I |X |x|B| |x|x|................|x|x|x|x| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
| | | | | | | | | | | | |..................| | | | | <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
After the patch, the 64k HPTE backed 64k PTE format is
as follows
0 1 2 3 4 5 6 7 8 9 10...........................63
: : : : : : : : : : : :
v v v v v v v v v v v v
,-,-,-,-,--,--,--,--,-,-,-,-,-,------------------,-,-,-,
|x|x|x| | | | |B |x| | |x|x|................|.|.|.|.| <- primary pte
'_'_'_'_'__'__'__'__'_'_'_'_'_'________________'_'_'_'_'
| | | | | | | | | | | | |..................|S|G|I|X| <- secondary pte
'_'_'_'_'__'__'__'__'_'_'_'_'__________________'_'_'_'_'
The above PTE changes is applicable to hugetlbpages aswell.
The patch does the following code changes:
a) moves the H_PAGE_F_SECOND and H_PAGE_F_GIX to 4k PTE
header since it is no more needed b the 64k PTEs.
b) abstracts out __real_pte() and __rpte_to_hidx() so the
caller need not know the bit location of the slot.
c) moves the slot bits to the secondary pte.
Reviewed-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 3 ++
arch/powerpc/include/asm/book3s/64/hash-64k.h | 29 +++++++++++-------------
arch/powerpc/include/asm/book3s/64/hash.h | 3 --
arch/powerpc/mm/hash64_64k.c | 23 ++++++++-----------
arch/powerpc/mm/hugetlbpage-hash64.c | 18 ++++++---------
5 files changed, 33 insertions(+), 43 deletions(-)
@@ -12,7 +12,7 @@*/#define H_PAGE_COMBO _RPAGE_RPN0 /* this is a combo 4k page */#define H_PAGE_4K_PFN _RPAGE_RPN1 /* PFN is for a single 4k page */-#define H_PAGE_BUSY _RPAGE_RPN42 /* software: PTE & hash are busy */+#define H_PAGE_BUSY _RPAGE_RPN44 /* software: PTE & hash are busy *//**WeneedtodifferentiatebetweenexplicithugepageandTHPhuge
@@ -103,8 +103,8 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,*Onhashinsertfailureweuseoldptevalueandwedon't*wantslotinformationthereifwehaveainsertfailure.*/-old_pte&=~(H_PAGE_HASHPTE|H_PAGE_F_GIX|H_PAGE_F_SECOND);-new_pte&=~(H_PAGE_HASHPTE|H_PAGE_F_GIX|H_PAGE_F_SECOND);+old_pte&=~H_PAGE_HASHPTE;+new_pte&=~H_PAGE_HASHPTE;
Shouldn't we set old/new_pte.slot = invalid? via rpte.hidx
by resetting the H_PAGE_HASHPTE flag, we are invalidating
slot information. Would that not be sufficient?
I think i misunderstood you question. Yes rpte.hidx will have
to be reset to invalid. The code does that further down in that
function.
if (!(old_pte & H_PAGE_COMBO))
rpte.hidx = ~0x0UL;
RP
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2017-10-07 10:02:56
Ram Pai [off-list ref] writes:
Total 32 keys are available on power7 and above. However
pkey 0,1 are reserved. So effectively we have 30 pkeys.
On 4K kernels, we do not have 5 bits in the PTE to
represent all the keys; we only have 3bits.Two of those
keys are reserved; pkey 0 and pkey 1. So effectively we
have 6 pkeys.
This patch keeps track of reserved keys, allocated keys
and keys that are currently free.
Also it adds skeletal functions and macros, that the
architecture-independent code expects to be available.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 9 ++++
arch/powerpc/include/asm/mmu_context.h | 1 +
arch/powerpc/include/asm/pkeys.h | 72 ++++++++++++++++++++++++=
This doesn't build for me, with pseries_le_defconfig. I assume it built
for you. So something has changed upstream maybe?
In file included from ../include/linux/pkeys.h:8:0,
from ../mm/mprotect.c:26:
../mm/mprotect.c: In function =E2=80=98do_mprotect_pkey=E2=80=99:
../arch/powerpc/include/asm/pkeys.h:27:29: error: =E2=80=98VM_PKEY_BIT0=E2=
=80=99 undeclared (first use in this function)
#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
^
../mm/mprotect.c:470:6: note: in expansion of macro =E2=80=98ARCH_VM_PKEY_F=
LAGS=E2=80=99
ARCH_VM_PKEY_FLAGS;
^~~~~~~~~~~~~~~~~~
../arch/powerpc/include/asm/pkeys.h:27:29: note: each undeclared identifier=
is reported only once for each function it appears in
#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
^
../mm/mprotect.c:470:6: note: in expansion of macro =E2=80=98ARCH_VM_PKEY_F=
LAGS=E2=80=99
ARCH_VM_PKEY_FLAGS;
^~~~~~~~~~~~~~~~~~
../arch/powerpc/include/asm/pkeys.h:27:44: error: =E2=80=98VM_PKEY_BIT1=E2=
=80=99 undeclared (first use in this function)
#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
^
../mm/mprotect.c:470:6: note: in expansion of macro =E2=80=98ARCH_VM_PKEY_F=
LAGS=E2=80=99
ARCH_VM_PKEY_FLAGS;
^~~~~~~~~~~~~~~~~~
../arch/powerpc/include/asm/pkeys.h:27:59: error: =E2=80=98VM_PKEY_BIT2=E2=
=80=99 undeclared (first use in this function)
#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
^
../mm/mprotect.c:470:6: note: in expansion of macro =E2=80=98ARCH_VM_PKEY_F=
LAGS=E2=80=99
ARCH_VM_PKEY_FLAGS;
^~~~~~~~~~~~~~~~~~
../arch/powerpc/include/asm/pkeys.h:28:5: error: =E2=80=98VM_PKEY_BIT3=E2=
=80=99 undeclared (first use in this function)
VM_PKEY_BIT3 | VM_PKEY_BIT4)
^
../mm/mprotect.c:470:6: note: in expansion of macro =E2=80=98ARCH_VM_PKEY_F=
LAGS=E2=80=99
ARCH_VM_PKEY_FLAGS;
^~~~~~~~~~~~~~~~~~
../arch/powerpc/include/asm/pkeys.h:28:20: error: =E2=80=98VM_PKEY_BIT4=E2=
=80=99 undeclared (first use in this function)
VM_PKEY_BIT3 | VM_PKEY_BIT4)
^
../mm/mprotect.c:470:6: note: in expansion of macro =E2=80=98ARCH_VM_PKEY_F=
LAGS=E2=80=99
ARCH_VM_PKEY_FLAGS;
^~~~~~~~~~~~~~~~~~
../scripts/Makefile.build:311: recipe for target 'mm/mprotect.o' failed
cheers
On Sat, Oct 07, 2017 at 09:02:55PM +1100, Michael Ellerman wrote:
Ram Pai [off-list ref] writes:
quoted
Total 32 keys are available on power7 and above. However
pkey 0,1 are reserved. So effectively we have 30 pkeys.
On 4K kernels, we do not have 5 bits in the PTE to
represent all the keys; we only have 3bits.Two of those
keys are reserved; pkey 0 and pkey 1. So effectively we
have 6 pkeys.
This patch keeps track of reserved keys, allocated keys
and keys that are currently free.
Also it adds skeletal functions and macros, that the
architecture-independent code expects to be available.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 9 ++++
arch/powerpc/include/asm/mmu_context.h | 1 +
arch/powerpc/include/asm/pkeys.h | 72 ++++++++++++++++++++++++++++--
arch/powerpc/mm/mmu_context_book3s64.c | 2 +
arch/powerpc/mm/pkeys.c | 28 ++++++++++++
5 files changed, 108 insertions(+), 4 deletions(-)
This doesn't build for me, with pseries_le_defconfig. I assume it built
for you. So something has changed upstream maybe?
Yes. :(
The following commit upstream broke my patches.
df3735c5b40fad8d0d28eb8ab065fe955b3347ee
Will fix and send you a patch.
RP
In file included from ../include/linux/pkeys.h:8:0,
from ../mm/mprotect.c:26:
../mm/mprotect.c: In function ‘do_mprotect_pkey’:
../arch/powerpc/include/asm/pkeys.h:27:29: error: ‘VM_PKEY_BIT0’ undeclared (first use in this function)
#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
^
On Fri, 8 Sep 2017 15:44:51 -0700
Ram Pai [off-list ref] wrote:
Total 32 keys are available on power7 and above. However
pkey 0,1 are reserved. So effectively we have 30 pkeys.
On 4K kernels, we do not have 5 bits in the PTE to
represent all the keys; we only have 3bits.Two of those
keys are reserved; pkey 0 and pkey 1. So effectively we
have 6 pkeys.
This patch keeps track of reserved keys, allocated keys
and keys that are currently free.
Also it adds skeletal functions and macros, that the
architecture-independent code expects to be available.
Signed-off-by: Ram Pai <redacted>
I ended up reviewing v7 of the patch. Is this v8?
I think the comments still apply to this revision
Balbir Singh.
On Fri, 8 Sep 2017 15:44:52 -0700
Ram Pai [off-list ref] wrote:
quoted hunk
Implements helper functions to read and write the key related
registers; AMR, IAMR, UAMOR.
AMR register tracks the read,write permission of a key
IAMR register tracks the execute permission of a key
UAMOR register enables and disables a key
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 31 ++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)
On Fri, 8 Sep 2017 15:44:53 -0700
Ram Pai [off-list ref] wrote:
quoted hunk
Introduce helper functions that can initialize the bits in the AMR,
IAMR and UAMOR register; the bits that correspond to the given pkey.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/pkeys.h | 1 +
arch/powerpc/mm/pkeys.c | 46 ++++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 0 deletions(-)
On Fri, 8 Sep 2017 15:44:54 -0700
Ram Pai [off-list ref] wrote:
quoted hunk
cleanup the bits corresponding to a key in the AMR, and IAMR
register, when the key is newly allocated/activated or is freed.
We dont want some residual bits cause the hardware enforce
unintended behavior when the key is activated or freed.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
On Wed, Oct 18, 2017 at 02:17:35PM +1100, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:44:52 -0700
Ram Pai [off-list ref] wrote:
quoted
Implements helper functions to read and write the key related
registers; AMR, IAMR, UAMOR.
AMR register tracks the read,write permission of a key
IAMR register tracks the execute permission of a key
UAMOR register enables and disables a key
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 31 ++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)
On Fri, 8 Sep 2017 15:44:57 -0700
Ram Pai [off-list ref] wrote:
quoted hunk
powerpc has hardware support to disable execute on a pkey.
This patch enables the ability to create execute-disabled
keys.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/uapi/asm/mman.h | 6 ++++++
arch/powerpc/mm/pkeys.c | 16 ++++++++++++++++
2 files changed, 22 insertions(+), 0 deletions(-)
On Fri, 8 Sep 2017 15:44:58 -0700
Ram Pai [off-list ref] wrote:
quoted hunk
Store and restore the AMR, IAMR and UAMOR register state of the task
before scheduling out and after scheduling in, respectively.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/pkeys.h | 4 +++
arch/powerpc/include/asm/processor.h | 5 ++++
arch/powerpc/kernel/process.c | 10 ++++++++
arch/powerpc/mm/pkeys.c | 39 ++++++++++++++++++++++++++++++++++
4 files changed, 58 insertions(+), 0 deletions(-)
Just define two variants of thread_pkey_regs_save() based on
CONFIG_PPC64_MEMORY_PROTECTION_KEYS and remove the #ifdefs from process.c
Ditto for the lines below
@@ -149,3 +149,42 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,init_amr(pkey,new_amr_bits);return0;}++voidthread_pkey_regs_save(structthread_struct*thread)+{+if(!pkey_inited)+return;++/* @TODO skip saving any registers if the thread+*hasnotusedanykeysyet.+*/
Comment style is broken
+
+ thread->amr = read_amr();
+ thread->iamr = read_iamr();
+ thread->uamor = read_uamor();
+}
+
+void thread_pkey_regs_restore(struct thread_struct *new_thread,
+ struct thread_struct *old_thread)
+{
+ if (!pkey_inited)
+ return;
+
+ /* @TODO just reset uamor to zero if the new_thread
+ * has not used any keys yet.
+ */
Comment style is broken.
+
+ if (old_thread->amr != new_thread->amr)
+ write_amr(new_thread->amr);
+ if (old_thread->iamr != new_thread->iamr)
+ write_iamr(new_thread->iamr);
+ if (old_thread->uamor != new_thread->uamor)
+ write_uamor(new_thread->uamor);
Is this order correct? Ideally, You want to write the uamor first
but since we are in supervisor state, I think we can get away
with this order. Do we want to expose the uamor to user space
for it to modify the AMR directly?
On Fri, 8 Sep 2017 15:44:59 -0700
Ram Pai [off-list ref] wrote:
quoted hunk
This patch provides the implementation of execute-only pkey.
The architecture-independent layer expects the arch-dependent
layer, to support the ability to create and enable a special
key which has execute-only permission.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 1 +
arch/powerpc/include/asm/pkeys.h | 9 ++++-
arch/powerpc/mm/pkeys.c | 57 ++++++++++++++++++++++++++++++
3 files changed, 66 insertions(+), 1 deletions(-)
If uamor for key 0 is 0x10 for example or 0x01 it's a bug.
The above check might miss it.
+
+ return !(read_amr() & ((AMR_RD_BIT|AMR_WR_BIT) << pkey_shift));
+}
+
+int __execute_only_pkey(struct mm_struct *mm)
+{
+ bool need_to_set_mm_pkey = false;
+ int execute_only_pkey = mm->context.execute_only_pkey;
+ int ret;
+
+ /* Do we need to assign a pkey for mm's execute-only maps? */
+ if (execute_only_pkey == -1) {
+ /* Go allocate one to use, which might fail */
+ execute_only_pkey = mm_pkey_alloc(mm);
+ if (execute_only_pkey < 0)
+ return -1;
+ need_to_set_mm_pkey = true;
+ }
+
+ /*
+ * We do not want to go through the relatively costly
+ * dance to set AMR if we do not need to. Check it
+ * first and assume that if the execute-only pkey is
+ * readwrite-disabled than we do not have to set it
+ * ourselves.
+ */
+ if (!need_to_set_mm_pkey &&
+ !pkey_allows_readwrite(execute_only_pkey))
+ return execute_only_pkey;
+
+ /*
+ * Set up AMR so that it denies access for everything
+ * other than execution.
+ */
+ ret = __arch_set_user_pkey_access(current, execute_only_pkey,
+ (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE));
+ /*
+ * If the AMR-set operation failed somehow, just return
+ * 0 and effectively disable execute-only support.
+ */
+ if (ret) {
+ mm_set_pkey_free(mm, execute_only_pkey);
+ return -1;
+ }
+
+ /* We got one, store it and use it from here on out */
+ if (need_to_set_mm_pkey)
+ mm->context.execute_only_pkey = execute_only_pkey;
+ return execute_only_pkey;
+}
Looks good otherwise
Acked-by: Balbir Singh <bsingharora@gmail.com>
On Fri, 8 Sep 2017 15:45:00 -0700
Ram Pai [off-list ref] wrote:
quoted hunk
arch-independent code expects the arch to map
a pkey into the vma's protection bit setting.
The patch provides that ability.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/mman.h | 8 +++++++-
arch/powerpc/include/asm/pkeys.h | 18 ++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletions(-)
Assuming that there is a linear order between VM_PKEY_BIT4 to
VM_PKEY_BIT0, the conditional checks can be removed
(pkey & 0x1fUL) << VM_PKEY_BIT0?
Balbir Singh
We don't want pkey_inited to be present in all functions, why do we need
a conditional branch for all functions. Even if we do, it should be a jump
label. I would rather we just removed !pkey_inited unless really really
required.
@@ -142,11 +152,14 @@ static inline int execute_only_pkey(struct mm_struct *mm) return __execute_only_pkey(mm); }-+extern int __arch_override_mprotect_pkey(struct vm_area_struct *vma,+ int prot, int pkey); static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot, int pkey) {- return 0;+ if (!pkey_inited)+ return 0;+ return __arch_override_mprotect_pkey(vma, prot, pkey); } extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
@@ -245,3 +245,50 @@ int __execute_only_pkey(struct mm_struct *mm)mm->context.execute_only_pkey=execute_only_pkey;returnexecute_only_pkey;}++staticinlineboolvma_is_pkey_exec_only(structvm_area_struct*vma)+{+/* Do this check first since the vm_flags should be hot */+if((vma->vm_flags&(VM_READ|VM_WRITE|VM_EXEC))!=VM_EXEC)+returnfalse;++return(vma_pkey(vma)==vma->vm_mm->context.execute_only_pkey);+}++/*+*Thisshouldonlybecalledfor*plain*mprotectcalls.
What's a plain mprotect call?
+ */
+int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,
+ int pkey)
+{
+ /*
+ * Is this an mprotect_pkey() call? If so, never
+ * override the value that came from the user.
+ */
+ if (pkey != -1)
+ return pkey;
If the user specified a key, we always use it? Presumably the user
got it from pkey_alloc(), in other cases, the user was lazy and used
-1 in the mprotect call?
+
+ /*
+ * If the currently associated pkey is execute-only,
+ * but the requested protection requires read or write,
+ * move it back to the default pkey.
+ */
+ if (vma_is_pkey_exec_only(vma) &&
+ (prot & (PROT_READ|PROT_WRITE)))
+ return 0;
+
+ /*
+ * the requested protection is execute-only. Hence
+ * lets use a execute-only pkey.
+ */
+ if (prot == PROT_EXEC) {
+ pkey = execute_only_pkey(vma->vm_mm);
+ if (pkey > 0)
+ return pkey;
+ }
+
+ /*
+ * nothing to override.
+ */
+ return vma_pkey(vma);
+}
On Fri, 8 Sep 2017 15:45:02 -0700
Ram Pai [off-list ref] wrote:
quoted hunk
map the key protection bits of the vma to the pkey bits in
the PTE.
The Pte bits used for pkey are 3,4,5,6 and 57. The first
four bits are the same four bits that were freed up initially
in this patch series. remember? :-) Without those four bits
this patch would'nt be possible.
BUT, On 4k kernel, bit 3, and 4 could not be freed up. remember?
Hence we have to be satisfied with 5,6 and 7.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 25 ++++++++++++++++++++++++-
arch/powerpc/include/asm/mman.h | 8 ++++++++
arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++
3 files changed, 44 insertions(+), 1 deletions(-)
@@ -232,6 +232,7 @@ unsigned long htab_convert_pte_flags(unsigned long pteflags)*/rflags|=HPTE_R_M;+rflags|=pte_to_hpte_pkey_bits(pteflags);returnrflags;}
On Fri, 8 Sep 2017 15:45:05 -0700
Ram Pai [off-list ref] wrote:
quoted hunk
helper function that checks if the read/write/execute is allowed
on the pte.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 4 +++
arch/powerpc/include/asm/pkeys.h | 12 +++++++++++
arch/powerpc/mm/pkeys.c | 28 ++++++++++++++++++++++++++
3 files changed, 44 insertions(+), 0 deletions(-)
@@ -292,3 +292,31 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,*/returnvma_pkey(vma);}++staticboolpkey_access_permitted(intpkey,boolwrite,boolexecute)+{+intpkey_shift;+u64amr;++if(!pkey)+returntrue;
Why would we have pkey set to 0, it's reserved. Why do we return true?
On Wed, Oct 18, 2017 at 02:42:56PM +1100, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:44:57 -0700
Ram Pai [off-list ref] wrote:
quoted
powerpc has hardware support to disable execute on a pkey.
This patch enables the ability to create execute-disabled
keys.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/uapi/asm/mman.h | 6 ++++++
arch/powerpc/mm/pkeys.c | 16 ++++++++++++++++
2 files changed, 22 insertions(+), 0 deletions(-)
Will these values every change? It's good to have I guess.
quoted
+
/* disable the pkey system till everything
* is in place. A patch further down the
* line will enable it.
@@ -120,10 +128,18 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey, unsigned long init_val) { u64 new_amr_bits = 0x0ul;+ u64 new_iamr_bits = 0x0ul; if (!is_pkey_enabled(pkey)) return -EINVAL;+ if ((init_val & PKEY_DISABLE_EXECUTE)) {+ if (!pkey_execute_disable_support)+ return -EINVAL;+ new_iamr_bits |= IAMR_EX_BIT;+ }+ init_iamr(pkey, new_iamr_bits);+
Where do we check the reserved keys?
The main gate keeper against spurious keys are the system calls.
sys_pkey_mprotect(), sys_pkey_free() and sys_pkey_modify() are the one
that will check against reserved and unallocated keys. Once it has
passed the check, all other internal functions trust the key values
provided to them. I can put in additional checks but that will
unnecessarily chew a few cpu cycles.
Agree?
BTW: you raise a good point though, I may have missed guarding against
unallocated or reserved keys in sys_pkey_modify(). That was a power
specific system call that I have introduced to change the permissions on
a key.
RP
On Wed, Oct 18, 2017 at 02:24:03PM +1100, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:44:53 -0700
Ram Pai [off-list ref] wrote:
quoted
Introduce helper functions that can initialize the bits in the AMR,
IAMR and UAMOR register; the bits that correspond to the given pkey.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/pkeys.h | 1 +
arch/powerpc/mm/pkeys.c | 46 ++++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 0 deletions(-)
Do we need to check for reserved keys or that is at a layer above?
These routines blindly trust the caller. The assumption is the
system calls which are the gate-keepers for the keys validate
the keys before calling any lower level functions.
On Wed, Oct 18, 2017 at 02:49:14PM +1100, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:44:58 -0700
Ram Pai [off-list ref] wrote:
quoted
Store and restore the AMR, IAMR and UAMOR register state of the task
before scheduling out and after scheduling in, respectively.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/pkeys.h | 4 +++
arch/powerpc/include/asm/processor.h | 5 ++++
arch/powerpc/kernel/process.c | 10 ++++++++
arch/powerpc/mm/pkeys.c | 39 ++++++++++++++++++++++++++++++++++
4 files changed, 58 insertions(+), 0 deletions(-)
Just define two variants of thread_pkey_regs_save() based on
CONFIG_PPC64_MEMORY_PROTECTION_KEYS and remove the #ifdefs from process.c
Ditto for the lines below
@@ -149,3 +149,42 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,init_amr(pkey,new_amr_bits);return0;}++voidthread_pkey_regs_save(structthread_struct*thread)+{+if(!pkey_inited)+return;++/* @TODO skip saving any registers if the thread+*hasnotusedanykeysyet.+*/
Comment style is broken
ok. this time i will fix them. It misses by radar screen because
checkpatch.pl does not complain.
quoted
+
+ thread->amr = read_amr();
+ thread->iamr = read_iamr();
+ thread->uamor = read_uamor();
+}
+
+void thread_pkey_regs_restore(struct thread_struct *new_thread,
+ struct thread_struct *old_thread)
+{
+ if (!pkey_inited)
+ return;
+
+ /* @TODO just reset uamor to zero if the new_thread
+ * has not used any keys yet.
+ */
Comment style is broken.
quoted
+
+ if (old_thread->amr != new_thread->amr)
+ write_amr(new_thread->amr);
+ if (old_thread->iamr != new_thread->iamr)
+ write_iamr(new_thread->iamr);
+ if (old_thread->uamor != new_thread->uamor)
+ write_uamor(new_thread->uamor);
Is this order correct? Ideally, You want to write the uamor first
but since we are in supervisor state, I think we can get away
with this order.
we could be in hypervisor state too, as is the case when we run
a powernv kernel.
But..does it matter in which order they are written? if
the thread is in the kernel, it cannot execute any instructions
in userspace. So it wont see a intermediate state. right?
or am i getting this wrong?
Do we want to expose the uamor to user space
for it to modify the AMR directly?
sorry I did not understand the comment. UAMOR cannot
be accessed from usespace. and there are no system calls
currently to help userspace to program the UAMOR on its
behalf.
On Wed, Oct 18, 2017 at 03:15:22PM +1100, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:44:59 -0700
Ram Pai [off-list ref] wrote:
quoted
This patch provides the implementation of execute-only pkey.
The architecture-independent layer expects the arch-dependent
layer, to support the ability to create and enable a special
key which has execute-only permission.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 1 +
arch/powerpc/include/asm/pkeys.h | 9 ++++-
arch/powerpc/mm/pkeys.c | 57 ++++++++++++++++++++++++++++++
3 files changed, 66 insertions(+), 1 deletions(-)
If uamor for key 0 is 0x10 for example or 0x01 it's a bug.
The above check might miss it.
The specs says both the bits corresponding to a key are set or
reset, cannot be anything else.
cut-n-paste from the ISA...
----------------------------------------------------
Software must ensure that both bits of each even/odd
bit pair of the AMOR contain the same value. -- i.e.,
the contents of register RS for mtspr specifying the
AMOR must be such that (RS)2n = (RS)2n+1 for every
n in the range 0:31 - and like for the UAMOR.
---------------------------------------------------------
quoted
+
+ return !(read_amr() & ((AMR_RD_BIT|AMR_WR_BIT) << pkey_shift));
+}
+
+int __execute_only_pkey(struct mm_struct *mm)
+{
+ bool need_to_set_mm_pkey = false;
+ int execute_only_pkey = mm->context.execute_only_pkey;
+ int ret;
+
+ /* Do we need to assign a pkey for mm's execute-only maps? */
+ if (execute_only_pkey == -1) {
+ /* Go allocate one to use, which might fail */
+ execute_only_pkey = mm_pkey_alloc(mm);
+ if (execute_only_pkey < 0)
+ return -1;
+ need_to_set_mm_pkey = true;
+ }
+
+ /*
+ * We do not want to go through the relatively costly
+ * dance to set AMR if we do not need to. Check it
+ * first and assume that if the execute-only pkey is
+ * readwrite-disabled than we do not have to set it
+ * ourselves.
+ */
+ if (!need_to_set_mm_pkey &&
+ !pkey_allows_readwrite(execute_only_pkey))
+ return execute_only_pkey;
+
+ /*
+ * Set up AMR so that it denies access for everything
+ * other than execution.
+ */
+ ret = __arch_set_user_pkey_access(current, execute_only_pkey,
+ (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE));
+ /*
+ * If the AMR-set operation failed somehow, just return
+ * 0 and effectively disable execute-only support.
+ */
+ if (ret) {
+ mm_set_pkey_free(mm, execute_only_pkey);
+ return -1;
+ }
+
+ /* We got one, store it and use it from here on out */
+ if (need_to_set_mm_pkey)
+ mm->context.execute_only_pkey = execute_only_pkey;
+ return execute_only_pkey;
+}
Looks good otherwise
Acked-by: Balbir Singh <bsingharora@gmail.com>
On Wed, Oct 18, 2017 at 03:27:33PM +1100, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:45:00 -0700
Ram Pai [off-list ref] wrote:
quoted
arch-independent code expects the arch to map
a pkey into the vma's protection bit setting.
The patch provides that ability.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/mman.h | 8 +++++++-
arch/powerpc/include/asm/pkeys.h | 18 ++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletions(-)
Assuming that there is a linear order between VM_PKEY_BIT4 to
VM_PKEY_BIT0, the conditional checks can be removed
(pkey & 0x1fUL) << VM_PKEY_BIT0?
yes. currently the are linear. But I am afraid it will break without
notice someday when someone decides to change the values of VM_PKEY_BITx to
be non-contiguous. I can put a BUILD_ASSERTION I suppose. But thought
this will be safe.
RP
We don't want pkey_inited to be present in all functions, why do we need
a conditional branch for all functions. Even if we do, it should be a jump
label. I would rather we just removed !pkey_inited unless really really
required.
No. we really really need it. For example when we build a kernel with
PROTECTION_KEYS config enabled and run that kernel on a older processor
or on a system where the key feature is not enabled in the device tree,
we have fail all the calls that get called-in by the arch-neutral code.
Hence we need this check.
BTW: jump labels are awkward IMHO, unless absolutely needed.
@@ -142,11 +152,14 @@ static inline int execute_only_pkey(struct mm_struct *mm) return __execute_only_pkey(mm); }-+extern int __arch_override_mprotect_pkey(struct vm_area_struct *vma,+ int prot, int pkey); static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot, int pkey) {- return 0;+ if (!pkey_inited)+ return 0;+ return __arch_override_mprotect_pkey(vma, prot, pkey); } extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
@@ -245,3 +245,50 @@ int __execute_only_pkey(struct mm_struct *mm)mm->context.execute_only_pkey=execute_only_pkey;returnexecute_only_pkey;}++staticinlineboolvma_is_pkey_exec_only(structvm_area_struct*vma)+{+/* Do this check first since the vm_flags should be hot */+if((vma->vm_flags&(VM_READ|VM_WRITE|VM_EXEC))!=VM_EXEC)+returnfalse;++return(vma_pkey(vma)==vma->vm_mm->context.execute_only_pkey);+}++/*+*Thisshouldonlybecalledfor*plain*mprotectcalls.
What's a plain mprotect call?
there is sys_mprotect() and now there is a sys_pkey_mprotect() call.
The 'plain' one is the former.
quoted
+ */
+int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,
+ int pkey)
+{
+ /*
+ * Is this an mprotect_pkey() call? If so, never
+ * override the value that came from the user.
+ */
+ if (pkey != -1)
+ return pkey;
If the user specified a key, we always use it? Presumably the user
got it from pkey_alloc(), in other cases, the user was lazy and used
-1 in the mprotect call?
in the plain sys_mprotect() key is not specified. In that case this
function gets called with a -1.
quoted
+
+ /*
+ * If the currently associated pkey is execute-only,
+ * but the requested protection requires read or write,
+ * move it back to the default pkey.
+ */
+ if (vma_is_pkey_exec_only(vma) &&
+ (prot & (PROT_READ|PROT_WRITE)))
+ return 0;
+
+ /*
+ * the requested protection is execute-only. Hence
+ * lets use a execute-only pkey.
+ */
+ if (prot == PROT_EXEC) {
+ pkey = execute_only_pkey(vma->vm_mm);
+ if (pkey > 0)
+ return pkey;
+ }
+
+ /*
+ * nothing to override.
+ */
+ return vma_pkey(vma);
+}
On Wed, Oct 18, 2017 at 03:39:11PM +1100, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:45:02 -0700
Ram Pai [off-list ref] wrote:
quoted
map the key protection bits of the vma to the pkey bits in
the PTE.
The Pte bits used for pkey are 3,4,5,6 and 57. The first
four bits are the same four bits that were freed up initially
in this patch series. remember? :-) Without those four bits
this patch would'nt be possible.
BUT, On 4k kernel, bit 3, and 4 could not be freed up. remember?
Hence we have to be satisfied with 5,6 and 7.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 25 ++++++++++++++++++++++++-
arch/powerpc/include/asm/mman.h | 8 ++++++++
arch/powerpc/include/asm/pkeys.h | 12 ++++++++++++
3 files changed, 44 insertions(+), 1 deletions(-)
On Wed, Oct 18, 2017 at 03:48:31PM +1100, Balbir Singh wrote:
On Fri, 8 Sep 2017 15:45:05 -0700
Ram Pai [off-list ref] wrote:
quoted
helper function that checks if the read/write/execute is allowed
on the pte.
Signed-off-by: Ram Pai <redacted>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 4 +++
arch/powerpc/include/asm/pkeys.h | 12 +++++++++++
arch/powerpc/mm/pkeys.c | 28 ++++++++++++++++++++++++++
3 files changed, 44 insertions(+), 0 deletions(-)
@@ -292,3 +292,31 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot,*/returnvma_pkey(vma);}++staticboolpkey_access_permitted(intpkey,boolwrite,boolexecute)+{+intpkey_shift;+u64amr;++if(!pkey)+returntrue;
Why would we have pkey set to 0, it's reserved. Why do we return true?
pkey 0 is reserved in some weird sense. it is the default key which is
omnipresent, which cannot be allocated or freed, but can be used any time and
allows read/write/execute at all times.