Re: [PATCH 03/15] [swiotlb] Add swiotlb_register_engine function.

[Chris Wright <chrisw@sous-sol.org>]

* Konrad Rzeszutek Wilk (konrad.wilk@oracle.com) wrote:
> We set the internal iommu_sw pointer to the passed in swiotlb_engine
> structure. Obviously we also check if the existing iommu_sw is
> set and if so, call iommu_sw->release before the switch-over.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  include/linux/swiotlb.h |    2 +
>  lib/swiotlb.c           |   48 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 781c3aa..3bc3c42 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -118,6 +118,8 @@ struct swiotlb_engine {
>  	void* (*bus_to_virt)(struct device *hwdev, dma_addr_t address);
>  };
>  
> +int swiotlb_register_engine(struct swiotlb_engine *iommu);
> +
>  extern void
>  *swiotlb_alloc_coherent(struct device *hwdev, size_t size,
>  			dma_addr_t *dma_handle, gfp_t flags);
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index e6d9e32..e84f269 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -97,6 +97,12 @@ static phys_addr_t *io_tlb_orig_addr;
>   */
>  static DEFINE_SPINLOCK(io_tlb_lock);
>  
> +/*
> + * The software IOMMU this library will utilize.
> + */
> +struct swiotlb_engine *iommu_sw;
> +EXPORT_SYMBOL(iommu_sw);

should be EXPORT_SYMBOL_GPL

>  static int late_alloc;
>  
>  static int __init
> @@ -126,6 +132,48 @@ static dma_addr_t swiotlb_virt_to_bus(struct device *hwdev,
>  	return phys_to_dma(hwdev, virt_to_phys(address));
>  }
>  
> +/*
> + * Register a software IO TLB engine.
> + *
> + * The registration allows the software IO TLB functions in the
> + * swiotlb library to function properly.
> + *
> + * All the values in the iotlb structure must be set.
> + *
> + * If the registration fails, it is assumed that the caller will free
> + * all of the resources allocated in the swiotlb_engine structure.
> + */
> +int swiotlb_register_engine(struct swiotlb_engine *iommu)
> +{
> +	if (!iommu || !iommu->name || !iommu->release) {
> +		printk(KERN_ERR "DMA: Trying to register a SWIOTLB engine" \
> +				" improperly!");
> +		return -EINVAL;
> +	}
> +
> +	if (iommu_sw && iommu_sw->name) {

According to above, you can't have !iommu_sw->name.

> +		int retval = -EINVAL;
> +
> +		/* 'release' must check for out-standing DMAs and flush them
> +		 *  out or fail. */
> +		if (iommu_sw->release)
> +			retval = iommu_sw->release(iommu_sw);

Same here, you can't have !iommu_sw->release, just call unconditionally.

> +
> +		if (retval) {
> +			printk(KERN_ERR "DMA: %s cannot be released!\n",
> +				iommu_sw->name);
> +			return retval;
> +		}
> +		printk(KERN_INFO "DMA: Replacing [%s] with [%s]\n",
> +			iommu_sw->name, iommu->name);
> +	}
> +
> +	iommu_sw = iommu;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(swiotlb_register_engine);
> +
>  void swiotlb_print_info(void)
>  {
>  	unsigned long bytes = io_tlb_nslabs << IO_TLB_SHIFT;