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

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-07-15 at 22:31 +0530, Vijay Kilari wrote:
> Sorry. I may not be clear.
> 
> In Linux when MSIx is enabled.
> Device is created first and its_device->its_collection is set for
> first onine cpu
> and mapvi is called with collection set in its_create_device() as below.
[...]
> When affinity is set, movi is sent with collection id selected
> for the cpu_mask.

OK, so at start of day all events for a given device end up mapped to a
single processor, but as individual interrupts are rebalanced they will
then become spread out among different CPUs, that makes sense.

I'm not sure I follow the scheme which Linux is using to achieve that
behaviour though, so I've CC'd Marc.

It seems to me from looking at the Linux code that its_dev->collection
doesn't, as one might expect, contain the collection associated with the
device (and therefore all Events of that device), but rather it contains
the collection corresponding to a single Event which is the one which
most recently had its affinity changed, i.e. the one with an potentially
outstanding INV.

So its_send_movi sends the MOVI command which ends up calling INV on
that collection, which at this point is the _old_ collection. Then it
stores the new collection for that Event in its_dev->collection.

What I don't follow is how set_lpi_config copes with this, since it
always sends the INV for an Event to the corresponding its_dev's
collection, but after a bunch of Events/IRQs have been rebalanced there
doesn't seem to be any guarantee that this would corresponding to the
current collection associated with the interrupt which has been
reconfigured.

Marc, have I misunderstood what the Linux ITS driver is trying to
achieve or how it goes about it?

Perhaps lpi_set_config (and by extension its_(un)mask_irq) is only
called at start of day?

I'm not sure if Xen is going to (want to) follow the way Linux arranges
this stuff, but I would very much like to at least understand it, since
it may have implications...

Cheers,
Ian
> 
> struct its_device *its_create_device(struct its_node *its, u32 dev_id,
>                                                      int nvecs)
> {
> ....
>     /* Bind the device to the first possible CPU */
>     cpu = cpumask_first(cpu_online_mask);
>     dev->collection = &its->collections[cpu];
> ....
> }
> 
> int its_alloc_device_irq(struct its_device *dev, u32 id,
>                                   int *hwirq, unsigned int *irq)
> {
> ...
>     /* Map the GIC irq ID to the device */
>     its_send_mapvi(dev, *hwirq, id);
> ...
> }
> 
> When affinity is set, movi is sent with collection id selected
> for the cpu_mask.
> 
> static int its_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
>                                     bool force)
> {
>     unsigned int cpu = cpumask_any_and(mask_val, cpu_online_mask);
>     ...
>     target_col = &its_dev->its->collections[cpu];
>     ....
>     its_send_movi(its_dev, target_col, id);
>     its_dev->collection = target_col;
>     ...
> }
> 
> So, collection id to Event/LPI mapping is not stored.
> 
> Regards
> Vijay