[PATCH 0/2] x86/context-tracking: Remove last remaining calls to exception_enter/exception_exit()

[PATCH 0/2] x86/context-tracking: Remove last remaining calls to exception_enter/exception_exit()

[Frederic Weisbecker]

Thanks to the cleanups from Andy Lutomirski over the years, those calls
can now be removed. This will allow for nice things in the future for
x86 support on full nohz:

* Remove TIF_NOHZ and use a per-cpu switch to enable, disable context
  tracking.

* Avoid context tracking on housekeepers.

* Dynamically enable/disable context tracking on CPU on runtime and
  therefore allow runtime enable/disable of nohz_full

* Make nohz_full a property of cpuset.

Frederic Weisbecker (2):
  x86/context-tracking: Remove exception_enter/exit() from
    do_page_fault()
  x86/context-tracking: Remove exception_enter/exit() from
    KVM_PV_REASON_PAGE_NOT_PRESENT async page fault

 arch/x86/kernel/kvm.c |  4 ----
 arch/x86/mm/fault.c   | 39 ++++++++++++---------------------------
 2 files changed, 12 insertions(+), 31 deletions(-)

-- 
2.23.0

[PATCH 1/2] x86/context-tracking: Remove exception_enter/exit() from do_page_fault()

[Frederic Weisbecker]

do_page_fault(), like other exceptions, is already covered by
user_enter() and user_exit() when the exception triggers in userspace.

As explained in 8c84014f3bbb112d07e73f30a10ac8a3a72f8649
("x86/entry: Remove exception_enter() from most trap handlers"),
exception_enter/exit() only remained to handle possible page fault from
kernel mode while context tracking is in CONTEXT_USER mode, ie: on
kernel entry before we manage to call user_exit(). And the only known
offender was do_fast_syscall_32() fetching EBP register from where
vDSO stashed it.

Meanwhile this got fixed with 9999c8c01f34c918a57d6e5ba2f5d8b79aa04801
("x86/entry: Call enter_from_user_mode() with IRQs off") that moved
enter_from_user_mode() before the call to get_user().

So we can safely remove it now.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/x86/mm/fault.c | 39 ++++++++++++---------------------------
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 304d31d8cbbc..2b4ab2862eda 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1486,27 +1486,6 @@ void do_user_addr_fault(struct pt_regs *regs,
 }
 NOKPROBE_SYMBOL(do_user_addr_fault);
 
-/*
- * Explicitly marked noinline such that the function tracer sees this as the
- * page_fault entry point.
- */
-static noinline void
-__do_page_fault(struct pt_regs *regs, unsigned long hw_error_code,
-		unsigned long address)
-{
-	prefetchw(&current->mm->mmap_sem);
-
-	if (unlikely(kmmio_fault(regs, address)))
-		return;
-
-	/* Was the fault on kernel-controlled part of the address space? */
-	if (unlikely(fault_in_kernel_space(address)))
-		do_kern_addr_fault(regs, hw_error_code, address);
-	else
-		do_user_addr_fault(regs, hw_error_code, address);
-}
-NOKPROBE_SYMBOL(__do_page_fault);
-
 static __always_inline void
 trace_page_fault_entries(struct pt_regs *regs, unsigned long error_code,
 			 unsigned long address)
@@ -1521,13 +1500,19 @@ trace_page_fault_entries(struct pt_regs *regs, unsigned long error_code,
 }
 
 dotraplinkage void
-do_page_fault(struct pt_regs *regs, unsigned long error_code, unsigned long address)
+do_page_fault(struct pt_regs *regs, unsigned long hw_error_code,
+		unsigned long address)
 {
-	enum ctx_state prev_state;
+	prefetchw(&current->mm->mmap_sem);
+	trace_page_fault_entries(regs, hw_error_code, address);
 
-	prev_state = exception_enter();
-	trace_page_fault_entries(regs, error_code, address);
-	__do_page_fault(regs, error_code, address);
-	exception_exit(prev_state);
+	if (unlikely(kmmio_fault(regs, address)))
+		return;
+
+	/* Was the fault on kernel-controlled part of the address space? */
+	if (unlikely(fault_in_kernel_space(address)))
+		do_kern_addr_fault(regs, hw_error_code, address);
+	else
+		do_user_addr_fault(regs, hw_error_code, address);
 }
 NOKPROBE_SYMBOL(do_page_fault);
-- 
2.23.0

[tip: x86/asm] x86/context-tracking: Remove exception_enter/exit() from do_page_fault()

[tip-bot2 for Frederic Weisbecker]

The following commit has been merged into the x86/asm branch of tip:

Commit-ID:     ee6352b2c47a24234398e06381edd93a8e965976
Gitweb:        https://git.kernel.org/tip/ee6352b2c47a24234398e06381edd93a8e965976
Author:        Frederic Weisbecker <frederic@kernel.org>
AuthorDate:    Fri, 27 Dec 2019 17:36:11 +01:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 07 Jan 2020 08:11:23 +01:00

x86/context-tracking: Remove exception_enter/exit() from do_page_fault()

do_page_fault(), like other exceptions, is already covered by
user_enter() and user_exit() when the exception triggers in userspace.

As explained in:

  8c84014f3bbb11 ("x86/entry: Remove exception_enter() from most trap handlers")

exception_enter/exit() only remained to handle possible page fault from
kernel mode while context tracking is in CONTEXT_USER mode, ie: on
kernel entry before we manage to call user_exit(). The only known
offender was do_fast_syscall_32() fetching EBP register from where
vDSO stashed it.

Meanwhile this got fixed in:

  9999c8c01f34c9 ("x86/entry: Call enter_from_user_mode() with IRQs off")

that moved enter_from_user_mode() before the call to get_user().

So we can safely remove it now.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Jim Mattson <jmattson@google.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Wanpeng Li <wanpengli@tencent.com>
Link: https://lkml.kernel.org/r/20191227163612.10039-2-frederic@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/mm/fault.c | 39 ++++++++++++---------------------------
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 304d31d..2b4ab28 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1486,27 +1486,6 @@ good_area:
 }
 NOKPROBE_SYMBOL(do_user_addr_fault);
 
-/*
- * Explicitly marked noinline such that the function tracer sees this as the
- * page_fault entry point.
- */
-static noinline void
-__do_page_fault(struct pt_regs *regs, unsigned long hw_error_code,
-		unsigned long address)
-{
-	prefetchw(&current->mm->mmap_sem);
-
-	if (unlikely(kmmio_fault(regs, address)))
-		return;
-
-	/* Was the fault on kernel-controlled part of the address space? */
-	if (unlikely(fault_in_kernel_space(address)))
-		do_kern_addr_fault(regs, hw_error_code, address);
-	else
-		do_user_addr_fault(regs, hw_error_code, address);
-}
-NOKPROBE_SYMBOL(__do_page_fault);
-
 static __always_inline void
 trace_page_fault_entries(struct pt_regs *regs, unsigned long error_code,
 			 unsigned long address)
@@ -1521,13 +1500,19 @@ trace_page_fault_entries(struct pt_regs *regs, unsigned long error_code,
 }
 
 dotraplinkage void
-do_page_fault(struct pt_regs *regs, unsigned long error_code, unsigned long address)
+do_page_fault(struct pt_regs *regs, unsigned long hw_error_code,
+		unsigned long address)
 {
-	enum ctx_state prev_state;
+	prefetchw(&current->mm->mmap_sem);
+	trace_page_fault_entries(regs, hw_error_code, address);
 
-	prev_state = exception_enter();
-	trace_page_fault_entries(regs, error_code, address);
-	__do_page_fault(regs, error_code, address);
-	exception_exit(prev_state);
+	if (unlikely(kmmio_fault(regs, address)))
+		return;
+
+	/* Was the fault on kernel-controlled part of the address space? */
+	if (unlikely(fault_in_kernel_space(address)))
+		do_kern_addr_fault(regs, hw_error_code, address);
+	else
+		do_user_addr_fault(regs, hw_error_code, address);
 }
 NOKPROBE_SYMBOL(do_page_fault);

[PATCH 2/2] x86/context-tracking: Remove exception_enter/exit() from KVM_PV_REASON_PAGE_NOT_PRESENT async page fault

[Frederic Weisbecker]

This is a leftover. Page faults, just like most other exceptions,
are protected inside user_exit() / user_enter() calls in x86 entry code
when we fault from userspace. So this pair of calls is now useless.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Wanpeng Li <wanpengli@tencent.com>
Cc: Jim Mattson <jmattson@google.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kernel/kvm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 32ef1ee733b7..81045aabb6f4 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -245,17 +245,13 @@ NOKPROBE_SYMBOL(kvm_read_and_reset_pf_reason);
 dotraplinkage void
 do_async_page_fault(struct pt_regs *regs, unsigned long error_code, unsigned long address)
 {
-	enum ctx_state prev_state;
-
 	switch (kvm_read_and_reset_pf_reason()) {
 	default:
 		do_page_fault(regs, error_code, address);
 		break;
 	case KVM_PV_REASON_PAGE_NOT_PRESENT:
 		/* page is swapped out by the host. */
-		prev_state = exception_enter();
 		kvm_async_pf_task_wait((u32)address, !user_mode(regs));
-		exception_exit(prev_state);
 		break;
 	case KVM_PV_REASON_PAGE_READY:
 		rcu_irq_enter();
-- 
2.23.0

[tip: x86/asm] x86/context-tracking: Remove exception_enter/exit() from KVM_PV_REASON_PAGE_NOT_PRESENT async page fault

[tip-bot2 for Frederic Weisbecker]

The following commit has been merged into the x86/asm branch of tip:

Commit-ID:     50cc02e599ef1e049473358604dc07d32b751e2c
Gitweb:        https://git.kernel.org/tip/50cc02e599ef1e049473358604dc07d32b751e2c
Author:        Frederic Weisbecker <frederic@kernel.org>
AuthorDate:    Fri, 27 Dec 2019 17:36:12 +01:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 07 Jan 2020 08:11:23 +01:00

x86/context-tracking: Remove exception_enter/exit() from KVM_PV_REASON_PAGE_NOT_PRESENT async page fault

This is a leftover. Page faults, just like most other exceptions,
are protected inside user_exit() / user_enter() calls in x86 entry code
when we fault from userspace. So this pair of calls is now superfluous.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Jim Mattson <jmattson@google.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Wanpeng Li <wanpengli@tencent.com>
Link: https://lkml.kernel.org/r/20191227163612.10039-3-frederic@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/kvm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 32ef1ee..81045aa 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -245,17 +245,13 @@ NOKPROBE_SYMBOL(kvm_read_and_reset_pf_reason);
 dotraplinkage void
 do_async_page_fault(struct pt_regs *regs, unsigned long error_code, unsigned long address)
 {
-	enum ctx_state prev_state;
-
 	switch (kvm_read_and_reset_pf_reason()) {
 	default:
 		do_page_fault(regs, error_code, address);
 		break;
 	case KVM_PV_REASON_PAGE_NOT_PRESENT:
 		/* page is swapped out by the host. */
-		prev_state = exception_enter();
 		kvm_async_pf_task_wait((u32)address, !user_mode(regs));
-		exception_exit(prev_state);
 		break;
 	case KVM_PV_REASON_PAGE_READY:
 		rcu_irq_enter();

Re: [PATCH 0/2] x86/context-tracking: Remove last remaining calls to exception_enter/exception_exit()

[Peter Zijlstra]

On Fri, Dec 27, 2019 at 05:36:10PM +0100, Frederic Weisbecker wrote:
> Thanks to the cleanups from Andy Lutomirski over the years, those calls
> can now be removed. This will allow for nice things in the future for
> x86 support on full nohz:
> 
> * Remove TIF_NOHZ and use a per-cpu switch to enable, disable context
>   tracking.
> 
> * Avoid context tracking on housekeepers.
> 
> * Dynamically enable/disable context tracking on CPU on runtime and
>   therefore allow runtime enable/disable of nohz_full
> 
> * Make nohz_full a property of cpuset.
> 
> Frederic Weisbecker (2):
>   x86/context-tracking: Remove exception_enter/exit() from
>     do_page_fault()
>   x86/context-tracking: Remove exception_enter/exit() from
>     KVM_PV_REASON_PAGE_NOT_PRESENT async page fault

Thanks, these look good to me.

Did I tell you about my 'TODO' item vs text_poke_sync() ? Basically, we
can avoid sending the IPI to NOHZ_FULL user CPUs, provided we make their
enter_from_user_mode() do the sync_core().

Re: [PATCH 0/2] x86/context-tracking: Remove last remaining calls to exception_enter/exception_exit()

[Frederic Weisbecker]

On Mon, Jan 06, 2020 at 04:05:46PM +0100, Peter Zijlstra wrote:
> On Fri, Dec 27, 2019 at 05:36:10PM +0100, Frederic Weisbecker wrote:
> > Thanks to the cleanups from Andy Lutomirski over the years, those calls
> > can now be removed. This will allow for nice things in the future for
> > x86 support on full nohz:
> > 
> > * Remove TIF_NOHZ and use a per-cpu switch to enable, disable context
> >   tracking.
> > 
> > * Avoid context tracking on housekeepers.
> > 
> > * Dynamically enable/disable context tracking on CPU on runtime and
> >   therefore allow runtime enable/disable of nohz_full
> > 
> > * Make nohz_full a property of cpuset.
> > 
> > Frederic Weisbecker (2):
> >   x86/context-tracking: Remove exception_enter/exit() from
> >     do_page_fault()
> >   x86/context-tracking: Remove exception_enter/exit() from
> >     KVM_PV_REASON_PAGE_NOT_PRESENT async page fault
> 
> Thanks, these look good to me.
> 
> Did I tell you about my 'TODO' item vs text_poke_sync() ? Basically, we
> can avoid sending the IPI to NOHZ_FULL user CPUs, provided we make their
> enter_from_user_mode() do the sync_core().

Yeah I believe I saw some email about that somewhere. I hope I didn't miss
a patch about that. Anyway, whether past or future patch, I'll happily review that :-)