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

Sean Christopherson
2022-11-10T16:00:18Z

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