Hi Will,
the code looks good overall, I just have some questions below.
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?
> + /* 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?
Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
