Re: [PATCH] irqchip/mips-gic: Fix IRQs in gic_dev_domain

From: Qais Yousef
Date: Sat Jun 25 2016 - 05:14:26 EST


On 23/06/2016 19:11, Jason Cooper wrote:
Qais,

On Tue, May 24, 2016 at 11:43:07AM +0100, Qais Yousef wrote:
Hmm I certainly did test this on real hardware with GIC. Are you using the
new dev domain? The idea is that GIC is logically divided and shouldn't be
used directly. Sorry I'm travelling and can't check the code.
Any update on this patch? Should I stop tracking it?

Apologies I didn't realise I was holding this up. I thought I received an email that this was committed so I didn't take a look.

I just had a look and we set the hwirq for the IPI domain in the same function. My memory failed me and I thought maybe the problem was due to someone trying to access the gic_irq_domain directly. I don't know why when I tested this on Malta I didn't see this problem.

Anyways I'm OK with this change if that's what you're asking.

Thanks,
Qais


thx,

Jason.

On 23 May 2016 12:06, "Harvey Hunt" <harvey.hunt@xxxxxxxxxx> wrote:

When allocating a new device IRQ, gic_dev_domain_alloc() correctly calls
irq_domain_set_hwirq_and_chip(), but gic_irq_domain_alloc() does not. This
means that gic_irq_domain believes all IRQs from the dev domain have an
hwirq of 0 and creates incorrect mappings in the linear_revmap. As
gic_irq_domain is a parent of the gic_dev_domain, this leads to an
inability to boot on devices with a GIC. Excerpt of the error:

[ 2.297649] irq 0: nobody cared (try booting with the "irqpoll" option)
...
[ 2.436963] handlers:
[ 2.439492] Disabling IRQ #0

Fix this by calling irq_domain_set_hwirq_and_chip() for both the dev and
irq domain.

Now that we are modifying the parent domain, be sure to clear it up in
case of an allocation error.

Fixes: c98c1822ee13 ("irqchip/mips-gic: Add device hierarchy domain")
Fixes: 2af70a962070 ("irqchip/mips-gic: Add a IPI hierarchy domain")
Signed-off-by: Harvey Hunt <harvey.hunt@xxxxxxxxxx>
Tested-by: Govindraj Raja <Govindraj.Raja@xxxxxxxxxx> # On Pistachio SoC
Reviewed-by: Matt Redfearn <matt.redfearn@xxxxxxxxxx>
Cc: <linux-mips@xxxxxxxxxxxxxx>
Cc: <linux-kernel@xxxxxxxxxxxxxxx>
Cc: Qais Yousef <qsyousef@xxxxxxxxx>
---
drivers/irqchip/irq-mips-gic.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-mips-gic.c
b/drivers/irqchip/irq-mips-gic.c
index 4dffccf..40fb120 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -734,6 +734,12 @@ static int gic_irq_domain_alloc(struct irq_domain *d,
unsigned int virq,
/* verify that it doesn't conflict with an IPI irq */
if (test_bit(spec->hwirq, ipi_resrv))
return -EBUSY;
+
+ hwirq = GIC_SHARED_TO_HWIRQ(spec->hwirq);
+
+ return irq_domain_set_hwirq_and_chip(d, virq, hwirq,
+
&gic_level_irq_controller,
+ NULL);
} else {
base_hwirq = find_first_bit(ipi_resrv, gic_shared_intrs);
if (base_hwirq == gic_shared_intrs) {
@@ -855,10 +861,14 @@ static int gic_dev_domain_alloc(struct irq_domain
*d, unsigned int virq,

&gic_level_irq_controller,
NULL);
if (ret)
- return ret;
+ goto error;
}

return 0;
+
+error:
+ irq_domain_free_irqs_parent(d, virq, nr_irqs);
+ return ret;
}

void gic_dev_domain_free(struct irq_domain *d, unsigned int virq,
--
2.8.2