diff mbox

[RFC,v2,08/11] ARM64: mm: Swap PTE_FILE and PTE_PROT_NONE bits.

Message ID 1368006763-30774-9-git-send-email-steve.capper@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Steve Capper May 8, 2013, 9:52 a.m. UTC
Under ARM64, PTEs can be broadly categorised as follows:
   - Present and valid: Bit #0 is set. The PTE is valid and memory
     access to the region may fault.

   - Present and invalid: Bit #0 is clear and bit #1 is set.
     Represents present memory with PROT_NONE protection. The PTE
     is an invalid entry, and the user fault handler will raise a
     SIGSEGV.

   - Not present (file): Bits #0 and #1 are clear, bit #2 is set.
     Memory represented has been paged out. The PTE is an invalid
     entry, and the fault handler will try and re-populate the
     memory where necessary.

Huge PTEs are block descriptors that have bit #1 clear. If we wish
to represent PROT_NONE huge PTEs we then run into a problem as
there is no way to distinguish between regular and huge PTEs if we
set bit #1.

As huge PTEs are always present, the meaning of bits #1 and #2 can
be swapped for invalid PTEs. This patch swaps the PTE_FILE and
PTE_PROT_NONE constants, allowing us to represent PROT_NONE huge
PTEs.

Signed-off-by: Steve Capper <steve.capper@linaro.org>
---
 arch/arm64/include/asm/pgtable.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Christopher Covington May 8, 2013, 4:17 p.m. UTC | #1
Hi Steve,

On 05/08/2013 05:52 AM, Steve Capper wrote:
> Under ARM64, PTEs can be broadly categorised as follows:
>    - Present and valid: Bit #0 is set. The PTE is valid and memory
>      access to the region may fault.
> 
>    - Present and invalid: Bit #0 is clear and bit #1 is set.
>      Represents present memory with PROT_NONE protection. The PTE
>      is an invalid entry, and the user fault handler will raise a
>      SIGSEGV.
> 
>    - Not present (file): Bits #0 and #1 are clear, bit #2 is set.
>      Memory represented has been paged out. The PTE is an invalid
>      entry, and the fault handler will try and re-populate the
>      memory where necessary.
> 
> Huge PTEs are block descriptors that have bit #1 clear. If we wish
> to represent PROT_NONE huge PTEs we then run into a problem as
> there is no way to distinguish between regular and huge PTEs if we
> set bit #1.
> 
> As huge PTEs are always present, the meaning of bits #1 and #2 can
> be swapped for invalid PTEs. This patch swaps the PTE_FILE and
> PTE_PROT_NONE constants, allowing us to represent PROT_NONE huge
> PTEs.

[...]

> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h

[...]

> @@ -306,8 +306,8 @@ extern pgd_t idmap_pg_dir[PTRS_PER_PGD];
>  
>  /*
>   * Encode and decode a file entry:
> - *	bits 0-1:	present (must be zero)
> - *	bit  2:		PTE_FILE
> + *	bits 0 & 2:	present (must be zero)

Consider using punctuation like "bits 0, 2" here to disambiguate from the
binary and operation.

[...]

Christopher
Will Deacon May 8, 2013, 4:40 p.m. UTC | #2
On Wed, May 08, 2013 at 10:52:40AM +0100, Steve Capper wrote:
> Under ARM64, PTEs can be broadly categorised as follows:
>    - Present and valid: Bit #0 is set. The PTE is valid and memory
>      access to the region may fault.
> 
>    - Present and invalid: Bit #0 is clear and bit #1 is set.
>      Represents present memory with PROT_NONE protection. The PTE
>      is an invalid entry, and the user fault handler will raise a
>      SIGSEGV.
> 
>    - Not present (file): Bits #0 and #1 are clear, bit #2 is set.
>      Memory represented has been paged out. The PTE is an invalid
>      entry, and the fault handler will try and re-populate the
>      memory where necessary.
> 
> Huge PTEs are block descriptors that have bit #1 clear. If we wish
> to represent PROT_NONE huge PTEs we then run into a problem as
> there is no way to distinguish between regular and huge PTEs if we
> set bit #1.
> 
> As huge PTEs are always present, the meaning of bits #1 and #2 can
> be swapped for invalid PTEs. This patch swaps the PTE_FILE and
> PTE_PROT_NONE constants, allowing us to represent PROT_NONE huge
> PTEs.
> 
> Signed-off-by: Steve Capper <steve.capper@linaro.org>
> ---
>  arch/arm64/include/asm/pgtable.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index b1a1b59..e245260 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -25,8 +25,8 @@
>   * Software defined PTE bits definition.
>   */
>  #define PTE_VALID		(_AT(pteval_t, 1) << 0)
> -#define PTE_PROT_NONE		(_AT(pteval_t, 1) << 1)	/* only when !PTE_VALID */
> -#define PTE_FILE		(_AT(pteval_t, 1) << 2)	/* only when !pte_present() */
> +#define PTE_FILE		(_AT(pteval_t, 1) << 1)	/* only when !pte_present() */
> +#define PTE_PROT_NONE		(_AT(pteval_t, 1) << 2)	/* only when !PTE_VALID */
>  #define PTE_DIRTY		(_AT(pteval_t, 1) << 55)
>  #define PTE_SPECIAL		(_AT(pteval_t, 1) << 56)
>  
> @@ -306,8 +306,8 @@ extern pgd_t idmap_pg_dir[PTRS_PER_PGD];
>  
>  /*
>   * Encode and decode a file entry:
> - *	bits 0-1:	present (must be zero)
> - *	bit  2:		PTE_FILE
> + *	bits 0 & 2:	present (must be zero)
> + *	bit  1:		PTE_FILE
>   *	bits 3-63:	file offset / PAGE_SIZE
>   */
>  #define pte_file(pte)		(pte_val(pte) & PTE_FILE)

Can you update the comment describing swp entries too please? I *think* the
__SWP_* defines can remain untouched, but the comment is now wrong.

Will
Steve Capper May 9, 2013, 8:15 a.m. UTC | #3
On Wed, May 08, 2013 at 12:17:35PM -0400, Christopher Covington wrote:
> Hi Steve,
> 
> On 05/08/2013 05:52 AM, Steve Capper wrote:
 
 [...]
> 
> > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> 
 [...]
> 
> > @@ -306,8 +306,8 @@ extern pgd_t idmap_pg_dir[PTRS_PER_PGD];
> >  
> >  /*
> >   * Encode and decode a file entry:
> > - *	bits 0-1:	present (must be zero)
> > - *	bit  2:		PTE_FILE
> > + *	bits 0 & 2:	present (must be zero)
> 
> Consider using punctuation like "bits 0, 2" here to disambiguate from the
> binary and operation.
> 

Hi Christopher,
Thanks, I now have:
       bits 0, 2:      present (must both be zero)

Cheers,
Steve Capper May 9, 2013, 8:22 a.m. UTC | #4
On Wed, May 08, 2013 at 05:40:19PM +0100, Will Deacon wrote:
> On Wed, May 08, 2013 at 10:52:40AM +0100, Steve Capper wrote:

 [...] 

> Can you update the comment describing swp entries too please? I *think* the
> __SWP_* defines can remain untouched, but the comment is now wrong.
> 
> Will

Ta Will, I've now changed the other comment.
The __SWP_* entries look ok to me. I'm going to run some swap tests though
to see if anything crops up.

Cheers,
Catalin Marinas May 16, 2013, 2:58 p.m. UTC | #5
On Wed, May 08, 2013 at 10:52:40AM +0100, Steve Capper wrote:
> Under ARM64, PTEs can be broadly categorised as follows:
>    - Present and valid: Bit #0 is set. The PTE is valid and memory
>      access to the region may fault.
> 
>    - Present and invalid: Bit #0 is clear and bit #1 is set.
>      Represents present memory with PROT_NONE protection. The PTE
>      is an invalid entry, and the user fault handler will raise a
>      SIGSEGV.
> 
>    - Not present (file): Bits #0 and #1 are clear, bit #2 is set.
>      Memory represented has been paged out. The PTE is an invalid
>      entry, and the fault handler will try and re-populate the
>      memory where necessary.
> 
> Huge PTEs are block descriptors that have bit #1 clear. If we wish
> to represent PROT_NONE huge PTEs we then run into a problem as
> there is no way to distinguish between regular and huge PTEs if we
> set bit #1.
> 
> As huge PTEs are always present, the meaning of bits #1 and #2 can
> be swapped for invalid PTEs. This patch swaps the PTE_FILE and
> PTE_PROT_NONE constants, allowing us to represent PROT_NONE huge
> PTEs.

I guess we'll never get a huge_(pte|pmd)_file() (but we can shift the
file bits up anyway).

> Signed-off-by: Steve Capper <steve.capper@linaro.org>

Apart from the comments you already got:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
diff mbox

Patch

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index b1a1b59..e245260 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -25,8 +25,8 @@ 
  * Software defined PTE bits definition.
  */
 #define PTE_VALID		(_AT(pteval_t, 1) << 0)
-#define PTE_PROT_NONE		(_AT(pteval_t, 1) << 1)	/* only when !PTE_VALID */
-#define PTE_FILE		(_AT(pteval_t, 1) << 2)	/* only when !pte_present() */
+#define PTE_FILE		(_AT(pteval_t, 1) << 1)	/* only when !pte_present() */
+#define PTE_PROT_NONE		(_AT(pteval_t, 1) << 2)	/* only when !PTE_VALID */
 #define PTE_DIRTY		(_AT(pteval_t, 1) << 55)
 #define PTE_SPECIAL		(_AT(pteval_t, 1) << 56)
 
@@ -306,8 +306,8 @@  extern pgd_t idmap_pg_dir[PTRS_PER_PGD];
 
 /*
  * Encode and decode a file entry:
- *	bits 0-1:	present (must be zero)
- *	bit  2:		PTE_FILE
+ *	bits 0 & 2:	present (must be zero)
+ *	bit  1:		PTE_FILE
  *	bits 3-63:	file offset / PAGE_SIZE
  */
 #define pte_file(pte)		(pte_val(pte) & PTE_FILE)