Hi Bharat,
On 11/08/2017 05:50 PM, Bharat Kumar Gogada wrote:
> Hi Jean,
>
> +static struct iommu_process *
> +iommu_process_alloc(struct iommu_domain *domain, struct task_struct
> +*task) {
> + int err;
> + int pasid;
> + struct iommu_process *process;
> +
> + if (WARN_ON(!domain->ops->process_alloc ||
> !domain->ops->process_free))
> + return ERR_PTR(-ENODEV);
> +
> + process = domain->ops->process_alloc(task);
> + if (IS_ERR(process))
> + return process;
> + if (!process)
> + return ERR_PTR(-ENOMEM);
> +
> + process->pid = get_task_pid(task, PIDTYPE_PID);
> + process->release = domain->ops->process_free;
> + INIT_LIST_HEAD(&process->domains);
> + kref_init(&process->kref);
> +
> + if (!process->pid) {
> + err = -EINVAL;
> + goto err_free_process;
> + }
> +
> + idr_preload(GFP_KERNEL);
> + spin_lock(&iommu_process_lock);
> + pasid = idr_alloc_cyclic(&iommu_process_idr, process,
> domain->min_pasid,
> + domain->max_pasid + 1, GFP_ATOMIC);
> If EP supports only one pasid; domain->min_pasid=1 and domain->max_pasid=0.
> When idr_alloc_cyclic is called it invokes idr_get_free_cmn function
> where we have following condition. (Based on kernel 4.14-rc6)
> if (!radix_tree_tagged(root, IDR_FREE))
> start = max(start, maxindex + 1);
> if (start > max)
> return ERR_PTR(-ENOSPC);
> Here max is being assigned zero by the time this function is invoked,
> this value is based on domain->max_pasid.
> This condition fails and ENOSPC is returned.
>
> In this case even though hardware supports PASID, BIND flow fails.
It should fail, since we're reserving PASID 0 for non-PASID transactions
with S1DSS=0b10. In addition, the SMMUv3 specification does not allow
using PASID with a single entry. See the description of S1CDMax in 5.2
Stream Table Entry:
"when this field is 0, the substreams of the STE are disabled and one CD
is available. (The minimum useful number of substreams is 2.) Any
transaction with a SubstreamID will be terminated with an abort and a
C_BAD_SUBSTREAMID event recorded."
> Any reason why pasid allocation moved to idr allocations rather than
> bitmap allocations as in v1 patches ?
Yes, idr provides a convenient way to quickly retrieve the context
associated with a PASID, when handling a fault. v1 had the allocation in a
bitmap and storing in a rb-tree. By using an idr we combine both and rely
on a well-tested infrastructure.
Note that in the future we might need to go back to handcrafting the PASID
allocation, but it will probably still be based on idr.
Thanks,
Jean
