| Message ID | 1455948760-24710-8-git-send-email-paulus@samba.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Paul Mackerras <paulus@samba.org> writes: > This moves the _PAGE_EXEC, _PAGE_RW and _PAGE_USER bits around in > the Linux PTE on 64-bit Book 3S systems to correspond with the bit > positions used in radix mode by PowerISA v3.0 CPUs. This also adds > a _PAGE_READ bit corresponding to the read permission bit in the > radix PTE. _PAGE_READ is currently unused but could possibly be used > in future to improve pte_protnone(). > > Signed-off-by: Paul Mackerras <paulus@samba.org> > --- > arch/powerpc/include/asm/book3s/64/hash.h | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h > index c8eba0e..0fc750c 100644 > --- a/arch/powerpc/include/asm/book3s/64/hash.h > +++ b/arch/powerpc/include/asm/book3s/64/hash.h > @@ -13,9 +13,12 @@ > * We could create separate kernel read-only if we used the 3 PP bits > * combinations that newer processors provide but we currently don't. > */ > -#define _PAGE_BIT_SWAP_TYPE 2 > -#define _PAGE_USER 0x00004 /* page may be accessed by userspace */ > -#define _PAGE_EXEC 0x00008 /* execute permission */ > +#define _PAGE_BIT_SWAP_TYPE 0 > + > +#define _PAGE_EXEC 0x00001 /* execute permission */ > +#define _PAGE_RW 0x00002 /* read & write access allowed */ > +#define _PAGE_READ 0x00004 /* read access allowed */ > +#define _PAGE_USER 0x00008 /* page may be accessed by userspace */ > #define _PAGE_GUARDED 0x00010 /* G: guarded (side-effect) page */ > /* M (memory coherence) is always set in the HPTE, so we don't need it here */ > #define _PAGE_COHERENT 0x0 > @@ -23,7 +26,6 @@ > #define _PAGE_WRITETHRU 0x00040 /* W: cache write-through */ > #define _PAGE_DIRTY 0x00080 /* C: page changed */ > #define _PAGE_ACCESSED 0x00100 /* R: page referenced */ > -#define _PAGE_RW 0x00200 /* software: user write access allowed */ > #define _PAGE_BUSY 0x00800 /* software: PTE & hash are busy */ > #define _PAGE_SPECIAL 0x10000 /* software: special page */ > For radix, _PAGE_USER is the inverse of this right ?. Ie, we set that bit position to 1 to indicate privileged access only. Also can you use constants like #define _PAGE_USER PPC_BIT(60) -aneesh
On Sun, Feb 21, 2016 at 01:00:54PM +0530, Aneesh Kumar K.V wrote: > Paul Mackerras <paulus@samba.org> writes: > > > This moves the _PAGE_EXEC, _PAGE_RW and _PAGE_USER bits around in > > the Linux PTE on 64-bit Book 3S systems to correspond with the bit > > positions used in radix mode by PowerISA v3.0 CPUs. This also adds > > a _PAGE_READ bit corresponding to the read permission bit in the > > radix PTE. _PAGE_READ is currently unused but could possibly be used > > in future to improve pte_protnone(). > > > > Signed-off-by: Paul Mackerras <paulus@samba.org> > > --- > > arch/powerpc/include/asm/book3s/64/hash.h | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h > > index c8eba0e..0fc750c 100644 > > --- a/arch/powerpc/include/asm/book3s/64/hash.h > > +++ b/arch/powerpc/include/asm/book3s/64/hash.h > > @@ -13,9 +13,12 @@ > > * We could create separate kernel read-only if we used the 3 PP bits > > * combinations that newer processors provide but we currently don't. > > */ > > -#define _PAGE_BIT_SWAP_TYPE 2 > > -#define _PAGE_USER 0x00004 /* page may be accessed by userspace */ > > -#define _PAGE_EXEC 0x00008 /* execute permission */ > > +#define _PAGE_BIT_SWAP_TYPE 0 > > + > > +#define _PAGE_EXEC 0x00001 /* execute permission */ > > +#define _PAGE_RW 0x00002 /* read & write access allowed */ > > +#define _PAGE_READ 0x00004 /* read access allowed */ > > +#define _PAGE_USER 0x00008 /* page may be accessed by userspace */ > > #define _PAGE_GUARDED 0x00010 /* G: guarded (side-effect) page */ > > /* M (memory coherence) is always set in the HPTE, so we don't need it here */ > > #define _PAGE_COHERENT 0x0 > > @@ -23,7 +26,6 @@ > > #define _PAGE_WRITETHRU 0x00040 /* W: cache write-through */ > > #define _PAGE_DIRTY 0x00080 /* C: page changed */ > > #define _PAGE_ACCESSED 0x00100 /* R: page referenced */ > > -#define _PAGE_RW 0x00200 /* software: user write access allowed */ > > #define _PAGE_BUSY 0x00800 /* software: PTE & hash are busy */ > > #define _PAGE_SPECIAL 0x10000 /* software: special page */ > > > > > For radix, _PAGE_USER is the inverse of this right ?. Ie, we set that > bit position to 1 to indicate privileged access only. Right, we'll need a follow-on patch that changes this to a _PAGE_PRIV bit and changes the logic that uses _PAGE_USER either that or a _PAGE_PRIV bit. But at least we have the bit position reserved now. > Also can you use constants like > #define _PAGE_USER PPC_BIT(60) I'd really rather not - that is harder for the casual reader to parse, because they then have to go off and find out what exactly PPC_BIT does. The only time that using PPC_BIT would help is when checking that the bit definitions match the Power ISA, and that's presumably done by intelligent people that can handle backwards bit numbering in their heads. :) Paul.
On Mon, 2016-02-22 at 09:36 +1100, Paul Mackerras wrote: > On Sun, Feb 21, 2016 at 01:00:54PM +0530, Aneesh Kumar K.V wrote: > > Paul Mackerras <paulus@samba.org> writes: > > > > Also can you use constants like > > #define _PAGE_USER PPC_BIT(60) > > I'd really rather not - that is harder for the casual reader to parse, > because they then have to go off and find out what exactly PPC_BIT > does. The only time that using PPC_BIT would help is when checking > that the bit definitions match the Power ISA, and that's presumably > done by intelligent people that can handle backwards bit numbering in > their heads. :) Yep agree 100%. Using PPC_BIT() means every time someone sees that defintion they need to think about what the conversion is and whether it's right, ie. for the entire future history of this code. On the other hand not using PPC_BIT() means the person who writes the definition needs to think about it and do the correct conversion, but only once. cheers
diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h index c8eba0e..0fc750c 100644 --- a/arch/powerpc/include/asm/book3s/64/hash.h +++ b/arch/powerpc/include/asm/book3s/64/hash.h @@ -13,9 +13,12 @@ * We could create separate kernel read-only if we used the 3 PP bits * combinations that newer processors provide but we currently don't. */ -#define _PAGE_BIT_SWAP_TYPE 2 -#define _PAGE_USER 0x00004 /* page may be accessed by userspace */ -#define _PAGE_EXEC 0x00008 /* execute permission */ +#define _PAGE_BIT_SWAP_TYPE 0 + +#define _PAGE_EXEC 0x00001 /* execute permission */ +#define _PAGE_RW 0x00002 /* read & write access allowed */ +#define _PAGE_READ 0x00004 /* read access allowed */ +#define _PAGE_USER 0x00008 /* page may be accessed by userspace */ #define _PAGE_GUARDED 0x00010 /* G: guarded (side-effect) page */ /* M (memory coherence) is always set in the HPTE, so we don't need it here */ #define _PAGE_COHERENT 0x0 @@ -23,7 +26,6 @@ #define _PAGE_WRITETHRU 0x00040 /* W: cache write-through */ #define _PAGE_DIRTY 0x00080 /* C: page changed */ #define _PAGE_ACCESSED 0x00100 /* R: page referenced */ -#define _PAGE_RW 0x00200 /* software: user write access allowed */ #define _PAGE_BUSY 0x00800 /* software: PTE & hash are busy */ #define _PAGE_SPECIAL 0x10000 /* software: special page */
This moves the _PAGE_EXEC, _PAGE_RW and _PAGE_USER bits around in the Linux PTE on 64-bit Book 3S systems to correspond with the bit positions used in radix mode by PowerISA v3.0 CPUs. This also adds a _PAGE_READ bit corresponding to the read permission bit in the radix PTE. _PAGE_READ is currently unused but could possibly be used in future to improve pte_protnone(). Signed-off-by: Paul Mackerras <paulus@samba.org> --- arch/powerpc/include/asm/book3s/64/hash.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)