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 11:49 +0200, Julien Grall wrote:
> Hi Ian,
> 
> On 15/07/2015 11:32, Ian Campbell wrote:
> >>>> 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?
> 
> Yes,
> 
> > 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.
> 
> No, vid is not specific to a domain but a device. The virtual event ID 
> is always the same as the physical event ID (See your design document 
> [1]). Furthermore, all the usage of the irq_to_vid in this series are 
> for physical command (see lpi_set_config within this patch).
> 
> >
> > 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.
> 
> On v3 I said: "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)."
> 
> Sorry if it was confusing.

It was me who was confusing the properties of vid with those of vlpi,
sorry.

Not helped by
http://xenbits.xen.org/people/ianc/vits/draftG.html#virtual-lpi-injection confusingly using "virq" instead of "vid".

Ian.

> >>>> 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?
> >
> >> You could do round robin on its_device... It would be exactly the same
> >
> > Routing all LPIs associated with a given its_device to the same
> > collection is not exactly the same as round robin-ing all LPIs from the
> > device over the collections.
> 
> Yes, sorry I was a bit lax on the writing. I wanted to meant that there 
> is not much difference to do it.
> 
> >> and save 2 byte if not more with the alignment per irq_desc.
> >
> > If this is a concern then I would say we would either want a separate
> > array of per-pLPI information which we do not want in irq_desc because
> > it is irq specific, or do add a pointer to its_desc which points to an
> > array of per-event information.
> 
> That would be a good solution. Although, as I said, I don't really care 
> for Xen 4.6. It's more an optimization for 4.7.
> 
> Regards,
> 
> [1] http://xenbits.xen.org/people/ianc/vits/draftG.html#event-id-event
>