Re: [PATCH v7 09/10] iommu/dma-reserved-iommu: iommu_msi_mapping_translate_msg

[Marc Zyngier]

On 19/04/16 17:56, Eric Auger wrote:
> Introduce iommu_msi_mapping_translate_msg whose role consists in
> detecting whether the device's MSIs must to be mapped into an IOMMU.
> It case it must, the function overrides the MSI msg originally composed
> and replaces the doorbell's PA by a pre-allocated and pre-mapped reserved
> IOVA. In case the corresponding PA region is not found, the function
> returns an error.
> 
> This function is likely to be called in some code that cannot sleep. This
> is the reason why the allocation/mapping is done separately.
> 
> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx>
> 
> ---
> 
> v7: creation
> ---
>  drivers/iommu/dma-reserved-iommu.c | 69 ++++++++++++++++++++++++++++++++++++++
>  include/linux/dma-reserved-iommu.h | 27 +++++++++++++++
>  2 files changed, 96 insertions(+)
> 
> diff --git a/drivers/iommu/dma-reserved-iommu.c b/drivers/iommu/dma-reserved-iommu.c
> index 907a17f..603ee45 100644
> --- a/drivers/iommu/dma-reserved-iommu.c
> +++ b/drivers/iommu/dma-reserved-iommu.c
> @@ -18,6 +18,14 @@
>  #include <linux/iommu.h>
>  #include <linux/iova.h>
>  #include <linux/msi.h>
> +#include <linux/irq.h>
> +
> +#ifdef CONFIG_PHYS_ADDR_T_64BIT
> +#define msg_to_phys_addr(msg) \
> +	(((phys_addr_t)((msg)->address_hi) << 32) | (msg)->address_lo)
> +#else
> +#define msg_to_phys_addr(msg)	((msg)->address_lo)
> +#endif
>  
>  struct reserved_iova_domain {
>  	struct iova_domain *iovad;
> @@ -351,3 +359,64 @@ struct iommu_domain *iommu_msi_mapping_desc_to_domain(struct msi_desc *desc)
>  	return d;
>  }
>  EXPORT_SYMBOL_GPL(iommu_msi_mapping_desc_to_domain);
> +
> +static dma_addr_t iommu_find_reserved_iova(struct iommu_domain *domain,
> +				    phys_addr_t addr, size_t size)
> +{
> +	unsigned long  base_pfn, end_pfn, nb_iommu_pages, order, flags;
> +	size_t iommu_page_size, binding_size;
> +	struct iommu_reserved_binding *b;
> +	phys_addr_t aligned_base, offset;
> +	dma_addr_t iova = DMA_ERROR_CODE;
> +	struct iova_domain *iovad;
> +
> +	spin_lock_irqsave(&domain->reserved_lock, flags);
> +
> +	iovad = (struct iova_domain *)domain->reserved_iova_cookie;
> +
> +	if (!iovad)
> +		goto unlock;
> +
> +	order = iova_shift(iovad);
> +	base_pfn = addr >> order;
> +	end_pfn = (addr + size - 1) >> order;
> +	aligned_base = base_pfn << order;
> +	offset = addr - aligned_base;
> +	nb_iommu_pages = end_pfn - base_pfn + 1;
> +	iommu_page_size = 1 << order;
> +	binding_size = nb_iommu_pages * iommu_page_size;
> +
> +	b = find_reserved_binding(domain, aligned_base, binding_size);
> +	if (b && (b->addr <= aligned_base) &&
> +		(aligned_base + binding_size <=  b->addr + b->size))
> +		iova = b->iova + offset + aligned_base - b->addr;
> +unlock:
> +	spin_unlock_irqrestore(&domain->reserved_lock, flags);
> +	return iova;
> +}
> +
> +int iommu_msi_mapping_translate_msg(struct irq_data *data, struct msi_msg *msg)
> +{

I'm really not keen on passing a full irq_data to something that doesn't
really care about it. What this really needs is the device that
generates the MSIs, which makes a lot more sense (you get the device and
the address from the msi_msg).

Or am I getting it wrong?

> +	struct iommu_domain *d;
> +	struct msi_desc *desc;
> +	dma_addr_t iova;
> +
> +	desc = irq_data_get_msi_desc(data);
> +	if (!desc)
> +		return -EINVAL;
> +
> +	d = iommu_msi_mapping_desc_to_domain(desc);
> +	if (!d)
> +		return 0;
> +
> +	iova = iommu_find_reserved_iova(d, msg_to_phys_addr(msg),
> +					sizeof(phys_addr_t));
> +
> +	if (iova == DMA_ERROR_CODE)
> +		return -EINVAL;
> +
> +	msg->address_lo = lower_32_bits(iova);
> +	msg->address_hi = upper_32_bits(iova);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(iommu_msi_mapping_translate_msg);
> diff --git a/include/linux/dma-reserved-iommu.h b/include/linux/dma-reserved-iommu.h
> index 8373929..04e1912f 100644
> --- a/include/linux/dma-reserved-iommu.h
> +++ b/include/linux/dma-reserved-iommu.h
> @@ -20,6 +20,8 @@
>  
>  struct iommu_domain;
>  struct msi_desc;
> +struct irq_data;
> +struct msi_msg;
>  
>  #ifdef CONFIG_IOMMU_DMA_RESERVED
>  
> @@ -82,6 +84,25 @@ void iommu_put_reserved_iova(struct iommu_domain *domain, phys_addr_t addr);
>   */
>  struct iommu_domain *iommu_msi_mapping_desc_to_domain(struct msi_desc *desc);
>  
> +/**
> + * iommu_msi_mapping_translate_msg: in case the MSI transaction is translated
> + * by an IOMMU, the msg address must be an IOVA instead of a physical address.
> + * This function overwrites the original MSI message containing the doorbell
> + * physical address, result of the primary composition, with the doorbell IOVA.
> + *
> + * The doorbell physical address must be bound previously to an IOVA using
> + * iommu_get_reserved_iova
> + *
> + * @data: irq data handle
> + * @msg: original msi message containing the PA to be overwritten with
> + * the IOVA
> + *
> + * return 0 if the MSI does not need to be mapped or when the PA/IOVA
> + * were successfully swapped; return -EINVAL if the addresses need
> + * to be swapped but not IOMMU binding is found
> + */
> +int iommu_msi_mapping_translate_msg(struct irq_data *data, struct msi_msg *msg);
> +
>  #else
>  
>  static inline int
> @@ -111,5 +132,11 @@ iommu_msi_mapping_desc_to_domain(struct msi_desc *desc)
>  	return NULL;
>  }
>  
> +static inline int iommu_msi_mapping_translate_msg(struct irq_data *data,
> +						  struct msi_msg *msg)
> +{
> +	return 0;
> +}
> +
>  #endif	/* CONFIG_IOMMU_DMA_RESERVED */
>  #endif	/* __DMA_RESERVED_IOMMU_H */
> 

Thanks,

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