Re: [PATCH v7 09/11] iommu/vt-d: Add bind guest PASID support
Hi, On 10/29/19 12:11 PM, Jacob Pan wrote: On Tue, 29 Oct 2019 10:54:48 +0800 Lu Baolu wrote: Hi, On 10/29/19 6:29 AM, Jacob Pan wrote: Hi Baolu, Appreciate the thorough review, comments inline. You are welcome. On Sat, 26 Oct 2019 10:01:19 +0800 Lu Baolu wrote: Hi, [...] +* allow multiple bind calls with the same PASID and pdev. +*/ + sdev->users++; + goto out; + } I remember I ever pointed this out before. But I forgot how we addressed it. So forgive me if this has been addressed. What if we have a valid bound svm but @dev doesn't belong to it (a.k.a. @dev not in svm->devs list)? If we are binding a new device to an existing/active PASID, the code will allocate a new sdev and add that to the svm->devs list. But allocating a new sdev and adding device is in below else branch, so it will never reach there, right? No, allocating sdev is outside else branch. Oh, yes! Please ignore it. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 09/11] iommu/vt-d: Add bind guest PASID support
On Tue, 29 Oct 2019 10:54:48 +0800
Lu Baolu wrote:
> Hi,
>
> On 10/29/19 6:29 AM, Jacob Pan wrote:
> > Hi Baolu,
> >
> > Appreciate the thorough review, comments inline.
>
> You are welcome.
>
> >
> > On Sat, 26 Oct 2019 10:01:19 +0800
> > Lu Baolu wrote:
> >
> >> Hi,
> >>
>
> [...]
>
> >>> + * allow multiple bind calls with the
> >>> same PASID and pdev.
> >>> + */
> >>> + sdev->users++;
> >>> + goto out;
> >>> + }
> >>
> >> I remember I ever pointed this out before. But I forgot how we
> >> addressed it. So forgive me if this has been addressed.
> >>
> >> What if we have a valid bound svm but @dev doesn't belong to it
> >> (a.k.a. @dev not in svm->devs list)?
> >>
> > If we are binding a new device to an existing/active PASID, the code
> > will allocate a new sdev and add that to the svm->devs list.
>
> But allocating a new sdev and adding device is in below else branch,
> so it will never reach there, right?
>
No, allocating sdev is outside else branch.
> >>> + } else {
> >>> + /* We come here when PASID has never been bond to
> >>> a device. */
> >>> + svm = kzalloc(sizeof(*svm), GFP_KERNEL);
> >>> + if (!svm) {
> >>> + ret = -ENOMEM;
> >>> + goto out;
> >>> + }
> >>> + /* REVISIT: upper layer/VFIO can track host
> >>> process that bind the PASID.
> >>> + * ioasid_set = mm might be sufficient for vfio
> >>> to check pasid VMM
> >>> + * ownership.
> >>> + */
> >>> + svm->mm = get_task_mm(current);
> >>> + svm->pasid = data->hpasid;
> >>> + if (data->flags & IOMMU_SVA_GPASID_VAL) {
> >>> + svm->gpasid = data->gpasid;
> >>> + svm->flags |= SVM_FLAG_GUEST_PASID;
> >>> + }
> >>> + ioasid_set_data(data->hpasid, svm);
> >>> + INIT_LIST_HEAD_RCU(&svm->devs);
> >>> + INIT_LIST_HEAD(&svm->list);
> >>> +
> >>> + mmput(svm->mm);
> >>> + }
> >>
> >> A blank line, please.
> > looks good.
> >>
> >>> + sdev = kzalloc(sizeof(*sdev), GFP_KERNEL);
> >>> + if (!sdev) {
> >>> + if (list_empty(&svm->devs))
> >>> + kfree(svm);
> >>
> >> This is dangerous. This might leave a wild pointer bound with
> >> gpasid.
> > why is that? can you please explain?
> > if the list is empty that means we just allocated the new svm, no
> > users. why can't we free it here?
>
> svm has been associated with the pasid private data. It needs to be
> unbound from pasid before getting freed. Otherwise, a wild pointer
> will be left.
>
> ioasid_set_data(pasid, NULL);
> kfree(svm);
>
Right, I need to clear the private data here. Thanks!
> >
> >>> + ret = -ENOMEM;
> >>> + goto out;
> >>> + }
> >>> + sdev->dev = dev;
> >>> + sdev->users = 1;
> >>> +
> >>> + /* Set up device context entry for PASID if not enabled
> >>> already */
> >>> + ret = intel_iommu_enable_pasid(iommu, sdev->dev);
> >>> + if (ret) {
> >>> + dev_err(dev, "Failed to enable PASID
> >>> capability\n");
> >>> + kfree(sdev);
> >>> + goto out;
> >>> + }
> >>> +
> >>> + /*
> >>> + * For guest bind, we need to set up PASID table entry as
> >>> follows:
> >>> + * - FLPM matches guest paging mode
> >>> + * - turn on nested mode
> >>> + * - SL guest address width matching
> >>> + */
> >>> + ret = intel_pasid_setup_nested(iommu,
> >>> + dev,
> >>> + (pgd_t *)data->gpgd,
> >>> + data->hpasid,
> >>> + &data->vtd,
> >>> + ddomain,
> >>> + data->addr_width);
> >>> + if (ret) {
> >>> + dev_err(dev, "Failed to set up PASID %llu in
> >>> nested mode, Err %d\n",
> >>> + data->hpasid, ret);
> >>
> >> This error handling is insufficient. You should at least:
> >>
> >> 1. free sdev
> > already done below
> >
> >> 2. if list_empty(&svm->devs)
> >>unbound the svm from gpasid
> >>free svm
> >>
> > yes, agreed.
> >
> >> The same for above error handling. Add a branch for error recovery
> >> at the end of function might help here.
> >>
> > not sure which code is the same as above? could you point it out?
>
> Above last comment. :-)
>
Got it.
> >>> + kfree(sdev);
> >>> + goto out;
> >>> + }
> >>> + svm->flags |= SVM_FLAG_GUEST_MODE;
> >>> +
> >>> + init_rcu_head(&sdev->rcu);
> >>> + list_add_rcu(&sdev->list, &svm->devs);
> >>> + out:
> >>> + mutex_unlock(&pasid_mutex);
> >>> + return ret;
> >>> +}
> >>> +
> >>> +int intel_svm_unbind_gpasid(struct device *dev, int pasid)
> >>> +{
> >>> + struct intel_svm_dev *sdev;
> >>> + struct intel_iommu *iommu;
> >>> + struct intel_svm *svm;
> >>> + int ret = -EINVAL;
> >>> +
> >>> + mutex_lock(&pasid_mutex);
> >>> + iommu = intel_svm_device_to_iommu(dev);
> >>> + if (!iommu)
>
Re: [PATCH v7 09/11] iommu/vt-d: Add bind guest PASID support
Hi,
On 10/29/19 6:29 AM, Jacob Pan wrote:
Hi Baolu,
Appreciate the thorough review, comments inline.
You are welcome.
On Sat, 26 Oct 2019 10:01:19 +0800
Lu Baolu wrote:
Hi,
[...]
+* allow multiple bind calls with the same
PASID and pdev.
+*/
+ sdev->users++;
+ goto out;
+ }
I remember I ever pointed this out before. But I forgot how we
addressed it. So forgive me if this has been addressed.
What if we have a valid bound svm but @dev doesn't belong to it
(a.k.a. @dev not in svm->devs list)?
If we are binding a new device to an existing/active PASID, the code
will allocate a new sdev and add that to the svm->devs list.
But allocating a new sdev and adding device is in below else branch, so
it will never reach there, right?
+ } else {
+ /* We come here when PASID has never been bond to
a device. */
+ svm = kzalloc(sizeof(*svm), GFP_KERNEL);
+ if (!svm) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ /* REVISIT: upper layer/VFIO can track host
process that bind the PASID.
+* ioasid_set = mm might be sufficient for vfio to
check pasid VMM
+* ownership.
+*/
+ svm->mm = get_task_mm(current);
+ svm->pasid = data->hpasid;
+ if (data->flags & IOMMU_SVA_GPASID_VAL) {
+ svm->gpasid = data->gpasid;
+ svm->flags |= SVM_FLAG_GUEST_PASID;
+ }
+ ioasid_set_data(data->hpasid, svm);
+ INIT_LIST_HEAD_RCU(&svm->devs);
+ INIT_LIST_HEAD(&svm->list);
+
+ mmput(svm->mm);
+ }
A blank line, please.
looks good.
+ sdev = kzalloc(sizeof(*sdev), GFP_KERNEL);
+ if (!sdev) {
+ if (list_empty(&svm->devs))
+ kfree(svm);
This is dangerous. This might leave a wild pointer bound with gpasid.
why is that? can you please explain?
if the list is empty that means we just allocated the new svm, no
users. why can't we free it here?
svm has been associated with the pasid private data. It needs to be
unbound from pasid before getting freed. Otherwise, a wild pointer will
be left.
ioasid_set_data(pasid, NULL);
kfree(svm);
+ ret = -ENOMEM;
+ goto out;
+ }
+ sdev->dev = dev;
+ sdev->users = 1;
+
+ /* Set up device context entry for PASID if not enabled
already */
+ ret = intel_iommu_enable_pasid(iommu, sdev->dev);
+ if (ret) {
+ dev_err(dev, "Failed to enable PASID
capability\n");
+ kfree(sdev);
+ goto out;
+ }
+
+ /*
+* For guest bind, we need to set up PASID table entry as
follows:
+* - FLPM matches guest paging mode
+* - turn on nested mode
+* - SL guest address width matching
+*/
+ ret = intel_pasid_setup_nested(iommu,
+ dev,
+ (pgd_t *)data->gpgd,
+ data->hpasid,
+ &data->vtd,
+ ddomain,
+ data->addr_width);
+ if (ret) {
+ dev_err(dev, "Failed to set up PASID %llu in
nested mode, Err %d\n",
+ data->hpasid, ret);
This error handling is insufficient. You should at least:
1. free sdev
already done below
2. if list_empty(&svm->devs)
unbound the svm from gpasid
free svm
yes, agreed.
The same for above error handling. Add a branch for error recovery at
the end of function might help here.
not sure which code is the same as above? could you point it out?
Above last comment. :-)
+ kfree(sdev);
+ goto out;
+ }
+ svm->flags |= SVM_FLAG_GUEST_MODE;
+
+ init_rcu_head(&sdev->rcu);
+ list_add_rcu(&sdev->list, &svm->devs);
+ out:
+ mutex_unlock(&pasid_mutex);
+ return ret;
+}
+
+int intel_svm_unbind_gpasid(struct device *dev, int pasid)
+{
+ struct intel_svm_dev *sdev;
+ struct intel_iommu *iommu;
+ struct intel_svm *svm;
+ int ret = -EINVAL;
+
+ mutex_lock(&pasid_mutex);
+ iommu = intel_svm_device_to_iommu(dev);
+ if (!iommu)
+ goto out;
Make it symmetrical with bind function.
if (WARN_ON(!iommu))
goto out;
sounds good.
+
+ svm = ioasid_find(NULL, pasid, NULL);
+ if (IS_ERR_OR_NULL(svm)) {
+ ret = PTR_ERR(svm);
If svm == NULL, this function will return success. This is not
expected, right?
good catch, will fix.
+ goto out;
+ }
+
+ for_each_svm_dev(svm, dev) {
+ ret = 0;
+ sdev->users--;
+
Re: [PATCH v7 09/11] iommu/vt-d: Add bind guest PASID support
Hi, On 10/28/19 2:03 PM, Tian, Kevin wrote: .dev_disable_feat = intel_iommu_dev_disable_feat, .is_attach_deferred = intel_iommu_is_attach_deferred, .pgsize_bitmap = INTEL_IOMMU_PGSIZES, +#ifdef CONFIG_INTEL_IOMMU_SVM + .sva_bind_gpasid= intel_svm_bind_gpasid, + .sva_unbind_gpasid = intel_svm_unbind_gpasid, +#endif again, pure PASID management logic should be separated from SVM. I am not following, these two functions are SVM functionality, not pure PASID management which is already separated in ioasid.c I should say pure "scalable mode" logic. Above callbacks are not related to host SVM per se. They are serving gpasid requests from guest side, thus part of generic scalable mode capability. Currently these two callbacks are for sva only and the patch has been queued by Joerg for the next rc1. It could be extended to be generic. But it deserves a separated patch. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 03/11] iommu/vt-d: Add custom allocator for IOASID
Hi, On 10/29/19 6:49 AM, Jacob Pan wrote: I'm not sure whether tying above logic to SVA is the right approach. If vcmd interface doesn't work, the whole SM mode doesn't make sense which is based on PASID-granular protection (SVA is only one usage atop). If the only remaining usage of SM is to map gIOVA using reserved PASID#0, then why not disabling SM and just fallback to legacy mode? Based on that I prefer to disabling the SM mode completely (better through an interface), and move the logic out of CONFIG_INTEL_ IOMMU_SVM Unfortunately, it is dangerous to disable SM after boot. SM uses different root/device contexts and pasid table formats. Disabling SM after boot requires changing above from SM format into legacy format. You are correct. Since ioasid registration failure is a rare case. How about moving this part of code up to the early stage of intel_iommu_init() and returning error if hardware present vcmd capability but software fails to register a custom ioasid allocator? It makes sense to me. sounds good to me too, the earlier the less to clean up. Actually, we even could return error directly and abort the iommu initialization. The registration of custom ioasid allocator fails only when memory runs out or software is buggy. In either cases, we should abort iommu initialization. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/arm-smmu: fix "hang" when games exit
On 2019-10-28 10:38 pm, Rob Clark wrote: On Mon, Oct 28, 2019 at 3:20 PM Will Deacon wrote: Hi Rob, On Mon, Oct 07, 2019 at 01:49:06PM -0700, Rob Clark wrote: From: Rob Clark When games, browser, or anything using a lot of GPU buffers exits, there can be many hundreds or thousands of buffers to unmap and free. If the GPU is otherwise suspended, this can cause arm-smmu to resume/suspend for each buffer, resulting 5-10 seconds worth of reprogramming the context bank (arm_smmu_write_context_bank()/arm_smmu_write_s2cr()/etc). To the user it would appear that the system just locked up. A simple solution is to use pm_runtime_put_autosuspend() instead, so we don't immediately suspend the SMMU device. Please can you reword the subject to be a bit more useful? The commit message is great, but the subject is a bit like "fix bug in code" to me. yeah, not the best $subject, but I wasn't quite sure how to fit something better in a reasonable # of chars.. maybe something like: "iommu/arm-smmu: optimize unmap but avoiding toggling runpm state"? FWIW, I'd be inclined to frame it as something like "avoid pathological RPM behaviour for unmaps". Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 03/11] iommu/vt-d: Add custom allocator for IOASID
On Fri, 25 Oct 2019 06:31:04 +
"Tian, Kevin" wrote:
> > From: Jacob Pan [mailto:jacob.jun@linux.intel.com]
> > Sent: Friday, October 25, 2019 3:55 AM
> >
> > When VT-d driver runs in the guest, PASID allocation must be
> > performed via virtual command interface. This patch registers a
> > custom IOASID allocator which takes precedence over the default
> > XArray based allocator. The resulting IOASID allocation will always
> > come from the host. This ensures that PASID namespace is system-
> > wide.
> >
> > Signed-off-by: Lu Baolu
> > Signed-off-by: Liu, Yi L
> > Signed-off-by: Jacob Pan
> > ---
> > drivers/iommu/Kconfig | 1 +
> > drivers/iommu/intel-iommu.c | 67
> > +
> > include/linux/intel-iommu.h | 2 ++
> > 3 files changed, 70 insertions(+)
> >
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index fd50ddbf..961fe5795a90 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -211,6 +211,7 @@ config INTEL_IOMMU_SVM
> > bool "Support for Shared Virtual Memory with Intel IOMMU"
> > depends on INTEL_IOMMU && X86
> > select PCI_PASID
> > + select IOASID
> > select MMU_NOTIFIER
> > help
> > Shared Virtual Memory (SVM) provides a facility for
> > devices diff --git a/drivers/iommu/intel-iommu.c
> > b/drivers/iommu/intel-iommu.c index 3f974919d3bd..ced1d89ef977
> > 100644 --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -1706,6 +1706,9 @@ static void free_dmar_iommu(struct intel_iommu
> > *iommu)
> > if (ecap_prs(iommu->ecap))
> > intel_svm_finish_prq(iommu);
> > }
> > + if (ecap_vcs(iommu->ecap) && vccap_pasid(iommu->vccap))
> > +
> > ioasid_unregister_allocator(&iommu->pasid_allocator); +
> > #endif
> > }
> >
> > @@ -4910,6 +4913,44 @@ static int __init
> > probe_acpi_namespace_devices(void)
> > return 0;
> > }
> >
> > +#ifdef CONFIG_INTEL_IOMMU_SVM
> > +static ioasid_t intel_ioasid_alloc(ioasid_t min, ioasid_t max,
> > void *data) +{
> > + struct intel_iommu *iommu = data;
> > + ioasid_t ioasid;
> > +
> > + /*
> > +* VT-d virtual command interface always uses the full 20
> > bit
> > +* PASID range. Host can partition guest PASID range based
> > on
> > +* policies but it is out of guest's control.
> > +*/
> > + if (min < PASID_MIN || max > intel_pasid_max_id)
> > + return INVALID_IOASID;
> > +
> > + if (vcmd_alloc_pasid(iommu, &ioasid))
> > + return INVALID_IOASID;
> > +
> > + return ioasid;
> > +}
> > +
> > +static void intel_ioasid_free(ioasid_t ioasid, void *data)
> > +{
> > + struct intel_iommu *iommu = data;
> > +
> > + if (!iommu)
> > + return;
> > + /*
> > +* Sanity check the ioasid owner is done at upper layer,
> > e.g. VFIO
> > +* We can only free the PASID when all the devices are
> > unbond.
>
> unbond -> unbound
>
will fix
> > +*/
> > + if (ioasid_find(NULL, ioasid, NULL)) {
> > + pr_alert("Cannot free active IOASID %d\n", ioasid);
> > + return;
> > + }
> > + vcmd_free_pasid(iommu, ioasid);
> > +}
> > +#endif
> > +
> > int __init intel_iommu_init(void)
> > {
> > int ret = -ENODEV;
> > @@ -5020,6 +5061,32 @@ int __init intel_iommu_init(void)
> >"%s", iommu->name);
> > iommu_device_set_ops(&iommu->iommu,
> > &intel_iommu_ops);
> > iommu_device_register(&iommu->iommu);
> > +#ifdef CONFIG_INTEL_IOMMU_SVM
> > + if (ecap_vcs(iommu->ecap) &&
> > vccap_pasid(iommu->vccap)) {
> > + pr_info("Register custom PASID
> > allocator\n");
> > + /*
> > +* Register a custom ASID allocator if we
> > are running
> > +* in a guest, the purpose is to have a
> > system wide PASID
> > +* namespace among all PASID users.
> > +* There can be multiple vIOMMUs in each
> > guest but only
> > +* one allocator is active. All vIOMMU
> > allocators will
> > +* eventually be calling the same host
> > allocator.
> > +*/
> > + iommu->pasid_allocator.alloc =
> > intel_ioasid_alloc;
> > + iommu->pasid_allocator.free =
> > intel_ioasid_free;
> > + iommu->pasid_allocator.pdata = (void
> > *)iommu;
> > + ret = ioasid_register_allocator(&iommu-
> > >pasid_allocator);
> > + if (ret) {
> > + pr_warn("Custom PASID allocator
> > registeration failed\n");
>
> registration
will fix
Thanks!
>
> > + /*
> > +* Disable scalable mode on this
> > IOMMU if there
> > +* is no custom allocator. Mixing
> > SM capable vIOMMU
> > +* and non-SM vIOMMU are n
Re: [PATCH v7 03/11] iommu/vt-d: Add custom allocator for IOASID
On Fri, 25 Oct 2019 15:52:39 +
"Tian, Kevin" wrote:
> > From: Lu Baolu [mailto:baolu...@linux.intel.com]
> > Sent: Friday, October 25, 2019 10:39 PM
> >
> > Hi,
> >
> > On 10/25/19 2:40 PM, Tian, Kevin wrote:
> > ioasid_register_allocator(&iommu->pasid_allocator);
> > + if (ret) {
> > + pr_warn("Custom PASID
> > allocator registeration failed\n");
> > + /*
> > + * Disable scalable mode on
> > this IOMMU if there
> > + * is no custom allocator.
> > Mixing SM capable vIOMMU
> > + * and non-SM vIOMMU are not
> > supported.
> > + */
> > + intel_iommu_sm = 0;
> > >>> It's insufficient to disable scalable mode by only clearing
> > >>> intel_iommu_sm. The DMA_RTADDR_SMT bit in root entry has
> > >>> already
> > >> been
> > >>> set. Probably, you need to
> > >>>
> > >>> for each iommu
> > >>> clear DMA_RTADDR_SMT in root entry
> > >>>
> > >>> Alternatively, since vSVA is the only customer of this custom
> > >>> PASID allocator, is it possible to only disable SVA here?
> > >>>
> > >> Yeah, I think disable SVA is better. We can still do gIOVA in
> > >> SM. I guess we need to introduce a flag for sva_enabled.
> > > I'm not sure whether tying above logic to SVA is the right
> > > approach. If vcmd interface doesn't work, the whole SM mode
> > > doesn't make sense which is based on PASID-granular protection
> > > (SVA is only one usage atop). If the only remaining usage of SM
> > > is to map gIOVA using reserved PASID#0, then why not disabling SM
> > > and just fallback to legacy mode?
> > >
> > > Based on that I prefer to disabling the SM mode completely (better
> > > through an interface), and move the logic out of CONFIG_INTEL_
> > > IOMMU_SVM
> > >
> >
> > Unfortunately, it is dangerous to disable SM after boot. SM uses
> > different root/device contexts and pasid table formats. Disabling SM
> > after boot requires changing above from SM format into legacy
> > format.
>
> You are correct.
>
> >
> > Since ioasid registration failure is a rare case. How about moving
> > this part of code up to the early stage of intel_iommu_init() and
> > returning error if hardware present vcmd capability but software
> > fails to register a custom ioasid allocator?
> >
>
> It makes sense to me.
>
sounds good to me too, the earlier the less to clean up.
> Thanks
> Kevin
[Jacob Pan]
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 04/11] iommu/vt-d: Replace Intel specific PASID allocator with IOASID
On Fri, 25 Oct 2019 06:41:16 +
"Tian, Kevin" wrote:
> > From: Jacob Pan [mailto:jacob.jun@linux.intel.com]
> > Sent: Friday, October 25, 2019 3:55 AM
> >
> > Make use of generic IOASID code to manage PASID allocation,
> > free, and lookup. Replace Intel specific code.
> >
> > Signed-off-by: Jacob Pan
>
> better push this patch separately. It's a generic cleanup.
>
True but might be more efficient to have this cleanup patch paved way.
Since the follow up new guest SVA code uses the new API. So I wanted to
get rid of the old code completely.
> > ---
> > drivers/iommu/intel-iommu.c | 12 ++--
> > drivers/iommu/intel-pasid.c | 36
> > drivers/iommu/intel-svm.c |
> > 39 +++ 3 files changed, 29
> > insertions(+), 58 deletions(-)
> >
> > diff --git a/drivers/iommu/intel-iommu.c
> > b/drivers/iommu/intel-iommu.c index ced1d89ef977..2ea09b988a23
> > 100644 --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -5311,7 +5311,7 @@ static void auxiliary_unlink_device(struct
> > dmar_domain *domain,
> > domain->auxd_refcnt--;
> >
> > if (!domain->auxd_refcnt && domain->default_pasid > 0)
> > - intel_pasid_free_id(domain->default_pasid);
> > + ioasid_free(domain->default_pasid);
> > }
> >
> > static int aux_domain_add_dev(struct dmar_domain *domain,
> > @@ -5329,10 +5329,10 @@ static int aux_domain_add_dev(struct
> > dmar_domain *domain,
> > if (domain->default_pasid <= 0) {
> > int pasid;
> >
> > - pasid = intel_pasid_alloc_id(domain, PASID_MIN,
> > -
> > pci_max_pasids(to_pci_dev(dev)),
> > -GFP_KERNEL);
> > - if (pasid <= 0) {
> > + /* No private data needed for the default pasid */
> > + pasid = ioasid_alloc(NULL, PASID_MIN,
> > pci_max_pasids(to_pci_dev(dev)) - 1,
> > + NULL);
> > + if (pasid == INVALID_IOASID) {
> > pr_err("Can't allocate default pasid\n");
> > return -ENODEV;
> > }
> > @@ -5368,7 +5368,7 @@ static int aux_domain_add_dev(struct
> > dmar_domain *domain,
> > spin_unlock(&iommu->lock);
> > spin_unlock_irqrestore(&device_domain_lock, flags);
> > if (!domain->auxd_refcnt && domain->default_pasid > 0)
> > - intel_pasid_free_id(domain->default_pasid);
> > + ioasid_free(domain->default_pasid);
> >
> > return ret;
> > }
> > diff --git a/drivers/iommu/intel-pasid.c
> > b/drivers/iommu/intel-pasid.c index d81e857d2b25..e79d680fe300
> > 100644 --- a/drivers/iommu/intel-pasid.c
> > +++ b/drivers/iommu/intel-pasid.c
> > @@ -26,42 +26,6 @@
> > */
> > static DEFINE_SPINLOCK(pasid_lock);
> > u32 intel_pasid_max_id = PASID_MAX;
> > -static DEFINE_IDR(pasid_idr);
> > -
> > -int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp)
> > -{
> > - int ret, min, max;
> > -
> > - min = max_t(int, start, PASID_MIN);
> > - max = min_t(int, end, intel_pasid_max_id);
> > -
> > - WARN_ON(in_interrupt());
> > - idr_preload(gfp);
> > - spin_lock(&pasid_lock);
> > - ret = idr_alloc(&pasid_idr, ptr, min, max, GFP_ATOMIC);
> > - spin_unlock(&pasid_lock);
> > - idr_preload_end();
> > -
> > - return ret;
> > -}
> > -
> > -void intel_pasid_free_id(int pasid)
> > -{
> > - spin_lock(&pasid_lock);
> > - idr_remove(&pasid_idr, pasid);
> > - spin_unlock(&pasid_lock);
> > -}
> > -
> > -void *intel_pasid_lookup_id(int pasid)
> > -{
> > - void *p;
> > -
> > - spin_lock(&pasid_lock);
> > - p = idr_find(&pasid_idr, pasid);
> > - spin_unlock(&pasid_lock);
> > -
> > - return p;
> > -}
> >
> > int vcmd_alloc_pasid(struct intel_iommu *iommu, unsigned int
> > *pasid) {
> > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> > index 9b159132405d..a9a7f85a09bc 100644
> > --- a/drivers/iommu/intel-svm.c
> > +++ b/drivers/iommu/intel-svm.c
> > @@ -17,6 +17,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include "intel-pasid.h"
> > @@ -318,16 +319,15 @@ int intel_svm_bind_mm(struct device *dev, int
> > *pasid, int flags, struct svm_dev_
> > if (pasid_max > intel_pasid_max_id)
> > pasid_max = intel_pasid_max_id;
> >
> > - /* Do not use PASID 0 in caching mode (virtualised
> > IOMMU) */
> > - ret = intel_pasid_alloc_id(svm,
> > - !!cap_caching_mode(iommu->cap),
> > - pasid_max - 1,
> > GFP_KERNEL);
> > - if (ret < 0) {
> > + /* Do not use PASID 0, reserved for RID to PASID */
> > + svm->pasid = ioasid_alloc(NULL, PASID_MIN,
> > + pasid_max - 1, svm);
> > + if (svm->pasid == INVALID_IOASID) {
> > kfree(svm);
> > kfree(sdev);
> > +
Re: [PATCH v7 06/11] iommu/vt-d: Avoid duplicated code for PASID setup
On Fri, 25 Oct 2019 06:42:54 +
"Tian, Kevin" wrote:
> > From: Jacob Pan [mailto:jacob.jun@linux.intel.com]
> > Sent: Friday, October 25, 2019 3:55 AM
> >
> > After each setup for PASID entry, related translation caches must be
> > flushed.
> > We can combine duplicated code into one function which is less error
> > prone.
> >
> > Signed-off-by: Jacob Pan
>
> similarly, it doesn't need to be in this series.
Technically true, it is in this series so that we can use the combined
function.
>
> > ---
> > drivers/iommu/intel-pasid.c | 48
> > +--- -
> > 1 file changed, 18 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/iommu/intel-pasid.c
> > b/drivers/iommu/intel-pasid.c index e79d680fe300..ffbd416ed3b8
> > 100644 --- a/drivers/iommu/intel-pasid.c
> > +++ b/drivers/iommu/intel-pasid.c
> > @@ -485,6 +485,21 @@ void intel_pasid_tear_down_entry(struct
> > intel_iommu *iommu,
> > devtlb_invalidation_with_pasid(iommu, dev, pasid);
> > }
> >
> > +static void pasid_flush_caches(struct intel_iommu *iommu,
> > + struct pasid_entry *pte,
> > + int pasid, u16 did)
> > +{
> > + if (!ecap_coherent(iommu->ecap))
> > + clflush_cache_range(pte, sizeof(*pte));
> > +
> > + if (cap_caching_mode(iommu->cap)) {
> > + pasid_cache_invalidation_with_pasid(iommu, did,
> > pasid);
> > + iotlb_invalidation_with_pasid(iommu, did, pasid);
> > + } else {
> > + iommu_flush_write_buffer(iommu);
> > + }
> > +}
> > +
> > /*
> > * Set up the scalable mode pasid table entry for first only
> > * translation type.
> > @@ -530,16 +545,7 @@ int intel_pasid_setup_first_level(struct
> > intel_iommu *iommu,
> > /* Setup Present and PASID Granular Transfer Type: */
> > pasid_set_translation_type(pte, 1);
> > pasid_set_present(pte);
> > -
> > - if (!ecap_coherent(iommu->ecap))
> > - clflush_cache_range(pte, sizeof(*pte));
> > -
> > - if (cap_caching_mode(iommu->cap)) {
> > - pasid_cache_invalidation_with_pasid(iommu, did,
> > pasid);
> > - iotlb_invalidation_with_pasid(iommu, did, pasid);
> > - } else {
> > - iommu_flush_write_buffer(iommu);
> > - }
> > + pasid_flush_caches(iommu, pte, pasid, did);
> >
> > return 0;
> > }
> > @@ -603,16 +609,7 @@ int intel_pasid_setup_second_level(struct
> > intel_iommu *iommu,
> > */
> > pasid_set_sre(pte);
> > pasid_set_present(pte);
> > -
> > - if (!ecap_coherent(iommu->ecap))
> > - clflush_cache_range(pte, sizeof(*pte));
> > -
> > - if (cap_caching_mode(iommu->cap)) {
> > - pasid_cache_invalidation_with_pasid(iommu, did,
> > pasid);
> > - iotlb_invalidation_with_pasid(iommu, did, pasid);
> > - } else {
> > - iommu_flush_write_buffer(iommu);
> > - }
> > + pasid_flush_caches(iommu, pte, pasid, did);
> >
> > return 0;
> > }
> > @@ -646,16 +643,7 @@ int intel_pasid_setup_pass_through(struct
> > intel_iommu *iommu,
> > */
> > pasid_set_sre(pte);
> > pasid_set_present(pte);
> > -
> > - if (!ecap_coherent(iommu->ecap))
> > - clflush_cache_range(pte, sizeof(*pte));
> > -
> > - if (cap_caching_mode(iommu->cap)) {
> > - pasid_cache_invalidation_with_pasid(iommu, did,
> > pasid);
> > - iotlb_invalidation_with_pasid(iommu, did, pasid);
> > - } else {
> > - iommu_flush_write_buffer(iommu);
> > - }
> > + pasid_flush_caches(iommu, pte, pasid, did);
> >
> > return 0;
> > }
> > --
> > 2.7.4
>
[Jacob Pan]
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 08/11] iommu/vt-d: Misc macro clean up for SVM
On Sat, 26 Oct 2019 09:00:51 +0800
Lu Baolu wrote:
> Hi,
>
> On 10/25/19 3:55 AM, Jacob Pan wrote:
> > Use combined macros for_each_svm_dev() to simplify SVM device
> > iteration and error checking.
> >
> > Suggested-by: Andy Shevchenko
> > Signed-off-by: Jacob Pan
> > Reviewed-by: Eric Auger
> > ---
> > drivers/iommu/intel-svm.c | 89
> > ++- 1 file changed, 42
> > insertions(+), 47 deletions(-)
> >
> > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> > index a9a7f85a09bc..a18b02a9709d 100644
> > --- a/drivers/iommu/intel-svm.c
> > +++ b/drivers/iommu/intel-svm.c
> > @@ -212,6 +212,10 @@ static const struct mmu_notifier_ops
> > intel_mmuops = { static DEFINE_MUTEX(pasid_mutex);
> > static LIST_HEAD(global_svm_list);
> >
> > +#define for_each_svm_dev(svm, dev) \
> > + list_for_each_entry(sdev, &svm->devs, list) \
> > + if (dev == sdev->dev) \
> > +
> > int intel_svm_bind_mm(struct device *dev, int *pasid, int flags,
> > struct svm_dev_ops *ops) {
> > struct intel_iommu *iommu =
> > intel_svm_device_to_iommu(dev); @@ -257,15 +261,13 @@ int
> > intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct
> > svm_dev_ goto out; }
> >
> > - list_for_each_entry(sdev, &svm->devs,
> > list) {
> > - if (dev == sdev->dev) {
> > - if (sdev->ops != ops) {
> > - ret = -EBUSY;
> > - goto out;
> > - }
> > - sdev->users++;
> > - goto success;
> > + for_each_svm_dev(svm, dev) {
> > + if (sdev->ops != ops) {
> > + ret = -EBUSY;
> > + goto out;
> > }
> > + sdev->users++;
> > + goto success;
> > }
> >
> > break;
> > @@ -402,50 +404,43 @@ int intel_svm_unbind_mm(struct device *dev,
> > int pasid) goto out;
> >
> > svm = ioasid_find(NULL, pasid, NULL);
> > - if (IS_ERR(svm)) {
> > + if (IS_ERR_OR_NULL(svm)) {
> > ret = PTR_ERR(svm);
> > goto out;
> > }
> >
> > - if (!svm)
> > - goto out;
>
> If svm == NULL here, this function will return success. This isn't
> expected, right?
>
you are right, should handle separately.
Thanks!
> Others looks good to me.
>
> Reviewed-by: Lu Baolu
>
> Best regards,
> baolu
>
> > -
> > - list_for_each_entry(sdev, &svm->devs, list) {
> > - if (dev == sdev->dev) {
> > - ret = 0;
> > - sdev->users--;
> > - if (!sdev->users) {
> > - list_del_rcu(&sdev->list);
> > - /* Flush the PASID cache and IOTLB
> > for this device.
> > -* Note that we do depend on the
> > hardware *not* using
> > -* the PASID any more. Just as we
> > depend on other
> > -* devices never using PASIDs that
> > they have no right
> > -* to use. We have a *shared*
> > PASID table, because it's
> > -* large and has to be physically
> > contiguous. So it's
> > -* hard to be as defensive as we
> > might like. */
> > - intel_pasid_tear_down_entry(iommu,
> > dev, svm->pasid);
> > - intel_flush_svm_range_dev(svm,
> > sdev, 0, -1, 0);
> > - kfree_rcu(sdev, rcu);
> > -
> > - if (list_empty(&svm->devs)) {
> > - /* Clear private data so
> > that free pass check */
> > -
> > ioasid_set_data(svm->pasid, NULL);
> > - ioasid_free(svm->pasid);
> > - if (svm->mm)
> > -
> > mmu_notifier_unregister(&svm->notifier, svm->mm); -
> > - list_del(&svm->list);
> > -
> > - /* We mandate that no page
> > faults may be outstanding
> > -* for the PASID when
> > intel_svm_unbind_mm() is called.
> > -* If that is not obeyed,
> > subtle errors will happen.
> > -* Let's make them less
> > subtle... */
> > - memset(svm, 0x6b,
> > sizeof(*svm));
> > - kfree(svm);
> > - }
> > + for_each_svm_dev(svm, dev) {
> > + ret = 0;
> > + sdev->users--;
> > + if (!sdev->users) {
> > + list_de
Re: [PATCH v7 09/11] iommu/vt-d: Add bind guest PASID support
Hi Baolu,
Appreciate the thorough review, comments inline.
On Sat, 26 Oct 2019 10:01:19 +0800
Lu Baolu wrote:
> Hi,
>
> On 10/25/19 3:55 AM, Jacob Pan wrote:
> > When supporting guest SVA with emulated IOMMU, the guest PASID
> > table is shadowed in VMM. Updates to guest vIOMMU PASID table
> > will result in PASID cache flush which will be passed down to
> > the host as bind guest PASID calls.
> >
> > For the SL page tables, it will be harvested from device's
> > default domain (request w/o PASID), or aux domain in case of
> > mediated device.
> >
> > .-. .---.
> > | vIOMMU| | Guest process CR3, FL only|
> > | | '---'
> > ./
> > | PASID Entry |--- PASID cache flush -
> > '-' |
> > | | V
> > | |CR3 in GPA
> > '-'
> > Guest
> > --| Shadow |--|
> >vv v
> > Host
> > .-. .--.
> > | pIOMMU| | Bind FL for GVA-GPA |
> > | | '--'
> > ./ |
> > | PASID Entry | V (Nested xlate)
> > '\.--.
> > | | |SL for GPA-HPA, default domain|
> > | | '--'
> > '-'
> > Where:
> > - FL = First level/stage one page tables
> > - SL = Second level/stage two page tables
> >
> > Signed-off-by: Jacob Pan
> > Signed-off-by: Liu, Yi L
> > ---
> > drivers/iommu/intel-iommu.c | 4 +
> > drivers/iommu/intel-svm.c | 184
> >
> > include/linux/intel-iommu.h | 8 +- include/linux/intel-svm.h |
> > 17 4 files changed, 212 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/intel-iommu.c
> > b/drivers/iommu/intel-iommu.c index acd1ac787d8b..5fab32fbc4b4
> > 100644 --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -6026,6 +6026,10 @@ const struct iommu_ops intel_iommu_ops = {
> > .dev_disable_feat = intel_iommu_dev_disable_feat,
> > .is_attach_deferred =
> > intel_iommu_is_attach_deferred, .pgsize_bitmap =
> > INTEL_IOMMU_PGSIZES, +#ifdef CONFIG_INTEL_IOMMU_SVM
> > + .sva_bind_gpasid= intel_svm_bind_gpasid,
> > + .sva_unbind_gpasid = intel_svm_unbind_gpasid,
> > +#endif
> > };
> >
> > static void quirk_iommu_igfx(struct pci_dev *dev)
> > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> > index a18b02a9709d..ae13a310cf96 100644
> > --- a/drivers/iommu/intel-svm.c
> > +++ b/drivers/iommu/intel-svm.c
> > @@ -216,6 +216,190 @@ static LIST_HEAD(global_svm_list);
> > list_for_each_entry(sdev, &svm->devs, list) \
> > if (dev == sdev->dev) \
>
> Add an indent tab please.
>
looks good.
> >
> > +int intel_svm_bind_gpasid(struct iommu_domain *domain,
> > + struct device *dev,
> > + struct iommu_gpasid_bind_data *data)
> > +{
> > + struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
> > + struct dmar_domain *ddomain;
> > + struct intel_svm_dev *sdev;
> > + struct intel_svm *svm;
> > + int ret = 0;
> > +
> > + if (WARN_ON(!iommu) || !data)
> > + return -EINVAL;
> > +
> > + if (data->version != IOMMU_GPASID_BIND_VERSION_1 ||
> > + data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
>
> Alignment should match open parenthesis.
>
> Run "scripts/checkpatch.pl --strict" for all in this patch. I will
> ignore others.
>
it was my editor's setting :), will do.
> > + return -EINVAL;
> > +
> > + if (dev_is_pci(dev)) {
> > + /* VT-d supports devices with full 20 bit PASIDs
> > only */
> > + if (pci_max_pasids(to_pci_dev(dev)) != PASID_MAX)
> > + return -EINVAL;
> > + }
> > +
> > + /*
> > +* We only check host PASID range, we have no knowledge to
> > check
> > +* guest PASID range nor do we use the guest PASID.
> > +*/
> > + if (data->hpasid <= 0 || data->hpasid >= PASID_MAX)
> > + return -EINVAL;
> > +
> > + ddomain = to_dmar_domain(domain);
> > + /* REVISIT:
> > +* Sanity check adddress width and paging mode support
>
> s/adddress/address/g
>
good catch, I will add the check for paging mode and this comment is no
longer needed.
> > +* width matching in two dimensions:
> > +* 1. paging mode CPU <= IOMMU
> > +* 2. address width Guest <= Host.
> > +*/ > + mutex_lock(&pasid_mutex);
> > + svm = ioasid_find(NULL, data->hpasid, NULL);
> > + if (IS_ERR(svm)) {
> > + ret = PTR_ERR(svm);
> > + goto out;
> > + }
>
> A blank line looks better.
>
true.
> > + if (svm) {
> > +
Re: [PATCH v2] iommu/arm-smmu: fix "hang" when games exit
Hi Rob,
On Mon, Oct 07, 2019 at 01:49:06PM -0700, Rob Clark wrote:
> From: Rob Clark
>
> When games, browser, or anything using a lot of GPU buffers exits, there
> can be many hundreds or thousands of buffers to unmap and free. If the
> GPU is otherwise suspended, this can cause arm-smmu to resume/suspend
> for each buffer, resulting 5-10 seconds worth of reprogramming the
> context bank (arm_smmu_write_context_bank()/arm_smmu_write_s2cr()/etc).
> To the user it would appear that the system just locked up.
>
> A simple solution is to use pm_runtime_put_autosuspend() instead, so we
> don't immediately suspend the SMMU device.
Please can you reword the subject to be a bit more useful? The commit
message is great, but the subject is a bit like "fix bug in code" to me.
> Signed-off-by: Rob Clark
> ---
> v1: original
> v2: unconditionally use autosuspend, rather than deciding based on what
> consumer does
>
> drivers/iommu/arm-smmu.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 3f1d55fb43c4..b7b41f5001bc 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -289,7 +289,7 @@ static inline int arm_smmu_rpm_get(struct arm_smmu_device
> *smmu)
> static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
> {
> if (pm_runtime_enabled(smmu->dev))
> - pm_runtime_put(smmu->dev);
> + pm_runtime_put_autosuspend(smmu->dev);
> }
>
> static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> @@ -1445,6 +1445,9 @@ static int arm_smmu_attach_dev(struct iommu_domain
> *domain, struct device *dev)
> /* Looks ok, so add the device to the domain */
> ret = arm_smmu_domain_add_master(smmu_domain, fwspec);
Please can you put a comment here explaining what this is doing? An abridged
version of the commit message is fine.
> + pm_runtime_set_autosuspend_delay(smmu->dev, 20);
> + pm_runtime_use_autosuspend(smmu->dev);
Cheers,
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 08/10] iommu/io-pgtable-arm: Rationalise TTBRn handling
On 28/10/2019 15:09, Steven Price wrote: [...] --- a/drivers/iommu/io-pgtable-arm-v7s.c +++ b/drivers/iommu/io-pgtable-arm-v7s.c @@ -822,15 +822,13 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg, /* Ensure the empty pgd is visible before any actual TTBR write */ wmb(); - /* TTBRs */ - cfg->arm_v7s_cfg.ttbr[0] = virt_to_phys(data->pgd) | - ARM_V7S_TTBR_S | ARM_V7S_TTBR_NOS | - (cfg->coherent_walk ? - (ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_WBWA) | - ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_WBWA)) : - (ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_NC) | - ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_NC))); - cfg->arm_v7s_cfg.ttbr[1] = 0; + /* TTBR */ + cfg->arm_v7s_cfg.ttbr = virt_to_phys(data->pgd) | ARM_V7S_TTBR_S | + (cfg->coherent_walk ? (ARM_V7S_TTBR_NOS | + ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_WBWA) | + ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_WBWA)) : +(ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_NC) | + ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_NC))); ARM_V7S_TTBR_NOS seems to have sneaked into the cfg->coherent_walk condition here - which you haven't mentioned in the commit log, so it doesn't look like it should be in this commit. Ah, yes, it's taken a while to remember whether this was something important that got muddled up in rebasing, but it's actually just trivial cleanup. For !coherent_walk, the non-cacheable output attribute makes shareable accesses implicitly outer-shareable, so setting TTBR.NOS for that case actually does nothing except look misleading. Thus this is essentially just a cosmetic change included in the reformatting for clarity and consistency with the LPAE version. I'll call that out in the commit message, thanks for spotting! Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] drivers: iommu: hyperv: Make HYPERV_IOMMU only available on x86
On Thu, Oct 17, 2019 at 08:57:03AM +0800, Boqun Feng wrote: Currently hyperv-iommu is implemented in a x86 specific way, for example, apic is used. So make the HYPERV_IOMMU Kconfig depend on X86 as a preparation for enabling HyperV on architecture other than x86. Cc: Lan Tianyu Cc: Michael Kelley Cc: linux-hyp...@vger.kernel.org Signed-off-by: Boqun Feng (Microsoft) Queued up for hyperv-fixes, thanks! -- Thanks, Sasha ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 11/11] iommu/vt-d: Add svm/sva invalidate function
On Fri, 25 Oct 2019 07:27:26 +
"Tian, Kevin" wrote:
> > From: Jacob Pan [mailto:jacob.jun@linux.intel.com]
> > Sent: Friday, October 25, 2019 3:55 AM
> >
> > When Shared Virtual Address (SVA) is enabled for a guest OS via
> > vIOMMU, we need to provide invalidation support at IOMMU API and
> > driver
> > level. This patch adds Intel VT-d specific function to implement
> > iommu passdown invalidate API for shared virtual address.
> >
> > The use case is for supporting caching structure invalidation
> > of assigned SVM capable devices. Emulated IOMMU exposes queue
> > invalidation capability and passes down all descriptors from the
> > guest to the physical IOMMU.
>
> specifically you may clarify that only invalidations related to
> first-level page table is passed down, because it's guest
> structure being bound to the first-level. other descriptors
> are emulated or translated into other necessary operations.
>
Sounds good, will do.
> >
> > The assumption is that guest to host device ID mapping should be
> > resolved prior to calling IOMMU driver. Based on the device handle,
> > host IOMMU driver can replace certain fields before submit to the
> > invalidation queue.
>
> what is device ID? it's a bit confusing term here.
>
Device ID meant requester IDs, or guest to host PCI BDF mapping should
be resolved such that passdown invalidation is targeting host PCI
device. I will rephrase.
> >
> > Signed-off-by: Jacob Pan
> > Signed-off-by: Ashok Raj
> > Signed-off-by: Liu, Yi L
> > ---
> > drivers/iommu/intel-iommu.c | 170
> >
> > 1 file changed, 170 insertions(+)
> >
> > diff --git a/drivers/iommu/intel-iommu.c
> > b/drivers/iommu/intel-iommu.c index 5fab32fbc4b4..a73e76d6457a
> > 100644 --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -5491,6 +5491,175 @@ static void
> > intel_iommu_aux_detach_device(struct iommu_domain *domain,
> > aux_domain_remove_dev(to_dmar_domain(domain), dev);
> > }
> >
> > +/*
> > + * 2D array for converting and sanitizing IOMMU generic TLB
> > granularity to
> > + * VT-d granularity. Invalidation is typically included in the
> > unmap operation
> > + * as a result of DMA or VFIO unmap. However, for assigned device
> > where guest
> > + * could own the first level page tables without being shadowed by
> > QEMU. In
> > + * this case there is no pass down unmap to the host IOMMU as a
> > result of unmap
> > + * in the guest. Only invalidations are trapped and passed down.
> > + * In all cases, only first level TLB invalidation (request with
> > PASID) can be
> > + * passed down, therefore we do not include IOTLB granularity for
> > request
> > + * without PASID (second level).
> > + *
> > + * For an example, to find the VT-d granularity encoding for IOTLB
> > + * type and page selective granularity within PASID:
> > + * X: indexed by iommu cache type
> > + * Y: indexed by enum iommu_inv_granularity
> > + * [IOMMU_CACHE_INV_TYPE_IOTLB][IOMMU_INV_GRANU_ADDR]
> > + *
> > + * Granu_map array indicates validity of the table. 1: valid, 0:
> > invalid
> > + *
> > + */
> > +const static int
> > inv_type_granu_map[IOMMU_CACHE_INV_TYPE_NR][IOMMU_INV_GRAN
> > U_NR] = {
> > + /* PASID based IOTLB, support PASID selective and page
> > selective */
> > + {0, 1, 1},
> > + /* PASID based dev TLBs, only support all PASIDs or single
> > PASID */
> > + {1, 1, 0},
>
> I forgot previous discussion. is it necessary to pass down dev TLB
> invalidation requests? Can it be handled by host iOMMU driver
> automatically?
>
> > + /* PASID cache */
> > + {1, 1, 0}
> > +};
> > +
> > +const static u64
> > inv_type_granu_table[IOMMU_CACHE_INV_TYPE_NR][IOMMU_INV_GRAN
> > U_NR] = {
> > + /* PASID based IOTLB */
> > + {0, QI_GRAN_NONG_PASID, QI_GRAN_PSI_PASID},
> > + /* PASID based dev TLBs */
> > + {QI_DEV_IOTLB_GRAN_ALL, QI_DEV_IOTLB_GRAN_PASID_SEL, 0},
> > + /* PASID cache */
> > + {QI_PC_ALL_PASIDS, QI_PC_PASID_SEL, 0},
> > +};
> > +
> > +static inline int to_vtd_granularity(int type, int granu, u64
> > *vtd_granu) +{
> > + if (type >= IOMMU_CACHE_INV_TYPE_NR || granu >=
> > IOMMU_INV_GRANU_NR ||
> > + !inv_type_granu_map[type][granu])
> > + return -EINVAL;
> > +
> > + *vtd_granu = inv_type_granu_table[type][granu];
> > +
> > + return 0;
> > +}
> > +
> > +static inline u64 to_vtd_size(u64 granu_size, u64 nr_granules)
> > +{
> > + u64 nr_pages = (granu_size * nr_granules) >>
> > VTD_PAGE_SHIFT; +
> > + /* VT-d size is encoded as 2^size of 4K pages, 0 for 4k, 9
> > for 2MB, etc.
> > +* IOMMU cache invalidate API passes granu_size in bytes,
> > and number of
> > +* granu size in contiguous memory.
> > +*/
> > + return order_base_2(nr_pages);
> > +}
> > +
> > +#ifdef CONFIG_INTEL_IOMMU_SVM
> > +static int intel_iommu_sva_invalidate(struct iommu_domain *domain,
> > + struct device *dev, struct
> > iommu_cache_invalidate_info *inv_info)
Re: [PATCH v7 11/11] iommu/vt-d: Add svm/sva invalidate function
On Mon, 28 Oct 2019 06:06:33 +
"Tian, Kevin" wrote:
> > >>> + /* PASID based dev TLBs, only support all PASIDs or single
> > >>> PASID */
> > >>> + {1, 1, 0},
> > >>
> > >> I forgot previous discussion. is it necessary to pass down dev
> > >> TLB invalidation
> > >> requests? Can it be handled by host iOMMU driver automatically?
> > >
> > > On host SVA, when a memory is unmapped, driver callback will
> > > invalidate dev IOTLB explicitly. So I guess we need to pass down
> > > it for guest case. This is also required for guest iova over 1st
> > > level usage as far as can see.
> > >
> >
> > Sorry, I confused guest vIOVA and guest vSVA. For guest vIOVA, no
> > device TLB invalidation pass down. But currently for guest vSVA,
> > device TLB invalidation is passed down. Perhaps we can avoid
> > passing down dev TLB flush just like what we are doing for guest
> > IOVA.
>
> I think dev TLB is fully handled within IOMMU driver today. It doesn't
> require device driver to explicit toggle. With this then we can fully
> virtualize guest dev TLB invalidation request to save one syscall,
> since the host is supposed to flush dev TLB when serving the earlier
> IOTLB invalidation pass-down.
In the previous discussions, we thought about making IOTLB flush
inclusive, where IOTLB flush would always include device TLB flush. But
we thought such behavior cannot be assumed for all VMMs, some may still
do explicit dev TLB flush. So for completeness, we included dev TLB
here.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 09/11] iommu/vt-d: Add bind guest PASID support
On Mon, 28 Oct 2019 06:03:36 + "Tian, Kevin" wrote: > > > > + .sva_bind_gpasid= intel_svm_bind_gpasid, > > > > + .sva_unbind_gpasid = intel_svm_unbind_gpasid, > > > > +#endif > > > > > > again, pure PASID management logic should be separated from SVM. > > > > > I am not following, these two functions are SVM functionality, not > > pure PASID management which is already separated in ioasid.c > > I should say pure "scalable mode" logic. Above callbacks are not > related to host SVM per se. They are serving gpasid requests from > guest side, thus part of generic scalable mode capability. Got your point, but we are sharing data structures with host SVM, it is very difficult and inefficient to separate the two. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dma: Add support for DMA_ATTR_SYS_CACHE
On Mon, Oct 28, 2019 at 12:37:28PM +0100, Christoph Hellwig wrote: > On Mon, Oct 28, 2019 at 11:24:58AM +, Will Deacon wrote: > > Agreed. The way I /think/ it works is that on many SoCs there is a > > system/last-level cache (LLC) which effectively sits in front of memory for > > all masters. Even if a device isn't coherent with the CPU caches, we still > > want to be able to allocate into the LLC. Why this doesn't happen > > automatically is beyond me, but it appears that on these Qualcomm designs > > you actually have to set the memory attributes up in the page-table to > > ensure that the resulting memory transactions are non-cacheable for the CPU > > but cacheable for the LLC. Without any changes, the transactions are > > non-cacheable in both of them which assumedly has a performance cost. > > > > But you can see that I'm piecing things together myself here. Isaac? > > If that is the case it sounds like we'd want to drive this through > DT properties, not the driver API. But again, without an actual consumer > it pretty much is a moot point anyway. I think there's certainly a DT aspect to it so that the driver knows that the SoC is hooked up this way, but I agree that we need a consumer so that we can figure out how dynamic this needs to be. If it's just a fire-and-forget thing then it needn't escape the IOMMU layer, but I fear that it's probably more flexible than that. If nothing shows up for 5.6, I'll send a patch to remove it (since Jordan said there was something coming soon [1]) Will [1] http://lkml.kernel.org/r/20191024153832.ga7...@jcrouse1-lnx.qualcomm.com ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dma: Add support for DMA_ATTR_SYS_CACHE
On Mon, Oct 28, 2019 at 11:59:04AM +, Robin Murphy wrote: > On 28/10/2019 11:24, Will Deacon wrote: > >Hi Christoph, > > > >On Mon, Oct 28, 2019 at 08:41:56AM +0100, Christoph Hellwig wrote: > >>On Sat, Oct 26, 2019 at 03:12:57AM -0700, isa...@codeaurora.org wrote: > >>>On 2019-10-25 22:30, Christoph Hellwig wrote: > The definition makes very little sense. > >>>Can you please clarify what part doesn’t make sense, and why? > >> > >>It looks like complete garbage to me. That might just be because it > >>uses tons of terms I've never heard of of and which aren't used anywhere > >>in the DMA API. It also might be because it doesn't explain how the > >>flag might actually be practically useful. > > > >Agreed. The way I /think/ it works is that on many SoCs there is a > >system/last-level cache (LLC) which effectively sits in front of memory for > >all masters. Even if a device isn't coherent with the CPU caches, we still > >want to be able to allocate into the LLC. Why this doesn't happen > >automatically is beyond me, but it appears that on these Qualcomm designs > >you actually have to set the memory attributes up in the page-table to > >ensure that the resulting memory transactions are non-cacheable for the CPU > >but cacheable for the LLC. Without any changes, the transactions are > >non-cacheable in both of them which assumedly has a performance cost. > > > >But you can see that I'm piecing things together myself here. Isaac? > > FWIW, that's pretty much how Pratik and Jordan explained it to me - the LLC > sits directly in front of memory and is more or less transparent, although > it might treat CPU and device accesses slightly differently (I don't > remember exactly how the inner cacheablility attribute interacts). Certain > devices don't get much benefit from the LLC, hence the desire for > finer-grained control of their outer allocation policy to avoid more > thrashing than necessary. Furthermore, for stuff in the video/GPU/display > area certain jobs benefit more than others, hence the desire to go even > finer-grained than a per-device control in order to maximise LLC > effectiveness. Robin's description is correct. And we did have a patch for an in-kernel user but it got lost in the wash. I'm hoping Sharat can get a respin in time for 5.5. https://lore.kernel.org/linux-arm-msm/1538744915-25490-8-git-send-email-smase...@codeaurora.org/ Jordan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 08/10] iommu/io-pgtable-arm: Rationalise TTBRn handling
On 25/10/2019 19:08, Robin Murphy wrote:
> TTBR1 values have so far been redundant since no users implement any
> support for split address spaces. Crucially, though, one of the main
> reasons for wanting to do so is to be able to manage each half entirely
> independently, e.g. context-switching one set of mappings without
> disturbing the other. Thus it seems unlikely that tying two tables
> together in a single io_pgtable_cfg would ever be particularly desirable
> or useful.
>
> Streamline the configs to just a single conceptual TTBR value
> representing the allocated table. This paves the way for future users to
> support split address spaces by simply allocating a table and dealing
> with the detailed TTBRn logistics themselves.
>
> Signed-off-by: Robin Murphy
> ---
> drivers/iommu/arm-smmu-v3.c| 2 +-
> drivers/iommu/arm-smmu.c | 9 -
> drivers/iommu/io-pgtable-arm-v7s.c | 16 +++-
> drivers/iommu/io-pgtable-arm.c | 5 ++---
> drivers/iommu/ipmmu-vmsa.c | 2 +-
> drivers/iommu/msm_iommu.c | 4 ++--
> drivers/iommu/mtk_iommu.c | 4 ++--
> drivers/iommu/qcom_iommu.c | 3 +--
> include/linux/io-pgtable.h | 4 ++--
> 9 files changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 3f20e548f1ec..da31e607698f 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2170,7 +2170,7 @@ static int arm_smmu_domain_finalise_s1(struct
> arm_smmu_domain *smmu_domain,
> }
>
> cfg->cd.asid= (u16)asid;
> - cfg->cd.ttbr= pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
> + cfg->cd.ttbr= pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
> cfg->cd.tcr = pgtbl_cfg->arm_lpae_s1_cfg.tcr;
> cfg->cd.mair= pgtbl_cfg->arm_lpae_s1_cfg.mair;
> return 0;
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 2bc3e93b11e6..a249e4e49ead 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -534,13 +534,12 @@ static void arm_smmu_init_context_bank(struct
> arm_smmu_domain *smmu_domain,
> /* TTBRs */
> if (stage1) {
> if (cfg->fmt == ARM_SMMU_CTX_FMT_AARCH32_S) {
> - cb->ttbr[0] = pgtbl_cfg->arm_v7s_cfg.ttbr[0];
> - cb->ttbr[1] = pgtbl_cfg->arm_v7s_cfg.ttbr[1];
> + cb->ttbr[0] = pgtbl_cfg->arm_v7s_cfg.ttbr;
> + cb->ttbr[1] = 0;
> } else {
> - cb->ttbr[0] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
> + cb->ttbr[0] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
> cb->ttbr[0] |= FIELD_PREP(TTBRn_ASID, cfg->asid);
> - cb->ttbr[1] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1];
> - cb->ttbr[1] |= FIELD_PREP(TTBRn_ASID, cfg->asid);
> + cb->ttbr[1] = FIELD_PREP(TTBRn_ASID, cfg->asid);
> }
> } else {
> cb->ttbr[0] = pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
> diff --git a/drivers/iommu/io-pgtable-arm-v7s.c
> b/drivers/iommu/io-pgtable-arm-v7s.c
> index 7c3bd2c3cdca..4d2c1e7f67c4 100644
> --- a/drivers/iommu/io-pgtable-arm-v7s.c
> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> @@ -822,15 +822,13 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct
> io_pgtable_cfg *cfg,
> /* Ensure the empty pgd is visible before any actual TTBR write */
> wmb();
>
> - /* TTBRs */
> - cfg->arm_v7s_cfg.ttbr[0] = virt_to_phys(data->pgd) |
> -ARM_V7S_TTBR_S | ARM_V7S_TTBR_NOS |
> -(cfg->coherent_walk ?
> -(ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_WBWA) |
> - ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_WBWA)) :
> -(ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_NC) |
> - ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_NC)));
> - cfg->arm_v7s_cfg.ttbr[1] = 0;
> + /* TTBR */
> + cfg->arm_v7s_cfg.ttbr = virt_to_phys(data->pgd) | ARM_V7S_TTBR_S |
> + (cfg->coherent_walk ? (ARM_V7S_TTBR_NOS |
> + ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_WBWA) |
> + ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_WBWA)) :
> + (ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_NC) |
> + ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_NC)));
ARM_V7S_TTBR_NOS seems to have sneaked into the cfg->coherent_walk
condition here - which you haven't mentioned in the commit log, so it
doesn't look like it should be in this commit.
Steve
> return &data->iop;
>
> out_free_data:
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 1795df8f7a51..bc0841040ebe 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -872,9 +872,8 @@ arm_64_lp
Re: [PATCH] iommu/intel: Use fallback generic_device_group() for ACPI devices
Hi Chris, Just a quick scan of the dmesg attached in the bugzilla. There are 6 devices reported in ANDD. [0.458662] DMAR: ANDD device: 1 name: \_SB.PCI0.I2C0 [0.458683] DMAR: ANDD device: 2 name: \_SB.PCI0.I2C1 [0.458704] DMAR: ANDD device: 3 name: \_SB.PCI0.SPI0 [0.458724] DMAR: ANDD device: 4 name: \_SB.PCI0.SPI1 [0.458745] DMAR: ANDD device: 6 name: \_SB.PCI0.UA01 [0.458766] DMAR: ANDD device: 7 name: \_SB.PCI0.SDHC On 10/5/19 4:55 AM, Chris Wilson wrote: [2.073922] DMAR: ACPI device "INT33C2:00" under DMAR at fed91000 as 00:15.1 [2.073983] DMAR: ACPI device "INT33C3:00" under DMAR at fed91000 as 00:15.2 [2.074027] DMAR: ACPI device "INT33C0:00" under DMAR at fed91000 as 00:15.3 [2.074072] DMAR: ACPI device "INT33C1:00" under DMAR at fed91000 as 00:15.4 Four of them have been mapped to pci devices with device# 15: [2.074114] DMAR: Failed to find handle for ACPI object \_SB.PCI0.UA01 [2.074156] DMAR: Failed to find handle for ACPI object \_SB.PCI0.SDHC And 2 failed to be probed due to lack of ACPI objects. [2.074208] DMAR: No ATSR found [2.074572] DMAR: dmar0: Using Queued invalidation [2.074629] DMAR: dmar1: Using Queued invalidation [2.110029] pci :00:00.0: Adding to iommu group 0 [2.115703] pci :00:02.0: Adding to iommu group 1 [2.116221] pci :00:03.0: Adding to iommu group 2 [2.116759] pci :00:14.0: Adding to iommu group 3 [2.117276] pci :00:16.0: Adding to iommu group 4 [2.117762] pci :00:1b.0: Adding to iommu group 5 [2.118264] pci :00:1c.0: Adding to iommu group 6 [2.118733] pci :00:1c.2: Adding to iommu group 7 [2.119289] pci :00:1d.0: Adding to iommu group 8 [2.119846] pci :00:1f.0: Adding to iommu group 9 [2.119960] pci :00:1f.2: Adding to iommu group 9 [2.120073] pci :00:1f.3: Adding to iommu group 9 [2.120549] pci :06:00.0: Adding to iommu group 10 IOMMU pci bus scan didn't see pci devices with device# 15. Are these hidden devices? Do you mind posting the output of lspci command? With this fix, do these devices work well? Best regards, baolu [2.120631] [ cut here ] [2.120681] WARNING: CPU: 2 PID: 1 at drivers/iommu/iommu.c:1275 pci_device_group+0x109/0x120 [2.120723] Modules linked in: [2.120744] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.4.0-rc1-CI-CI_DRM_7000+ #1 [2.120782] Hardware name: Dell Inc. XPS 12-9Q33/XPS 12-9Q33, BIOS A04 12/03/2013 [2.120821] RIP: 0010:pci_device_group+0x109/0x120 [2.120848] Code: e9 ff ff 48 85 c0 48 89 c5 75 bd 48 8d 74 24 10 4c 89 e7 e8 49 ea ff ff 48 85 c0 48 89 c5 75 a8 e8 fc ee ff ff 48 89 c5 eb 9e <0f> 0b 48 c7 c5 ea ff ff ff eb 93 e8 37 5f a7 ff 0f 1f 80 00 00 00 [2.120933] RSP: :c9037cd0 EFLAGS: 00010202 [2.120961] RAX: 81639810 RBX: ffea RCX: [2.120996] RDX: RSI: 403efd19 RDI: 88811c08 [2.121031] RBP: 88811c08 R08: 88811a5188f8 R09: fffe [2.121066] R10: ca7d066a R11: 2161dc90 R12: 888118320a58 [2.121100] R13: 888119fc1e50 R14: 0001 R15: 888119fc2300 [2.121136] FS: () GS:88811b90() knlGS: [2.121176] CS: 0010 DS: ES: CR0: 80050033 [2.121205] CR2: CR3: 05210001 CR4: 001606e0 [2.121240] Call Trace: [2.121264] iommu_group_get_for_dev+0x77/0x210 [2.121295] intel_iommu_add_device+0x54/0x1c0 [2.121323] iommu_probe_device+0x43/0xc0 [2.121350] intel_iommu_init+0x11fb/0x12c9 [2.121383] ? set_debug_rodata+0xc/0xc [2.121410] ? set_debug_rodata+0xc/0xc [2.121434] ? e820__memblock_setup+0x5b/0x5b [2.121458] ? pci_iommu_init+0x11/0x3a [2.121471] ? rcu_read_lock_sched_held+0x4d/0x80 [2.121471] pci_iommu_init+0x11/0x3a [2.121471] do_one_initcall+0x58/0x2ff [2.121471] ? set_debug_rodata+0xc/0xc [2.121471] ? rcu_read_lock_sched_held+0x4d/0x80 [2.121471] kernel_init_freeable+0x137/0x1c7 [2.121471] ? rest_init+0x250/0x250 [2.121471] kernel_init+0x5/0x100 [2.121471] ret_from_fork+0x3a/0x50 [2.121471] irq event stamp: 1252438 [2.121471] hardirqs last enabled at (1252437): [] __slab_alloc.isra.84.constprop.89+0x4d/0x70 [2.121471] hardirqs last disabled at (1252438): [] trace_hardirqs_off_thunk+0x1a/0x20 [2.121471] softirqs last enabled at (1252382): [] __do_softirq+0x385/0x47f [2.121471] softirqs last disabled at (1252375): [] irq_exit+0xba/0xc0 [2.121471] ---[ end trace 610717c918cf08f3 ]--- [2.121974] DMAR: ACPI name space devices didn't probe correctly [2.122069] DMAR: Intel(R) Virtualization Technology for Directed I/O Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111906 Signed-off-by: Chris Wilson Cc: Joerg Roedel --- Ta
Re: [PATCH v2 2/3] iommu/dma: wire-up new dma map op .get_virt_addr
On 24/10/2019 13:41, Laurentiu Tudor wrote:
From: Laurentiu Tudor
Add an implementation of the newly introduced dma map op in the
generic DMA IOMMU generic glue layer and wire it up.
Signed-off-by: Laurentiu Tudor
---
drivers/iommu/dma-iommu.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index f321279baf9e..15e76232d697 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1091,6 +1091,21 @@ static unsigned long iommu_dma_get_merge_boundary(struct
device *dev)
return (1UL << __ffs(domain->pgsize_bitmap)) - 1;
}
+static void *iommu_dma_get_virt_addr(struct device *dev, dma_addr_t dma_handle)
+{
+ struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
Note that we'd generally use the iommu_get_dma_domain() fastpath...
+
+ if (domain) {
...wherein we can also assume that the device having iommu_dma_ops
assigned at all implies that a DMA ops domain is in place.
Robin.
+ phys_addr_t phys;
+
+ phys = iommu_iova_to_phys(domain, dma_handle);
+ if (phys)
+ return phys_to_virt(phys);
+ }
+
+ return NULL;
+}
+
static const struct dma_map_ops iommu_dma_ops = {
.alloc = iommu_dma_alloc,
.free = iommu_dma_free,
@@ -1107,6 +1122,7 @@ static const struct dma_map_ops iommu_dma_ops = {
.map_resource = iommu_dma_map_resource,
.unmap_resource = iommu_dma_unmap_resource,
.get_merge_boundary = iommu_dma_get_merge_boundary,
+ .get_virt_addr = iommu_dma_get_virt_addr,
};
/*
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants
On 24/10/2019 13:41, Laurentiu Tudor wrote:
From: Laurentiu Tudor
Introduce a few new dma unmap and sync variants that, on top of the
original variants, return the virtual address corresponding to the
input dma address.
In order to implement this a new dma map op is added and used:
void *get_virt_addr(dev, dma_handle);
It does the actual conversion of an input dma address to the output
virtual address.
At this point, I think it might be better to just change the prototype
of the .unmap_page/.sync_single_for_cpu callbacks themselves. In cases
where .get_virt_addr would be non-trivial, it's most likely duplicating
work that the relevant callback has to do anyway (i.e. where the virtual
and/or physical address is needed internally for a cache maintenance or
bounce buffer operation). It would also help avoid any possible
ambiguity about whether .get_virt_addr returns the VA corresponding
dma_handle (if one exists) rather than the VA of the buffer *mapped to*
dma_handle, which for a bounce-buffering implementation would be
different, and the one you actually need - a naive
phys_to_virt(dma_to_phys(dma_handle)) would lead you to the wrong place
(in fact it looks like DPAA2 would currently go wrong with
"swiotlb=force" and the SMMU disabled or in passthrough).
One question there is whether we'd want careful special-casing to avoid
introducing overhead where unmap/sync are currently complete no-ops, or
whether an extra phys_to_virt() or so in those paths would be tolerable.
Signed-off-by: Laurentiu Tudor
---
include/linux/dma-mapping.h | 55 +
1 file changed, 55 insertions(+)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4a1c4fca475a..ae7bb8a84b9d 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -132,6 +132,7 @@ struct dma_map_ops {
u64 (*get_required_mask)(struct device *dev);
size_t (*max_mapping_size)(struct device *dev);
unsigned long (*get_merge_boundary)(struct device *dev);
+ void *(*get_virt_addr)(struct device *dev, dma_addr_t dma_handle);
};
#define DMA_MAPPING_ERROR (~(dma_addr_t)0)
@@ -304,6 +305,21 @@ static inline void dma_unmap_page_attrs(struct device
*dev, dma_addr_t addr,
debug_dma_unmap_page(dev, addr, size, dir);
}
+static inline struct page *
+dma_unmap_page_attrs_desc(struct device *dev, dma_addr_t addr, size_t size,
+ enum dma_data_direction dir, unsigned long attrs)
+{
+ const struct dma_map_ops *ops = get_dma_ops(dev);
+ void *ptr = NULL;
+
+ if (ops && ops->get_virt_addr)
+ ptr = ops->get_virt_addr(dev, addr);
Note that this doesn't work for dma-direct, but for the sake of arm64 at
least it almost certainly wants to.
Robin.
+ dma_unmap_page_attrs(dev, addr, size, dir, attrs);
+
+ return ptr ? virt_to_page(ptr) : NULL;
+}
+
/*
* dma_maps_sg_attrs returns 0 on error and > 0 on success.
* It should never return a value < 0.
@@ -390,6 +406,21 @@ static inline void dma_sync_single_for_cpu(struct device
*dev, dma_addr_t addr,
debug_dma_sync_single_for_cpu(dev, addr, size, dir);
}
+static inline void *
+dma_sync_single_for_cpu_desc(struct device *dev, dma_addr_t addr, size_t size,
+enum dma_data_direction dir)
+{
+ const struct dma_map_ops *ops = get_dma_ops(dev);
+ void *ptr = NULL;
+
+ if (ops && ops->get_virt_addr)
+ ptr = ops->get_virt_addr(dev, addr);
+
+ dma_sync_single_for_cpu(dev, addr, size, dir);
+
+ return ptr;
+}
+
static inline void dma_sync_single_for_device(struct device *dev,
dma_addr_t addr, size_t size,
enum dma_data_direction dir)
@@ -500,6 +531,12 @@ static inline void dma_sync_single_for_cpu(struct device
*dev, dma_addr_t addr,
size_t size, enum dma_data_direction dir)
{
}
+
+static inline void *
+dma_sync_single_for_cpu_desc(struct device *dev, dma_addr_t addr, size_t size,
+enum dma_data_direction dir)
+{
+}
static inline void dma_sync_single_for_device(struct device *dev,
dma_addr_t addr, size_t size, enum dma_data_direction dir)
{
@@ -594,6 +631,21 @@ static inline void dma_unmap_single_attrs(struct device
*dev, dma_addr_t addr,
return dma_unmap_page_attrs(dev, addr, size, dir, attrs);
}
+static inline void *
+dma_unmap_single_attrs_desc(struct device *dev, dma_addr_t addr, size_t size,
+ enum dma_data_direction dir, unsigned long attrs)
+{
+ const struct dma_map_ops *ops = get_dma_ops(dev);
+ void *ptr = NULL;
+
+ if (ops && ops->get_virt_addr)
+ ptr = ops->get_virt_addr(dev, addr);
+
+ dma_unmap_single_attrs(dev, addr, size, dir, attrs);
+
+ return ptr;
+}
+
static inline voi
Re: [PATCH v2 1/3] dma-mapping: introduce new dma unmap and sync api variants
On Thu, Oct 24, 2019 at 12:41:41PM +, Laurentiu Tudor wrote: > From: Laurentiu Tudor > > Introduce a few new dma unmap and sync variants that, on top of the > original variants, return the virtual address corresponding to the > input dma address. > In order to implement this a new dma map op is added and used: > void *get_virt_addr(dev, dma_handle); > It does the actual conversion of an input dma address to the output > virtual address. We'll definitively need an implementation for dma-direct at least as well. Also as said previously we need a dma_can_unmap_by_dma_addr() or similar helper that tells the driver beforehand if this works, so that the driver can either use a sub-optimal workaround or fail the probe if this functionality isn't implemented. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dma: Add support for DMA_ATTR_SYS_CACHE
On 28/10/2019 11:24, Will Deacon wrote: Hi Christoph, On Mon, Oct 28, 2019 at 08:41:56AM +0100, Christoph Hellwig wrote: On Sat, Oct 26, 2019 at 03:12:57AM -0700, isa...@codeaurora.org wrote: On 2019-10-25 22:30, Christoph Hellwig wrote: The definition makes very little sense. Can you please clarify what part doesn’t make sense, and why? It looks like complete garbage to me. That might just be because it uses tons of terms I've never heard of of and which aren't used anywhere in the DMA API. It also might be because it doesn't explain how the flag might actually be practically useful. Agreed. The way I /think/ it works is that on many SoCs there is a system/last-level cache (LLC) which effectively sits in front of memory for all masters. Even if a device isn't coherent with the CPU caches, we still want to be able to allocate into the LLC. Why this doesn't happen automatically is beyond me, but it appears that on these Qualcomm designs you actually have to set the memory attributes up in the page-table to ensure that the resulting memory transactions are non-cacheable for the CPU but cacheable for the LLC. Without any changes, the transactions are non-cacheable in both of them which assumedly has a performance cost. But you can see that I'm piecing things together myself here. Isaac? FWIW, that's pretty much how Pratik and Jordan explained it to me - the LLC sits directly in front of memory and is more or less transparent, although it might treat CPU and device accesses slightly differently (I don't remember exactly how the inner cacheablility attribute interacts). Certain devices don't get much benefit from the LLC, hence the desire for finer-grained control of their outer allocation policy to avoid more thrashing than necessary. Furthermore, for stuff in the video/GPU/display area certain jobs benefit more than others, hence the desire to go even finer-grained than a per-device control in order to maximise LLC effectiveness. Robin. This is really just an extension of this patch that got mainlined, so that clients that use the DMA API can use IOMMU_QCOM_SYS_CACHE as well: https://patchwork.kernel.org/patch/10946099/ Any without a user in the same series it is a complete no-go anyway. IOMMU_QCOM_SYS_CACHE does not have any current users in the mainline, nor did it have it in the patch series in which it got merged, yet it is still present? Furthermore, there are plans to upstream support for one of our SoCs that may benefit from this, as seen here: https://www.spinics.net/lists/iommu/msg39608.html. Which means it should have never been merged. As a general policy we do not add code to the Linux kernel without actual users. Yes, in this case I was hoping a user would materialise via a different tree, but it didn't happen, hence my post last week about removing this altogether: https://lore.kernel.org/linux-iommu/20191024153832.ga7...@jcrouse1-lnx.qualcomm.com/T/#t which I suspect prompted this patch that unfortunately fails to solve the problem. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dma: Add support for DMA_ATTR_SYS_CACHE
On Mon, Oct 28, 2019 at 11:24:58AM +, Will Deacon wrote: > Agreed. The way I /think/ it works is that on many SoCs there is a > system/last-level cache (LLC) which effectively sits in front of memory for > all masters. Even if a device isn't coherent with the CPU caches, we still > want to be able to allocate into the LLC. Why this doesn't happen > automatically is beyond me, but it appears that on these Qualcomm designs > you actually have to set the memory attributes up in the page-table to > ensure that the resulting memory transactions are non-cacheable for the CPU > but cacheable for the LLC. Without any changes, the transactions are > non-cacheable in both of them which assumedly has a performance cost. > > But you can see that I'm piecing things together myself here. Isaac? If that is the case it sounds like we'd want to drive this through DT properties, not the driver API. But again, without an actual consumer it pretty much is a moot point anyway. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] dpaa2_eth: use new unmap and sync dma api variants
On Mon, Oct 28, 2019 at 10:55:05AM +, Laurentiu Tudor wrote:
> >> @@ -85,9 +75,10 @@ static void free_rx_fd(struct dpaa2_eth_priv *priv,
> >> sgt = vaddr + dpaa2_fd_get_offset(fd);
> >> for (i = 1; i < DPAA2_ETH_MAX_SG_ENTRIES; i++) {
> >> addr = dpaa2_sg_get_addr(&sgt[i]);
> >> - sg_vaddr = dpaa2_iova_to_virt(priv->iommu_domain, addr);
> >> - dma_unmap_page(dev, addr, DPAA2_ETH_RX_BUF_SIZE,
> >> - DMA_BIDIRECTIONAL);
> >> + sg_vaddr = page_to_virt
> >> + (dma_unmap_page_desc(dev, addr,
> >> + DPAA2_ETH_RX_BUF_SIZE,
> >> + DMA_BIDIRECTIONAL));
> >
> > This is doing virt -> page -> virt. Why not just have the new
> > function return the VA corresponding to the addr, which would
> > match the other functions?
>
> I'd really like that as it would get rid of the page_to_virt() calls but
> it will break the symmetry with the dma_map_page() API. I'll let the
> maintainers decide.
It would be symmetric with dma_map_single, though. Maybe we need
both variants?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dma: Add support for DMA_ATTR_SYS_CACHE
Hi Christoph, On Mon, Oct 28, 2019 at 08:41:56AM +0100, Christoph Hellwig wrote: > On Sat, Oct 26, 2019 at 03:12:57AM -0700, isa...@codeaurora.org wrote: > > On 2019-10-25 22:30, Christoph Hellwig wrote: > >> The definition makes very little sense. > > Can you please clarify what part doesn’t make sense, and why? > > It looks like complete garbage to me. That might just be because it > uses tons of terms I've never heard of of and which aren't used anywhere > in the DMA API. It also might be because it doesn't explain how the > flag might actually be practically useful. Agreed. The way I /think/ it works is that on many SoCs there is a system/last-level cache (LLC) which effectively sits in front of memory for all masters. Even if a device isn't coherent with the CPU caches, we still want to be able to allocate into the LLC. Why this doesn't happen automatically is beyond me, but it appears that on these Qualcomm designs you actually have to set the memory attributes up in the page-table to ensure that the resulting memory transactions are non-cacheable for the CPU but cacheable for the LLC. Without any changes, the transactions are non-cacheable in both of them which assumedly has a performance cost. But you can see that I'm piecing things together myself here. Isaac? > > This is > > really just an extension of this patch that got mainlined, so that clients > > that use the DMA API can use IOMMU_QCOM_SYS_CACHE as well: > > https://patchwork.kernel.org/patch/10946099/ > >> Any without a user in the same series it is a complete no-go anyway. > > IOMMU_QCOM_SYS_CACHE does not have any current users in the mainline, nor > > did it have it in the patch series in which it got merged, yet it is still > > present? Furthermore, there are plans to upstream support for one of our > > SoCs that may benefit from this, as seen here: > > https://www.spinics.net/lists/iommu/msg39608.html. > > Which means it should have never been merged. As a general policy we do > not add code to the Linux kernel without actual users. Yes, in this case I was hoping a user would materialise via a different tree, but it didn't happen, hence my post last week about removing this altogether: https://lore.kernel.org/linux-iommu/20191024153832.ga7...@jcrouse1-lnx.qualcomm.com/T/#t which I suspect prompted this patch that unfortunately fails to solve the problem. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 10/13] iommu/mediatek: Remove mtk_iommu_get_m4u_data api
Based on previous modifications in the patchset, A mtk_iommu_data
structure represent a iommu, we will add mtk_iommu_data to mtk_iommu_domain
to show the iommu which mtk_iommu_domain belongs to, so we can get
mtk_iommu_data by mtk_iommu_domain, don't use to "mtk_iommu_get_m4u_data"
any more.
Besides, there is a small SW adjustment, we will move alloc iommu_group
into "create_iommu_group"
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 74 +++
1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 42fad1cf73f3..8c06d2a793a7 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -121,6 +121,7 @@ struct mtk_iommu_domain {
u32 id;
struct iommu_domain domain;
struct iommu_group *group;
+ struct mtk_iommu_data *data;
struct list_headlist;
};
@@ -168,23 +169,6 @@ static LIST_HEAD(m4ulist); /* List all the M4U HWs */
#define for_each_m4u(data) list_for_each_entry(data, &m4ulist, list)
-/*
- * There may be 1 or 2 M4U HWs, But we always expect they are in the same
domain
- * for the performance.
- *
- * Here always return the mtk_iommu_data of the first probed M4U where the
- * iommu domain information is recorded.
- */
-static struct mtk_iommu_data *mtk_iommu_get_m4u_data(void)
-{
- struct mtk_iommu_data *data;
-
- for_each_m4u(data)
- return data;
-
- return NULL;
-}
-
static u32 get_domain_id(struct mtk_iommu_data *data, u32 portid)
{
u32 dom_id = 0;
@@ -403,6 +387,27 @@ static void mtk_iommu_config(struct mtk_iommu_data *data,
}
}
+static struct iommu_group *create_iommu_group(struct mtk_iommu_data *data,
+ struct device *dev)
+{
+ struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
+
+ /* Prepare for allocate mtk_iommu_domain */
+ data->m4u_group = mtk_iommu_get_group(dev);
+ if (!data->m4u_group) {
+ data->m4u_group = iommu_group_alloc();
+ if (IS_ERR(data->m4u_group))
+ dev_err(dev, "Failed to allocate M4U IOMMU group\n");
+ } else {
+ iommu_group_ref_get(data->m4u_group);
+ }
+
+ /* save the latest init device */
+ pgtable->init_dev = dev;
+
+ return data->m4u_group;
+}
+
static struct mtk_iommu_pgtable *create_pgtable(struct mtk_iommu_data *data)
{
struct mtk_iommu_pgtable *pgtable;
@@ -468,7 +473,7 @@ static int mtk_iommu_attach_pgtable(struct mtk_iommu_data
*data,
static struct iommu_domain *mtk_iommu_domain_alloc(unsigned type)
{
struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
- struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
+ struct mtk_iommu_data *data;
struct mtk_iommu_domain *dom;
struct device *dev;
@@ -477,6 +482,7 @@ static struct iommu_domain *mtk_iommu_domain_alloc(unsigned
type)
if (pgtable) {
dev = pgtable->init_dev;
+ data = dev_iommu_fwspec_get(dev)->iommu_priv;
if (!data->m4u_group) {
pr_err("%s, find m4u_group failed\n", __func__);
return NULL;
@@ -497,6 +503,7 @@ static struct iommu_domain *mtk_iommu_domain_alloc(unsigned
type)
if (dom->id >= data->plat_data->dom_cnt)
goto put_dma_cookie;
+ dom->data = data;
dom->group = data->m4u_group;
/* Update our support page sizes bitmap */
dom->domain.pgsize_bitmap = pgtable->cfg.pgsize_bitmap;
@@ -554,7 +561,8 @@ static int mtk_iommu_map(struct iommu_domain *domain,
unsigned long iova,
phys_addr_t paddr, size_t size, int prot)
{
struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
- struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
+ struct mtk_iommu_domain *dom = to_mtk_domain(domain);
+ struct mtk_iommu_data *data = dom->data;
/* The "4GB mode" M4U physically can not use the lower remap of Dram. */
if (data->enable_4GB)
@@ -575,27 +583,30 @@ static size_t mtk_iommu_unmap(struct iommu_domain *domain,
static void mtk_iommu_flush_iotlb_all(struct iommu_domain *domain)
{
- mtk_iommu_tlb_flush_all(mtk_iommu_get_m4u_data());
+ struct mtk_iommu_domain *dom = to_mtk_domain(domain);
+
+ mtk_iommu_tlb_flush_all(dom->data);
}
static void mtk_iommu_iotlb_sync(struct iommu_domain *domain,
struct iommu_iotlb_gather *gather)
{
- struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
+ struct mtk_iommu_domain *dom = to_mtk_domain(domain);
size_t length = gather->end - gather->start;
if (gather->start == ULONG_MAX)
return;
mtk_iommu_tlb_flush_range_syn
[PATCH 11/13] iommu/mediatek: Add iova reserved function
For multiple iommu_domains, we need to reserve some iova
regions, so we will add mtk_iommu_resv_iova_region structure.
It includes the start address and size of iova and iommu_resv_type.
Based on the function, we will realize multiple mtk_iommu_domains
Signed-off-by: Anan Sun
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 47 +++
drivers/iommu/mtk_iommu.h | 12 ++
2 files changed, 59 insertions(+)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 8c06d2a793a7..c0cd7da71c2c 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -697,6 +697,51 @@ static int mtk_iommu_of_xlate(struct device *dev, struct
of_phandle_args *args)
return iommu_fwspec_add_ids(dev, args->args, 1);
}
+/* reserve/dir-map iova region */
+static void mtk_iommu_get_resv_regions(struct device *dev,
+ struct list_head *head)
+{
+ struct mtk_iommu_data *data = dev_iommu_fwspec_get(dev)->iommu_priv;
+ unsigned int i, total_cnt = data->plat_data->resv_cnt;
+ const struct mtk_iommu_resv_iova_region *resv_data;
+ struct iommu_resv_region *region;
+ unsigned long base = 0;
+ size_t size = 0;
+ int prot = IOMMU_WRITE | IOMMU_READ;
+
+ resv_data = data->plat_data->resv_region;
+
+ for (i = 0; i < total_cnt; i++) {
+ size = 0;
+ if (resv_data[i].iova_size) {
+ base = (unsigned long)resv_data[i].iova_base;
+ size = resv_data[i].iova_size;
+ }
+ if (!size)
+ continue;
+
+ region = iommu_alloc_resv_region(base, size, prot,
+resv_data[i].type);
+ if (!region)
+ return;
+
+ list_add_tail(®ion->list, head);
+
+ dev_dbg(data->dev, "%s iova 0x%x ~ 0x%x\n",
+ (resv_data[i].type == IOMMU_RESV_DIRECT) ? "dm" : "rsv",
+ (unsigned int)base, (unsigned int)(base + size - 1));
+ }
+}
+
+static void mtk_iommu_put_resv_regions(struct device *dev,
+ struct list_head *head)
+{
+ struct iommu_resv_region *entry, *next;
+
+ list_for_each_entry_safe(entry, next, head, list)
+ kfree(entry);
+}
+
static const struct iommu_ops mtk_iommu_ops = {
.domain_alloc = mtk_iommu_domain_alloc,
.domain_free= mtk_iommu_domain_free,
@@ -711,6 +756,8 @@ static const struct iommu_ops mtk_iommu_ops = {
.remove_device = mtk_iommu_remove_device,
.device_group = mtk_iommu_device_group,
.of_xlate = mtk_iommu_of_xlate,
+ .get_resv_regions = mtk_iommu_get_resv_regions,
+ .put_resv_regions = mtk_iommu_put_resv_regions,
.pgsize_bitmap = SZ_4K | SZ_64K | SZ_1M | SZ_16M,
};
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index d8aef0d57b1a..10476b23adee 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -36,6 +36,12 @@ enum mtk_iommu_plat {
M4U_MT8183,
};
+struct mtk_iommu_resv_iova_region {
+ dma_addr_t iova_base;
+ size_t iova_size;
+ enum iommu_resv_typetype;
+};
+
/*
* reserved IOVA Domain for IOMMU users of HW limitation.
*/
@@ -68,6 +74,12 @@ struct mtk_iommu_plat_data {
u32 dom_cnt;
unsigned char larbid_remap[2][MTK_LARB_NR_MAX];
const struct mtk_domain_data*dom_data;
+ /*
+* reserve/dir-mapping iova region data
+* todo: for different reserve needs on multiple iommu domains
+*/
+ const unsigned int resv_cnt;
+ const struct mtk_iommu_resv_iova_region *resv_region;
};
struct mtk_iommu_domain;
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 06/13] iommu/mediatek: Change get the way of m4u_group
1. Redefine mtk_iommu_domain structure, it will include iommu_group
and iommu_domain. Different mtk_iommu_domains can be distinguished by
ID. When we realize multiple mtk_iommu_domains, every mtk_iommu_domain
can describe one iova region.
2. In theory, every device has one iommu_group, so this patch will
get iommu_group by checking device. All the devices belong to the same
m4u_group currently, so they also use the same mtk_iommu_domain(id=0).
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 46 +++
1 file changed, 46 insertions(+)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index f264fa8c16a0..27995b2b29a6 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -117,12 +117,16 @@
#define MTK_M4U_TO_PORT(id)((id) & 0x1f)
struct mtk_iommu_domain {
+ u32 id;
struct iommu_domain domain;
+ struct iommu_group *group;
+ struct list_headlist;
};
struct mtk_iommu_pgtable {
struct io_pgtable_cfg cfg;
struct io_pgtable_ops *iop;
+ struct list_headm4u_dom_v2;
};
static struct mtk_iommu_pgtable *share_pgtable;
@@ -173,6 +177,41 @@ static struct mtk_iommu_data *mtk_iommu_get_m4u_data(void)
return NULL;
}
+static u32 get_domain_id(void)
+{
+ /* only support one mtk_iommu_domain currently */
+ return 0;
+}
+
+static u32 mtk_iommu_get_domain_id(void)
+{
+ return get_domain_id();
+}
+
+static struct mtk_iommu_domain *get_mtk_domain(struct device *dev)
+{
+ struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
+ struct mtk_iommu_domain *dom;
+ u32 domain_id = mtk_iommu_get_domain_id();
+
+ list_for_each_entry(dom, &data->pgtable->m4u_dom_v2, list) {
+ if (dom->id == domain_id)
+ return dom;
+ }
+ return NULL;
+}
+
+static struct iommu_group *mtk_iommu_get_group(struct device *dev)
+{
+ struct mtk_iommu_domain *dom;
+
+ dom = get_mtk_domain(dev);
+ if (dom)
+ return dom->group;
+
+ return NULL;
+}
+
static struct mtk_iommu_pgtable *mtk_iommu_get_pgtable(void)
{
return share_pgtable;
@@ -334,6 +373,8 @@ static struct mtk_iommu_pgtable *create_pgtable(struct
mtk_iommu_data *data)
if (!pgtable)
return ERR_PTR(-ENOMEM);
+ INIT_LIST_HEAD(&pgtable->m4u_dom_v2);
+
pgtable->cfg = (struct io_pgtable_cfg) {
.quirks = IO_PGTABLE_QUIRK_ARM_NS |
IO_PGTABLE_QUIRK_NO_PERMS |
@@ -388,6 +429,7 @@ static int mtk_iommu_attach_pgtable(struct mtk_iommu_data
*data,
static struct iommu_domain *mtk_iommu_domain_alloc(unsigned type)
{
struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
+ struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
struct mtk_iommu_domain *dom;
if (type != IOMMU_DOMAIN_DMA)
@@ -405,12 +447,15 @@ static struct iommu_domain
*mtk_iommu_domain_alloc(unsigned type)
if (iommu_get_dma_cookie(&dom->domain))
goto free_dom;
+ dom->group = data->m4u_group;
+ dom->id = mtk_iommu_get_domain_id();
/* Update our support page sizes bitmap */
dom->domain.pgsize_bitmap = pgtable->cfg.pgsize_bitmap;
dom->domain.geometry.aperture_start = 0;
dom->domain.geometry.aperture_end = DMA_BIT_MASK(32);
dom->domain.geometry.force_aperture = true;
+ list_add_tail(&dom->list, &pgtable->m4u_dom_v2);
return &dom->domain;
@@ -566,6 +611,7 @@ static struct iommu_group *mtk_iommu_device_group(struct
device *dev)
}
/* All the client devices are in the same m4u iommu-group */
+ data->m4u_group = mtk_iommu_get_group(dev);
if (!data->m4u_group) {
data->m4u_group = iommu_group_alloc();
if (IS_ERR(data->m4u_group))
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 08/13] iommu/mediatek: Add mtk_domain_data structure
Add mtk_domain_data structure to describe how many iova regions
there are and the relevant the start and end address of each
iova region. The number of iova region is equal to the number
of mtk_iommu_domain. So we will use mtk_domain_data to initialize
the start and end iova of mtk_iommu_domain.
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 17 +++--
drivers/iommu/mtk_iommu.h | 17 +
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 0eacbc473374..8d68a1af8ed5 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -128,6 +128,12 @@ struct mtk_iommu_pgtable {
struct io_pgtable_ops *iop;
struct device *init_dev;
struct list_headm4u_dom_v2;
+ const struct mtk_domain_data*dom_region;
+};
+
+const struct mtk_domain_data single_dom = {
+ .min_iova = 0x0,
+ .max_iova = DMA_BIT_MASK(32)
};
static struct mtk_iommu_pgtable *share_pgtable;
@@ -406,6 +412,7 @@ static struct mtk_iommu_pgtable *create_pgtable(struct
mtk_iommu_data *data)
dev_err(data->dev, "Failed to alloc io pgtable\n");
return ERR_PTR(-EINVAL);
}
+ pgtable->dom_region = data->plat_data->dom_data;
dev_info(data->dev, "%s create pgtable done\n", __func__);
@@ -476,8 +483,10 @@ static struct iommu_domain
*mtk_iommu_domain_alloc(unsigned type)
/* Update our support page sizes bitmap */
dom->domain.pgsize_bitmap = pgtable->cfg.pgsize_bitmap;
- dom->domain.geometry.aperture_start = 0;
- dom->domain.geometry.aperture_end = DMA_BIT_MASK(32);
+ dom->domain.geometry.aperture_start =
+ pgtable->dom_region->min_iova;
+ dom->domain.geometry.aperture_end =
+ pgtable->dom_region->max_iova;
dom->domain.geometry.force_aperture = true;
list_add_tail(&dom->list, &pgtable->m4u_dom_v2);
@@ -958,6 +967,7 @@ static const struct mtk_iommu_plat_data mt2712_data = {
.has_bclk = true,
.has_vld_pa_rng = true,
.dom_cnt = 1,
+ .dom_data = &single_dom,
.larbid_remap[0] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
.inv_sel_reg = REG_MMU_INV_SEL,
};
@@ -965,6 +975,7 @@ static const struct mtk_iommu_plat_data mt2712_data = {
static const struct mtk_iommu_plat_data mt6779_data = {
.m4u_plat = M4U_MT6779,
.dom_cnt = 1,
+ .dom_data = &single_dom,
.larbid_remap[0] = {0, 1, 2, 3, 5, 7, 10, 9},
/* vp6a, vp6b, mdla/core2, mdla/edmc*/
.larbid_remap[1] = {2, 0, 3, 1},
@@ -981,6 +992,7 @@ static const struct mtk_iommu_plat_data mt8173_data = {
.has_bclk = true,
.reset_axi= true,
.dom_cnt = 1,
+ .dom_data = &single_dom,
.larbid_remap[0] = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */
.inv_sel_reg = REG_MMU_INV_SEL,
};
@@ -989,6 +1001,7 @@ static const struct mtk_iommu_plat_data mt8183_data = {
.m4u_plat = M4U_MT8183,
.reset_axi= true,
.dom_cnt = 1,
+ .dom_data = &single_dom,
.larbid_remap[0] = {0, 4, 5, 6, 7, 2, 3, 1},
.inv_sel_reg = REG_MMU_INV_SEL,
};
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index 6801f8496fcc..d8aef0d57b1a 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -36,6 +36,22 @@ enum mtk_iommu_plat {
M4U_MT8183,
};
+/*
+ * reserved IOVA Domain for IOMMU users of HW limitation.
+ */
+
+/*
+ * struct mtk_domain_data: domain configuration
+ * @min_iova: Start address of iova
+ * @max_iova: End address of iova
+ * Note: one user can only belong to one domain
+ */
+
+struct mtk_domain_data {
+ dma_addr_t min_iova;
+ dma_addr_t max_iova;
+};
+
struct mtk_iommu_plat_data {
enum mtk_iommu_plat m4u_plat;
boolhas_4gb_mode;
@@ -51,6 +67,7 @@ struct mtk_iommu_plat_data {
u32 m4u1_mask;
u32 dom_cnt;
unsigned char larbid_remap[2][MTK_LARB_NR_MAX];
+ const struct mtk_domain_data*dom_data;
};
struct mtk_iommu_domain;
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 12/13] iommu/mediatek: Change single domain to multiple domains
Based on one mtk_iommu_domain, this patch supports multiple
mtk_iommu_domains to realize different iova regions.
Every module has one smi_larb port, so we can create different
mtk_iommu_domains by smi_larb port define. So we will add port_mask
variable to mtk_domain_data, if some modules need special iova regions,
they can write smi_larb port which corresponding to themselves to
post_mask variable and specify the start and end address of iova region.
The form of port_mask can use "MTK_M4U_ID(larb, port)", larb and port can
refer to "mt-larb-port.h(ex: mt6779-larb-port.h)" file.
The architecture diagram is as below:
mtk_iommu_pgtable
|
mtk_domain_data
|
-
| | |
mtk_iommu_domain1 mtk_iommu_domain2 mtk_iommu_domain3
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 48 +--
drivers/iommu/mtk_iommu.h | 11 -
2 files changed, 51 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index c0cd7da71c2c..c33ea55a1841 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -130,6 +130,8 @@ struct mtk_iommu_pgtable {
struct io_pgtable_ops *iop;
struct device *init_dev;
struct list_headm4u_dom_v2;
+ spinlock_t domain_lock; /* lock for domain count */
+ u32 domain_count;
const struct mtk_domain_data*dom_region;
};
@@ -172,11 +174,15 @@ static LIST_HEAD(m4ulist);/* List all the M4U HWs
*/
static u32 get_domain_id(struct mtk_iommu_data *data, u32 portid)
{
u32 dom_id = 0;
- int i;
+ const struct mtk_domain_data *mtk_dom_array = data->plat_data->dom_data;
+ int i, j;
- /* only support one mtk_iommu_domain currently(dom_cnt = 1) */
- for (i = 0; i < data->plat_data->dom_cnt; i++)
- return i;
+ for (i = 0; i < data->plat_data->dom_cnt; i++) {
+ for (j = 0; j < MTK_MAX_PORT_NUM; j++) {
+ if (portid == mtk_dom_array[i].port_mask[j])
+ return i;
+ }
+ }
return dom_id;
}
@@ -416,6 +422,8 @@ static struct mtk_iommu_pgtable *create_pgtable(struct
mtk_iommu_data *data)
if (!pgtable)
return ERR_PTR(-ENOMEM);
+ spin_lock_init(&pgtable->domain_lock);
+ pgtable->domain_count = 0;
INIT_LIST_HEAD(&pgtable->m4u_dom_v2);
pgtable->cfg = (struct io_pgtable_cfg) {
@@ -476,6 +484,7 @@ static struct iommu_domain *mtk_iommu_domain_alloc(unsigned
type)
struct mtk_iommu_data *data;
struct mtk_iommu_domain *dom;
struct device *dev;
+ unsigned long flags;
if (type != IOMMU_DOMAIN_DMA)
return NULL;
@@ -503,18 +512,34 @@ static struct iommu_domain
*mtk_iommu_domain_alloc(unsigned type)
if (dom->id >= data->plat_data->dom_cnt)
goto put_dma_cookie;
+ spin_lock_irqsave(&pgtable->domain_lock, flags);
+ if (pgtable->domain_count >= data->plat_data->dom_cnt) {
+ spin_unlock_irqrestore(&pgtable->domain_lock, flags);
+ dev_err(dev, "%s, too many domain, count=%u\n",
+ __func__, pgtable->domain_count);
+ goto put_dma_cookie;
+ }
+ pgtable->domain_count++;
+ spin_unlock_irqrestore(&pgtable->domain_lock, flags);
dom->data = data;
dom->group = data->m4u_group;
+
/* Update our support page sizes bitmap */
dom->domain.pgsize_bitmap = pgtable->cfg.pgsize_bitmap;
dom->domain.geometry.aperture_start =
- pgtable->dom_region->min_iova;
+ pgtable->dom_region[dom->id].min_iova;
dom->domain.geometry.aperture_end =
- pgtable->dom_region->max_iova;
+ pgtable->dom_region[dom->id].max_iova;
dom->domain.geometry.force_aperture = true;
list_add_tail(&dom->list, &pgtable->m4u_dom_v2);
+ dev_info(dev, "%s: dom_id:%u, start:%pa, end:%pa, dom_cnt:%u\n",
+__func__, dom->id,
+&dom->domain.geometry.aperture_start,
+&dom->domain.geometry.aperture_end,
+pgtable->domain_count);
+
return &dom->domain;
put_dma_cookie:
@@ -527,9 +552,17 @@ static struct iommu_domain
*mtk_iommu_domain_alloc(unsigned type)
static void mtk_iommu_domain_free(struct iommu_domain *domain)
{
struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
+ unsigned long flags;
iommu_put_dma_cookie(domain);
kfree(to_mt
[PATCH 09/13] iommu/mediatek: Remove the usage of m4u_dom variable
This patch will remove the usage of the m4u_dom variable.
We have already redefined mtk_iommu_domain structure and it
includes iommu_domain, so m4u_dom variable will not be used.
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 27 ---
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 8d68a1af8ed5..42fad1cf73f3 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -113,6 +113,7 @@
* Get the local arbiter ID and the portid within the larb arbiter
* from mtk_m4u_id which is defined by MTK_M4U_ID.
*/
+#define MTK_M4U_ID(larb, port) (((larb) << 5) | (port))
#define MTK_M4U_TO_LARB(id)(((id) >> 5) & 0xf)
#define MTK_M4U_TO_PORT(id)((id) & 0x1f)
@@ -205,6 +206,22 @@ static u32 mtk_iommu_get_domain_id(struct device *dev)
return get_domain_id(data, portid);
}
+static struct iommu_domain *_get_mtk_domain(struct mtk_iommu_data *data,
+ u32 larbid, u32 portid)
+{
+ u32 domain_id;
+ u32 port_mask = MTK_M4U_ID(larbid, portid);
+ struct mtk_iommu_domain *dom;
+
+ domain_id = get_domain_id(data, port_mask);
+
+ list_for_each_entry(dom, &data->pgtable->m4u_dom_v2, list) {
+ if (dom->id == domain_id)
+ return &dom->domain;
+ }
+ return NULL;
+}
+
static struct mtk_iommu_domain *get_mtk_domain(struct device *dev)
{
struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
@@ -307,7 +324,7 @@ static const struct iommu_flush_ops mtk_iommu_flush_ops = {
static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
{
struct mtk_iommu_data *data = dev_id;
- struct mtk_iommu_domain *dom = data->m4u_dom;
+ struct iommu_domain *domain;
u32 int_state, regval, fault_iova, fault_pa;
unsigned int fault_larb, fault_port, sub_comm = 0;
bool layer, write;
@@ -342,7 +359,8 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
fault_larb = data->plat_data->larbid_remap[data->m4u_id][fault_larb];
- if (report_iommu_fault(&dom->domain, data->dev, fault_iova,
+ domain = _get_mtk_domain(data, fault_larb, fault_port);
+ if (report_iommu_fault(domain, data->dev, fault_iova,
write ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ)) {
dev_err_ratelimited(
data->dev,
@@ -512,16 +530,11 @@ static void mtk_iommu_domain_free(struct iommu_domain
*domain)
static int mtk_iommu_attach_device(struct iommu_domain *domain,
struct device *dev)
{
- struct mtk_iommu_domain *dom = to_mtk_domain(domain);
struct mtk_iommu_data *data = dev_iommu_fwspec_get(dev)->iommu_priv;
if (!data)
return -ENODEV;
- /* Update the pgtable base address register of the M4U HW */
- if (!data->m4u_dom)
- data->m4u_dom = dom;
-
mtk_iommu_config(data, dev, true);
return 0;
}
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 13/13] iommu/mediatek: Add multiple mtk_iommu_domain support for mt6779
For mt6779, it needs to support three mtk_iommu_domains, every
mtk_iommu_domain's iova space is different.
Three mtk_iommu_domains is as below:
1. Normal mtk_iommu_domain exclude 0x4000_~0x47ff_ and
0x7da0_~7fbf_.
2. CCU mtk_iommu_domain include 0x4000_~0x47ff_.
3. VPU mtk_iommu_domain 0x7da0_~0x7fbf_.
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 45 +--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index c33ea55a1841..882fe01ff770 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -140,6 +140,30 @@ const struct mtk_domain_data single_dom = {
.max_iova = DMA_BIT_MASK(32)
};
+/*
+ * related file: mt6779-larb-port.h
+ */
+const struct mtk_domain_data mt6779_multi_dom[] = {
+ /* normal domain */
+ {
+.min_iova = 0x0,
+.max_iova = DMA_BIT_MASK(32),
+ },
+ /* ccu domain */
+ {
+.min_iova = 0x4000,
+.max_iova = 0x4800 - 1,
+.port_mask = {MTK_M4U_ID(9, 21), MTK_M4U_ID(9, 22),
+ MTK_M4U_ID(12, 0), MTK_M4U_ID(12, 1)}
+ },
+ /* vpu domain */
+ {
+.min_iova = 0x7da0,
+.max_iova = 0x7fc0 - 1,
+.port_mask = {MTK_M4U_ID(13, 0)}
+ }
+};
+
static struct mtk_iommu_pgtable *share_pgtable;
static const struct iommu_ops mtk_iommu_ops;
@@ -1055,6 +1079,21 @@ static const struct dev_pm_ops mtk_iommu_pm_ops = {
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
};
+static const struct mtk_iommu_resv_iova_region mt6779_iommu_rsv_list[] = {
+ {
+ .dom_id = 0,
+ .iova_base = 0x4000,/* CCU */
+ .iova_size = 0x800,
+ .type = IOMMU_RESV_RESERVED,
+ },
+ {
+ .dom_id = 0,
+ .iova_base = 0x7da0,/* VPU/MDLA */
+ .iova_size = 0x270,
+ .type = IOMMU_RESV_RESERVED,
+ },
+};
+
static const struct mtk_iommu_plat_data mt2712_data = {
.m4u_plat = M4U_MT2712,
.has_4gb_mode = true,
@@ -1068,8 +1107,10 @@ static const struct mtk_iommu_plat_data mt2712_data = {
static const struct mtk_iommu_plat_data mt6779_data = {
.m4u_plat = M4U_MT6779,
- .dom_cnt = 1,
- .dom_data = &single_dom,
+ .resv_cnt = ARRAY_SIZE(mt6779_iommu_rsv_list),
+ .resv_region = mt6779_iommu_rsv_list,
+ .dom_cnt = ARRAY_SIZE(mt6779_multi_dom),
+ .dom_data = mt6779_multi_dom,
.larbid_remap[0] = {0, 1, 2, 3, 5, 7, 10, 9},
/* vp6a, vp6b, mdla/core2, mdla/edmc*/
.larbid_remap[1] = {2, 0, 3, 1},
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 03/13] iommu/mediatek: Add mtk_iommu_pgtable structure
Start with this patch, we will change the SW architecture
to support multiple domains. SW architecture will has a big change,
so we need to modify a little bit by more than one patch.
The new SW overall architecture is as below:
iommu0 iommu1
| |
---
|
mtk_iommu_pgtable
|
--
|| |
mtk_iommu_domain1 mtk_iommu_domain2 mtk_iommu_domain3
|| |
iommu_group1 iommu_group2iommu_group3
|| |
iommu_domain1iommu_domain2 iommu_domain3
|| |
iova region1(normal) iova region2(CCU) iova region3(VPU)
For current structure, no matter how many iommus there are,
they use the same page table to simplify the usage of module.
In order to make the software architecture more explicit, this
patch will create a global mtk_iommu_pgtable structure to describe
page table and all the iommus use it.
The diagram is as below:
mtk_iommu_data1(MM) mtk_iommu_data2(APU)
| |
| |
--mtk_iommu_pgtable-
We need to create global mtk_iommu_pgtable to include all the iova
regions firstly and special iova regions by divided based on it,
so the information of pgtable needs to be created in device_group.
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 84 +++
drivers/iommu/mtk_iommu.h | 1 +
2 files changed, 85 insertions(+)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index f2847e661137..fcbde6b0f58d 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -123,6 +123,12 @@ struct mtk_iommu_domain {
struct iommu_domain domain;
};
+struct mtk_iommu_pgtable {
+ struct io_pgtable_cfg cfg;
+ struct io_pgtable_ops *iop;
+};
+
+static struct mtk_iommu_pgtable *share_pgtable;
static const struct iommu_ops mtk_iommu_ops;
/*
@@ -170,6 +176,11 @@ static struct mtk_iommu_data *mtk_iommu_get_m4u_data(void)
return NULL;
}
+static struct mtk_iommu_pgtable *mtk_iommu_get_pgtable(void)
+{
+ return share_pgtable;
+}
+
static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
{
return container_of(dom, struct mtk_iommu_domain, domain);
@@ -322,6 +333,13 @@ static int mtk_iommu_domain_finalise(struct
mtk_iommu_domain *dom)
{
struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
+ if (data->pgtable) {
+ dom->cfg = data->pgtable->cfg;
+ dom->iop = data->pgtable->iop;
+ dom->domain.pgsize_bitmap = data->pgtable->cfg.pgsize_bitmap;
+ return 0;
+ }
+
dom->cfg = (struct io_pgtable_cfg) {
.quirks = IO_PGTABLE_QUIRK_ARM_NS |
IO_PGTABLE_QUIRK_NO_PERMS |
@@ -345,6 +363,61 @@ static int mtk_iommu_domain_finalise(struct
mtk_iommu_domain *dom)
return 0;
}
+static struct mtk_iommu_pgtable *create_pgtable(struct mtk_iommu_data *data)
+{
+ struct mtk_iommu_pgtable *pgtable;
+
+ pgtable = kzalloc(sizeof(*pgtable), GFP_KERNEL);
+ if (!pgtable)
+ return ERR_PTR(-ENOMEM);
+
+ pgtable->cfg = (struct io_pgtable_cfg) {
+ .quirks = IO_PGTABLE_QUIRK_ARM_NS |
+ IO_PGTABLE_QUIRK_NO_PERMS |
+ IO_PGTABLE_QUIRK_TLBI_ON_MAP |
+ IO_PGTABLE_QUIRK_ARM_MTK_EXT,
+ .pgsize_bitmap = mtk_iommu_ops.pgsize_bitmap,
+ .ias = 32,
+ .oas = 34,
+ .tlb = &mtk_iommu_flush_ops,
+ .iommu_dev = data->dev,
+ };
+
+ pgtable->iop = alloc_io_pgtable_ops(ARM_V7S, &pgtable->cfg, data);
+ if (!pgtable->iop) {
+ dev_err(data->dev, "Failed to alloc io pgtable\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ dev_info(data->dev, "%s create pgtable done\n", __func__);
+
+ return pgtable;
+}
+
+static int mtk_iommu_attach_pgtable(struct mtk_iommu_data *data,
+ struct device *dev)
+{
+ struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
+
+ /* create share pgtable */
+ if (!pgtable) {
+ pgtable = create_pgtable(data);
+ if (IS_ERR(pgtable)) {
+ dev_err(data->dev, "Failed to create pgtable\n");
+ return -EN
[PATCH 07/13] iommu/mediatek: Add smi_larb info about device
All the devices which used iommu are connected to SMI_larb port,
so when different devices driver execute initialization, iommu
can check larb_id and port_id to distinguish them and make
them match to iommu_group accordingly. We also add dom_cnt variable
to describe the number of mtk_iommu_domain.
Larb_id and port_id define can refer to "mt-larb-port.h(ex:
mt6779-larb-port.h)" file
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 50 ---
drivers/iommu/mtk_iommu.h | 1 +
2 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 27995b2b29a6..0eacbc473374 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -126,6 +126,7 @@ struct mtk_iommu_domain {
struct mtk_iommu_pgtable {
struct io_pgtable_cfg cfg;
struct io_pgtable_ops *iop;
+ struct device *init_dev;
struct list_headm4u_dom_v2;
};
@@ -177,22 +178,35 @@ static struct mtk_iommu_data *mtk_iommu_get_m4u_data(void)
return NULL;
}
-static u32 get_domain_id(void)
+static u32 get_domain_id(struct mtk_iommu_data *data, u32 portid)
{
- /* only support one mtk_iommu_domain currently */
- return 0;
+ u32 dom_id = 0;
+ int i;
+
+ /* only support one mtk_iommu_domain currently(dom_cnt = 1) */
+ for (i = 0; i < data->plat_data->dom_cnt; i++)
+ return i;
+
+ return dom_id;
}
-static u32 mtk_iommu_get_domain_id(void)
+static u32 mtk_iommu_get_domain_id(struct device *dev)
{
- return get_domain_id();
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+ struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
+ u32 portid = fwspec->ids[0];
+
+ return get_domain_id(data, portid);
}
static struct mtk_iommu_domain *get_mtk_domain(struct device *dev)
{
struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
struct mtk_iommu_domain *dom;
- u32 domain_id = mtk_iommu_get_domain_id();
+ u32 domain_id = mtk_iommu_get_domain_id(dev);
+
+ if (domain_id >= data->plat_data->dom_cnt)
+ return NULL;
list_for_each_entry(dom, &data->pgtable->m4u_dom_v2, list) {
if (dom->id == domain_id)
@@ -431,11 +445,18 @@ static struct iommu_domain
*mtk_iommu_domain_alloc(unsigned type)
struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
struct mtk_iommu_domain *dom;
+ struct device *dev;
if (type != IOMMU_DOMAIN_DMA)
return NULL;
- if (!pgtable) {
+ if (pgtable) {
+ dev = pgtable->init_dev;
+ if (!data->m4u_group) {
+ pr_err("%s, find m4u_group failed\n", __func__);
+ return NULL;
+ }
+ } else {
pr_err("%s, pgtable is not ready\n", __func__);
return NULL;
}
@@ -447,8 +468,11 @@ static struct iommu_domain
*mtk_iommu_domain_alloc(unsigned type)
if (iommu_get_dma_cookie(&dom->domain))
goto free_dom;
+ dom->id = mtk_iommu_get_domain_id(dev);
+ if (dom->id >= data->plat_data->dom_cnt)
+ goto put_dma_cookie;
+
dom->group = data->m4u_group;
- dom->id = mtk_iommu_get_domain_id();
/* Update our support page sizes bitmap */
dom->domain.pgsize_bitmap = pgtable->cfg.pgsize_bitmap;
@@ -459,6 +483,8 @@ static struct iommu_domain *mtk_iommu_domain_alloc(unsigned
type)
return &dom->domain;
+put_dma_cookie:
+ iommu_put_dma_cookie(&dom->domain);
free_dom:
kfree(dom);
return NULL;
@@ -619,6 +645,10 @@ static struct iommu_group *mtk_iommu_device_group(struct
device *dev)
} else {
iommu_group_ref_get(data->m4u_group);
}
+
+ /* save the latest init device */
+ pgtable->init_dev = dev;
+
return data->m4u_group;
}
@@ -927,12 +957,14 @@ static const struct mtk_iommu_plat_data mt2712_data = {
.has_4gb_mode = true,
.has_bclk = true,
.has_vld_pa_rng = true,
+ .dom_cnt = 1,
.larbid_remap[0] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
.inv_sel_reg = REG_MMU_INV_SEL,
};
static const struct mtk_iommu_plat_data mt6779_data = {
.m4u_plat = M4U_MT6779,
+ .dom_cnt = 1,
.larbid_remap[0] = {0, 1, 2, 3, 5, 7, 10, 9},
/* vp6a, vp6b, mdla/core2, mdla/edmc*/
.larbid_remap[1] = {2, 0, 3, 1},
@@ -948,6 +980,7 @@ static const struct mtk_iommu_plat_data mt8173_data = {
.has_4gb_mode = true,
.has_bclk = true,
.reset_axi= true,
+ .dom_cnt = 1,
.larbid_remap[0] = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */
.inv_sel_reg = REG_MMU_INV_SEL,
};
@@ -955,6 +988,7 @@ static const struct mtk_iommu_plat_dat
[PATCH 04/13] iommu/mediatek: Remove mtk_iommu_domain_finalise
We already have global mtk_iommu_pgtable structure to describe
page table and create it in group_device, "mtk_iommu_domain_finalise"
is as the same as that, so so we will remove mtk_iommu_domain_finalise.
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 48 ---
1 file changed, 10 insertions(+), 38 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index fcbde6b0f58d..3fa09b12e9f9 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -329,40 +329,6 @@ static void mtk_iommu_config(struct mtk_iommu_data *data,
}
}
-static int mtk_iommu_domain_finalise(struct mtk_iommu_domain *dom)
-{
- struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
-
- if (data->pgtable) {
- dom->cfg = data->pgtable->cfg;
- dom->iop = data->pgtable->iop;
- dom->domain.pgsize_bitmap = data->pgtable->cfg.pgsize_bitmap;
- return 0;
- }
-
- dom->cfg = (struct io_pgtable_cfg) {
- .quirks = IO_PGTABLE_QUIRK_ARM_NS |
- IO_PGTABLE_QUIRK_NO_PERMS |
- IO_PGTABLE_QUIRK_TLBI_ON_MAP |
- IO_PGTABLE_QUIRK_ARM_MTK_EXT,
- .pgsize_bitmap = mtk_iommu_ops.pgsize_bitmap,
- .ias = 32,
- .oas = 34,
- .tlb = &mtk_iommu_flush_ops,
- .iommu_dev = data->dev,
- };
-
- dom->iop = alloc_io_pgtable_ops(ARM_V7S, &dom->cfg, data);
- if (!dom->iop) {
- dev_err(data->dev, "Failed to alloc io pgtable\n");
- return -EINVAL;
- }
-
- /* Update our support page sizes bitmap */
- dom->domain.pgsize_bitmap = dom->cfg.pgsize_bitmap;
- return 0;
-}
-
static struct mtk_iommu_pgtable *create_pgtable(struct mtk_iommu_data *data)
{
struct mtk_iommu_pgtable *pgtable;
@@ -420,11 +386,17 @@ static int mtk_iommu_attach_pgtable(struct mtk_iommu_data
*data,
static struct iommu_domain *mtk_iommu_domain_alloc(unsigned type)
{
+ struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
struct mtk_iommu_domain *dom;
if (type != IOMMU_DOMAIN_DMA)
return NULL;
+ if (!pgtable) {
+ pr_err("%s, pgtable is not ready\n", __func__);
+ return NULL;
+ }
+
dom = kzalloc(sizeof(*dom), GFP_KERNEL);
if (!dom)
return NULL;
@@ -432,8 +404,10 @@ static struct iommu_domain
*mtk_iommu_domain_alloc(unsigned type)
if (iommu_get_dma_cookie(&dom->domain))
goto free_dom;
- if (mtk_iommu_domain_finalise(dom))
- goto put_dma_cookie;
+ dom->cfg = pgtable->cfg;
+ dom->iop = pgtable->iop;
+ /* Update our support page sizes bitmap */
+ dom->domain.pgsize_bitmap = pgtable->cfg.pgsize_bitmap;
dom->domain.geometry.aperture_start = 0;
dom->domain.geometry.aperture_end = DMA_BIT_MASK(32);
@@ -441,8 +415,6 @@ static struct iommu_domain *mtk_iommu_domain_alloc(unsigned
type)
return &dom->domain;
-put_dma_cookie:
- iommu_put_dma_cookie(&dom->domain);
free_dom:
kfree(dom);
return NULL;
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 01/13] dt-bindings: mediatek: Add bindings for MT6779
This patch adds description for MT6779 IOMMU. MT6779 has two iommus, they are MM_IOMMU and APU_IOMMU which use ARM Short-Descriptor translation format. The MT6779 IOMMU hardware diagram is as below, it is only a brief diagram about iommu, it don't focus on the part of smi_larb, so I don't describe the smi_larb detailedly. EMI | -- || MM_IOMMUAPU_IOMMU || SMI_COMMOM--- APU_BUS ||| SMI_LARB(0~11) SMI_LARB12(FAKE)SMI_LARB13(FAKE) ||| || -- || | | | Multimedia engine CCU VPU MDLA EMDA All the connections are hardware fixed, software can not adjust it. From the diagram above, MM_IOMMU provides mapping for multimedia engine, but CCU is connected with smi_common directly, we can take them as larb12. APU_IOMMU provides mapping for APU engine, we can take them larb13. Larb12 and Larb13 are fake larbs. Signed-off-by: Chao Hao --- .../bindings/iommu/mediatek,iommu.txt | 2 + include/dt-bindings/memory/mt6779-larb-port.h | 217 ++ 2 files changed, 219 insertions(+) create mode 100644 include/dt-bindings/memory/mt6779-larb-port.h diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt index ce59a505f5a4..c1ccd8582eb2 100644 --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt @@ -58,6 +58,7 @@ Required properties: - compatible : must be one of the following string: "mediatek,mt2701-m4u" for mt2701 which uses generation one m4u HW. "mediatek,mt2712-m4u" for mt2712 which uses generation two m4u HW. + "mediatek,mt6779-m4u" for mt6779 which uses generation two m4u HW. "mediatek,mt7623-m4u", "mediatek,mt2701-m4u" for mt7623 which uses generation one m4u HW. "mediatek,mt8173-m4u" for mt8173 which uses generation two m4u HW. @@ -78,6 +79,7 @@ Required properties: Specifies the mtk_m4u_id as defined in dt-binding/memory/mt2701-larb-port.h for mt2701, mt7623 dt-binding/memory/mt2712-larb-port.h for mt2712, + dt-binding/memory/mt6779-larb-port.h for mt6779, dt-binding/memory/mt8173-larb-port.h for mt8173, and dt-binding/memory/mt8183-larb-port.h for mt8183. diff --git a/include/dt-bindings/memory/mt6779-larb-port.h b/include/dt-bindings/memory/mt6779-larb-port.h new file mode 100644 index ..8b7f2d2446ea --- /dev/null +++ b/include/dt-bindings/memory/mt6779-larb-port.h @@ -0,0 +1,217 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2019 MediaTek Inc. + * Author: Chao Hao + */ + +#ifndef _DTS_IOMMU_PORT_MT6779_H_ +#define _DTS_IOMMU_PORT_MT6779_H_ + +#define MTK_M4U_ID(larb, port) (((larb) << 5) | (port)) + +#define M4U_LARB0_ID0 +#define M4U_LARB1_ID1 +#define M4U_LARB2_ID2 +#define M4U_LARB3_ID3 +#define M4U_LARB4_ID4 +#define M4U_LARB5_ID5 +#define M4U_LARB6_ID6 +#define M4U_LARB7_ID7 +#define M4U_LARB8_ID8 +#define M4U_LARB9_ID9 +#define M4U_LARB10_ID 10 +#define M4U_LARB11_ID 11 +#define M4U_LARB12_ID 12 +#define M4U_LARB13_ID 13 + +/* larb0 */ +#define M4U_PORT_DISP_POSTMASK0 MTK_M4U_ID(M4U_LARB0_ID, 0) +#define M4U_PORT_DISP_OVL0_HDR MTK_M4U_ID(M4U_LARB0_ID, 1) +#define M4U_PORT_DISP_OVL1_HDR MTK_M4U_ID(M4U_LARB0_ID, 2) +#define M4U_PORT_DISP_OVL0 MTK_M4U_ID(M4U_LARB0_ID, 3) +#define M4U_PORT_DISP_OVL1 MTK_M4U_ID(M4U_LARB0_ID, 4) +#define M4U_PORT_DISP_PVRIC0MTK_M4U_ID(M4U_LARB0_ID, 5) +#define M4U_PORT_DISP_RDMA0 MTK_M4U_ID(M4U_LARB0_ID, 6) +#define M4U_PORT_DISP_WDMA0 MTK_M4U_ID(M4U_LARB0_ID, 7) +#define M4U_PORT_DISP_FAKE0 MTK_M4U_ID(M4U_LARB0_ID, 8) + +/* larb1 */ +#define M4U_PORT_DISP_OVL0_2L_HDR MTK_M4U_ID(M4U_LARB1_ID, 0) +#define M4U_PORT_DISP_OVL1_2L_HDR MTK_M4U_ID(M4U_LARB1_ID, 1) +#define M4U_PORT_DISP_OVL0_2L MTK_M4U_ID(M4U_LARB1_ID, 2) +#define M4U_PORT_DISP_OVL1_2L MTK_M4U_ID(M4U_LARB1_ID, 3) +#define M4U_PORT_DISP_RDMA1 MTK_M4U_ID(M4U_LARB1_ID, 4) +#define M4U_PORT_MDP_PVRIC0 MTK_M4U_ID(M4U_LARB1_ID, 5) +#define M4U_PORT_MDP_PVRIC
[PATCH 00/13] MT6779 IOMMU SUPPORT
This patchset adds mt6779 iommu support and adjusts mtk iommu software architecture mainly. 1. Add mt6779 basic function, such as smi larb port define, registers define and so on. 2. In addition, this patchset will adjust current mtk iommu SW architecture mainly to adapt all the mtk platforms: Firstly, mt6779 iommu can support more HW modules, but some modules have special requirements for iova region, for example, CCU only can access 0x4000_~0x47ff_, VPU only can access 0x7da0_~0x7fbf_. Current architecture only support one iommu domain(include 0~4GB), all the modules allocate iova from 0~4GB region, so it doesn't ensure to allocate expected iova region for special module(CCU and VPU). In order to resolve the problem, we will create different iommu domains for special module, and every domain can include iova region which module needs. Secondly, all the iommus share one page table for current architecture by "mtk_iommu_get_m4u_data", to make the architecture look clearly, we will create a global page table firstly(mtk_iommu_pgtable), and the all the iommus can use it. One page table can include 4GB iova space, so multiple iommu domains are created based on the same page table. New SW architecture diagram is as below: iommu0 iommu1 || -- | mtk_iommu_pgtable | -- || | mtk_iommu_domain1 mtk_iommu_domain2 mtk_iommu_domain3 || | iommu_group1 iommu_group2iommu_group3 || | iommu_domain1iommu_domain2 iommu_domain3 || | iova region1(normal) iova region2(CCU) iova region3(VPU) This patchset depends on "Improve tlb range flush"[1] and based on v5.4-rc1. [1]http://lists.infradead.org/pipermail/linux-mediatek/2019-October/024207.html Chao Hao (13): dt-bindings: mediatek: Add bindings for MT6779 iommu/mediatek: Add mt6779 IOMMU basic support iommu/mediatek: Add mtk_iommu_pgtable structure iommu/mediatek: Remove mtk_iommu_domain_finalise iommu/mediatek: Remove pgtable info in mtk_iommu_domain iommu/mediatek: Change get the way of m4u_group iommu/mediatek: Add smi_larb info about device iommu/mediatek: Add mtk_domain_data structure iommu/mediatek: Remove the usage of m4u_dom variable iommu/mediatek: Remove mtk_iommu_get_m4u_data api iommu/mediatek: Add iova reserved function iommu/mediatek: Change single domain to multiple domains iommu/mediatek: Add multiple mtk_iommu_domain support for mt6779 .../bindings/iommu/mediatek,iommu.txt | 2 + drivers/iommu/mtk_iommu.c | 488 +++--- drivers/iommu/mtk_iommu.h | 50 +- include/dt-bindings/memory/mt6779-larb-port.h | 217 4 files changed, 685 insertions(+), 72 deletions(-) -- 2.18.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 05/13] iommu/mediatek: Remove pgtable info in mtk_iommu_domain
This patch will use mtk_iommu_pgtable to replace the part
of pgtable in mtk_iommu_domain, so we can remove the information
of pgtable in mtk_iommu_domain.
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 37 +
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 3fa09b12e9f9..f264fa8c16a0 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -117,9 +117,6 @@
#define MTK_M4U_TO_PORT(id)((id) & 0x1f)
struct mtk_iommu_domain {
- struct io_pgtable_cfg cfg;
- struct io_pgtable_ops *iop;
-
struct iommu_domain domain;
};
@@ -379,6 +376,10 @@ static int mtk_iommu_attach_pgtable(struct mtk_iommu_data
*data,
/* binding to pgtable */
data->pgtable = pgtable;
+ /* update HW settings */
+ writel(pgtable->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK,
+ data->base + REG_MMU_PT_BASE_ADDR);
+
dev_info(data->dev, "m4u%d attach_pgtable done!\n", data->m4u_id);
return 0;
@@ -404,8 +405,6 @@ static struct iommu_domain *mtk_iommu_domain_alloc(unsigned
type)
if (iommu_get_dma_cookie(&dom->domain))
goto free_dom;
- dom->cfg = pgtable->cfg;
- dom->iop = pgtable->iop;
/* Update our support page sizes bitmap */
dom->domain.pgsize_bitmap = pgtable->cfg.pgsize_bitmap;
@@ -422,11 +421,12 @@ static struct iommu_domain
*mtk_iommu_domain_alloc(unsigned type)
static void mtk_iommu_domain_free(struct iommu_domain *domain)
{
- struct mtk_iommu_domain *dom = to_mtk_domain(domain);
+ struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
- free_io_pgtable_ops(dom->iop);
iommu_put_dma_cookie(domain);
kfree(to_mtk_domain(domain));
+ free_io_pgtable_ops(pgtable->iop);
+ kfree(pgtable);
}
static int mtk_iommu_attach_device(struct iommu_domain *domain,
@@ -439,11 +439,8 @@ static int mtk_iommu_attach_device(struct iommu_domain
*domain,
return -ENODEV;
/* Update the pgtable base address register of the M4U HW */
- if (!data->m4u_dom) {
+ if (!data->m4u_dom)
data->m4u_dom = dom;
- writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK,
- data->base + REG_MMU_PT_BASE_ADDR);
- }
mtk_iommu_config(data, dev, true);
return 0;
@@ -463,7 +460,7 @@ static void mtk_iommu_detach_device(struct iommu_domain
*domain,
static int mtk_iommu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot)
{
- struct mtk_iommu_domain *dom = to_mtk_domain(domain);
+ struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
/* The "4GB mode" M4U physically can not use the lower remap of Dram. */
@@ -471,16 +468,16 @@ static int mtk_iommu_map(struct iommu_domain *domain,
unsigned long iova,
paddr |= BIT_ULL(32);
/* Synchronize with the tlb_lock */
- return dom->iop->map(dom->iop, iova, paddr, size, prot);
+ return pgtable->iop->map(pgtable->iop, iova, paddr, size, prot);
}
static size_t mtk_iommu_unmap(struct iommu_domain *domain,
unsigned long iova, size_t size,
struct iommu_iotlb_gather *gather)
{
- struct mtk_iommu_domain *dom = to_mtk_domain(domain);
+ struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
- return dom->iop->unmap(dom->iop, iova, size, gather);
+ return pgtable->iop->unmap(pgtable->iop, iova, size, gather);
}
static void mtk_iommu_flush_iotlb_all(struct iommu_domain *domain)
@@ -504,11 +501,11 @@ static void mtk_iommu_iotlb_sync(struct iommu_domain
*domain,
static phys_addr_t mtk_iommu_iova_to_phys(struct iommu_domain *domain,
dma_addr_t iova)
{
- struct mtk_iommu_domain *dom = to_mtk_domain(domain);
+ struct mtk_iommu_pgtable *pgtable = mtk_iommu_get_pgtable();
struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
phys_addr_t pa;
- pa = dom->iop->iova_to_phys(dom->iop, iova);
+ pa = pgtable->iop->iova_to_phys(pgtable->iop, iova);
if (data->enable_4GB && pa >= MTK_IOMMU_4GB_MODE_REMAP_BASE)
pa &= ~BIT_ULL(32);
@@ -850,8 +847,8 @@ static int __maybe_unused mtk_iommu_suspend(struct device
*dev)
static int __maybe_unused mtk_iommu_resume(struct device *dev)
{
struct mtk_iommu_data *data = dev_get_drvdata(dev);
+ struct mtk_iommu_pgtable *pgtable = data->pgtable;
struct mtk_iommu_suspend_reg *reg = &data->reg;
- struct mtk_iommu_domain *m4u_dom = data->m4u_dom;
void __iomem *base = data->base;
int ret;
@@ -869,8 +866,8
[PATCH 02/13] iommu/mediatek: Add mt6779 IOMMU basic support
1. Add mt6779 registers define for iommu.
2. Add mt6779_data define to support mt6779 iommu HW init.
3. There are two iommus, one is mm_iommu, the other is vpu_iommu.
MM_IOMMU is connected smi_larb to support multimedia engine to
access DRAM, and VPU_IOMMU is connected to APU_bus to support
VPU,MDLA,EDMA to access DRAM. MM_IOMMU and VPU_IOMMU use the same
page table to simplify design by "mtk_iommu_get_m4u_data".
4. For smi_larb6, it doesn't use mm_iommu, so we can distinguish
vpu_iommu by it when excutes iommu_probe.
5. For mt6779 APU_IOMMU fault id is irregular, so it was treated
specially.
Signed-off-by: Chao Hao
---
drivers/iommu/mtk_iommu.c | 91 +--
drivers/iommu/mtk_iommu.h | 10 -
2 files changed, 87 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 8ca2e99964fe..f2847e661137 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -38,12 +38,24 @@
#define REG_MMU_INVLD_END_A0x028
#define REG_MMU_INV_SEL0x038
+#define REG_MMU_INV_SEL_MT6779 0x02c
#define F_INVLD_EN0BIT(0)
#define F_INVLD_EN1BIT(1)
#define REG_MMU_STANDARD_AXI_MODE 0x048
+
+#define REG_MMU_MISC_CRTL_MT6779 0x048
+#define REG_MMU_STANDARD_AXI_MODE_MT6779 (BIT(3) | BIT(19))
+#define REG_MMU_COHERENCE_EN (BIT(0) | BIT(16))
+#define REG_MMU_IN_ORDER_WR_EN (BIT(1) | BIT(17))
+#define F_MMU_HALF_ENTRY_MODE_L(BIT(5) | BIT(21))
+#define F_MMU_BLOCKING_MODE_L (BIT(4) | BIT(20))
+
#define REG_MMU_DCM_DIS0x050
+#define REG_MMU_WR_LEN 0x054
+#define F_MMU_WR_THROT_DIS (BIT(5) | BIT(21))
+
#define REG_MMU_CTRL_REG 0x110
#define F_MMU_TF_PROT_TO_PROGRAM_ADDR (2 << 4)
#define F_MMU_PREFETCH_RT_REPLACE_MOD BIT(4)
@@ -88,10 +100,14 @@
#define REG_MMU1_INVLD_PA 0x148
#define REG_MMU0_INT_ID0x150
#define REG_MMU1_INT_ID0x154
+#define F_MMU_INT_ID_COMM_ID(a)(((a) >> 9) & 0x7)
+#define F_MMU_INT_ID_SUB_COMM_ID(a)(((a) >> 7) & 0x3)
#define F_MMU_INT_ID_LARB_ID(a)(((a) >> 7) & 0x7)
#define F_MMU_INT_ID_PORT_ID(a)(((a) >> 2) & 0x1f)
+#define F_MMU_INT_ID_COMM_APU_ID(a)((a) & 0x3)
+#define F_MMU_INT_ID_SUB_APU_ID(a) (((a) >> 2) & 0x3)
-#define MTK_PROTECT_PA_ALIGN 128
+#define MTK_PROTECT_PA_ALIGN 256
/*
* Get the local arbiter ID and the portid within the larb arbiter
@@ -165,7 +181,7 @@ static void mtk_iommu_tlb_flush_all(void *cookie)
for_each_m4u(data) {
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
- data->base + REG_MMU_INV_SEL);
+ data->base + data->plat_data->inv_sel_reg);
writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
wmb(); /* Make sure the tlb flush all done */
}
@@ -182,7 +198,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long
iova, size_t size,
for_each_m4u(data) {
spin_lock_irqsave(&data->tlb_lock, flags);
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
- data->base + REG_MMU_INV_SEL);
+ data->base + data->plat_data->inv_sel_reg);
writel_relaxed(iova, data->base + REG_MMU_INVLD_START_A);
writel_relaxed(iova + size - 1,
@@ -226,7 +242,7 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
struct mtk_iommu_data *data = dev_id;
struct mtk_iommu_domain *dom = data->m4u_dom;
u32 int_state, regval, fault_iova, fault_pa;
- unsigned int fault_larb, fault_port;
+ unsigned int fault_larb, fault_port, sub_comm = 0;
bool layer, write;
/* Read error info from registers */
@@ -242,17 +258,30 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
}
layer = fault_iova & F_MMU_FAULT_VA_LAYER_BIT;
write = fault_iova & F_MMU_FAULT_VA_WRITE_BIT;
- fault_larb = F_MMU_INT_ID_LARB_ID(regval);
fault_port = F_MMU_INT_ID_PORT_ID(regval);
+ if (data->plat_data->has_sub_comm[data->m4u_id]) {
+ /* m4u1 is VPU in mt6779.*/
+ if (data->m4u_id && data->plat_data->m4u_plat == M4U_MT6779) {
+ fault_larb = F_MMU_INT_ID_COMM_APU_ID(regval);
+ sub_comm = F_MMU_INT_ID_SUB_APU_ID(regval);
+ fault_port = 0; /* for mt6779 APU ID is irregular */
+ } else {
+ fault_larb = F_MMU_
Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
On 10/28/19 10:55 AM, Alexandre Torgue wrote:
> Hi Vlad,
>
> On 10/24/19 5:27 PM, Vladimir Murzin wrote:
>> Hi Alex,
>>
>> On 10/24/19 4:20 PM, Alexandre Torgue wrote:
>>> Hi Vlad,
>>>
>>> On 10/24/19 2:43 PM, Vladimir Murzin wrote:
On 10/17/19 10:46 AM, Vladimir Murzin wrote:
> I'm wondering if I've missed something with diff bellow (it was a long
> time ago when I touched DMA)?
Any comments on that? I can only build test it, so lack of testing
stopping me from sending it as a
proper patch :(
>>>
>>> I can make some tests tomorrow. Which particular setup I need to test:
>>> cortex M7 + cache + dma + xip ? Let me know.
>>
>> I assume xip implies dma-ranges in dt, then yes it looks like what we need.
>>
>
> I tested your patch on stm32h7431-eval board (cortex-M7). No issues reported.
> I use reserved dma memory pool for dma access ( Dcache disabled) and
> dma_alloc_coherent is ok. Note there is no "dma-ranges" for this board.
>
> I also tested it on stm32f469-disco (cortex-M4) which uses dma-ranges. No
> issues reported too. Note there is no data cache at all in this case.
Happy to hear it doesn't introduce regression for NOMM - thanks got giving it a
try!
@Daniele, it'd be handy to know if that fix issue for you...
Cheers
Vladimir
>
> regards
>
> Alex
>
>> Great thanks!
>>
>> Vladimir
>>
>>>
>>> regards
>>> alex
>>>
>
> diff --git a/arch/arm/mm/dma-mapping-nommu.c
> b/arch/arm/mm/dma-mapping-nommu.c
> index db92478..287ef89 100644
> --- a/arch/arm/mm/dma-mapping-nommu.c
> +++ b/arch/arm/mm/dma-mapping-nommu.c
> @@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev,
> size_t size,
> unsigned long attrs)
> {
> - void *ret = dma_alloc_from_global_coherent(size, dma_handle);
> + void *ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
> /*
> * dma_alloc_from_global_coherent() may fail because:
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 4a1c4fc..10918c5 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -162,7 +162,7 @@ int dma_release_from_dev_coherent(struct device *dev,
> int order, void *vaddr);
> int dma_mmap_from_dev_coherent(struct device *dev, struct
> vm_area_struct *vma,
> void *cpu_addr, size_t size, int *ret);
> -void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t
> *dma_handle);
> +void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
> dma_addr_t *dma_handle);
> int dma_release_from_global_coherent(int order, void *vaddr);
> int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void
> *cpu_addr,
> size_t size, int *ret);
> @@ -172,7 +172,7 @@ int dma_mmap_from_global_coherent(struct
> vm_area_struct *vma, void *cpu_addr,
> #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
> #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
> -static inline void *dma_alloc_from_global_coherent(ssize_t size,
> +static inline void *dma_alloc_from_global_coherent(struct device *dev,
> ssize_t size,
> dma_addr_t *dma_handle)
> {
> return NULL;
> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
> index 545e386..551b0eb 100644
> --- a/kernel/dma/coherent.c
> +++ b/kernel/dma/coherent.c
> @@ -123,8 +123,9 @@ int dma_declare_coherent_memory(struct device *dev,
> phys_addr_t phys_addr,
> return ret;
> }
> -static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
> - ssize_t size, dma_addr_t *dma_handle)
> +static void *__dma_alloc_from_coherent(struct device *dev,
> + struct dma_coherent_mem *mem,
> + ssize_t size, dma_addr_t *dma_handle)
> {
> int order = get_order(size);
> unsigned long flags;
> @@ -143,7 +144,7 @@ static void *__dma_alloc_from_coherent(struct
> dma_coherent_mem *mem,
> /*
> * Memory was found in the coherent area.
> */
> - *dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
> + *dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
> ret = mem->virt_base + (pageno << PAGE_SHIFT);
> spin_unlock_irqrestore(&mem->spinlock, flags);
> memset(ret, 0, size);
> @@ -175,17 +176,18 @@ int dma_alloc_from_dev_coherent(struct device *dev,
> ssize_t size,
> if (!mem)
> return 0;
> - *ret = __dma_alloc_from_coherent(mem, size, dma_handle);
> + *ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
> return 1;
> }
> -void *dma_alloc_fro
Re: [BUG] dma-ranges, reserved memory regions, dma_alloc_coherent: possible bug?
Hi Vlad,
On 10/24/19 5:27 PM, Vladimir Murzin wrote:
Hi Alex,
On 10/24/19 4:20 PM, Alexandre Torgue wrote:
Hi Vlad,
On 10/24/19 2:43 PM, Vladimir Murzin wrote:
On 10/17/19 10:46 AM, Vladimir Murzin wrote:
I'm wondering if I've missed something with diff bellow (it was a long time ago
when I touched DMA)?
Any comments on that? I can only build test it, so lack of testing stopping me
from sending it as a
proper patch :(
I can make some tests tomorrow. Which particular setup I need to test: cortex
M7 + cache + dma + xip ? Let me know.
I assume xip implies dma-ranges in dt, then yes it looks like what we need.
I tested your patch on stm32h7431-eval board (cortex-M7). No issues
reported. I use reserved dma memory pool for dma access ( Dcache
disabled) and dma_alloc_coherent is ok. Note there is no "dma-ranges"
for this board.
I also tested it on stm32f469-disco (cortex-M4) which uses dma-ranges.
No issues reported too. Note there is no data cache at all in this case.
regards
Alex
Great thanks!
Vladimir
regards
alex
diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
index db92478..287ef89 100644
--- a/arch/arm/mm/dma-mapping-nommu.c
+++ b/arch/arm/mm/dma-mapping-nommu.c
@@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t
size,
unsigned long attrs)
{
- void *ret = dma_alloc_from_global_coherent(size, dma_handle);
+ void *ret = dma_alloc_from_global_coherent(dev, size, dma_handle);
/*
* dma_alloc_from_global_coherent() may fail because:
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4a1c4fc..10918c5 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -162,7 +162,7 @@ int dma_release_from_dev_coherent(struct device *dev, int
order, void *vaddr);
int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct
*vma,
void *cpu_addr, size_t size, int *ret);
-void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
+void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
dma_addr_t *dma_handle);
int dma_release_from_global_coherent(int order, void *vaddr);
int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
size_t size, int *ret);
@@ -172,7 +172,7 @@ int dma_mmap_from_global_coherent(struct vm_area_struct
*vma, void *cpu_addr,
#define dma_release_from_dev_coherent(dev, order, vaddr) (0)
#define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
-static inline void *dma_alloc_from_global_coherent(ssize_t size,
+static inline void *dma_alloc_from_global_coherent(struct device *dev, ssize_t
size,
dma_addr_t *dma_handle)
{
return NULL;
diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
index 545e386..551b0eb 100644
--- a/kernel/dma/coherent.c
+++ b/kernel/dma/coherent.c
@@ -123,8 +123,9 @@ int dma_declare_coherent_memory(struct device *dev,
phys_addr_t phys_addr,
return ret;
}
-static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
- ssize_t size, dma_addr_t *dma_handle)
+static void *__dma_alloc_from_coherent(struct device *dev,
+ struct dma_coherent_mem *mem,
+ ssize_t size, dma_addr_t *dma_handle)
{
int order = get_order(size);
unsigned long flags;
@@ -143,7 +144,7 @@ static void *__dma_alloc_from_coherent(struct
dma_coherent_mem *mem,
/*
* Memory was found in the coherent area.
*/
- *dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
+ *dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
ret = mem->virt_base + (pageno << PAGE_SHIFT);
spin_unlock_irqrestore(&mem->spinlock, flags);
memset(ret, 0, size);
@@ -175,17 +176,18 @@ int dma_alloc_from_dev_coherent(struct device *dev,
ssize_t size,
if (!mem)
return 0;
- *ret = __dma_alloc_from_coherent(mem, size, dma_handle);
+ *ret = __dma_alloc_from_coherent(dev, mem, size, dma_handle);
return 1;
}
-void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
+void *dma_alloc_from_global_coherent(struct device *dev, ssize_t size,
+ dma_addr_t *dma_handle)
{
if (!dma_coherent_default_memory)
return NULL;
- return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
- dma_handle);
+ return __dma_alloc_from_coherent(dev, dma_coherent_default_memory, size,
+ dma_handle);
}
static int __dma_release_from_coherent(struct dma_coherent_mem *mem,
Thanks
Vladimir
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] dpaa2_eth: use new unmap and sync dma api variants
Hi Jonathan,
On 25.10.2019 19:12, Jonathan Lemon wrote:
>
>
> On 24 Oct 2019, at 5:41, Laurentiu Tudor wrote:
>
>> From: Laurentiu Tudor
>>
>> Convert this driver to usage of the newly introduced dma unmap and
>> sync DMA APIs. This will get rid of the unsupported direct usage of
>> iommu_iova_to_phys() API.
>>
>> Signed-off-by: Laurentiu Tudor
>> ---
>> .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 40 +++
>> .../net/ethernet/freescale/dpaa2/dpaa2-eth.h | 1 -
>> 2 files changed, 15 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
>> b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
>> index 19379bae0144..8c3391e6e598 100644
>> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
>> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
>> @@ -29,16 +29,6 @@ MODULE_LICENSE("Dual BSD/GPL");
>> MODULE_AUTHOR("Freescale Semiconductor, Inc");
>> MODULE_DESCRIPTION("Freescale DPAA2 Ethernet Driver");
>>
>> -static void *dpaa2_iova_to_virt(struct iommu_domain *domain,
>> - dma_addr_t iova_addr)
>> -{
>> - phys_addr_t phys_addr;
>> -
>> - phys_addr = domain ? iommu_iova_to_phys(domain, iova_addr) :
>> iova_addr;
>> -
>> - return phys_to_virt(phys_addr);
>> -}
>> -
>> static void validate_rx_csum(struct dpaa2_eth_priv *priv,
>> u32 fd_status,
>> struct sk_buff *skb)
>> @@ -85,9 +75,10 @@ static void free_rx_fd(struct dpaa2_eth_priv *priv,
>> sgt = vaddr + dpaa2_fd_get_offset(fd);
>> for (i = 1; i < DPAA2_ETH_MAX_SG_ENTRIES; i++) {
>> addr = dpaa2_sg_get_addr(&sgt[i]);
>> - sg_vaddr = dpaa2_iova_to_virt(priv->iommu_domain, addr);
>> - dma_unmap_page(dev, addr, DPAA2_ETH_RX_BUF_SIZE,
>> - DMA_BIDIRECTIONAL);
>> + sg_vaddr = page_to_virt
>> + (dma_unmap_page_desc(dev, addr,
>> + DPAA2_ETH_RX_BUF_SIZE,
>> + DMA_BIDIRECTIONAL));
>
> This is doing virt -> page -> virt. Why not just have the new
> function return the VA corresponding to the addr, which would
> match the other functions?
I'd really like that as it would get rid of the page_to_virt() calls but
it will break the symmetry with the dma_map_page() API. I'll let the
maintainers decide.
---
Best Regards, Laurentiu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/dma: Add support for DMA_ATTR_SYS_CACHE
On Sat, Oct 26, 2019 at 03:12:57AM -0700, isa...@codeaurora.org wrote: > On 2019-10-25 22:30, Christoph Hellwig wrote: >> The definition makes very little sense. > Can you please clarify what part doesn’t make sense, and why? It looks like complete garbage to me. That might just be because it uses tons of terms I've never heard of of and which aren't used anywhere in the DMA API. It also might be because it doesn't explain how the flag might actually be practically useful. > This is > really just an extension of this patch that got mainlined, so that clients > that use the DMA API can use IOMMU_QCOM_SYS_CACHE as well: > https://patchwork.kernel.org/patch/10946099/ >> Any without a user in the same series it is a complete no-go anyway. > IOMMU_QCOM_SYS_CACHE does not have any current users in the mainline, nor > did it have it in the patch series in which it got merged, yet it is still > present? Furthermore, there are plans to upstream support for one of our > SoCs that may benefit from this, as seen here: > https://www.spinics.net/lists/iommu/msg39608.html. Which means it should have never been merged. As a general policy we do not add code to the Linux kernel without actual users. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
