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.coop...@citrix.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
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
