Re: [PATCH 5/6] KVM: x86/VMX: add kvm_vmx_reinject_nmi_irq() for NMI/IRQ reinjection

Sean Christopherson
2022-11-10T20:53:18Z

On Thu, Nov 10, 2022, Li, Xin3 wrote:
> > > +#if IS_ENABLED(CONFIG_KVM_INTEL)
> > > +/*
> > > + * KVM VMX reinjects NMI/IRQ on its current stack, it's a sync
> >
> > Please use a verb other than "reinject". There is no event injection of any kind,
> > KVM is simply making a function call. KVM already uses "inject" and "reinject"
> > for KVM where KVM is is literally injecting events into the guest.
> >
> > The "kvm_vmx" part is also weird IMO. The function is in x86's
> > traps/exceptions namespace, not the KVM VMX namespace.
>
> right, "kvm_vmx" doesn't look good per your explanation.
>
> >
> > Maybe exc_raise_nmi_or_irq()?
>
> It's good for me.
>
> >
> > > + * call thus the values in the pt_regs structure are not used in
> > > + * executing NMI/IRQ handlers,
> >
> > Won't this break stack traces to some extent?
> >
>
> The pt_regs structure, and its IP/CS, is NOT part of the call stack, thus
> I don't see a problem. No?

bool nmi_cpu_backtrace(struct pt_regs *regs)
{
int cpu = smp_processor_id();
unsigned long flags;

if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
/*
* Allow nested NMI backtraces while serializing
* against other CPUs.
*/
printk_cpu_sync_get_irqsave(flags);
if (!READ_ONCE(backtrace_idle) && regs && cpu_in_idle(instruction_pointer(regs))) {
pr_warn("NMI backtrace for cpu %d skipped: idling at %pS\n",
cpu, (void *)instruction_pointer(regs));
} else {
pr_warn("NMI backtrace for cpu %d\n", cpu);
if (regs)
show_regs(regs); <============================== HERE!!!
else
dump_stack();
}
printk_cpu_sync_put_irqrestore(flags);
cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
return true;
}

return false;
}