On Tue, May 19, 2015 at 04:24:35PM +0100, Joerg Roedel wrote:
> Hi Will,
Hi Joerg,
> the code looks good overall, I just have some questions below.
Great, thanks for having a look. I'll still need to post a v2 to address
some of the other comments I've had.
> On Fri, May 08, 2015 at 07:00:45PM +0100, Will Deacon wrote:
> > +static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device
> > *dev)
> > +{
> > + int ret = 0;
> > + struct arm_smmu_device *smmu;
> > + struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> > + struct arm_smmu_group *smmu_group = arm_smmu_group_get(dev);
> > +
> > + if (!smmu_group)
> > + return -ENOENT;
> > +
> > + /* Already attached to a different domain? */
> > + if (smmu_group->domain && smmu_group->domain != smmu_domain)
> > + return -EEXIST;
> > +
> > + smmu = smmu_group->smmu;
> > + mutex_lock(&smmu_domain->init_mutex);
> > +
> > + if (!smmu_domain->smmu) {
> > + smmu_domain->smmu = smmu;
> > + ret = arm_smmu_domain_finalise(domain);
> > + if (ret) {
> > + smmu_domain->smmu = NULL;
> > + goto out_unlock;
> > + }
> > + } else if (smmu_domain->smmu != smmu) {
> > + dev_err(dev,
> > + "cannot attach to SMMU %s (upstream of %s)\n",
> > + dev_name(smmu_domain->smmu->dev),
> > + dev_name(smmu->dev));
> > + ret = -ENXIO;
> > + goto out_unlock;
> > + }
>
> This looks like all devices in a domain need to be behind the same SMMU
> device, right?
Yes, that's correct. This matches what we do for the arm-smmu driver
already and is mainly brought about by the potential for hardware
differences between different SMMU instances in the same SoC.
> > + /* Page sizes */
> > + if (reg & IDR5_GRAN64K)
> > + pgsize_bitmap |= SZ_64K | SZ_512M;
> > + if (reg & IDR5_GRAN16K)
> > + pgsize_bitmap |= SZ_16K | SZ_32M;
> > + if (reg & IDR5_GRAN4K)
> > + pgsize_bitmap |= SZ_4K | SZ_2M | SZ_1G;
> > +
> > + arm_smmu_ops.pgsize_bitmap &= pgsize_bitmap;
>
> So this could effictivly lead to a zero pgsize_bitmap when there are
> SMMUs in the system with support for different page sizes, no?
Indeed, if there is no common page size then we end up not being able to
support any. I tried to resolve this by moving the bitmap out of the
iommu_ops and into the iommu_domain, but you weren't fond of that idea ;)
Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
