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

[Sean Christopherson <seanjc@xxxxxxxxxx>]

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