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

[Robin Murphy <robin.murphy@arm.com>]

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@arm.com>
>>>> ---
>>>>    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.