From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 12/24] xen/arm: Release IRQ routed to a domain when it's destroying Date: Mon, 23 Feb 2015 15:49:30 +0000 Message-ID: <54EB4C0A.1040504@linaro.org> References: <1421159133-31526-1-git-send-email-julien.grall@linaro.org> <1421159133-31526-13-git-send-email-julien.grall@linaro.org> <1424449912.30924.346.camel@citrix.com> <54E771C7.4000003@linaro.org> <1424705123.27930.159.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YPvGh-0005ly-AN for xen-devel@lists.xenproject.org; Mon, 23 Feb 2015 15:49:59 +0000 Received: by mail-wi0-f182.google.com with SMTP id l15so18259208wiw.3 for ; Mon, 23 Feb 2015 07:49:58 -0800 (PST) In-Reply-To: <1424705123.27930.159.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org Hi Ian, On 23/02/15 15:25, Ian Campbell wrote: > On Fri, 2015-02-20 at 17:41 +0000, Julien Grall wrote: > >>>> + /* TODO: Handle eviction from LRs. For now, deny remove if the IRQ >>>> + * is inflight and not disabled. >>> >>> If we are ungracefully killing a guest doesn't this risk ending up with >>> an undestroyable domain? i.e. if the guest is paused with an inflight >>> IRQ and then destroyed, how does the inflight IRQ ever become >>> not-inflight again? A similar argument could apply if the guest has e.g. >>> crashed, paniced or is otherwise not processing any more interrupts or >>> generating EOIs for existing ones. >> >> No, this will fall on the "is_dying" part. During domain destruction, >> the hypervisor will release all the IRQ still assigned to the guest one >> by one. >> >>> We need to be able to kill a guest in such a state somehow. >> >> The TODO is here for running domain where we try to deassign an inflight >> IRQ. > > I see. I think either expand the comment to say "for a running domain" > or, probably better, put this bit of code (and the comment) in an else > of the is_dying and get rid of the goto in the is_dying==true case. I will do. >>>> + for ( i = 0; i < (d->arch.vgic.nr_spis); i++ ) >>>> + { >>>> + struct pending_irq *p = &d->arch.vgic.pending_irqs[i]; >>> >>> Is there not a helper for this lookup? If so it should be used. >> >> The irq_pending code is adding extra-check. But I guess we don't care >> for domain destruction? > > I don't think so. Ok. Regards, -- Julien Grall