diff mbox

[RFC,v2,11/27] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW

Message ID 20180710222639.8241-12-yu-cheng.yu@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yu, Yu-cheng July 10, 2018, 10:26 p.m. UTC
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(+)

Comments

Dave Hansen July 10, 2018, 10:44 p.m. UTC | #1
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.
Nadav Amit July 10, 2018, 11:23 p.m. UTC | #2
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
Dave Hansen July 10, 2018, 11:52 p.m. UTC | #3
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.
Peter Zijlstra July 11, 2018, 8:48 a.m. UTC | #4
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 mbox

Patch

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