Re: [Xen-devel] [PATCH v2 06/21] xen/arm: Allow virq != irq

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Ian,

On 09/09/14 06:29, Ian Campbell wrote:
> Mostly spelling/grammar nits, a couple of questions but mostly looks
> good.
>
> On Thu, 2014-07-31 at 16:00 +0100, Julien Grall wrote:
> > Actually Xen is assuming that the virtual IRQ will always be equal the IRQ.
> "be equal to" or better "always be the same as".
>
> > Modify, route_guest_irq to take the virtual IRQ in parameter and let Xen
> Spurious comma.
>
> > assigned a different IRQ number. Also store the vIRQ in the desc action to
> "assign".
>
> > retrieve easily the IRQ target when we need to inject the interrupt.
> "easily retrieve" (or "be able to easily retrieve")
>
> > As DOM0 will get most the device, the vIRQ is equal to the IRQ.
> "most of the devices".
>
> I suppose the sentence should end with "in that case" or something?
Yes. I will fix all the grammar nits in the next version.

> > @@ -122,18 +129,20 @@ void __cpuinit init_secondary_IRQ(void)
> >       BUG_ON(init_local_irq_data() < 0);
> >   }
> >
> > -static inline struct domain *irq_get_domain(struct irq_desc *desc)
> > +static inline struct irq_guest *irq_get_guest_info(struct irq_desc *desc)
> >   {
> >       ASSERT(spin_is_locked(&desc->lock));
> > -
> > -    if ( !test_bit(_IRQ_GUEST, &desc->status) )
> > -        return dom_xen;
> > -
> > +    ASSERT(test_bit(_IRQ_GUEST, &desc->status));
> >       ASSERT(desc->action != NULL);
> >
> >       return desc->action->dev_id;
> >   }
> >
> > +static inline struct domain *irq_get_domain(struct irq_desc *desc)
> > +{
> > +    return irq_get_guest_info(desc)->d;
> Previously irq_get_domain could return dom_xen for !_IRQ_GUEST domains.
> What happened to this logic? If it is never possible to get here with a
> Xen owned IRQ then I think that is worth mentioning in the commit log
> and/or a comment.
This was a safe guard in case of developer were misusing the function. 
Actually every callers of this function are checking that the IRQ is 
assigned to a guest before calling irq_get_domain.
I will mention it in the commit log.

> > @@ -206,7 +215,7 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, 
> > int is_fiq)
> >
> >           /* the irq cannot be a PPI, we only support delivery of SPIs to
> >            * guests */
> > -        vgic_vcpu_inject_spi(d, irq);
> > +        vgic_vcpu_inject_spi(info->d, info->virq);
> Could this function take an irq_guest* or are there other callers?
I haven't thought about this possibility. This is the only caller, so I 
will pass irq_guest.
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel