Re: [Xen-devel] [RFC PATCH v3 07/18] xen/arm: ITS: implement hw_irq_controller for LPIs

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 26/06/2015 16:25, Vijay Kilari wrote:
> Hi Julien,
Hi Vijay,


> > > +    its_send_inv(its_dev, col, virq);
> > > +}
> > > +
> > > +void its_set_affinity(struct irq_desc *desc, int cpu)
> > > +{
> > > +    struct its_device *its_dev = get_irq_device(desc);
> > > +    struct its_collection *target_col;
> > > +
> > > +    /* Physical collection id */
> > > +    target_col = &its_dev->its->collections[cpu];
> > > +    its_send_movi(its_dev, target_col, irq_to_virq(desc));
> > The field "virq" in the structure irq_guest refers to the guest virtual
> > IRQ and not the event ID. As Ian suggested in the proposal [1], please
> > use an union to make this code clears.
>     Apart from adding union, do you recommend to add macros irq_to_vid()
> and irq_to_virq() and use appropriately?
If it makes to code clearer yes. Although it may not be necessary if we 
move the value into irq_desc (see below).
[..]

> > > diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> > > index 2dd43ee..9dbdf7d 100644
> > > --- a/xen/arch/arm/irq.c
> > > +++ b/xen/arch/arm/irq.c
> > > @@ -36,6 +36,7 @@ struct irq_guest
> > >   {
> > >       struct domain *d;
> > >       unsigned int virq;
> > > +    struct its_device *dev;
> > I know that this was suggested in the proposal [1]. But the goal of
> > irq_guest is to store anything specific to the guest. The event ID and
> > the its_device assigned are known when the device is added to Xen and
> > hence can be set in irq_desc (with a small memory impact, but we have
> > plenty of memory on ARM64).
> Do you mean adding its_device to irq_desc instead of irq_guest?
> If so, irq_desc is common for both x86 & ARM.
There is an arch specific structure for irq_desc (see arch_irq_desc).

Regards,

--
Julien Grall

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