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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux