Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756416Ab3EVPZR (ORCPT ); Wed, 22 May 2013 11:25:17 -0400 Received: from nat28.tlf.novell.com ([130.57.49.28]:60215 "EHLO nat28.tlf.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753030Ab3EVPZP convert rfc822-to-8bit (ORCPT ); Wed, 22 May 2013 11:25:15 -0400 Message-Id: <519CFF7602000078000D82E6@nat28.tlf.novell.com> X-Mailer: Novell GroupWise Internet Agent 12.0.2 Date: Wed, 22 May 2013 16:25:10 +0100 From: "Jan Beulich" To: "Konrad Rzeszutek Wilk" Cc: "David Vrabel" , "Stefano Stabellini" , "Thomas Gleixner" , "xen-devel@lists.xensource.com" , "Chien Yen" , "Feng Jin" , "Yuval Shaia" , "Zhenzhong Duan" , "Ingo Molnar" , "linux-kernel@vger.kernel.org" , "H. Peter Anvin" Subject: Re: [Xen-devel] [PATCH] xen: reuse the same pirq allocated when driver load first time References: <20130520175706.GA27973@phenom.dumpdata.com> <20130520203855.GA30616@phenom.dumpdata.com> <519B474E.4000202@citrix.com> <20130521134059.GE492@phenom.dumpdata.com> <20130521204245.GA7073@phenom.dumpdata.com> <20130521224125.GA3483@phenom.dumpdata.com> <519CAE0302000078000D807A@nat28.tlf.novell.com> <20130522151437.GC8162@phenom.dumpdata.com> In-Reply-To: <20130522151437.GC8162@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 8BIT Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2335 Lines: 49 >>> On 22.05.13 at 17:14, Konrad Rzeszutek Wilk wrote: > The physdev_unmap_pirq (from PHYSDEVOP_unmap_pirq), only has this > check: > if (domain_pirq_to_emuirq(d, pirq) != IRQ_UNBOUND) > > and since the arch.hvm.emuirq is IRQ_UNBOUND (-1), it does not > call unmap_domain_pirq_emuirq. It probably shouldn't, but it should > at least remove the info->arch.pirq = PIRQ_ALLOCATED as we are > telling the hypervisor: "hey, I am done with this, return to the > pool." But since that is not cleared, the PHYSDEVOP_get_free_pirq > will skip this pirq as arch.pirq is still set to PIRQ_ALLOCATED. Okay, that clarifies it quite a bit. For one, I'll leave any of the emuirq stuff to Stefano, who wrote this originally. And then, from the beginning of this thread, I'm not convinced that freeing a pirq is really the right thing here: unmap_pirq() is the counterpart of map_pirq(), not get_free_pirq(). I would think that is a guest allocates a pirq and then unmaps it without first mapping it, it's the guest's fault that it now lost one pirq resource. It should not have allocated one in the first place if it didn't mean to use it for anything. >> I see none at all, unmap_domain_pirq() has a <= 0 check, and >> unmap_domain_pirq_emuirq() again doesn't appear to have any. > > The 'unmap_domain_pirq' path would be if dom0 (so QEMU) did the > unmap for the guest. That is via the PHYSDEVOP_unmap_pirq. And > I think if that path was taken (as Stefano suggests QEMU should > do when a MSI or MSI-X driver is unloaded and zero is writen as > an PIRQ), we would end up calling clear_domain_irq_pirq, which > would set arch.pirq = 0. > > Or to a negative value as you pointed out later. Which then > means we won't be ever able to re-use the PIRQ (as > PHYSDEVOP_get_free_pirq or rather get_free_pirq would skip over it > as arch.pirq != 0). That setting to a negative value is not causing the slot to be permanently lost, it merely defers its freeing. It was the only way I could find back then to reasonably handle an unmap being done before the matched unbind. Jan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/