From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 06/21] xen/arm: Allow virq != irq Date: Tue, 09 Sep 2014 11:42:06 -0700 Message-ID: <540F49FE.9020500@linaro.org> References: <1406818852-31856-1-git-send-email-julien.grall@linaro.org> <1406818852-31856-7-git-send-email-julien.grall@linaro.org> <1410269384.8217.178.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" 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 1XRQMl-0005Bn-7x for xen-devel@lists.xenproject.org; Tue, 09 Sep 2014 18:42:11 +0000 Received: by mail-qa0-f48.google.com with SMTP id v10so765662qac.21 for ; Tue, 09 Sep 2014 11:42:08 -0700 (PDT) In-Reply-To: <1410269384.8217.178.camel@kazak.uk.xensource.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 09/09/14 06:29, Ian Campbell wrote: > Mostly spelling/grammar nits, a couple of questions but mostly looks > good. > > On Thu, 2014-07-31 at 16:00 +0100, Julien Grall wrote: >> Actually Xen is assuming that the virtual IRQ will always be equal the IRQ. > > "be equal to" or better "always be the same as". > >> >> Modify, route_guest_irq to take the virtual IRQ in parameter and let Xen > > Spurious comma. > >> assigned a different IRQ number. Also store the vIRQ in the desc action to > > "assign". > >> retrieve easily the IRQ target when we need to inject the interrupt. > > "easily retrieve" (or "be able to easily retrieve") > >> As DOM0 will get most the device, the vIRQ is equal to the IRQ. > > "most of the devices". > > I suppose the sentence should end with "in that case" or something? Yes. I will fix all the grammar nits in the next version. >> @@ -122,18 +129,20 @@ void __cpuinit init_secondary_IRQ(void) >> BUG_ON(init_local_irq_data() < 0); >> } >> >> -static inline struct domain *irq_get_domain(struct irq_desc *desc) >> +static inline struct irq_guest *irq_get_guest_info(struct irq_desc *desc) >> { >> ASSERT(spin_is_locked(&desc->lock)); >> - >> - if ( !test_bit(_IRQ_GUEST, &desc->status) ) >> - return dom_xen; >> - >> + ASSERT(test_bit(_IRQ_GUEST, &desc->status)); >> ASSERT(desc->action != NULL); >> >> return desc->action->dev_id; >> } >> >> +static inline struct domain *irq_get_domain(struct irq_desc *desc) >> +{ >> + return irq_get_guest_info(desc)->d; > > Previously irq_get_domain could return dom_xen for !_IRQ_GUEST domains. > What happened to this logic? If it is never possible to get here with a > Xen owned IRQ then I think that is worth mentioning in the commit log > and/or a comment. This was a safe guard in case of developer were misusing the function. Actually every callers of this function are checking that the IRQ is assigned to a guest before calling irq_get_domain. I will mention it in the commit log. > >> @@ -206,7 +215,7 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq) >> >> /* the irq cannot be a PPI, we only support delivery of SPIs to >> * guests */ >> - vgic_vcpu_inject_spi(d, irq); >> + vgic_vcpu_inject_spi(info->d, info->virq); > > Could this function take an irq_guest* or are there other callers? I haven't thought about this possibility. This is the only caller, so I will pass irq_guest. Regards, -- Julien Grall