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

[Marc Zyngier]

On 23/04/2019 14:19, Robin Murphy wrote:
> 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?

I think that'd be a useful thing to do, as most architectures do not
require this dynamic mapping of MSIs.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...