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

[Julien Grall <julien.grall@xxxxxxx>]

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
> > directly?
> 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.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel