Re: [Xen-devel] [PATCH v4 05/17] xen/arm: ITS: implement hw_irq_controller for LPIs

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2015-07-15 at 19:45 +0530, Vijay Kilari wrote:
> On Wed, Jul 15, 2015 at 3:02 PM, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> > On Wed, 2015-07-15 at 10:26 +0200, Julien Grall wrote:
> >
> >> >>> @@ -149,7 +173,7 @@ int gic_route_irq_to_guest(struct domain *d, 
> >> >>> unsigned
> >> >>> int virq,
> >> >>>             test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) )
> >> >>>            goto out;
> >> >>>
> >> >>> -    desc->handler = gic_hw_ops->gic_guest_irq_type;
> >> >>> +    desc->handler = get_guest_hw_irq_controller(desc->irq);
> >> >>>        set_bit(_IRQ_GUEST, &desc->status);
> >> >>>
> >> >>>        gic_set_irq_properties(desc, cpumask_of(v_target->processor),
> >> >>> priority);
> >> >>> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> >> >>> index 2dd43ee..ba8528a 100644
> >> >>> --- a/xen/arch/arm/irq.c
> >> >>> +++ b/xen/arch/arm/irq.c
> >> >>> @@ -35,7 +35,13 @@ static DEFINE_SPINLOCK(local_irqs_type_lock);
> >> >>>    struct irq_guest
> >> >>>    {
> >> >>>        struct domain *d;
> >> >>> -    unsigned int virq;
> >> >>> +    union
> >> >>> +    {
> >> >>> +        /* virq refer to virtual irq in case of spi */
> >> >>> +        unsigned int virq;
> >> >>> +        /* virq refer to event ID in case of lpi */
> >> >>> +        unsigned int vid;
> >> >>
> >> >>
> >> >> Why can't we store the event ID in the irq_guest? As said on v3, this 
> >> >> is not
> >> >
> >> > Are you referring to irq_desc in above statement?
> >>
> >> Yes sorry.
> >
> > I'm afraid I don't follow your suggestion here, are you suggesting that
> > the vid field added above should be moved to irq_desc?
> >
> > But the vid _is_ domain specific, it is the virtual event ID which is
> > per-domain (it's the thing looked up in the ITT to get a vLPI to be
> > injected). I think it is a pretty direct analogue of the virq field used
> > for non-LPI irq_guest structs.
> >
> > If we had need for the physical event id then that would like belong in
> > the irq_desc.
> >
> > Your proposal on v3 looks to be around moving the its_device pointer to
> > the irq_desc, which appears to have been done here, along with turning
> > the virq+vid into a union as requested there too.
> >
> >> >> It has been suggested by Ian to move col_id in the its_device in the
> >> >> previous version [4]. Any reason to not doing it?
> >> >
> >> > In round robin fashion each plpi is attached to col_id. So storing
> >> > in its_device is not possible. In linux latest col_id is stored in 
> >> > its_device
> >> > structure for which set_affinity is called.
> >
> > Are you saying that in Linux all Events/LPIs associated with a given ITS
> > device are routed to the same collection?
> 
>  Not associated with same collection. Events are associated with cpu
> for which cpu_mask is set and the collection id of that cpu is stored
> in the its_device,

Surely storing the collection ID is precisely equivalent to storing the
cpu mask of the one CPU to which that collection ID is routed?

But that is orthogonal to my question anyway, so let me try again:

Are you saying that in Linux all Events/LPIs associated with a given ITS
device are routed to the same CPU?

>  which is later used for SYNC.


> So effectively it_device
> does not store collection id associated for all Events of that device.

Right above you said "and the collection id of that cpu is stored in the
its_device", which contradicts what you now say here. (Unless it_device
is not a typo of its_device but a separate thing?)

Ian.


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