+#ifdef CONFIG_IRQ_HAS_MULTIPLE_ACTION
+    new->next = desc->action;
+    smp_mb();
+#endif
+
+    desc->action = new;
+    smp_mb();

Aren't smp_wmb() sufficient on both places? If not, I think comments
want adding.

smp_wmb() will be sufficient but I think the barrier could be dropped at all
as __setup_irq() is called only in setup_irq() and __setup_irq() call is wrapped
by spinlock_{un}lock_irqsave() where spinlock_unlock_*() will put barrier.

+    return 0;
+}
+
+void irq_set_affinity(struct irq_desc *desc, const cpumask_t *cpu_mask)
+{
+    if ( desc != NULL )

Can desc really be NULL here?

It can't as irq_desc[] isn't dynamically allocated.

 Isn't desc->lock required to be held?

It is and it is called in setup_irq() which calls spin_lock_irqsave().
Anyway, I think it could be dropped at all and use 'desc->handler->set_affinity(desc, cpu_mask);'
explicitly in setup_irq().

+    spin_lock_irqsave(&desc->lock, flags);
+
+    disabled = (desc->action == NULL);
+
+    if ( test_bit(_IRQ_GUEST, &desc->status) )
+    {
+        spin_unlock_irqrestore(&desc->lock, flags);
+        /*
+         * TODO: would be nice to have functionality to print which domain owns
+         *       an IRQ.
+         */
+        printk(XENLOG_ERR "ERROR: IRQ %u is already in use by a domain\n", irq);
+        return -EBUSY;
+    }
+
+    rc = __setup_irq(desc, irqflags, new);
+    if ( rc )
+        goto err;
+
+    /* First time the IRQ is setup */
+    if ( disabled )
+    {
+        /* disable irq by default */
+        set_bit(_IRQ_DISABLED, &desc->status);

Shouldn't this be set when we make it here?

It should be. I'll drop the setting of _IRQ_DISABLED.

+        /* route interrupt to xen */
+        intc_route_irq_to_xen(desc, IRQ_NO_PRIORITY);
+
+        /*
+         * we don't care for now which CPU will receive the
+         * interrupt
+         *
+         * TODO: Handle case where IRQ is setup on different CPU than
+         * the targeted CPU and the priority.
+         */
+        irq_set_affinity(desc, cpumask_of(smp_processor_id()));
+        desc->handler->startup(desc);
+        /* enable irq */
+        clear_bit(_IRQ_DISABLED, &desc->status);

Now it turns out this is really done twice: Once in aplic_irq_enable(),
and once here.

Agree, this is a job of *_startup()->*_aplic_irq_enable(). I'll drop that too.

~ Oleksii