Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Borislav Petkov <bp@alien8.de>, ira.weiny@intel.com
Cc:     Rik van Riel <riel@surriel.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        linux-kernel@vger.kernel.org, kernel-team@fb.com,
        Frederic Weisbecker <frederic@kernel.org>,
        Juergen Gross <jgross@suse.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [RFC PATCH 1/5] entry: Pass pt_regs to
 irqentry_exit_cond_resched()
In-Reply-To: <87fsi5qa49.ffs@tglx>
References: <20220805173009.3128098-1-ira.weiny@intel.com>
 <20220805173009.3128098-2-ira.weiny@intel.com> <YvDnkALyHl77R/Ug@zn.tnic>
 <87fsi5qa49.ffs@tglx>
Date:   Wed, 10 Aug 2022 09:25:48 +0200
Message-ID: <87a68cr24j.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Wed, Aug 10 2022 at 01:18, Thomas Gleixner wrote:
> Both clearly demonstrate how well tested this XEN_PV muck is which means
> we should just delete it.
>
> Anyway. This wants the fix below.

which is not sufficient. If CONFIG_PREEMPT_DYNAMIC is set then
CONFIG_PREEMPTION is set too, which makes the preemtible hypercall magic
a complete NOP. So if the dynamic call is disabled, then such a several
seconds (sic!) hypercall cannot be preempted at all.

Something like the below then. Oh well...

Thanks,

        tglx
---
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -253,7 +253,8 @@ SYSCALL_DEFINE0(ni_syscall)
 }
 
 #ifdef CONFIG_XEN_PV
-#ifndef CONFIG_PREEMPTION
+
+#if !defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC)
 /*
  * Some hypercalls issued by the toolstack can take many 10s of
  * seconds. Allow tasks running hypercalls via the privcmd driver to
@@ -283,9 +284,18 @@ static __always_inline void restore_inhc
 {
 	__this_cpu_write(xen_in_preemptible_hcall, inhcall);
 }
+
+static __always_inline void xenpv_irqentry_exit_cond_resched(void)
+{
+	instrumentation_begin();
+	raw_irqentry_exit_cond_resched();
+	trace_hardirqs_on();
+	instrumentation_end();
+}
 #else
 static __always_inline bool get_and_clear_inhcall(void) { return false; }
 static __always_inline void restore_inhcall(bool inhcall) { }
+static __always_inline void xenpv_irqentry_exit_cond_resched(void) { }
 #endif
 
 static void __xen_pv_evtchn_do_upcall(struct pt_regs *regs)
@@ -306,11 +316,11 @@ static void __xen_pv_evtchn_do_upcall(st
 
 	instrumentation_begin();
 	run_sysvec_on_irqstack_cond(__xen_pv_evtchn_do_upcall, regs);
+	instrumentation_end();
 
 	inhcall = get_and_clear_inhcall();
 	if (inhcall && !WARN_ON_ONCE(state.exit_rcu)) {
-		irqentry_exit_cond_resched();
-		instrumentation_end();
+		xenpv_irqentry_exit_cond_resched();
 		restore_inhcall(inhcall);
 	} else {
 		instrumentation_end();
--- a/include/linux/entry-common.h
+++ b/include/linux/entry-common.h
@@ -416,6 +416,7 @@ irqentry_state_t noinstr irqentry_enter(
  * Conditional reschedule with additional sanity checks.
  */
 void raw_irqentry_exit_cond_resched(void);
+
 #ifdef CONFIG_PREEMPT_DYNAMIC
 #if defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 #define irqentry_exit_cond_resched_dynamic_enabled	raw_irqentry_exit_cond_resched
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -194,7 +194,7 @@ bool xen_running_on_version_or_later(uns
 void xen_efi_runtime_setup(void);
 
 
-#if defined(CONFIG_XEN_PV) && !defined(CONFIG_PREEMPTION)
+#if defined(CONFIG_XEN_PV) && (!defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC))
 
 DECLARE_PER_CPU(bool, xen_in_preemptible_hcall);
 
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -385,6 +385,7 @@ void raw_irqentry_exit_cond_resched(void
 			preempt_schedule_irq();
 	}
 }
+
 #ifdef CONFIG_PREEMPT_DYNAMIC
 #if defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 DEFINE_STATIC_CALL(irqentry_exit_cond_resched, raw_irqentry_exit_cond_resched);