On Mon, Mar 20, 2017 at 10:21 AM, Sricharan R <sricha...@codeaurora.org> 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, <sricha...@codeaurora.org> 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 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu