On Mon, Mar 20, 2017 at 10:21 AM, Sricharan R <sricharan@xxxxxxxxxxxxxx> wrote:
> Hi Rob,
>
> sorry for the delayed response. Was not there mostly last week.
>
>
> On 3/13/2017 11:49 PM, Rob Clark wrote:
>>
>> On Mon, Mar 13, 2017 at 9:38 AM, <sricharan@xxxxxxxxxxxxxx> wrote:
>>>
>>> Hi Rob,
>>>
>>> [..]
>>>
>>>
>>>> +static int qcom_iommu_init_domain(struct iommu_domain *domain,
>>>> + struct qcom_iommu_dev *qcom_iommu,
>>>> + struct iommu_fwspec *fwspec)
>>>> +{
>>>> + struct qcom_iommu_domain *qcom_domain =
>>>> to_qcom_iommu_domain(domain);
>>>> + struct io_pgtable_ops *pgtbl_ops;
>>>> + struct io_pgtable_cfg pgtbl_cfg;
>>>> + int i, ret = 0;
>>>> + u32 reg;
>>>> +
>>>> + mutex_lock(&qcom_domain->init_mutex);
>>>> + if (qcom_domain->iommu)
>>>> + goto out_unlock;
>>>> +
>>>> + pgtbl_cfg = (struct io_pgtable_cfg) {
>>>> + .pgsize_bitmap = qcom_iommu_ops.pgsize_bitmap,
>>>> + .ias = 32,
>>>> + .oas = 40,
>>>> + .tlb = &qcom_gather_ops,
>>>> + .iommu_dev = qcom_iommu->dev,
>>>> + };
>>>> +
>>>> + qcom_domain->iommu = qcom_iommu;
>>>> + pgtbl_ops = alloc_io_pgtable_ops(ARM_32_LPAE_S1, &pgtbl_cfg,
>>>> fwspec);
>>>
>>>
>>>
>>> So why not pass in the ctx pointer itself
>>> that we get below as a cookie ? That would basically
>>> avoid iterating through the list in the tlb_ops ?
>>
>>
>> The issue is that one domain might be attached to a device with multiple
>> ctx's.
>>
>
> ok, interesting, what's the usecase for this ? Dynamic domain
> has 1-ctx shared with multiple domains, but this seems to be
> the inverse.
For example, the gpu has the "user" and "priv" contexts. And at least
for now we attach a single domain. (Actually we might not be
triggering the gpu yet to use the "user" context, so it hasn't really
been an issue yet.. that may possibly anger arm-smmu in the future..)
>> Although perhaps __to_ctx() could be made a bit more clever. I was
>> mostly in "make it work, optimize later" mode ;-)
>>
>> Note also, I'm thinking (both for qcom_iommu and arm-smmu) that we
>> want to move pgtbl alloc into _domain_alloc().. or at least that would
>> allow the driver to iommu_map/unmap() before attaching the domain.
>> (Partly this depends on how the iommu task and/or dynamic domain stuff
>> works out.. but one way or another we want to be able to map things to
>> pagetables that aren't the currently attached pagetables)
>
>
> ok, the dynamic domain patches i remember was tweaking the attach path
> to make this work. So are you trying to avoid that and simple do only
> a dynamic_domain_alloc instead ?
>
I probably need to look more closely at the dynamic domain stuff.
Although I'm not sure that we'll enable per-process pagetables on
anything earlier than a5xx so possibly we only care about arm-smmu for
that..
BR,
-R
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
