| Message ID | 20180710222639.8241-12-yu-cheng.yu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/10/2018 03:26 PM, Yu-cheng Yu wrote: > + /* > + * On platforms before CET, other threads could race to > + * create a RO and _PAGE_DIRTY_HW PMD again. However, > + * on CET platforms, this is safe without a TLB flush. > + */ If I didn't work for Intel, I'd wonder what the heck CET is and what the heck it has to do with _PAGE_DIRTY_HW. I think we need a better comment than this. How about: Some processors can _start_ a write, but end up seeing a read-only PTE by the time they get to getting the Dirty bit. In this case, they will set the Dirty bit, leaving a read-only, Dirty PTE which looks like a Shadow Stack PTE. However, this behavior has been improved and will *not* occur on processors supporting Shadow Stacks. Without this guarantee, a transition to a non-present PTE and flush the TLB would be needed.
at 6:44 PM, Dave Hansen <dave.hansen@linux.intel.com> wrote: > On 07/10/2018 03:26 PM, Yu-cheng Yu wrote: >> + /* >> + * On platforms before CET, other threads could race to >> + * create a RO and _PAGE_DIRTY_HW PMD again. However, >> + * on CET platforms, this is safe without a TLB flush. >> + */ > > If I didn't work for Intel, I'd wonder what the heck CET is and what the > heck it has to do with _PAGE_DIRTY_HW. I think we need a better comment > than this. How about: > > Some processors can _start_ a write, but end up seeing > a read-only PTE by the time they get to getting the > Dirty bit. In this case, they will set the Dirty bit, > leaving a read-only, Dirty PTE which looks like a Shadow > Stack PTE. > > However, this behavior has been improved and will *not* occur on > processors supporting Shadow Stacks. Without this guarantee, a > transition to a non-present PTE and flush the TLB would be > needed. Interesting. Does that regard the knights landing bug or something more general? Will the write succeed or trigger a page-fault in this case? [ I know it is not related to the patch, but I would appreciate if you share your knowledge ] Regards, Nadav
On 07/10/2018 04:23 PM, Nadav Amit wrote: > at 6:44 PM, Dave Hansen <dave.hansen@linux.intel.com> wrote: > >> On 07/10/2018 03:26 PM, Yu-cheng Yu wrote: >>> + /* >>> + * On platforms before CET, other threads could race to >>> + * create a RO and _PAGE_DIRTY_HW PMD again. However, >>> + * on CET platforms, this is safe without a TLB flush. >>> + */ >> >> If I didn't work for Intel, I'd wonder what the heck CET is and what the >> heck it has to do with _PAGE_DIRTY_HW. I think we need a better comment >> than this. How about: >> >> Some processors can _start_ a write, but end up seeing >> a read-only PTE by the time they get to getting the >> Dirty bit. In this case, they will set the Dirty bit, >> leaving a read-only, Dirty PTE which looks like a Shadow >> Stack PTE. >> >> However, this behavior has been improved and will *not* occur on >> processors supporting Shadow Stacks. Without this guarantee, a >> transition to a non-present PTE and flush the TLB would be >> needed. > > Interesting. Does that regard the knights landing bug or something more > general? It's more general. > Will the write succeed or trigger a page-fault in this case? It will trigger a page fault.
On Tue, Jul 10, 2018 at 03:44:32PM -0700, Dave Hansen wrote: > On 07/10/2018 03:26 PM, Yu-cheng Yu wrote: > > + /* > > + * On platforms before CET, other threads could race to > > + * create a RO and _PAGE_DIRTY_HW PMD again. However, > > + * on CET platforms, this is safe without a TLB flush. > > + */ > > If I didn't work for Intel, I'd wonder what the heck CET is and what the > heck it has to do with _PAGE_DIRTY_HW. I think we need a better comment And Changelog, the provided one is abysmal. > than this. How about: > > Some processors can _start_ a write, but end up seeing > a read-only PTE by the time they get to getting the > Dirty bit. In this case, they will set the Dirty bit, > leaving a read-only, Dirty PTE which looks like a Shadow > Stack PTE. > > However, this behavior has been improved and will *not* occur on > processors supporting Shadow Stacks. Without this guarantee, a > transition to a non-present PTE and flush the TLB would be > needed. I'm still struggling. I think I get the first paragraph, but then what?
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h index ecbd3539a864..456a864aa605 100644 --- a/arch/x86/include/asm/pgtable.h +++ b/arch/x86/include/asm/pgtable.h @@ -1170,7 +1170,18 @@ static inline pte_t ptep_get_and_clear_full(struct mm_struct *mm, static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addr, pte_t *ptep) { + pte_t pte; + clear_bit(_PAGE_BIT_RW, (unsigned long *)&ptep->pte); + pte = *ptep; + + /* + * On platforms before CET, other threads could race to + * create a RO and _PAGE_DIRTY_HW PTE again. However, + * on CET platforms, this is safe without a TLB flush. + */ + pte = pte_move_flags(pte, _PAGE_DIRTY_HW, _PAGE_DIRTY_SW); + set_pte_at(mm, addr, ptep, pte); } #define flush_tlb_fix_spurious_fault(vma, address) do { } while (0) @@ -1220,7 +1231,18 @@ static inline pud_t pudp_huge_get_and_clear(struct mm_struct *mm, static inline void pmdp_set_wrprotect(struct mm_struct *mm, unsigned long addr, pmd_t *pmdp) { + pmd_t pmd; + clear_bit(_PAGE_BIT_RW, (unsigned long *)pmdp); + pmd = *pmdp; + + /* + * On platforms before CET, other threads could race to + * create a RO and _PAGE_DIRTY_HW PMD again. However, + * on CET platforms, this is safe without a TLB flush. + */ + pmd = pmd_move_flags(pmd, _PAGE_DIRTY_HW, _PAGE_DIRTY_SW); + set_pmd_at(mm, addr, pmdp, pmd); } #define pud_write pud_write
Update ptep_set_wrprotect() and pmdp_set_wrprotect() for _PAGE_DIRTY_SW. Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> --- arch/x86/include/asm/pgtable.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)