PTE fixes for ARM LPAE
v5 cover PTE fixes for ARM LPAE
This series fixes a couple of problems that I encountered on ARM with LPAE.
1) Some pte accessors can have their results cancelled out by a downcast. This is addressed by the first patch.
2) It is impossible to distinguish between clean writeable ptes and read only ptes. This is addressed by the second patch.
Notable changes in this series from V4->V5:
o) Rather than switch to L_PTE_WRITE semantics, we move L_PTE_RDONLY to a software bit. This allows for a much smaller patch set that does not affect the 2-level code.
Notable changes in this series from V3->V4:
o) Rebased to take into account the BE LPAE fix in 3.16-rc1.
o) THP logic added as this was missing.
This series has been tested with libhugetlbfs unit tests, ltp mm tests and a custom THP test to test what happens when PROT_NONE THPs are faulted. I have tested on an Arndale board with LPAE and classic MMU (with classic MMU, only ltp mm tests were run). Testing was performed against 3.16-rc2.
Any comments/feedback would be appreciated.
Cheers,
--
Steve
Steve Capper (2):
arm: mm: Introduce {pte,pmd}_isset and {pte,pmd}_isclear
arm: mm: Modify pte_write and pmd_write logic for LPAE
arch/arm/include/asm/pgtable-3level-hwdef.h | 1 + arch/arm/include/asm/pgtable-3level.h | 45 ++++++++++++++++++----------- arch/arm/include/asm/pgtable.h | 18 +++++++----- arch/arm/mm/proc-v7-3level.S | 9 ++++-- 4 files changed, 47 insertions(+), 26 deletions(-)
Signature · 1 line
1.9.3
PATCH v5 1/2 arm: mm: Introduce {pte, pmd}_isset and {pte, pmd}_isclear
Long descriptors on ARM are 64 bits, and some pte functions such as pte_dirty return a bitwise-and of a flag with the pte value. If the flag to be tested resides in the upper 32 bits of the pte, then we run into the danger of the result being dropped if downcast.
For example:
gather_stats(page, md, pte_dirty(*pte), 1);
where pte_dirty(*pte) is downcast to an int.
This patch introduces a new macro pte_isset which performs the bitwise and, then performs a double logical invert (where needed) to ensure predictable downcasting. The logical inverse pte_isclear is also introduced.
Equivalent pmd functions for Transparent HugePages have also been added.
---
Changed in V5: routed pmd_trans_splitting through the pmd_isset logic.
--- arch/arm/include/asm/pgtable-3level.h | 12 ++++++++---- arch/arm/include/asm/pgtable.h | 18 +++++++++++------- 2 files changed, 19 insertions(+), 11 deletions(-)
arch/arm/include/asm/pgtable-3level.h +8 -4
@@ -207,17 +207,21 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)#define pte_huge (pte) (pte_val(pte) && !(pte_val(pte) & PTE_TABLE_BIT))#define pte_mkhuge (pte) (__pte(pte_val(pte) & ~PTE_TABLE_BIT)) -#define pmd_young (pmd) (pmd_val(pmd) & PMD_SECT_AF) +#define pmd_isset (pmd, val) ((u32)(val) == (val) ? pmd_val(pmd) & (val) \ + : !!(pmd_val(pmd) & (val))) +#define pmd_isclear (pmd, val) (!(pmd_val(pmd) & (val))) + +#define pmd_young (pmd) (pmd_isset((pmd), PMD_SECT_AF))#define __HAVE_ARCH_PMD_WRITE -#define pmd_write (pmd) (!(pmd_val(pmd) & PMD_SECT_RDONLY)) +#define pmd_write (pmd) (pmd_isclear((pmd), PMD_SECT_RDONLY))#define pmd_hugewillfault (pmd) (!pmd_young(pmd) || !pmd_write(pmd))#define pmd_thp_or_huge (pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd))#ifdef CONFIG_TRANSPARENT_HUGEPAGE -#define pmd_trans_huge (pmd) (pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT)) -#define pmd_trans_splitting (pmd) (pmd_val(pmd) & PMD_SECT_SPLITTING) +#define pmd_trans_huge (pmd) (pmd_val(pmd) && pmd_isclear((pmd), PMD_TABLE_BIT)) +#define pmd_trans_splitting (pmd) (pmd_isset((pmd), PMD_SECT_SPLITTING))#endif #define PMD_BIT_FUNC (fn,op) \
arch/arm/include/asm/pgtable.h +11 -7
@@ -214,18 +214,22 @@ static inline pte_t *pmd_page_vaddr(pmd_t pmd)#define pte_clear (mm,addr,ptep) set_pte_ext(ptep, __pte(0), 0) +#define pte_isset (pte, val) ((u32)(val) == (val) ? pte_val(pte) & (val) \ + : !!(pte_val(pte) & (val))) +#define pte_isclear (pte, val) (!(pte_val(pte) & (val))) +#define pte_none (pte) (!pte_val(pte)) -#define pte_present (pte) (pte_val(pte) & L_PTE_PRESENT) -#define pte_valid (pte) (pte_val(pte) & L_PTE_VALID) +#define pte_present (pte) (pte_isset((pte), L_PTE_PRESENT)) +#define pte_valid (pte) (pte_isset((pte), L_PTE_VALID))#define pte_accessible (mm, pte) (mm_tlb_flush_pending(mm) ? pte_present(pte) : pte_valid(pte)) -#define pte_write (pte) (!(pte_val(pte) & L_PTE_RDONLY)) -#define pte_dirty (pte) (pte_val(pte) & L_PTE_DIRTY) -#define pte_young (pte) (pte_val(pte) & L_PTE_YOUNG) -#define pte_exec (pte) (!(pte_val(pte) & L_PTE_XN)) +#define pte_write (pte) (pte_isclear((pte), L_PTE_RDONLY)) +#define pte_dirty (pte) (pte_isset((pte), L_PTE_DIRTY)) +#define pte_young (pte) (pte_isset((pte), L_PTE_YOUNG)) +#define pte_exec (pte) (pte_isclear((pte), L_PTE_XN))#define pte_special (pte) (0)#define pte_valid_user (pte) \ - (pte_valid(pte) && (pte_val(pte) & L_PTE_USER) && pte_young(pte)) + (pte_valid(pte) && pte_isset((pte), L_PTE_USER) && pte_young(pte))#if __LINUX_ARM_ARCH__ <6 static inline void __sync_icache_dcache (pte_t pteval)
Signature · 1 line
1.9.3
PATCH v5 1/2 arm: mm: Introduce {pte,pmd}_isset and {pte,pmd}_isclear
Hi Steve,
Quoted 16 lines · Steve Capper
Long descriptors on ARM are 64 bits, and some pte functions such as
pte_dirtyreturn a bitwise-and of a flag with the pte value. If the flag to be tested resides in the upper 32 bits of the pte, then we run into the danger of the result being dropped if downcast.For example:
gather_stats(page, md, pte_dirty(*pte), 1);where
pte_dirty(*pte)is downcast to an int.This patch introduces a new macro
pte_issetwhich performs the bitwise and, then performs a double logical invert (where needed) to ensure predictable downcasting. The logical inversepte_isclearis also introduced.Equivalent pmd functions for Transparent HugePages have also been added.
[...]
Quoted 16 lines
arch/arm/include/asm/pgtable-3level.h +5
-1@@ -207,17 +207,21 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)#define pte_huge (pte) (pte_val(pte) && !(pte_val(pte) & PTE_TABLE_BIT))#define pte_mkhuge (pte) (__pte(pte_val(pte) & ~PTE_TABLE_BIT)) -#define pmd_young (pmd) (pmd_val(pmd) & PMD_SECT_AF) +#define pmd_isset (pmd, val) ((u32)(val) == (val) ? pmd_val(pmd) & (val) \ + : !!(pmd_val(pmd) & (val))) +#define pmd_isclear (pmd, val) (!(pmd_val(pmd) & (val))) + +#define pmd_young (pmd) (pmd_isset((pmd), PMD_SECT_AF))#define __HAVE_ARCH_PMD_WRITE
arch/arm/include/asm/pgtable-3level.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 223
Quoted 10 lines
arch/arm/include/asm/pgtable-3level.h +2
-3-#define pmd_write (pmd) (!(pmd_val(pmd) & PMD_SECT_RDONLY)) +#define pmd_write (pmd) (pmd_isclear((pmd), PMD_SECT_RDONLY))#define pmd_hugewillfault (pmd) (!pmd_young(pmd) || !pmd_write(pmd))#define pmd_thp_or_huge (pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd))#ifdef CONFIG_TRANSPARENT_HUGEPAGE -#define pmd_trans_huge (pmd) (pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT)) -#define pmd_trans_splitting (pmd) (pmd_val(pmd) & PMD_SECT_SPLITTING) +#define pmd_trans_huge (pmd) (pmd_val(pmd) && pmd_isclear((pmd), PMD_TABLE_BIT))
Why isn't this just pmd_present(pmd) && !pmd_table(pmd)? Put another way, I see to have forgotten why we need PMD_TABLE_BIT instead of just using pmd_table and pmd_sect to work out whether we have a table or a block.
Will
PATCH v5 1/2 arm: mm: Introduce {pte,pmd}_isset and {pte,pmd}_isclear
Quoted 1 line · Will Deacon
Hi Steve,
Hey Will,
Quoted 44 lines · Steve Capper
Quoted 16 lines
Long descriptors on ARM are 64 bits, and some pte functions such as
pte_dirtyreturn a bitwise-and of a flag with the pte value. If the flag to be tested resides in the upper 32 bits of the pte, then we run into the danger of the result being dropped if downcast.For example:
gather_stats(page, md, pte_dirty(*pte), 1);where
pte_dirty(*pte)is downcast to an int.This patch introduces a new macro
pte_issetwhich performs the bitwise and, then performs a double logical invert (where needed) to ensure predictable downcasting. The logical inversepte_isclearis also introduced.Equivalent pmd functions for Transparent HugePages have also been added.
[...]
Quoted 22 lines
arch/arm/include/asm/pgtable-3level.h +6
-2@@ -207,17 +207,21 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)#define pte_huge (pte) (pte_val(pte) && !(pte_val(pte) & PTE_TABLE_BIT))#define pte_mkhuge (pte) (__pte(pte_val(pte) & ~PTE_TABLE_BIT)) -#define pmd_young (pmd) (pmd_val(pmd) & PMD_SECT_AF) +#define pmd_isset (pmd, val) ((u32)(val) == (val) ? pmd_val(pmd) & (val) \ + : !!(pmd_val(pmd) & (val))) +#define pmd_isclear (pmd, val) (!(pmd_val(pmd) & (val))) + +#define pmd_young (pmd) (pmd_isset((pmd), PMD_SECT_AF))#define __HAVE_ARCH_PMD_WRITE -#define pmd_write (pmd) (!(pmd_val(pmd) & PMD_SECT_RDONLY)) +#define pmd_write (pmd) (pmd_isclear((pmd), PMD_SECT_RDONLY))#define pmd_hugewillfault (pmd) (!pmd_young(pmd) || !pmd_write(pmd))#define pmd_thp_or_huge (pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd))
Quoted 9 lines
arch/arm/include/asm/pgtable-3level.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 223
Quoted 4 lines
arch/arm/include/asm/pgtable-3level.h +1
-2#ifdef CONFIG_TRANSPARENT_HUGEPAGE -#define pmd_trans_huge (pmd) (pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT)) -#define pmd_trans_splitting (pmd) (pmd_val(pmd) & PMD_SECT_SPLITTING) +#define pmd_trans_huge (pmd) (pmd_val(pmd) && pmd_isclear((pmd), PMD_TABLE_BIT))Why isn't this just
pmd_present(pmd)&& !pmd_table(pmd)? Put another way, I see to have forgotten why we needPMD_TABLE_BITinstead of just usingpmd_tableandpmd_sectto work out whether we have a table or a block.
If we use pmd_sect, we are testing for block entries, but THPs are allowed to be faulting entries (i.e. PROT_NONE). A non-zero pmd that does not have the table bit set can safely be assumed to be a PROT_NONE THP.
Cheers,
Signature · 1 line
Steve
PATCH v5 1/2 arm: mm: Introduce {pte,pmd}_isset and {pte,pmd}_isclear
Quoted 12 lines · Steve Capper
Quoted 10 lines · Will Deacon
arch/arm/include/asm/pgtable-3level.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 223
Quoted 4 lines · Steve Capper
#ifdef
CONFIG_TRANSPARENT_HUGEPAGEarch/arm/include/asm/pgtable-3level.h +1
-2-#define pmd_trans_huge (pmd) (pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT)) -#define pmd_trans_splitting (pmd) (pmd_val(pmd) & PMD_SECT_SPLITTING) +#define pmd_trans_huge (pmd) (pmd_val(pmd) && pmd_isclear((pmd), PMD_TABLE_BIT))Why isn't this just
pmd_present(pmd)&& !pmd_table(pmd)? Put another way, I see to have forgotten why we needPMD_TABLE_BITinstead of just usingpmd_tableandpmd_sectto work out whether we have a table or a block.
Quoted 3 lines
If we use
pmd_sect, we are testing for block entries, but THPs are allowed to be faulting entries (i.e.PROT_NONE). A non-zero pmd that does not have the table bit set can safely be assumed to be aPROT_NONETHP.
Ah yes, I forgot about faulting entries. We should still be able to use pmd_table(pmd) as opposed to PMD_TABLE_BIT though, right?
Will
PATCH v5 1/2 arm: mm: Introduce {pte,pmd}_isset and {pte,pmd}_isclear
Quoted 13 lines · Will Deacon
Quoted 12 lines · Steve Capper
Quoted 10 lines · Will Deacon
arch/arm/include/asm/pgtable-3level.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 223
Quoted 4 lines · Steve Capper
#ifdef
CONFIG_TRANSPARENT_HUGEPAGEarch/arm/include/asm/pgtable-3level.h +1
-2-#define pmd_trans_huge (pmd) (pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT)) -#define pmd_trans_splitting (pmd) (pmd_val(pmd) & PMD_SECT_SPLITTING) +#define pmd_trans_huge (pmd) (pmd_val(pmd) && pmd_isclear((pmd), PMD_TABLE_BIT))Why isn't this just
pmd_present(pmd)&& !pmd_table(pmd)? Put another way, I see to have forgotten why we needPMD_TABLE_BITinstead of just usingpmd_tableandpmd_sectto work out whether we have a table or a block.
Quoted 6 lines
Quoted 3 lines
If we use
pmd_sect, we are testing for block entries, but THPs are allowed to be faulting entries (i.e.PROT_NONE). A non-zero pmd that does not have the table bit set can safely be assumed to be aPROT_NONETHP.Ah yes, I forgot about faulting entries. We should still be able to use
pmd_table(pmd)as opposed toPMD_TABLE_BITthough, right?
Yes, that should be okay, thanks I'll simplify this.
Quoted 2 lines
Will
PATCH v5 2/2 arm: mm: Modify pte_write and pmd_write logic for LPAE
In reply to Steve CapperFor LPAE, we have the following means for encoding writable or dirty ptes:
L_PTE_DIRTY L_PTE_RDONLY
!pte_dirty && !pte_write 0 1
!pte_dirty && pte_write 0 1
pte_dirty && !pte_write 1 1
pte_dirty && pte_write 1 0So we can't distinguish between writeable clean ptes and read only ptes. This can cause problems with ptes being incorrectly flagged as read only when they are writeable but not dirty.
This patch renumbers L_PTE_RDONLY from AP[2] to a software bit #58, and adds additional logic to set AP[2] whenever the pte is read only or not dirty. That way we can distinguish between clean writeable ptes and read only ptes.
HugeTLB pages will use this new logic automatically.
We need to add some logic to Transparent HugePages to ensure that they correctly interpret the revised pgprot permissions (L_PTE_RDONLY has moved and no longer matches PMD_SECT_RDONLY). In the process of revising THP, the names of the PMD software bits have been prefixed with L_ to make them easier to distinguish from their hardware bit counterparts.
---
Changed in V5 -> rather than re-introduce L_PTE_WRITE we change L_PTE_RDONLY to be a software bit #58. This allows us to leave the two level code alone.
--- arch/arm/include/asm/pgtable-3level-hwdef.h | 1 + arch/arm/include/asm/pgtable-3level.h | 37 +++++++++++++++++------------ arch/arm/mm/proc-v7-3level.S | 9 +++++-- 3 files changed, 30 insertions(+), 17 deletions(-)
arch/arm/include/asm/pgtable-3level-hwdef.h +1 -0
@@ -72,6 +72,7 @@#define PTE_TABLE_BIT (_AT(pteval_t, 1) << 1)#define PTE_BUFFERABLE (_AT(pteval_t, 1) << 2)/* AttrIndx[0] */ #define PTE_CACHEABLE (_AT(pteval_t, 1) << 3)/* AttrIndx[1] */ +#define PTE_AP2 (_AT(pteval_t, 1) << 7)/* AP[2] */ #define PTE_EXT_SHARED (_AT(pteval_t, 3) << 8)/* SH[1:0], inner shareable */ #define PTE_EXT_AF (_AT(pteval_t, 1) << 10)/* Access Flag */ #define PTE_EXT_NG (_AT(pteval_t, 1) << 11)/* nG */
arch/arm/include/asm/pgtable-3level.h +22 -15
@@ -79,18 +79,19 @@#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] */ #define L_PTE_SHARED (_AT(pteval_t, 3) << 8)/* SH[1:0], inner shareable */ #define L_PTE_YOUNG (_AT(pteval_t, 1) << 10)/* AF */ #define L_PTE_XN (_AT(pteval_t, 1) << 54)/* XN */ #define L_PTE_DIRTY (_AT(pteval_t, 1) << 55)/* unused */ #define L_PTE_SPECIAL (_AT(pteval_t, 1) << 56)/* unused */ #define L_PTE_NONE (_AT(pteval_t, 1) << 57)/* PROT_NONE */ +#define L_PTE_RDONLY (_AT(pteval_t, 1) << 58)/* READ ONLY */ -#define PMD_SECT_VALID (_AT(pmdval_t, 1) << 0) -#define PMD_SECT_DIRTY (_AT(pmdval_t, 1) << 55) -#define PMD_SECT_SPLITTING (_AT(pmdval_t, 1) << 56) -#define PMD_SECT_NONE (_AT(pmdval_t, 1) << 57) +#define L_PMD_SECT_VALID (_AT(pmdval_t, 1) << 0) +#define L_PMD_SECT_DIRTY (_AT(pmdval_t, 1) << 55) +#define L_PMD_SECT_SPLITTING (_AT(pmdval_t, 1) << 56) +#define L_PMD_SECT_NONE (_AT(pmdval_t, 1) << 57) +#define L_PMD_SECT_RDONLY (_AT(pteval_t, 1) << 58)/* * To be used in assembly code with the upper page attributes.
@@ -214,24 +215,25 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)#define pmd_young (pmd) (pmd_isset((pmd), PMD_SECT_AF))#define __HAVE_ARCH_PMD_WRITE -#define pmd_write (pmd) (pmd_isclear((pmd), PMD_SECT_RDONLY)) +#define pmd_write (pmd) (pmd_isclear((pmd), L_PMD_SECT_RDONLY)) +#define pmd_dirty (pmd) (pmd_isset((pmd), L_PMD_SECT_DIRTY))#define pmd_hugewillfault (pmd) (!pmd_young(pmd) || !pmd_write(pmd))#define pmd_thp_or_huge (pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd))#ifdef CONFIG_TRANSPARENT_HUGEPAGE #define pmd_trans_huge (pmd) (pmd_val(pmd) && pmd_isclear((pmd), PMD_TABLE_BIT)) -#define pmd_trans_splitting (pmd) (pmd_isset((pmd), PMD_SECT_SPLITTING)) +#define pmd_trans_splitting (pmd) (pmd_isset((pmd), L_PMD_SECT_SPLITTING))#endif #define PMD_BIT_FUNC (fn,op) \ static inline pmd_t pmd_##fn(pmd_t pmd) { pmd_val(pmd) op; return pmd; } -PMD_BIT_FUNC(wrprotect, |= PMD_SECT_RDONLY); +PMD_BIT_FUNC(wrprotect, |= L_PMD_SECT_RDONLY); PMD_BIT_FUNC(mkold, &= ~PMD_SECT_AF); -PMD_BIT_FUNC(mksplitting, |= PMD_SECT_SPLITTING); -PMD_BIT_FUNC(mkwrite, &= ~PMD_SECT_RDONLY); -PMD_BIT_FUNC(mkdirty, |= PMD_SECT_DIRTY); +PMD_BIT_FUNC(mksplitting, |= L_PMD_SECT_SPLITTING); +PMD_BIT_FUNC(mkwrite, &= ~L_PMD_SECT_RDONLY); +PMD_BIT_FUNC(mkdirty, |= L_PMD_SECT_DIRTY); PMD_BIT_FUNC(mkyoung, |= PMD_SECT_AF);#define pmd_mkhuge (pmd) (__pmd(pmd_val(pmd) & ~PMD_TABLE_BIT))
@@ -245,8 +247,8 @@ PMD_BIT_FUNC(mkyoung, |= PMD_SECT_AF);static inline pmd_t pmd_modify (pmd_t pmd,pgprot_t newprot) { -const pmdval_t mask =PMD_SECT_USER |PMD_SECT_XN |PMD_SECT_RDONLY | -PMD_SECT_VALID |PMD_SECT_NONE ; +const pmdval_t mask =PMD_SECT_USER |PMD_SECT_XN |L_PMD_SECT_RDONLY | +L_PMD_SECT_VALID |L_PMD_SECT_NONE ;pmd_val (pmd) = (pmd_val (pmd) & ~mask) | (pgprot_val (newprot) & mask);return pmd; }
@@ -257,8 +259,13 @@ static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr,BUG_ON (addr >=TASK_SIZE );/* create a faulting entry if PROT_NONE protected */ -if (pmd_val (pmd) &PMD_SECT_NONE ) -pmd_val (pmd) &= ~PMD_SECT_VALID ; +if (pmd_val (pmd) &L_PMD_SECT_NONE ) +pmd_val (pmd) &= ~L_PMD_SECT_VALID ; + +if (pmd_write (pmd) &&pmd_dirty (pmd)) +pmd_val (pmd) &= ~PMD_SECT_RDONLY ; +else +pmd_val (pmd) |=PMD_SECT_RDONLY ; *pmdp =__pmd (pmd_val (pmd) | PMD_SECT_nG);flush_pmd_entry (pmdp);
arch/arm/mm/proc-v7-3level.S +7 -2
@@ -86,8 +86,13 @@ ENTRY(cpu_v7_set_pte_ext) tst rh, #1 << (57 - 32) @ L_PTE_NONE bicne rl, #L_PTE_VALID bne 1f - tst rh, #1 << (55 - 32) @ L_PTE_DIRTY - orreq rl, #L_PTE_RDONLY + + eor ip, rh, #1 << (55 - 32) @ toggle L_PTE_DIRTY in temp reg to + @ test for !L_PTE_DIRTY || L_PTE_RDONLY + tst ip, #1 << (55 - 32) | 1 << (58 - 32) + orrne rl, #PTE_AP2 + biceq rl, #PTE_AP2 + 1: strd r2, r3, [r0] ALT_SMP(W(nop)) ALT_UP (mcr p15, 0, r0, c7, c10, 1) @ flush_pte
Signature · 1 line
1.9.3
PATCH v5 2/2 arm: mm: Modify pte_write and pmd_write logic for LPAE
Hey Steve,
Quoted 26 lines · Steve Capper
For LPAE, we have the following means for encoding writable or dirty ptes:
L_PTE_DIRTY L_PTE_RDONLY !pte_dirty && !pte_write 0 1 !pte_dirty && pte_write 0 1 pte_dirty && !pte_write 1 1 pte_dirty && pte_write 1 0So we can't distinguish between writeable clean ptes and read only ptes. This can cause problems with ptes being incorrectly flagged as read only when they are writeable but not dirty.
This patch renumbers
L_PTE_RDONLYfrom AP[2] to a software bit #58, and adds additional logic to set AP[2] whenever the pte is read only or not dirty. That way we can distinguish between clean writeable ptes and read only ptes.HugeTLB pages will use this new logic automatically.
We need to add some logic to Transparent HugePages to ensure that they correctly interpret the revised pgprot permissions (
L_PTE_RDONLYhas moved and no longer matchesPMD_SECT_RDONLY). In the process of revising THP, the names of the PMD software bits have been prefixed with L_ to make them easier to distinguish from their hardware bit counterparts.
Quoted 5 lines
Signed-off-by Steve Capper <[email protected]>---
Changed in V5 -> rather than re-introduceL_PTE_WRITEwe changeL_PTE_RDONLYto be a software bit #58. This allows us to leave the two level code alone.
Damnit, you owe me a beer every time I have to review this patch ;) At least proc-macros.S is unscathed this time around!
Quoted 11 lines
--- arch/arm/include/asm/pgtable-3level-hwdef.h | 1 + arch/arm/include/asm/pgtable-3level.h | 37 +++++++++++++++++------------ arch/arm/mm/proc-v7-3level.S | 9 +++++-- 3 files changed, 30 insertions(+), 17 deletions(-)arch/arm/include/asm/pgtable-3level-hwdef.h +0
-0
arch/arm/include/asm/pgtable-3level-hwdef.h line 75
Quoted 4 lines
arch/arm/include/asm/pgtable-3level-hwdef.h +1
-0@@ -72,6 +72,7 @@#define PTE_TABLE_BIT (_AT(pteval_t, 1) << 1)#define PTE_BUFFERABLE (_AT(pteval_t, 1) << 2)/* AttrIndx[0] */ #define PTE_CACHEABLE (_AT(pteval_t, 1) << 3)/* AttrIndx[1] */ +#define PTE_AP2 (_AT(pteval_t, 1) << 7)/* AP[2] */
PTE_RDONLY? AP2 is pretty cryptic.
Quoted 9 lines
arch/arm/include/asm/pgtable-3level-hwdef.h +0
-0#define PTE_EXT_SHARED (_AT(pteval_t, 3) << 8)/* SH[1:0], inner shareable */ #define PTE_EXT_AF (_AT(pteval_t, 1) << 10)/* Access Flag */ #define PTE_EXT_NG (_AT(pteval_t, 1) << 11)/* nG */ arch/arm/include/asm/pgtable-3level.h +0
-0@@ -79,18 +79,19 @@#define L_PTE_PRESENT (_AT(pteval_t, 3) << 0)/* Present */
arch/arm/include/asm/pgtable-3level.h line 88
Quoted 10 lines
arch/arm/include/asm/pgtable-3level.h +1
-1#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] */ #define L_PTE_SHARED (_AT(pteval_t, 3) << 8)/* SH[1:0], inner shareable */ #define L_PTE_YOUNG (_AT(pteval_t, 1) << 10)/* AF */ #define L_PTE_XN (_AT(pteval_t, 1) << 54)/* XN */ #define L_PTE_DIRTY (_AT(pteval_t, 1) << 55)/* unused */ #define L_PTE_SPECIAL (_AT(pteval_t, 1) << 56)/* unused */ #define L_PTE_NONE (_AT(pteval_t, 1) << 57)/* PROT_NONE */ +#define L_PTE_RDONLY (_AT(pteval_t, 1) << 58)/* READ ONLY */
Can you fix those `unused' comments above while you're here, please? Whilst they are unused by the hardware, they're very much used by Linux. Well, SPECIAL isn't yet, but it could be...
arch/arm/include/asm/pgtable-3level.h line 94
Quoted 9 lines
arch/arm/include/asm/pgtable-3level.h +5
-4-#define PMD_SECT_VALID (_AT(pmdval_t, 1) << 0) -#define PMD_SECT_DIRTY (_AT(pmdval_t, 1) << 55) -#define PMD_SECT_SPLITTING (_AT(pmdval_t, 1) << 56) -#define PMD_SECT_NONE (_AT(pmdval_t, 1) << 57) +#define L_PMD_SECT_VALID (_AT(pmdval_t, 1) << 0) +#define L_PMD_SECT_DIRTY (_AT(pmdval_t, 1) << 55) +#define L_PMD_SECT_SPLITTING (_AT(pmdval_t, 1) << 56) +#define L_PMD_SECT_NONE (_AT(pmdval_t, 1) << 57) +#define L_PMD_SECT_RDONLY (_AT(pteval_t, 1) << 58)
Boo-hoo, that's our last remaining software bit for LPAE. It's a sad day.
Quoted 7 lines
arch/arm/include/asm/pgtable-3level.h +0
-1/* * To be used in assembly code with the upper page attributes. @@ -214,24 +215,25 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)#define pmd_young (pmd) (pmd_isset((pmd), PMD_SECT_AF))#define __HAVE_ARCH_PMD_WRITE -#define pmd_write (pmd) (pmd_isclear((pmd), PMD_SECT_RDONLY))
arch/arm/include/asm/pgtable-3level.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 226
Quoted 10 lines
arch/arm/include/asm/pgtable-3level.h +3
-1+#define pmd_write (pmd) (pmd_isclear((pmd), L_PMD_SECT_RDONLY)) +#define pmd_dirty (pmd) (pmd_isset((pmd), L_PMD_SECT_DIRTY))#define pmd_hugewillfault (pmd) (!pmd_young(pmd) || !pmd_write(pmd))#define pmd_thp_or_huge (pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd))#ifdef CONFIG_TRANSPARENT_HUGEPAGE #define pmd_trans_huge (pmd) (pmd_val(pmd) && pmd_isclear((pmd), PMD_TABLE_BIT)) -#define pmd_trans_splitting (pmd) (pmd_isset((pmd), PMD_SECT_SPLITTING)) +#define pmd_trans_splitting (pmd) (pmd_isset((pmd), L_PMD_SECT_SPLITTING))
I take it we only call this on something we've decided is huge already (as opposed to, e.g. a swap entry)?
Quoted 29 lines
arch/arm/include/asm/pgtable-3level.h +6
-6#endif #define PMD_BIT_FUNC (fn,op) \ static inline pmd_t pmd_##fn(pmd_t pmd) { pmd_val(pmd) op; return pmd; } -PMD_BIT_FUNC(wrprotect, |= PMD_SECT_RDONLY); +PMD_BIT_FUNC(wrprotect, |= L_PMD_SECT_RDONLY); PMD_BIT_FUNC(mkold, &= ~PMD_SECT_AF); -PMD_BIT_FUNC(mksplitting, |= PMD_SECT_SPLITTING); -PMD_BIT_FUNC(mkwrite, &= ~PMD_SECT_RDONLY); -PMD_BIT_FUNC(mkdirty, |= PMD_SECT_DIRTY); +PMD_BIT_FUNC(mksplitting, |= L_PMD_SECT_SPLITTING); +PMD_BIT_FUNC(mkwrite, &= ~L_PMD_SECT_RDONLY); +PMD_BIT_FUNC(mkdirty, |= L_PMD_SECT_DIRTY); PMD_BIT_FUNC(mkyoung, |= PMD_SECT_AF);#define pmd_mkhuge (pmd) (__pmd(pmd_val(pmd) & ~PMD_TABLE_BIT))@@ -245,8 +247,8 @@ PMD_BIT_FUNC(mkyoung, |= PMD_SECT_AF);static inline pmd_t pmd_modify (pmd_t pmd,pgprot_t newprot) { -const pmdval_t mask =PMD_SECT_USER |PMD_SECT_XN |PMD_SECT_RDONLY | -PMD_SECT_VALID |PMD_SECT_NONE ; +const pmdval_t mask =PMD_SECT_USER |PMD_SECT_XN |L_PMD_SECT_RDONLY | +L_PMD_SECT_VALID |L_PMD_SECT_NONE ;pmd_val (pmd) = (pmd_val (pmd) & ~mask) | (pgprot_val (newprot) & mask);return pmd; }
arch/arm/include/asm/pgtable-3level.h static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr, · line 266
Quoted 10 lines
arch/arm/include/asm/pgtable-3level.h +5
-2@@ -257,8 +259,13 @@ static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr,BUG_ON (addr >=TASK_SIZE );/* create a faulting entry if PROT_NONE protected */ -if (pmd_val (pmd) &PMD_SECT_NONE ) -pmd_val (pmd) &= ~PMD_SECT_VALID ; +if (pmd_val (pmd) &L_PMD_SECT_NONE ) +pmd_val (pmd) &= ~L_PMD_SECT_VALID ; + +if (pmd_write (pmd) &&pmd_dirty (pmd)) +pmd_val (pmd) &= ~PMD_SECT_RDONLY ;
pmd_mkwrite?
arch/arm/include/asm/pgtable-3level.h static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr, · line 268
Quoted 2 lines
arch/arm/include/asm/pgtable-3level.h +2
-0+else +pmd_val (pmd) |=PMD_SECT_RDONLY ;
pmd_wrprotect?
Quoted 8 lines
arch/arm/include/asm/pgtable-3level.h +0
-0*pmdp =__pmd (pmd_val (pmd) | PMD_SECT_nG);flush_pmd_entry (pmdp);arch/arm/mm/proc-v7-3level.S +0
-0
arch/arm/mm/proc-v7-3level.S ENTRY(cpu_v7_set_pte_ext) · line 91
Quoted 8 lines
arch/arm/mm/proc-v7-3level.S +3
-2@@ -86,8 +86,13 @@ ENTRY(cpu_v7_set_pte_ext) tst rh, #1 << (57 - 32) @ L_PTE_NONE bicne rl, #L_PTE_VALID bne 1f - tst rh, #1 << (55 - 32) @ L_PTE_DIRTY - orreq rl, #L_PTE_RDONLY + + eor ip, rh, #1 << (55 - 32) @ toggle L_PTE_DIRTY in temp reg to + @ test for !L_PTE_DIRTY || L_PTE_RDONLY
Since you already set RDONLY for pmds based on dirty, can we do the same for ptes and avoid this check here...?
arch/arm/mm/proc-v7-3level.S ENTRY(cpu_v7_set_pte_ext) · line 94
Quoted 3 lines
arch/arm/mm/proc-v7-3level.S +3
-0+ tst ip, #1 << (55 - 32) | 1 << (58 - 32) + orrne rl, #PTE_AP2 + biceq rl, #PTE_AP2
... Then use Russell's ubfx trick to copy L_PTE_RDONLY into PTE_RDONLY?
Will
PATCH v5 2/2 arm: mm: Modify pte_write and pmd_write logic for LPAE
Quoted 29 lines · Will Deacon
Hey Steve,
Quoted 26 lines · Steve Capper
For LPAE, we have the following means for encoding writable or dirty ptes:
L_PTE_DIRTY L_PTE_RDONLY !pte_dirty && !pte_write 0 1 !pte_dirty && pte_write 0 1 pte_dirty && !pte_write 1 1 pte_dirty && pte_write 1 0So we can't distinguish between writeable clean ptes and read only ptes. This can cause problems with ptes being incorrectly flagged as read only when they are writeable but not dirty.
This patch renumbers
L_PTE_RDONLYfrom AP[2] to a software bit #58, and adds additional logic to set AP[2] whenever the pte is read only or not dirty. That way we can distinguish between clean writeable ptes and read only ptes.HugeTLB pages will use this new logic automatically.
We need to add some logic to Transparent HugePages to ensure that they correctly interpret the revised pgprot permissions (
L_PTE_RDONLYhas moved and no longer matchesPMD_SECT_RDONLY). In the process of revising THP, the names of the PMD software bits have been prefixed with L_ to make them easier to distinguish from their hardware bit counterparts.
Quoted 8 lines
Quoted 5 lines
Signed-off-by Steve Capper <[email protected]>---
Changed in V5 -> rather than re-introduceL_PTE_WRITEwe changeL_PTE_RDONLYto be a software bit #58. This allows us to leave the two level code alone.Damnit, you owe me a beer every time I have to review this patch ;) At least
proc-macros.Sis unscathed this time around!
It's thirsty work writing and testing it too, I have a cunning plan that can fix both our problems.... :-)
Quoted 7 lines
Quoted 7 lines
--- arch/arm/include/asm/pgtable-3level-hwdef.h | 1 + arch/arm/include/asm/pgtable-3level.h | 37 +++++++++++++++++------------ arch/arm/mm/proc-v7-3level.S | 9 +++++-- 3 files changed, 30 insertions(+), 17 deletions(-)
Quoted 11 lines
arch/arm/include/asm/pgtable-3level-hwdef.h line 75
Quoted 9 lines
arch/arm/include/asm/pgtable-3level-hwdef.h +1
-0@@ -72,6 +72,7 @@#define PTE_TABLE_BIT (_AT(pteval_t, 1) << 1)#define PTE_BUFFERABLE (_AT(pteval_t, 1) << 2)/* AttrIndx[0] */ #define PTE_CACHEABLE (_AT(pteval_t, 1) << 3)/* AttrIndx[1] */ +#define PTE_AP2 (_AT(pteval_t, 1) << 7)/* AP[2] */
PTE_RDONLY? AP2 is pretty cryptic.
That is probably better, but could it be confused with L_PRE_RDONLY? (Please see below for the pmd analogue).
Quoted 16 lines
Quoted 16 lines
arch/arm/include/asm/pgtable-3level-hwdef.h +0
-0#define PTE_EXT_SHARED (_AT(pteval_t, 3) << 8)/* SH[1:0], inner shareable */ #define PTE_EXT_AF (_AT(pteval_t, 1) << 10)/* Access Flag */ #define PTE_EXT_NG (_AT(pteval_t, 1) << 11)/* nG */ arch/arm/include/asm/pgtable-3level.h +0
-1@@ -79,18 +79,19 @@#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] */ #define L_PTE_SHARED (_AT(pteval_t, 3) << 8)/* SH[1:0], inner shareable */ #define L_PTE_YOUNG (_AT(pteval_t, 1) << 10)/* AF */ #define L_PTE_XN (_AT(pteval_t, 1) << 54)/* XN */
Quoted 8 lines
arch/arm/include/asm/pgtable-3level.h line 88
Quoted 4 lines
arch/arm/include/asm/pgtable-3level.h +1
-0#define L_PTE_DIRTY (_AT(pteval_t, 1) << 55)/* unused */ #define L_PTE_SPECIAL (_AT(pteval_t, 1) << 56)/* unused */ #define L_PTE_NONE (_AT(pteval_t, 1) << 57)/* PROT_NONE */ +#define L_PTE_RDONLY (_AT(pteval_t, 1) << 58)/* READ ONLY */ Can you fix those `unused' comments above while you're here, please? Whilst they are unused by the hardware, they're very much used by Linux. Well, SPECIAL isn't yet, but it could be...
Sure.
Quoted 26 lines
arch/arm/include/asm/pgtable-3level.h line 94
Quoted 10 lines
arch/arm/include/asm/pgtable-3level.h +5
-4-#define PMD_SECT_VALID (_AT(pmdval_t, 1) << 0) -#define PMD_SECT_DIRTY (_AT(pmdval_t, 1) << 55) -#define PMD_SECT_SPLITTING (_AT(pmdval_t, 1) << 56) -#define PMD_SECT_NONE (_AT(pmdval_t, 1) << 57) +#define L_PMD_SECT_VALID (_AT(pmdval_t, 1) << 0) +#define L_PMD_SECT_DIRTY (_AT(pmdval_t, 1) << 55) +#define L_PMD_SECT_SPLITTING (_AT(pmdval_t, 1) << 56) +#define L_PMD_SECT_NONE (_AT(pmdval_t, 1) << 57) +#define L_PMD_SECT_RDONLY (_AT(pteval_t, 1) << 58)Boo-hoo, that's our last remaining software bit for LPAE. It's a sad day.
Quoted 13 lines
arch/arm/include/asm/pgtable-3level.h +2
-1/* * To be used in assembly code with the upper page attributes. @@ -214,24 +215,25 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)#define pmd_young (pmd) (pmd_isset((pmd), PMD_SECT_AF))#define __HAVE_ARCH_PMD_WRITE -#define pmd_write (pmd) (pmd_isclear((pmd), PMD_SECT_RDONLY)) +#define pmd_write (pmd) (pmd_isclear((pmd), L_PMD_SECT_RDONLY)) +#define pmd_dirty (pmd) (pmd_isset((pmd), L_PMD_SECT_DIRTY))#define pmd_hugewillfault (pmd) (!pmd_young(pmd) || !pmd_write(pmd))#define pmd_thp_or_huge (pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd))
Quoted 7 lines
arch/arm/include/asm/pgtable-3level.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 226
Quoted 4 lines
arch/arm/include/asm/pgtable-3level.h +1
-1#ifdef CONFIG_TRANSPARENT_HUGEPAGE #define pmd_trans_huge (pmd) (pmd_val(pmd) && pmd_isclear((pmd), PMD_TABLE_BIT)) -#define pmd_trans_splitting (pmd) (pmd_isset((pmd), PMD_SECT_SPLITTING)) +#define pmd_trans_splitting (pmd) (pmd_isset((pmd), L_PMD_SECT_SPLITTING))I take it we only call this on something we've decided is huge already (as opposed to, e.g. a swap entry)?
Yes that's correct, the sequence is pmd_trans_huge(blah), then later on pmd_trans_splitting(blah).
Quoted 43 lines
Quoted 30 lines
arch/arm/include/asm/pgtable-3level.h +6
-6#endif #define PMD_BIT_FUNC (fn,op) \ static inline pmd_t pmd_##fn(pmd_t pmd) { pmd_val(pmd) op; return pmd; } -PMD_BIT_FUNC(wrprotect, |= PMD_SECT_RDONLY); +PMD_BIT_FUNC(wrprotect, |= L_PMD_SECT_RDONLY); PMD_BIT_FUNC(mkold, &= ~PMD_SECT_AF); -PMD_BIT_FUNC(mksplitting, |= PMD_SECT_SPLITTING); -PMD_BIT_FUNC(mkwrite, &= ~PMD_SECT_RDONLY); -PMD_BIT_FUNC(mkdirty, |= PMD_SECT_DIRTY); +PMD_BIT_FUNC(mksplitting, |= L_PMD_SECT_SPLITTING); +PMD_BIT_FUNC(mkwrite, &= ~L_PMD_SECT_RDONLY); +PMD_BIT_FUNC(mkdirty, |= L_PMD_SECT_DIRTY); PMD_BIT_FUNC(mkyoung, |= PMD_SECT_AF);#define pmd_mkhuge (pmd) (__pmd(pmd_val(pmd) & ~PMD_TABLE_BIT))@@ -245,8 +247,8 @@ PMD_BIT_FUNC(mkyoung, |= PMD_SECT_AF);static inline pmd_t pmd_modify (pmd_t pmd,pgprot_t newprot) { -const pmdval_t mask =PMD_SECT_USER |PMD_SECT_XN |PMD_SECT_RDONLY | -PMD_SECT_VALID |PMD_SECT_NONE ; +const pmdval_t mask =PMD_SECT_USER |PMD_SECT_XN |L_PMD_SECT_RDONLY | +L_PMD_SECT_VALID |L_PMD_SECT_NONE ;pmd_val (pmd) = (pmd_val (pmd) & ~mask) | (pgprot_val (newprot) & mask);return pmd; }arch/arm/include/asm/pgtable-3level.h static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr, · line 266
Quoted 10 lines
arch/arm/include/asm/pgtable-3level.h +5
-2@@ -257,8 +259,13 @@ static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr,BUG_ON (addr >=TASK_SIZE );/* create a faulting entry if PROT_NONE protected */ -if (pmd_val (pmd) &PMD_SECT_NONE ) -pmd_val (pmd) &= ~PMD_SECT_VALID ; +if (pmd_val (pmd) &L_PMD_SECT_NONE ) +pmd_val (pmd) &= ~L_PMD_SECT_VALID ; + +if (pmd_write (pmd) &&pmd_dirty (pmd)) +pmd_val (pmd) &= ~PMD_SECT_RDONLY ;
pmd_mkwrite?
Quoted 4 lines
arch/arm/include/asm/pgtable-3level.h static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr, · line 268
Quoted 2 lines
arch/arm/include/asm/pgtable-3level.h +2
-0+else +pmd_val (pmd) |=PMD_SECT_RDONLY ;
pmd_wrprotect?
I'm setting the hardware bits here. I tried to distinguish:PMD_SECT_RDONLY and L_PMD_SECT_RDONLY. I think I should probably rename PMD_SECT_RDONLY to something else?
Quoted 12 lines
Quoted 12 lines
arch/arm/include/asm/pgtable-3level.h +0
-0*pmdp =__pmd (pmd_val (pmd) | PMD_SECT_nG);flush_pmd_entry (pmdp);arch/arm/mm/proc-v7-3level.S +0
-0@@ -86,8 +86,13 @@ ENTRY(cpu_v7_set_pte_ext) tst rh, #1 << (57 - 32) @ L_PTE_NONE bicne rl, #L_PTE_VALID bne 1f
Quoted 8 lines
arch/arm/mm/proc-v7-3level.S ENTRY(cpu_v7_set_pte_ext) · line 91
Quoted 5 lines
arch/arm/mm/proc-v7-3level.S +3
-2- tst rh, #1 << (55 - 32) @ L_PTE_DIRTY - orreq rl, #L_PTE_RDONLY + + eor ip, rh, #1 << (55 - 32) @ toggle L_PTE_DIRTY in temp reg to + @ test for !L_PTE_DIRTY || L_PTE_RDONLYSince you already set RDONLY for pmds based on dirty, can we do the same for ptes and avoid this check here...?
Isn't the pte check logically equivalent to the pmd case?
if (pmd_write(pmd) && pmd_dirty(pmd))
Quoted 7 lines
arch/arm/mm/proc-v7-3level.S ENTRY(cpu_v7_set_pte_ext) · line 94
Quoted 4 lines
arch/arm/mm/proc-v7-3level.S +3
-0+ tst ip, #1 << (55 - 32) | 1 << (58 - 32) + orrne rl, #PTE_AP2 + biceq rl, #PTE_AP2... Then use Russell's ubfx trick to copy
L_PTE_RDONLYintoPTE_RDONLY?
I will have another think about this.
Cheers,
Signature · 1 line
Steve
PATCH v5 2/2 arm: mm: Modify pte_write and pmd_write logic for LPAE
Quoted 14 lines · Steve Capper
Quoted 12 lines · Will Deacon
arch/arm/include/asm/pgtable-3level-hwdef.h line 75
Quoted 9 lines · Steve Capper
arch/arm/include/asm/pgtable-3level-hwdef.h +1
-0@@ -72,6 +72,7 @@#define PTE_TABLE_BIT (_AT(pteval_t, 1) << 1)#define PTE_BUFFERABLE (_AT(pteval_t, 1) << 2)/* AttrIndx[0] */ #define PTE_CACHEABLE (_AT(pteval_t, 1) << 3)/* AttrIndx[1] */ +#define PTE_AP2 (_AT(pteval_t, 1) << 7)/* AP[2] */
PTE_RDONLY? AP2 is pretty cryptic.
Quoted 2 lines
That is probably better, but could it be confused with
L_PRE_RDONLY? (Please see below for the pmd analogue).
Comments below, I seem to have fallen into my own trap.
Quoted 14 lines
Quoted 14 lines
arch/arm/include/asm/pgtable-3level-hwdef.h static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr, · line 266
Quoted 11 lines
arch/arm/include/asm/pgtable-3level-hwdef.h +5
-2@@ -257,8 +259,13 @@ static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr,BUG_ON (addr >=TASK_SIZE );/* create a faulting entry if PROT_NONE protected */ -if (pmd_val (pmd) &PMD_SECT_NONE ) -pmd_val (pmd) &= ~PMD_SECT_VALID ; +if (pmd_val (pmd) &L_PMD_SECT_NONE ) +pmd_val (pmd) &= ~L_PMD_SECT_VALID ; + +if (pmd_write (pmd) &&pmd_dirty (pmd)) +pmd_val (pmd) &= ~PMD_SECT_RDONLY ;
pmd_mkwrite?
Quoted 8 lines
Quoted 4 lines
arch/arm/include/asm/pgtable-3level-hwdef.h static inline void set_pmd_at(struct mm_struct *mm, unsigned long addr, · line 268
Quoted 2 lines
arch/arm/include/asm/pgtable-3level-hwdef.h +2
-0+else +pmd_val (pmd) |=PMD_SECT_RDONLY ;
pmd_wrprotect?I'm setting the hardware bits here. I tried to distinguish:
PMD_SECT_RDONLYandL_PMD_SECT_RDONLY. I think I should probably renamePMD_SECT_RDONLYto something else?
Ok, so that's actually a really good argument to keep AP2 as the name for both ptes and pmds. In that case, the comment we have " /* AP[2] */ is useless and should say "RDONLY". Then we have non-ambiguous symbol names alongside descriptive comments -- not something I'm used to in the kernel sources!
Will