| Message ID | 1348156605-30398-4-git-send-email-will.deacon@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 20, 2012 at 04:56:44PM +0100, Will Deacon wrote: > diff --git a/arch/arm/include/asm/pgtable-2level.h b/arch/arm/include/asm/pgtable-2level.h > index 2317a71..c44a1ec 100644 > --- a/arch/arm/include/asm/pgtable-2level.h > +++ b/arch/arm/include/asm/pgtable-2level.h > @@ -115,6 +115,7 @@ > * The PTE table pointer refers to the hardware entries; the "Linux" > * entries are stored 1024 bytes below. > */ > +#define L_PTE_VALID (_AT(pteval_t, 1) << 0) /* Valid */ > #define L_PTE_PRESENT (_AT(pteval_t, 1) << 0) > #define L_PTE_YOUNG (_AT(pteval_t, 1) << 1) > #define L_PTE_FILE (_AT(pteval_t, 1) << 2) /* only when !PRESENT */ > diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h > index b249035..e32311a 100644 > --- a/arch/arm/include/asm/pgtable-3level.h > +++ b/arch/arm/include/asm/pgtable-3level.h > @@ -67,7 +67,8 @@ > * These bits overlap with the hardware bits but the naming is preserved for > * consistency with the classic page table format. > */ > -#define L_PTE_PRESENT (_AT(pteval_t, 3) << 0) /* Valid */ > +#define L_PTE_VALID (_AT(pteval_t, 1) << 0) /* Valid */ > +#define L_PTE_PRESENT (_AT(pteval_t, 3) << 0) /* Present */ > #define L_PTE_FILE (_AT(pteval_t, 1) << 2) /* only when !PRESENT */ > #define L_PTE_USER (_AT(pteval_t, 1) << 6) /* AP[1] */ > #define L_PTE_RDONLY (_AT(pteval_t, 1) << 7) /* AP[2] */ > diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h > index 41dc31f..df206c1 100644 > --- a/arch/arm/include/asm/pgtable.h > +++ b/arch/arm/include/asm/pgtable.h > @@ -203,9 +203,7 @@ static inline pte_t *pmd_page_vaddr(pmd_t pmd) > #define pte_exec(pte) (!(pte_val(pte) & L_PTE_XN)) > #define pte_special(pte) (0) > > -#define pte_present_user(pte) \ > - ((pte_val(pte) & (L_PTE_PRESENT | L_PTE_USER)) == \ > - (L_PTE_PRESENT | L_PTE_USER)) > +#define pte_present_user(pte) (pte_present(pte) && (pte_val(pte) & L_PTE_USER)) This causes the compiler to do the "present" test and then, only if that evaluates true, to do the "user" test. That's not what we want, we want the compiler to combine the two tests into one (which it should be able to do so.)
Hi Russell, On Thu, Sep 20, 2012 at 11:21:05PM +0100, Russell King - ARM Linux wrote: > On Thu, Sep 20, 2012 at 04:56:44PM +0100, Will Deacon wrote: > > diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h > > index 41dc31f..df206c1 100644 > > --- a/arch/arm/include/asm/pgtable.h > > +++ b/arch/arm/include/asm/pgtable.h > > @@ -203,9 +203,7 @@ static inline pte_t *pmd_page_vaddr(pmd_t pmd) > > #define pte_exec(pte) (!(pte_val(pte) & L_PTE_XN)) > > #define pte_special(pte) (0) > > > > -#define pte_present_user(pte) \ > > - ((pte_val(pte) & (L_PTE_PRESENT | L_PTE_USER)) == \ > > - (L_PTE_PRESENT | L_PTE_USER)) > > +#define pte_present_user(pte) (pte_present(pte) && (pte_val(pte) & L_PTE_USER)) > > This causes the compiler to do the "present" test and then, only if that > evaluates true, to do the "user" test. That's not what we want, we want > the compiler to combine the two tests into one (which it should be able > to do so.) This is tricky for LPAE because PTEs with the following suffixes must all be treated as present: ...1xxxx01 /* L_PTE_USER | L_PTE_VALID -- will be used by hugetlb */ ...1xxxx10 /* L_PTE_USER | L_PTE_PRESENT[1] */ ...1xxxx11 /* L_PTE_USER | L_PTE_PRESENT */ so you need to check that (a) L_PTE_USER is set and (b) that at least one of the bottom two bits is set. How about something like: ((pte_val(pte) & (L_PTE_PRESENT | L_PTE_USER)) > L_PTE_USER) Will
diff --git a/arch/arm/include/asm/pgtable-2level.h b/arch/arm/include/asm/pgtable-2level.h index 2317a71..c44a1ec 100644 --- a/arch/arm/include/asm/pgtable-2level.h +++ b/arch/arm/include/asm/pgtable-2level.h @@ -115,6 +115,7 @@ * The PTE table pointer refers to the hardware entries; the "Linux" * entries are stored 1024 bytes below. */ +#define L_PTE_VALID (_AT(pteval_t, 1) << 0) /* Valid */ #define L_PTE_PRESENT (_AT(pteval_t, 1) << 0) #define L_PTE_YOUNG (_AT(pteval_t, 1) << 1) #define L_PTE_FILE (_AT(pteval_t, 1) << 2) /* only when !PRESENT */ diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h index b249035..e32311a 100644 --- a/arch/arm/include/asm/pgtable-3level.h +++ b/arch/arm/include/asm/pgtable-3level.h @@ -67,7 +67,8 @@ * These bits overlap with the hardware bits but the naming is preserved for * consistency with the classic page table format. */ -#define L_PTE_PRESENT (_AT(pteval_t, 3) << 0) /* Valid */ +#define L_PTE_VALID (_AT(pteval_t, 1) << 0) /* Valid */ +#define L_PTE_PRESENT (_AT(pteval_t, 3) << 0) /* Present */ #define L_PTE_FILE (_AT(pteval_t, 1) << 2) /* only when !PRESENT */ #define L_PTE_USER (_AT(pteval_t, 1) << 6) /* AP[1] */ #define L_PTE_RDONLY (_AT(pteval_t, 1) << 7) /* AP[2] */ diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h index 41dc31f..df206c1 100644 --- a/arch/arm/include/asm/pgtable.h +++ b/arch/arm/include/asm/pgtable.h @@ -203,9 +203,7 @@ static inline pte_t *pmd_page_vaddr(pmd_t pmd) #define pte_exec(pte) (!(pte_val(pte) & L_PTE_XN)) #define pte_special(pte) (0) -#define pte_present_user(pte) \ - ((pte_val(pte) & (L_PTE_PRESENT | L_PTE_USER)) == \ - (L_PTE_PRESENT | L_PTE_USER)) +#define pte_present_user(pte) (pte_present(pte) && (pte_val(pte) & L_PTE_USER)) #if __LINUX_ARM_ARCH__ < 6 static inline void __sync_icache_dcache(pte_t pteval) diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S index e37600b..e755e9f 100644 --- a/arch/arm/mm/proc-v7-2level.S +++ b/arch/arm/mm/proc-v7-2level.S @@ -100,7 +100,7 @@ ENTRY(cpu_v7_set_pte_ext) orrne r3, r3, #PTE_EXT_XN tst r1, #L_PTE_YOUNG - tstne r1, #L_PTE_PRESENT + tstne r1, #L_PTE_VALID moveq r3, #0 ARM( str r3, [r0, #2048]! ) diff --git a/arch/arm/mm/proc-v7-3level.S b/arch/arm/mm/proc-v7-3level.S index 8de0f1d..d23d067 100644 --- a/arch/arm/mm/proc-v7-3level.S +++ b/arch/arm/mm/proc-v7-3level.S @@ -65,7 +65,7 @@ ENDPROC(cpu_v7_switch_mm) */ ENTRY(cpu_v7_set_pte_ext) #ifdef CONFIG_MMU - tst r2, #L_PTE_PRESENT + tst r2, #L_PTE_VALID beq 1f tst r3, #1 << (55 - 32) @ L_PTE_DIRTY orreq r2, #L_PTE_RDONLY
For long-descriptor translation table formats, the ARMv7 architecture defines the last two bits of the second- and third-level descriptors to be: x0b - Invalid 01b - Block (second-level), Reserved (third-level) 11b - Table (second-level), Page (third-level) This allows us to define L_PTE_PRESENT as (3 << 0) and use this value to create ptes directly. However, when determining whether a given pte value is present in the low-level page table accessors, we only need to check the least significant bit of the descriptor, allowing us to write faulting, present entries which are required for PROT_NONE mappings. This patch introduces L_PTE_VALID, which can be used to test whether a pte should fault, and updates the low-level page table accessors accordingly. Signed-off-by: Will Deacon <will.deacon@arm.com> --- arch/arm/include/asm/pgtable-2level.h | 1 + arch/arm/include/asm/pgtable-3level.h | 3 ++- arch/arm/include/asm/pgtable.h | 4 +--- arch/arm/mm/proc-v7-2level.S | 2 +- arch/arm/mm/proc-v7-3level.S | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-)