Re: [RFC PATCH 1/5] entry: Pass pt_regs to irqentry_exit_cond_resched()

From: Borislav Petkov <bp@alien8.de>
To: 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
Subject: Re: [RFC PATCH 1/5] entry: Pass pt_regs to irqentry_exit_cond_resched()
Date: Mon, 8 Aug 2022 12:38:08 +0200	[thread overview]
Message-ID: <YvDnkALyHl77R/Ug@zn.tnic> (raw)
In-Reply-To: <20220805173009.3128098-2-ira.weiny@intel.com>

On Fri, Aug 05, 2022 at 10:30:05AM -0700, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> Auxiliary pt_regs space needs to be manipulated by the generic
> entry/exit code.
> 
> Ideally irqentry_exit() would take care of handling any auxiliary
> pt_regs on exit.  Unfortunately, irqentry_exit() is not the only exit
> from exception path.  The call to irqentry_exit_cond_resched() from
> xen_pv_evtchn_do_upcall() bypasses irqentry_exit().
> 
> Make irqentry_exit_cond_resched() symmetrical with irqentry_enter() by
> passing pt_regs to it.  This makes irqentry_exit_cond_resched() capable
> of handling auxiliary pt_regs in future patches.
> 
> Cc: Rik van Riel <riel@surriel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> ---
> Forward ported from PKS series:
> 	https://lore.kernel.org/lkml/20220419170649.1022246-19-ira.weiny@intel.com/
> ---
>  arch/arm64/include/asm/preempt.h |  2 +-
>  arch/arm64/kernel/entry-common.c |  4 ++--
>  arch/x86/entry/common.c          |  2 +-
>  include/linux/entry-common.h     | 17 ++++++++------
>  kernel/entry/common.c            | 13 +++++++----
>  kernel/sched/core.c              | 40 ++++++++++++++++----------------
>  6 files changed, 43 insertions(+), 35 deletions(-)

Why all this churn?

Why can't you add a parameter to irqentry_exit():

  noinstr void irqentry_exit(struct pt_regs *regs, irqentry_state_t state, bool cond_resched);

and then have all callers except xen_pv_evtchn_do_upcall() pass in false
and this way have all exit paths end up in irqentry_exit()?

And, ofc, move the true case which is the body of
raw_irqentry_exit_cond_resched() to irqentry_exit() and then get rid of
former.

xen_pv_evtchn_do_upcall() will, ofc, do:

        if (inhcall && !WARN_ON_ONCE(state.exit_rcu)) {
                irqentry_exit(regs, state, true);
                instrumentation_end();
                restore_inhcall(inhcall);
        } else {
                instrumentation_end();
                irqentry_exit(regs, state, false);

Hmmm?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette