Re: [PATCH 1/7] genirq/msi: Add a new field in msi_desc to store an IOMMU cookie

[Robin Murphy]

On 23/04/2019 12:46, Marc Zyngier wrote:
> On 23/04/2019 11:51, Julien Grall wrote:
> > On 4/23/19 11:23 AM, Marc Zyngier wrote:
> > > Hi Julien,
> >
> > Hi Marc,
> >
> > > On 18/04/2019 18:26, Julien Grall wrote:
> > > > When an MSI doorbell is located downstream of an IOMMU, it is required
> > > > to swizzle the physical address with an appropriately-mapped IOVA for any
> > > > device attached to one of our DMA ops domain.
> > > >
> > > > At the moment, the allocation of the mapping may be done when composing
> > > > the message. However, the composing may be done in non-preemtible
> > > > context while the allocation requires to be called from preemptible
> > > > context.
> > > >
> > > > A follow-up patch will split the current logic in two functions
> > > > requiring to keep an IOMMU cookie per MSI.
> > > >
> > > > This patch introduces a new field in msi_desc to store an IOMMU cookie
> > > > when CONFIG_IOMMU_DMA is selected.
> > > >
> > > > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > > > ---
> > > >    include/linux/msi.h | 3 +++
> > > >    1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/include/linux/msi.h b/include/linux/msi.h
> > > > index 7e9b81c3b50d..d7907feef1bb 100644
> > > > --- a/include/linux/msi.h
> > > > +++ b/include/linux/msi.h
> > > > @@ -77,6 +77,9 @@ struct msi_desc {
> > > >    	struct device			*dev;
> > > >    	struct msi_msg			msg;
> > > >    	struct irq_affinity_desc	*affinity;
> > > > +#ifdef CONFIG_IOMMU_DMA
> > > > +	const void			*iommu_cookie;
> > > > +#endif
> > > >    
> > > >    	union {
> > > >    		/* PCI MSI/X specific data */
> > >
> > > Given that this is the only member in this structure that is dependent
> > > on a config option, you could also add a couple of accessors that would
> > > do nothing when IOMMU_DMA is not selected (and use that in the DMA code).
> >
> > I haven't seen any use of the helpers so far because the DMA code is
> > also protected by IOMMU_DMA.
> >
> > I can add the helpers in the next version if you see any use outside of
> > the DMA code.
>
> There may not be any user user yet, but I'd surely like to see the
> accessors. This isn't very different from the stub functions you add in
> patch #2.

If you foresee this being useful in general, do you reckon it would be 
worth decoupling it under its own irqchip-layer Kconfig which can then 
be selected by IOMMU_DMA?

Robin.