> -----Original Message----- > From: Stefano Stabellini [mailto:sstabell...@kernel.org] > Sent: 2017年7月6日 1:58 > To: Wei Chen <wei.c...@arm.com> > Cc: Stefano Stabellini <sstabell...@kernel.org>; xen-devel@lists.xen.org; > Steve Capper <steve.cap...@arm.com>; Kaly Xin <kaly....@arm.com>; Julien Grall > <julien.gr...@arm.com>; nd <n...@arm.com> > Subject: RE: [Xen-devel] [PATCH 1/7] xen/arm: SMMU: Implement the add_device > callback in SMMU > > On Tue, 4 Jul 2017, Wei Chen wrote: > > Hi Stefano, > > > > > -----Original Message----- > > > From: Stefano Stabellini [mailto:sstabell...@kernel.org] > > > Sent: 2017年7月4日 5:58 > > > To: Wei Chen <wei.c...@arm.com> > > > Cc: xen-devel@lists.xen.org; sstabell...@kernel.org; Steve Capper > > > <steve.cap...@arm.com>; Kaly Xin <kaly....@arm.com>; Julien Grall > > > <julien.gr...@arm.com>; nd <n...@arm.com> > > > Subject: Re: [Xen-devel] [PATCH 1/7] xen/arm: SMMU: Implement the > add_device > > > callback in SMMU > > > > > > On Fri, 30 Jun 2017, Wei Chen wrote: > > > > This add_device callback function is taking care of adding a device > > > > to SMMU and make sure it is fully prepare to be used by the SMMU > > > > afterwards. > > > > > > > > In previous code, we don't implement the add_device callback in > > > > iommu_ops for ARM SMMU. We placed the work of add_device to > > > > assign_device callback. The function assign_device should not care > > > > about adding the device to an iommu_group. It might not even be > > > > able to decide how to do that. In this patch, we move this work > > > > back to add_device callback. > > > > > > > > This add_device callback is only called while we are handling all > > > > devices for constructing the Domain0. > > > > > > > > Signed-off-by: Wei Chen <wei.c...@arm.com> > > > > --- > > > > xen/drivers/passthrough/arm/smmu.c | 34 +++++++++++++++++++++++-------- > --- > > > > 1 file changed, 23 insertions(+), 11 deletions(-) > > > > > > > > diff --git a/xen/drivers/passthrough/arm/smmu.c > > > b/xen/drivers/passthrough/arm/smmu.c > > > > index 74c09b0..2efa52d 100644 > > > > --- a/xen/drivers/passthrough/arm/smmu.c > > > > +++ b/xen/drivers/passthrough/arm/smmu.c > > > > @@ -2591,6 +2591,26 @@ static void arm_smmu_destroy_iommu_domain(struct > > > iommu_domain *domain) > > > > xfree(domain); > > > > } > > > > > > > > +static int arm_smmu_xen_add_device(u8 devfn, struct device*dev) > > > > +{ > > > > + if (dt_device_is_protected(dev->of_node)) { > > > > + if (!dev->archdata.iommu) { > > > > + dev->archdata.iommu = xzalloc(struct > arm_smmu_xen_device); > > > > + if (!dev->archdata.iommu) > > > > + return -ENOMEM; > > > > + } > > > > + > > > > + if (!dev_iommu_group(dev)) > > > > + return arm_smmu_add_device(dev); > > > > + } > > > > + > > > > + /* > > > > + * Return 0 if the device is not protected to follow the > > > > behavior > > > > + * of PCI add device. > > > > > > What does this comment mean? > > > > > > > While I was looking at the iommu_add_device which is used by PCI > > counterpart. > > I found it will always return 0 even for device that are not protected by an > IOMMU. > > I would much prefer to keep platform devices have similar behavior as PCI > devices. > > > > So while I was implementing iommu_add_dt_device and arm_smmu_xen_add_device, > I > > returned 0 if the device is not protected by IOMMU. > > I understand now. Please rephrase to: > > "Return 0 (not an error) if the device is not protected by an SMMU, to > match the behavior of iommu_add_device." >
Ok, I will do it in next version. > > > > > > > > + */ > > > > + return 0; > > > > +} > > > > + > > > > static int arm_smmu_assign_dev(struct domain *d, u8 devfn, > > > > struct device *dev, u32 flag) > > > > { > > > > @@ -2600,17 +2620,8 @@ static int arm_smmu_assign_dev(struct domain *d, > u8 > > > devfn, > > > > > > > > xen_domain = dom_iommu(d)->arch.priv; > > > > > > > > - if (!dev->archdata.iommu) { > > > > - dev->archdata.iommu = xzalloc(struct > > > > arm_smmu_xen_device); > > > > - if (!dev->archdata.iommu) > > > > - return -ENOMEM; > > > > - } > > > > - > > > > - if (!dev_iommu_group(dev)) { > > > > - ret = arm_smmu_add_device(dev); > > > > - if (ret) > > > > - return ret; > > > > - } > > > > + if (!dev_iommu_group(dev)) > > > > + return -ENODEV; > > > > > > > > spin_lock(&xen_domain->lock); > > > > > > > > @@ -2784,6 +2795,7 @@ static const struct iommu_ops arm_smmu_iommu_ops = > { > > > > .teardown = arm_smmu_iommu_domain_teardown, > > > > .iotlb_flush = arm_smmu_iotlb_flush, > > > > .iotlb_flush_all = arm_smmu_iotlb_flush_all, > > > > + .add_device = arm_smmu_xen_add_device, > > > > .assign_device = arm_smmu_assign_dev, > > > > .reassign_device = arm_smmu_reassign_dev, > > > > .map_page = arm_smmu_map_page, > > > > -- > > > > 2.7.4 > > > > > > > > > > > > _______________________________________________ > > > > Xen-devel mailing list > > > > Xen-devel@lists.xen.org > > > > https://lists.xen.org/xen-devel > > > > > > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel