From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 2/2] xen/dt: Allow only IRQ translation that are mapped to main GIC Date: Wed, 16 Jul 2014 15:28:36 +0100 Message-ID: <53C68C14.6000702@linaro.org> References: <1404912223-9320-1-git-send-email-julien.grall@linaro.org> <1404912223-9320-2-git-send-email-julien.grall@linaro.org> <1405516449.15219.7.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.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1X7QCH-0001J5-Ix for xen-devel@lists.xenproject.org; Wed, 16 Jul 2014 14:28:41 +0000 Received: by mail-wi0-f179.google.com with SMTP id f8so1449457wiw.0 for ; Wed, 16 Jul 2014 07:28:39 -0700 (PDT) In-Reply-To: <1405516449.15219.7.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 On 16/07/14 14:14, Ian Campbell wrote: > On Wed, 2014-07-09 at 14:23 +0100, Julien Grall wrote: >> Xen is only able to handle one GIC controller. Some platform may contain >> other interrupt controller. >> >> Make sure to only translate IRQ mapped into the GIC handled by Xen. >> >> Signed-off-by: Julien Grall >> >> --- >> Changes in v3: >> - Add an ASSERT to check that dt_interrupt_controller is not >> NULL. >> >> Changes in v2: >> - Fix compilation... >> --- >> xen/common/device_tree.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c >> index 310635e..cc45bd1 100644 >> --- a/xen/common/device_tree.c >> +++ b/xen/common/device_tree.c >> @@ -1442,9 +1442,12 @@ int dt_irq_translate(const struct dt_raw_irq *raw, >> struct dt_irq *out_irq) >> { >> ASSERT(dt_irq_xlate != NULL); >> + ASSERT(dt_interrupt_controller != NULL); >> >> - /* TODO: Retrieve the right irq_xlate. This is only work for the gic */ >> + if ( raw->controller != dt_interrupt_controller ) >> + return -EINVAL; >> >> + /* TODO: Retrieve the right irq_xlate. This is only work for the gic */ > > "This only works for ...". > > Do you mean it to say "primary gic"? In which case I think it was in the > correct location before (i.e. before the check which enforced that). Which location are you talking about? The one in map_device? If so, it doesn't protect anything, but only avoid to assign an IRQ to DOM0 which is not routed to the primary interrupt controller. Checking the controller in the location will prevent platform_get_irq to translate IRQ not handled by the gic controller. Futhermore, it will avoid the new hypercall to list interrupt for a specific device node returning an invalid IRQ number. Regards, -- Julien Grall