From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCHv0 dont apply] RFC: kvm eoi PV using shared memory Date: Tue, 10 Apr 2012 18:14:42 +0300 Message-ID: <20120410151441.GA20259@redhat.com> References: <20120410132756.GA14101@redhat.com> <4F843DAA.1000808@redhat.com> <20120410142616.GB19556@redhat.com> <4F8444AE.4020504@redhat.com> <20120410145351.GE19556@redhat.com> <4F844B23.3050506@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:31624 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754575Ab2DJP0d (ORCPT ); Tue, 10 Apr 2012 11:26:33 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q3AFQWjH007007 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 10 Apr 2012 11:26:33 -0400 Content-Disposition: inline In-Reply-To: <4F844B23.3050506@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Apr 10, 2012 at 06:00:51PM +0300, Avi Kivity wrote: > On 04/10/2012 05:53 PM, Michael S. Tsirkin wrote: > > > > > > > > Yes. But we can and it's easier than figuring out priorities. > > > > I am guessing such collisions are rare, right? > > > > > > It's pretty easy, if there is something in IRR but > > > kvm_lapic_has_interrupt() returns -1, then we need to disable eoi avoidance. > > > > I only see kvm_apic_has_interrupt - is this what you mean? > > Yes, sorry. > > It's not clear whether to do the check in kvm_apic_has_interrupt() or > kvm_apic_get_interrupt() - the latter is called only after interrupts > are enabled, so it looks like a better place (EOIs while interrupts are > disabled have no effect). But need to make sure those functions are > actually called, since they're protected by KVM_REQ_EVENT. Sorry not sure what you mean by "make sure" - read the code carefully? > > > > I'll add a trace to make sure. > > > > > > > > > > + if (v != -1) > > > > > > + apic_set_vector(v, apic->regs + APIC_ISR); > > > > > > + } else { > > > > > > + eoi_set_pending_vector(vcpu, vector); > > > > > > + set_isr = false; > > > > > > > > > > Weird. Just set it normally. Remember that reading the ISR needs to > > > > > return the correct value. > > > > > > > > Marcelo said linux does not normally read ISR - not true? > > > > > > It's true and it's irrelevant. We aren't coding a feature to what linux > > > does now, but for what linux or another guest may do in the future. > > > > Right. So you think reading ISR has value > > in combination with PV EOI for future guests? > > I'm not arguing either way just curious. > > I don't. But we need to preserve the same interface the APIC has > presented for thousands of years (well, almost). Talk about overstatements :) > > > > > > Note this has no effect if the PV optimization is not enabled. > > > > > > > > > We need to process the avoided EOI before any APIC read/writes, to be > > > > > sure the guest sees the updated values. Same for IOAPIC, EOI affects > > > > > remote_irr. That may been we need to sample it after every exit, or > > > > > perhaps disable the feature for level-triggered interrupts. > > > > > > > > Disabling would be very sad. Can we sample on remote irr read? > > > > > > That can be done from another vcpu. > > > > We still can handle it, right? Where's the code that handles that read? > > Better to keep everything per-cpu. The code is in virt/kvm/ioapic.c Hmm. Disabling for level handles the ack notifiers issue as well, which I forgot about. It's a tough call. You think looking at TMR in kvm_get_apic_interrupt is safe? > > > > > Why do we care about > > > level-triggered interrupts? Everything uses MSI or edge-triggered > > > IOAPIC interrupts these days. > > > > Well lots of emulated devices don't yet. > > They probably should but it's nice to be able to > > test with e.g. e1000 emulation not just virtio. > > > e1000 doesn't support msi? qemu emulation doesn't. > > > > Besides, kvm_get_apic_interrupt > > simply doesn't know about the triggering mode at the moment. > > > > > -- > error compiling committee.c: too many arguments to function