Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 11 Nov 2022 10:15:12 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     "Li, Xin3" <xin3.li@intel.com>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "bp@alien8.de" <bp@alien8.de>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "hpa@zytor.com" <hpa@zytor.com>,
        "Christopherson,, Sean" <seanjc@google.com>,
        "pbonzini@redhat.com" <pbonzini@redhat.com>,
        "Tian, Kevin" <kevin.tian@intel.com>
Subject: Re: [RESEND PATCH 5/6] KVM: x86/VMX: add kvm_vmx_reinject_nmi_irq()
 for NMI/IRQ reinjection
Message-ID: <Y24SoNKZtj/NPSGy@hirez.programming.kicks-ass.net>
References: <20221110061545.1531-1-xin3.li@intel.com>
 <20221110061545.1531-6-xin3.li@intel.com>
 <Y2y+YgBUYuUHbPtd@hirez.programming.kicks-ass.net>
 <BN6PR1101MB2161976800EB14B74A24D9F3A8019@BN6PR1101MB2161.namprd11.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <BN6PR1101MB2161976800EB14B74A24D9F3A8019@BN6PR1101MB2161.namprd11.prod.outlook.com>
Precedence: bulk

On Thu, Nov 10, 2022 at 08:53:30PM +0000, Li, Xin3 wrote:

> > >  static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu)
> > > {
> > >  	u32 intr_info = vmx_get_intr_info(vcpu);
> > >  	unsigned int vector = intr_info & INTR_INFO_VECTOR_MASK;
> > > -	gate_desc *desc = (gate_desc *)host_idt_base + vector;
> > >
> > >  	if (KVM_BUG(!is_external_intr(intr_info), vcpu->kvm,
> > >  	    "KVM: unexpected VM-Exit interrupt info: 0x%x", intr_info))
> > >  		return;
> > >
> > > -	handle_interrupt_nmi_irqoff(vcpu, gate_offset(desc));
> > > +	handle_interrupt_nmi_irqoff(vcpu, vector);
> > >  	vcpu->arch.at_instruction_boundary = true;  }
> > 
> > How does any of this work? You're calling into entry/noinstr code from a
> > random context.
> 
> Can you please elaborate your concern a bit more?
> 
> We are here in handle_external_interrupt_irqoff () because an external
> interrupt happened when a guest was running and the CPU vm-exits to host
> to dispatch to the IRQ handler with IRQ disabled.

I don't speak virt (but this all sounds disguisting) -- but what appears
to be the case is you calling into entry code from regular kernel
context, which is odd at best.

Specifically, going by the fact that all this is not noinstr code, the
assumption is that RCU/lockdep/etc.. is all set-up and running. This
means you should not be calling DEFINE_IDTENTRY_*(func) functions
because those will try and set all that up again.

Granted, irqentry_{enter,exit}() do nest, but *yuck*.