Re: [PATCH v3 RESEND] x86/entry: Avoid redundant CR3 write on paranoid returns

[Yosry Ahmed]

On Mon, Jan 8, 2024 at 3:39 AM Brendan Jackman <jackmanb@xxxxxxxxxx> wrote:
>
> From: Lai Jiangshan <laijs@xxxxxxxxxxxxxxxxx>
>
> This path gets used called from:
>
> 1. #NMI return.
> 2. paranoid_exit (i.e. #MCE, #VC, #DB and #DF return)
>
> Contrary to the implication in commit 21e94459110252 ("x86/mm: Optimize
> RESTORE_CR3"), the kernel never modifies CR3 in any of these exceptions,
> except for switching from user to kernel pagetables under PTI. That
> means that most of the time when returning from an exception that
> interrupted the kernel no CR3 restore is necessary. Writing CR3 is
> expensive on some machines, so this commit avoids redundant writes.
>
> I said "most of the time" because the interrupt might have come during
> kernel entry before the user->kernel CR3 switch or the during exit after
> the kernel->user switch. In the former case skipping the restore might
> actually be be fine, but definitely not the latter. So we do still need
> to check the saved CR3 and restore it if it's a user CR3.
>
> Note this code is ONLY used for returning _to kernel code_. So the only
> times where the CR3 write is necessary are in those rather special cases
> mentioned above where we are in kernel _code_ but a userspace CR3.
>
> While changing this logic the macro is given a new name to clarify its
> usage, and a comment that was describing its behaviour at the call site
> is removed.  We can also simplify the code around the SET_NOFLUSH_BIT
> invocation as we no longer need to branch to it from above.
>
> Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxxxxx>
> [Rewrote commit message; responded to review comments]
> Signed-off-by: Brendan Jackman <jackmanb@xxxxxxxxxx>
> Change-Id: I6e56978c4753fb943a7897ff101f519514fa0827

The Change-Id line here needs to be deleted. Otherwise, it seems like
this patch keeps falling through the cracks :)

Is there anything that needs to be done here?

> ---
>
> Notes:
>     v1: https://lore.kernel.org/lkml/20230817121513.1382800-1-jackmanb@xxxxxxxxxx/
>
>     v1->v2: Rewrote some comments, added a proper commit message, cleaned up
>         the code per tglx's suggestion.
>
>         I've kept Lai as the Author. If you prefer for the blame to
>         record the last person that touched it then that's also fine
>         though, I can credit Lai as Co-developed-by.
>
>     v2: https://lore.kernel.org/lkml/20230920150443.1789000-1-jackmanb@xxxxxxxxxx/
>
>     v2->v3: Clarified the commit message per Dave's suggestion and renamed the
>         macro. I did not carry PeterZ's ack since I have made some changes.
>
>     original v3 (no responses):
>         https://lore.kernel.org/lkml/20231108171656.3444702-1-jackmanb@xxxxxxxxxx/