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

[Sean Christopherson <seanjc@google.com>]

On Wed, Nov 09, 2022, Xin Li 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.

Maybe exc_raise_nmi_or_irq()?

> + * 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?

> +static void handle_interrupt_nmi_irqoff(struct kvm_vcpu *vcpu, u32 vector)
>  {
> -	bool is_nmi = entry == (unsigned long)asm_exc_nmi_noist;
> -
> -	kvm_before_interrupt(vcpu, is_nmi ? KVM_HANDLING_NMI : KVM_HANDLING_IRQ);
> -	vmx_do_interrupt_nmi_irqoff(entry);
> +	kvm_before_interrupt(vcpu, vector == NMI_VECTOR ?
> +				   KVM_HANDLING_NMI : KVM_HANDLING_IRQ);
> +	kvm_vmx_reinject_nmi_irq(vector);

This is where I strongly object to kvm_vmx_reinject_nmi_irq().  This looks like
KVM is reinjecting the event into the guest, which is all kinds of confusing.

>  	kvm_after_interrupt(vcpu);
>  }