Re: [PATCH 3/5] x86/crash: Drop manual hooking of exception_table[]

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.11.2021 19:21, Andrew Cooper wrote:
> NMI hooking in the crash path has undergone several revisions since its
> introduction.  What we have now is not sufficiently different from the regular
> nmi_callback() mechanism to warrant special casing.
> 
> Use set_nmi_callback() directly, and do away with patching a read-only data
> structure via a read-write alias.  This also means that the
> vmx_vmexit_handler() can and should call do_nmi() directly, rather than
> indirecting through the exception table to pick up the crash path hook.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with one remark / concern:

> --- a/xen/arch/x86/crash.c
> +++ b/xen/arch/x86/crash.c
> @@ -36,10 +36,8 @@ static unsigned int crashing_cpu;
>  static DEFINE_PER_CPU_READ_MOSTLY(bool, crash_save_done);
>  
>  /* This becomes the NMI handler for non-crashing CPUs, when Xen is crashing. 
> */
> -static void noreturn do_nmi_crash(const struct cpu_user_regs *regs)
> +static int noreturn do_nmi_crash(const struct cpu_user_regs *regs, int cpu)
>  {
> -    unsigned int cpu = smp_processor_id();
> -
>      stac();
>  
>      /* nmi_shootdown_cpus() should ensure that this assertion is correct. */

Looks like this is the first instance of a noreturn function returning non-void.
Are you sufficiently certain that (older) compilers won't complain about missing
return statements (with a value)?

Jan