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;
Date:   Mon, 8 Aug 2022 10:34:19 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     Borislav Petkov <bp@alien8.de>
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()
Message-ID: <YvFJGxU0k492npYT@iweiny-desk3>
References: <20220805173009.3128098-1-ira.weiny@intel.com>
 <20220805173009.3128098-2-ira.weiny@intel.com>
 <YvDnkALyHl77R/Ug@zn.tnic>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <YvDnkALyHl77R/Ug@zn.tnic>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?sp1yGhP/yUayeFiC2tTvLNnRewXE41EQTNeInXNy04zTiAldsRcwyLBNNwq+?=
 =?us-ascii?Q?AhRGCLXhxaRaoPPU6h2YR4CEEzzt+0cIOvwyXHtNCW/peJn7faHN9x3OpfLz?=
 =?us-ascii?Q?EA6NSJFabsCOUjHKdKMnetNym5sPGSLMHGoQ/CiN9/+NR/AMv9/aM06K4ECI?=
 =?us-ascii?Q?idgGSh+W5HEjxgzNMZV+z5gLzi5OrVoe2m+pax3eyZ6dehZ0/2zkp9z0Q0yF?=
 =?us-ascii?Q?Ysn/00RYPLO/P23GALoGRLoxLddbRIRC9GFLraifPuYYGW/Pts6AR++2K0w5?=
 =?us-ascii?Q?boUwOX/OB8EWlOp1pO5D1/Gkm4WKr49zrWa2qfXglhLeLVOHAI7gaVkLYy9d?=
 =?us-ascii?Q?+Jivl12DZHJRfyERQjXX/y0BmWaWwlggfc8pYa2Mcb7Uh5Of8L2ylYS91BC+?=
 =?us-ascii?Q?D71LtU0NgczhDEnAcUySTDPG8kyf9yvoibeEThephhBmNk8pchmkA40PoSRF?=
 =?us-ascii?Q?F1KORvwFEVAhfC19hRuTev1Jw3iOrLuT6JRL7r6Nxv4i9o8EjrvuyDtSF1I2?=
 =?us-ascii?Q?yBW6ONwU6LeUiNHdiivisGygLilHZkcB4OvlslbBlK2m9mM7VnjtKGiSCB2u?=
 =?us-ascii?Q?VOSZyglS7JctFo/lzxreTWNoX4GWBe2RzBJTb5tK2PZJK7nyZcIO6vLTl6ww?=
 =?us-ascii?Q?SQfG4kBIB2BxwpdgCXCvaqt+fDeZMUr3o/m/xkuvbTdVjHcc/GyEvnYQCHA5?=
 =?us-ascii?Q?9BB4AYqW/9OAjQvm1L0My2BmGDyoVfTwdhCnhxc0amUe1OdA8dBY3Hxmsi6y?=
 =?us-ascii?Q?FyoiISynhK4u3pj29Sw34dTE2Xh/gcicO/nU8UuBdn1Z2Iiq8Z+95t/hNBEI?=
 =?us-ascii?Q?5jO/Qln9uSH52gRqoaSCTwFLNvI2SZulhf40je/AM2mtnY/k8M/YYvS5SC0v?=
 =?us-ascii?Q?G4F/v66iZyfoouaHvSzxvlhTnNaZdkNNnjI5k1tduFPCYOPLnGLAkrbQptt3?=
 =?us-ascii?Q?NKV4pZfgGYRsygIsA3cG6E8eXvtiNxqggKHNbUd5VFouO9OdUCaS5+w51NLy?=
 =?us-ascii?Q?V408US5JWt5DsCDDs7lhiFW610LmhNpwkT0677tl8Z+FTncr/OAiNSMeMg5y?=
 =?us-ascii?Q?2DZ9YsfU8PqVhdDPIogcnUvflkupsmEs8KaWk+ptsPOy3FmdKscm3leSQvLa?=
 =?us-ascii?Q?FKBkh2NDbJyqa/xPxTJnPMPDhL1HjMI4h9f5a3YwBY+4aDV71QyM0Ozv+0iy?=
 =?us-ascii?Q?sbzYmiohmRa2lmiC5eeWZTIpJwIka/z2GxLZsEsUNGqAWfHuwVZznI5ehQQh?=
 =?us-ascii?Q?1WC581UyTGhj/mqCKJmDKRd1OmmcAydN/hHiTa+h7j78a8kfD7fxkRPE8+zC?=
 =?us-ascii?Q?RHK1XPBSw4/+lh15CESCEZDPH+QZUZ057PdjQqyIKWmYNH5bS9Oqso/rdao4?=
 =?us-ascii?Q?+qjsYs+grjv0d40JQcxpAq6nt0jFXLjuMcScZ6WOFLvhTcnyfHR4Xy3cfi6a?=
 =?us-ascii?Q?w43GOWG1vMTjuZy//qsn48QMwjCLFpSILeznFyB/srINRWG+g6pOIjpjyOo+?=
 =?us-ascii?Q?NjhHuuiykciDzEGl9jaSrgFZQd9y2H9dPFcWeHvm1iXB2Hfg/M3HmOSZqS8V?=
 =?us-ascii?Q?eM//eIaktU5/sg1z3Vn+j9h9Nw9qvJaISBJCe9ob?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4626edf3-2f54-4011-0f7b-08da79643fd3
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6311.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Aug 2022 17:34:30.2564
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: /mKuSe7S/Vojz9ZlIDdnubxNteNUmDMUT95X9ybHWDvE8vw/Ih7982tIv0RHsblPe65RM2g03lG7Ioj4oIBKUw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR11MB1353
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Aug 08, 2022 at 12:38:08PM +0200, Borislav Petkov wrote:
> 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);

I thought about that but generally have been steered away from using bool
arguments like this.

> 
> 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.

Looking this over I think this may be a reasonable use of a flag like that.

I'm not sure if this series is going to land given the discussion of
performance.  Would this clean up be welcome on its own?  I'm willing to respin
this patch and submit if so.

Ira

> 
> 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