Re: [PATCH 8/9] VMX: work around lacking VNMI support

[Gleb Natapov <gleb@qumranet.com>]

On Sun, Sep 21, 2008 at 06:57:32PM +0200, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > Hi Jan,
> > 
> > On Fri, Sep 19, 2008 at 02:04:37PM +0200, Jan Kiszka wrote:
> >>  static void vmx_inject_irq(struct kvm_vcpu *vcpu, int irq)
> >>  {
> >>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> >> @@ -2356,6 +2384,29 @@ static void vmx_inject_nmi(struct kvm_vc
> >>  {
> >>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> >>  
> >> +	if (!cpu_has_virtual_nmis()) {
> >> +		int desc_size = is_long_mode(vcpu) ? 16 : 8;
> >> +		struct descriptor_table dt;
> >> +		gpa_t gpa;
> >> +		u64 desc;
> >> +
> >> +		/*
> >> +		 * Deny delivery if the NMI will not be handled by an
> >> +		 * interrupt gate (workaround depends on IRQ masking).
> >> +		 */
> >> +		vmx_get_idt(vcpu, &dt);
> >> +		if (!vcpu->arch.rmode.active && dt.limit
> >> +		    >= desc_size * (NMI_VECTOR + 1) - 1) {
> >> +			gpa = vcpu->arch.mmu.gva_to_gpa(vcpu,
> >> +					dt.base + desc_size * NMI_VECTOR);
> >> +			if (kvm_read_guest(vcpu->kvm, gpa, &desc, 8) == 0
> >> +			    && ((desc >> 40) & 0x7) != 0x6)
> >> +				return;
> >> +		}
> > 
> > Windows2003 sets NMI entry in IDT as a task gate (0x5) during hibernation and this check
> > prevents it from shutting down itself. It hangs in "It is save to turn
> > your computer now" screen.
> 
> Grmbl, what a weird guest...
This is a known trick that some OSes use.

> 
> Is this a regression of this patch because NMIs were considered broken
> by Windows on that host CPU so far?
> 
Nope. This is not a regression. Hibernation hangs in the same place with
the current git on this machine. It works on newer CPUs.


> > If I replace this part by:
> >     if(vmx->soft_vnmi_blocked)
> > 	  return;
> > It shut itself down properly.
> 
> OK, but that almost always evaluates to false here.
Without this check guest BSODs. CPU 0 send two NMI in a row to CPU 1 and
if second one is accepted something goes wrong.

--
			Gleb.