From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3] xen/passthrough: Support a single iommu_domain per xen domain per SMMU Date: Tue, 24 Mar 2015 18:04:50 +0000 Message-ID: <5511A742.3060805@linaro.org> References: <1427215377-7581-1-git-send-email-robert.vanvossen@dornerworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YaTCA-0000rK-1j for xen-devel@lists.xenproject.org; Tue, 24 Mar 2015 18:04:54 +0000 Received: by wibdy8 with SMTP id dy8so81899116wib.0 for ; Tue, 24 Mar 2015 11:04:52 -0700 (PDT) In-Reply-To: <1427215377-7581-1-git-send-email-robert.vanvossen@dornerworks.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Robbie VanVossen , xen-devel@lists.xenproject.org Cc: julien.grall@citrix.com, edgar.iglesias@xilinx.com, Josh.Whitehead@dornerworks.com, Ian.Campbell@citrix.com, Stefano.Stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org Hello Robert, On 24/03/2015 16:42, Robbie VanVossen wrote: > If multiple devices are being passed through to the same domain and they > share a single SMMU, then they only require a single iommu_domain. > > In arm_smmu_assign_dev, before a new iommu_domain is created, the > xen_domain->contexts is checked for any iommu_domains that are already > assigned to device that uses the same SMMU as the current device. If one > is found, attach the device to that iommu_domain. If a new one isn't > found, create a new iommu_domain just like before. > > The arm_smmu_deassign_dev function assumes that there is a single > device per iommu_domain. This meant that when the first device was > deassigned, the iommu_domain was freed and when another device was > deassigned a crash occured in xen. > > To fix this, a reference counter was added to the iommu_domain struct. > When an arm_smmu_xen_device references an iommu_domain, the > iommu_domains ref is incremented. When that reference is removed, the > iommu_domains ref is decremented. The iommu_domain will only be freed > when the ref is 0. > > Signed-off-by: Robbie VanVossen > > --- > Changed since v2: > * Fixed coding style > * Removed an unnecessary error message > * Added some helper functions to clean up the workflow in arm_smmu_assign_dev a bit > Changed since v1: > * Fixed coding style for comments > * Move increment/decrement outside of attach/detach functions > * Expanded xen_domain->lock to protect more of the assign/deassign > functions > * Removed iommu_domain add/remove_device functions > --- > xen/drivers/passthrough/arm/smmu.c | 99 ++++++++++++++++++++++++++---------- > 1 file changed, 71 insertions(+), 28 deletions(-) > > diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c > index a7a7da9..be0ecdc 100644 > --- a/xen/drivers/passthrough/arm/smmu.c > +++ b/xen/drivers/passthrough/arm/smmu.c > @@ -223,6 +223,7 @@ struct iommu_domain > /* Runtime SMMU configuration for this iommu_domain */ > struct arm_smmu_domain *priv; > > + atomic_t ref; > /* Used to link iommu_domain contexts for a same domain. > * There is at least one per-SMMU to used by the domain. > * */ > @@ -2564,12 +2565,45 @@ static void arm_smmu_iotlb_flush(struct domain *d, unsigned long gfn, > arm_smmu_iotlb_flush_all(d); > } > > +static struct iommu_domain *arm_smmu_get_domain(struct domain *d, > + struct device *dev) The indentation is wrong here. [..] > - spin_lock(&xen_domain->lock); > - /* Chain the new context to the domain */ > - list_add(&domain->list, &xen_domain->contexts); > - spin_unlock(&xen_domain->lock); > + /* Chain the new context to the domain */ > + list_add(&domain->list, &xen_domain->contexts); > + This line contains trailing white-space. Other than this 2 nits: Reviewed-by: Julien Grall Regards, -- Julien Grall