[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC PATCH 30/49] ARM: new VGIC: Add ENABLE registers handlers

Hi Andre,

On 23/02/18 15:18, Andre Przywara wrote:
+    irq_desc_t *desc;
+    int i;
+    unsigned long flags;
+    enum vgic_irq_config config;
+    for_each_set_bit( i, &val, len * 8 )
+    {
+        struct vgic_irq *irq;
+        irq = vgic_get_irq(vcpu->domain, vcpu, intid + i);
+        spin_lock_irqsave(&irq->irq_lock, flags);
+        irq->enabled = true;
+        if ( irq->hw )
+        {
+            /*
+             * The irq cannot be a PPI, we only support delivery
+             * of SPIs to guests.
+             */
+            ASSERT(irq->hwintid >= 32);
+            desc = irq_to_desc(irq->hwintid);

What is the rationale behind storing hwintid rather than the irq_desc

Well, this is because KVM does it this way, for abstraction reasons,
mostly. Looking over the users I see that mostly we are indeed after the
struct irq_desc. But it would also increase struct vgic_irq by 4 bytes ;-)

I could try to make to make the change, but am not fully convinced.

What are your arguments for that change?

To be honest, I don't have much arguments :). My main concern is using irq_to_desc wrongly when we will add support for routing PPI to domains. This would useful to support the virtual timer without the hack we currently have.

In the PPI context, irq_to_desc would always return the PPI irq_desc of the current CPU. I am not entirely if this will always be ok for us. But I might be over cautious :).

So I guess, we can keep it like that for now.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.