From mboxrd@z Thu Jan  1 00:00:00 1970
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH 03/15] [swiotlb] Add swiotlb_register_engine
	function.
Date: Tue, 19 Jan 2010 12:25:14 -0500
Message-ID: <20100119172514.GL11986@phenom.dumpdata.com>
References: <1263510064-16788-1-git-send-email-konrad.wilk@oracle.com>
	<1263510064-16788-2-git-send-email-konrad.wilk@oracle.com>
	<1263510064-16788-3-git-send-email-konrad.wilk@oracle.com>
	<1263510064-16788-4-git-send-email-konrad.wilk@oracle.com>
	<20100115014129.GC6021@sequoia.sous-sol.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Return-path: <xen-devel-bounces@lists.xensource.com>
Content-Disposition: inline
In-Reply-To: <20100115014129.GC6021@sequoia.sous-sol.org>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Chris Wright <chrisw@sous-sol.org>
Cc: Ian.Campbell@eu.citrix.com, jeremy@goop.org, xen-devel@lists.xensource.com, joerg.roedel@amd.com, fujita.tomonori@lab.ntt.co.jp, iommu@lists.linux-foundation.org, dwmw2@infradead.org, alex.williamson@hp.com
List-Id: xen-devel@lists.xenproject.org

> > +EXPORT_SYMBOL(iommu_sw);
> 
> should be EXPORT_SYMBOL_GPL

Yup.
> 
> >  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.

Yup. Artificats of previous implementation.
> 
> > +		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.

Ok.