From: Robin Murphy <robin.murphy@arm.com> To: Vivek Gautam <vivek.gautam@codeaurora.org>, iommu@lists.linux-foundation.org, linux-arm-msm@vger.kernel.org Cc: robdclark@gmail.com, will.deacon@arm.com, joro@8bytes.org, robh+dt@kernel.org, mark.rutland@arm.com, m.szyprowski@samsung.com, linux-kernel@vger.kernel.org, stanimir.varbanov@linaro.org, sricharan@codeaurora.org, sboyd@codeaurora.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] iommu/arm-smmu: Defer TLB flush in case of unmap op Date: Fri, 4 Aug 2017 18:04:40 +0100 [thread overview] Message-ID: <fd13c041-7dd8-140b-1d02-36ff2bc3a633@arm.com> (raw) In-Reply-To: <d9cdeeee-36a9-40d4-457e-b1a865c3b83f@codeaurora.org> On 03/08/17 06:35, Vivek Gautam wrote: > Hi Robin, > > > > On 08/02/2017 05:47 PM, Robin Murphy wrote: >> On 02/08/17 10:53, Vivek Gautam wrote: >>> We don't want to touch the TLB when smmu is suspended. >>> Defer it until resume. >>> >>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> >>> --- >>> >>> Hi all, >>> >>> Here's the small patch in response of suggestion to defer tlb operations >>> when smmu is in suspend state. >>> The patch stores the TLB requests in 'unmap' when the smmu device is >>> suspended. On resume, it checks all the pending TLB requests, and >>> performs the unmap over those. >>> >>> Right now, I have applied the patch on top of the pm runtime series. >>> Let me know what you think of the change. It will also be helpful if >>> somebody can please test a valid use case with this. >> The patch itself doesn't make much sense to me, but more crucially it's >> definitely broken in concept. We can't return from arm_smmu_unmap() >> without having actually unmapped anything, because that leaves the page >> tables out of sync with what the caller expects - they may immmediately >> reuse that IOVA to map something else for a different device and hit an >> unexpected failure from io-pgtable when the PTE turns out to be >> non-empty. > > To understand things bit more, > once we don't *unmap* in arm_smmu_unmap(), and leave the TLBs as is, > the next mapping can happen only with the *knowledge* of smmu, i.e., > smmu should be active at that time. > If that's true then, the _runtime()_resume() method will take care of > invalidating the TLBs when we call arm_smmu_unmap() from _runtime_resume(). > Is my understanding correct here? What I mean is that it's OK for arm_smmu_unmap() to defer the physical TLB maintenance for an unmap request if the SMMU is suspended, but it *must* still update the pagetable so that the given address is logically unmapped before returning. In other words, the place to make decisions based on the SMMU PM state would be in the .tlb_add_flush and .tlb_sync callbacks, rather than at the top level. >> However, if in general suspend *might* power-gate any part of the SMMU, >> then I don't think we have any guarantee of what state any TLBs could be >> in upon resume. Therefore any individual invalidations we skip while >> suspended are probably moot, since resume would almost certainly have to >> invalidate everything to get back to a safe state anyway. > > Right, in case when the suspend power-gates the SMMU, then > the TLB context is lost anyways. So resume path can freshly start. > This is something that exynos does at present. Yes, in general I don't think we can assume any SMMU state is preserved, so the only safe option would be for .runtime_resume to do the same thing as .resume, which does at least make things nice and simple. >> Conversely though, the situation that still concerns me is whether this >> can work at all for a distributed SMMU if things *don't* lose state. Say >> the GPU and its local TBU are in the same clock domain - if the GPU has >> just gone idle and we've clock-gated it, but "the SMMU" (i.e. the TCU) >> is still active servicing other devices, we will assume we can happily >> unmap GPU buffers and issue TLBIs, but what happens with entries held in >> the unclocked TBU's micro-TLB? > > We know of platforms we have that have shared TCU and multiple TBUs. > Each TBU is available in its own power domain, not in master's power > domain. > In such cases we may want to runtime_get() the TBUs, so that unmap() > call with > master clock gated gets through. > > Can we have a situation where the TBU and master are in the same power > domain, and the unmap is called when the master is not runtime active? > How will such a situation be handled? Having thought about it a bit more, I think the unmap-after-master-suspended case is only one facet of the problem - if we can power down individual TBUs/micro-TLBs without suspending the rest of the SMMU, do we also have any guarantee that such TLBs don't power back on full of valid-looking random junk? I'm starting to think the only way to be generally safe would be to globally invalidate all TLBs after any *master* is resumed, and I'm not even sure that's feasible :/ Robin.
WARNING: multiple messages have this Message-ID (diff)
From: robin.murphy@arm.com (Robin Murphy) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] iommu/arm-smmu: Defer TLB flush in case of unmap op Date: Fri, 4 Aug 2017 18:04:40 +0100 [thread overview] Message-ID: <fd13c041-7dd8-140b-1d02-36ff2bc3a633@arm.com> (raw) In-Reply-To: <d9cdeeee-36a9-40d4-457e-b1a865c3b83f@codeaurora.org> On 03/08/17 06:35, Vivek Gautam wrote: > Hi Robin, > > > > On 08/02/2017 05:47 PM, Robin Murphy wrote: >> On 02/08/17 10:53, Vivek Gautam wrote: >>> We don't want to touch the TLB when smmu is suspended. >>> Defer it until resume. >>> >>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> >>> --- >>> >>> Hi all, >>> >>> Here's the small patch in response of suggestion to defer tlb operations >>> when smmu is in suspend state. >>> The patch stores the TLB requests in 'unmap' when the smmu device is >>> suspended. On resume, it checks all the pending TLB requests, and >>> performs the unmap over those. >>> >>> Right now, I have applied the patch on top of the pm runtime series. >>> Let me know what you think of the change. It will also be helpful if >>> somebody can please test a valid use case with this. >> The patch itself doesn't make much sense to me, but more crucially it's >> definitely broken in concept. We can't return from arm_smmu_unmap() >> without having actually unmapped anything, because that leaves the page >> tables out of sync with what the caller expects - they may immmediately >> reuse that IOVA to map something else for a different device and hit an >> unexpected failure from io-pgtable when the PTE turns out to be >> non-empty. > > To understand things bit more, > once we don't *unmap* in arm_smmu_unmap(), and leave the TLBs as is, > the next mapping can happen only with the *knowledge* of smmu, i.e., > smmu should be active at that time. > If that's true then, the _runtime()_resume() method will take care of > invalidating the TLBs when we call arm_smmu_unmap() from _runtime_resume(). > Is my understanding correct here? What I mean is that it's OK for arm_smmu_unmap() to defer the physical TLB maintenance for an unmap request if the SMMU is suspended, but it *must* still update the pagetable so that the given address is logically unmapped before returning. In other words, the place to make decisions based on the SMMU PM state would be in the .tlb_add_flush and .tlb_sync callbacks, rather than at the top level. >> However, if in general suspend *might* power-gate any part of the SMMU, >> then I don't think we have any guarantee of what state any TLBs could be >> in upon resume. Therefore any individual invalidations we skip while >> suspended are probably moot, since resume would almost certainly have to >> invalidate everything to get back to a safe state anyway. > > Right, in case when the suspend power-gates the SMMU, then > the TLB context is lost anyways. So resume path can freshly start. > This is something that exynos does at present. Yes, in general I don't think we can assume any SMMU state is preserved, so the only safe option would be for .runtime_resume to do the same thing as .resume, which does at least make things nice and simple. >> Conversely though, the situation that still concerns me is whether this >> can work at all for a distributed SMMU if things *don't* lose state. Say >> the GPU and its local TBU are in the same clock domain - if the GPU has >> just gone idle and we've clock-gated it, but "the SMMU" (i.e. the TCU) >> is still active servicing other devices, we will assume we can happily >> unmap GPU buffers and issue TLBIs, but what happens with entries held in >> the unclocked TBU's micro-TLB? > > We know of platforms we have that have shared TCU and multiple TBUs. > Each TBU is available in its own power domain, not in master's power > domain. > In such cases we may want to runtime_get() the TBUs, so that unmap() > call with > master clock gated gets through. > > Can we have a situation where the TBU and master are in the same power > domain, and the unmap is called when the master is not runtime active? > How will such a situation be handled? Having thought about it a bit more, I think the unmap-after-master-suspended case is only one facet of the problem - if we can power down individual TBUs/micro-TLBs without suspending the rest of the SMMU, do we also have any guarantee that such TLBs don't power back on full of valid-looking random junk? I'm starting to think the only way to be generally safe would be to globally invalidate all TLBs after any *master* is resumed, and I'm not even sure that's feasible :/ Robin.
next prev parent reply other threads:[~2017-08-04 17:04 UTC|newest] Thread overview: 168+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-07-06 9:36 [PATCH V4 0/6] iommu/arm-smmu: Add runtime pm/sleep support Vivek Gautam 2017-07-06 9:36 ` Vivek Gautam 2017-07-06 9:37 ` [PATCH V4 1/6] iommu/arm-smmu: Fix the error path in arm_smmu_add_device Vivek Gautam 2017-07-06 9:37 ` Vivek Gautam 2017-07-06 9:37 ` [PATCH V4 2/6] iommu/arm-smmu: Add pm_runtime/sleep ops Vivek Gautam 2017-07-06 9:37 ` Vivek Gautam 2017-07-06 9:37 ` Vivek Gautam [not found] ` <1499333825-7658-3-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-12 22:58 ` Stephen Boyd 2017-07-12 22:58 ` Stephen Boyd 2017-07-12 22:58 ` Stephen Boyd [not found] ` <20170712225821.GB22780-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-12 23:01 ` Stephen Boyd 2017-07-12 23:01 ` Stephen Boyd 2017-07-12 23:01 ` Stephen Boyd 2017-07-13 3:57 ` Vivek Gautam 2017-07-13 3:57 ` Vivek Gautam 2017-07-06 9:37 ` [PATCH V4 3/6] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device Vivek Gautam 2017-07-06 9:37 ` Vivek Gautam [not found] ` <1499333825-7658-4-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-12 22:54 ` Stephen Boyd 2017-07-12 22:54 ` Stephen Boyd 2017-07-12 22:54 ` Stephen Boyd [not found] ` <20170712225459.GZ22780-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-13 5:13 ` Vivek Gautam 2017-07-13 5:13 ` Vivek Gautam 2017-07-13 5:13 ` Vivek Gautam [not found] ` <5ee0bacd-e557-a6c4-a897-844fb12ea6ae-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-13 5:35 ` Sricharan R 2017-07-13 5:35 ` Sricharan R 2017-07-13 5:35 ` Sricharan R [not found] ` <4dbc938c-ac88-9bd4-cf00-458008ae24c1-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-13 11:50 ` Rob Clark 2017-07-13 11:50 ` Rob Clark 2017-07-13 11:50 ` Rob Clark 2017-07-13 11:50 ` Rob Clark [not found] ` <CAF6AEGsFOtsOjt1sLNPSFLEcu-7d1zxCOhTeC+P8e0TDbb1dSA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2017-07-13 12:02 ` Marek Szyprowski 2017-07-13 12:02 ` Marek Szyprowski 2017-07-13 12:02 ` Marek Szyprowski 2017-07-13 12:02 ` Marek Szyprowski 2017-07-13 12:10 ` Rob Clark 2017-07-13 12:10 ` Rob Clark 2017-07-13 12:10 ` Rob Clark [not found] ` <CAF6AEGsfDewRUHLUbFKT1Q+8U2BkmFMHo4ZBSwSGspU3ktUY8g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2017-07-13 12:23 ` Marek Szyprowski 2017-07-13 12:23 ` Marek Szyprowski 2017-07-13 12:23 ` Marek Szyprowski 2017-07-13 12:23 ` Marek Szyprowski 2017-07-13 13:53 ` Sricharan R 2017-07-13 13:53 ` Sricharan R 2017-07-13 13:53 ` Sricharan R 2017-07-13 13:53 ` Sricharan R [not found] ` <60a56ae6-ed9d-57cd-130f-5bd9d32d4d58-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-13 14:55 ` Rob Clark 2017-07-13 14:55 ` Rob Clark 2017-07-13 14:55 ` Rob Clark 2017-07-13 14:55 ` Rob Clark 2017-07-14 17:07 ` Will Deacon 2017-07-14 17:07 ` Will Deacon 2017-07-14 17:07 ` Will Deacon 2017-07-14 17:42 ` Rob Clark 2017-07-14 17:42 ` Rob Clark 2017-07-14 17:42 ` Rob Clark 2017-07-14 18:06 ` Will Deacon 2017-07-14 18:06 ` Will Deacon 2017-07-14 18:06 ` Will Deacon 2017-07-14 18:25 ` Rob Clark 2017-07-14 18:25 ` Rob Clark 2017-07-14 18:25 ` Rob Clark 2017-07-14 19:01 ` Will Deacon 2017-07-14 19:01 ` Will Deacon 2017-07-14 19:01 ` Will Deacon [not found] ` <20170714190113.GE26488-5wv7dgnIgG8@public.gmane.org> 2017-07-14 19:34 ` Rob Clark 2017-07-14 19:34 ` Rob Clark 2017-07-14 19:34 ` Rob Clark 2017-07-14 19:34 ` Rob Clark 2017-07-14 19:36 ` Will Deacon 2017-07-14 19:36 ` Will Deacon 2017-07-14 19:36 ` Will Deacon 2017-07-14 19:39 ` Rob Clark 2017-07-14 19:39 ` Rob Clark 2017-07-14 19:39 ` Rob Clark 2017-07-17 11:46 ` Sricharan R 2017-07-17 11:46 ` Sricharan R 2017-07-17 11:46 ` Sricharan R [not found] ` <6cd287bb-25c0-a7bd-8d3c-a63b9da0fd25-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-17 12:28 ` Sricharan R 2017-07-17 12:28 ` Sricharan R 2017-07-17 12:28 ` Sricharan R 2017-07-17 12:28 ` Sricharan R 2017-07-24 15:31 ` Vivek Gautam 2017-07-24 15:31 ` Vivek Gautam 2017-07-24 15:31 ` Vivek Gautam [not found] ` <CAFp+6iFfu2-qrDDim7fzKKLqMcSVMmOr7esqBZ-xEeLTOOTNLA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2017-08-02 9:53 ` [PATCH] iommu/arm-smmu: Defer TLB flush in case of unmap op Vivek Gautam 2017-08-02 9:53 ` Vivek Gautam 2017-08-02 9:53 ` Vivek Gautam 2017-08-02 12:17 ` Robin Murphy 2017-08-02 12:17 ` Robin Murphy [not found] ` <35aeb7dd-4fe6-3175-2252-41c3c54873a9-5wv7dgnIgG8@public.gmane.org> 2017-08-03 5:35 ` Vivek Gautam 2017-08-03 5:35 ` Vivek Gautam 2017-08-03 5:35 ` Vivek Gautam 2017-08-04 17:04 ` Robin Murphy [this message] 2017-08-04 17:04 ` Robin Murphy 2017-08-07 7:44 ` Vivek Gautam 2017-08-07 7:44 ` Vivek Gautam 2017-08-07 7:44 ` Vivek Gautam 2017-09-06 5:37 ` [PATCH v2 1/1] " Vivek Gautam [not found] ` <1504676255-15980-1-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-09-13 11:04 ` Vivek Gautam 2017-10-13 19:08 ` Will Deacon 2017-11-20 17:17 ` Vivek Gautam 2017-08-07 8:27 ` [PATCH V4 3/6] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device Vivek Gautam 2017-08-07 8:27 ` Vivek Gautam 2017-08-07 8:27 ` Vivek Gautam 2017-08-07 8:27 ` Vivek Gautam 2017-08-07 12:29 ` Rob Clark 2017-08-07 12:29 ` Rob Clark 2017-08-07 12:29 ` Rob Clark [not found] ` <CAF6AEGsw2=nERuJ8UCBr_kTBS0TigaA9LL1Hxw3JmNiu4oycOA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2017-11-14 18:30 ` Vivek Gautam 2017-11-14 18:30 ` Vivek Gautam 2017-11-14 18:30 ` Vivek Gautam 2017-11-14 18:30 ` Vivek Gautam [not found] ` <CAFp+6iGyB-iVb+vyDr6Dzk1FG6baiNy_kZWjB3sm_GViDh6rnQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2017-11-27 22:22 ` Stephen Boyd 2017-11-27 22:22 ` Stephen Boyd 2017-11-27 22:22 ` Stephen Boyd 2017-11-27 22:22 ` Stephen Boyd [not found] ` <20171127222238.GF18379-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-11-27 23:43 ` Rob Clark 2017-11-27 23:43 ` Rob Clark 2017-11-27 23:43 ` Rob Clark 2017-11-27 23:43 ` Rob Clark 2017-11-28 13:43 ` Vivek Gautam 2017-11-28 13:43 ` Vivek Gautam 2017-11-28 13:43 ` Vivek Gautam [not found] ` <3a2f74e9-90cf-d843-d801-15eb614d7abe-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-11-28 20:05 ` Rob Clark 2017-11-28 20:05 ` Rob Clark 2017-11-28 20:05 ` Rob Clark 2017-11-28 20:05 ` Rob Clark 2017-07-13 13:57 ` Vivek Gautam 2017-07-13 13:57 ` Vivek Gautam 2017-07-13 13:57 ` Vivek Gautam 2017-07-13 13:57 ` Vivek Gautam [not found] ` <CAFp+6iFdogDfKbwWta3AMGu2GuZ9NaR+Dv373N7LwwrF5cFYwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2017-07-13 14:01 ` Vivek Gautam 2017-07-13 14:01 ` Vivek Gautam 2017-07-13 14:01 ` Vivek Gautam 2017-07-13 14:01 ` Vivek Gautam 2017-07-13 6:48 ` Stephen Boyd 2017-07-13 6:48 ` Stephen Boyd 2017-07-13 6:48 ` Stephen Boyd 2017-07-13 9:50 ` Robin Murphy 2017-07-13 9:50 ` Robin Murphy 2017-07-13 11:53 ` Rob Clark 2017-07-13 11:53 ` Rob Clark 2017-07-13 11:53 ` Rob Clark 2017-07-06 9:37 ` [PATCH V4 4/6] iommu/arm-smmu: Add the device_link between masters and smmu Vivek Gautam 2017-07-06 9:37 ` Vivek Gautam [not found] ` <1499333825-7658-5-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-12 22:55 ` Stephen Boyd 2017-07-12 22:55 ` Stephen Boyd 2017-07-12 22:55 ` Stephen Boyd [not found] ` <20170712225547.GA22780-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-13 3:59 ` Vivek Gautam 2017-07-13 3:59 ` Vivek Gautam 2017-07-13 3:59 ` Vivek Gautam 2017-07-06 9:37 ` [PATCH V4 5/6] iommu/arm-smmu: Add support for MMU40x/500 clocks Vivek Gautam 2017-07-06 9:37 ` Vivek Gautam [not found] ` <1499333825-7658-6-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-10 3:37 ` Rob Herring 2017-07-10 3:37 ` Rob Herring 2017-07-10 3:37 ` Rob Herring 2017-07-11 5:18 ` Vivek Gautam 2017-07-11 5:18 ` Vivek Gautam [not found] ` <1499333825-7658-1-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-06 9:37 ` [PATCH V4 6/6] iommu/arm-smmu: Add support for qcom,msm8996-smmu-v2 clocks Vivek Gautam 2017-07-06 9:37 ` [PATCH V4 6/6] iommu/arm-smmu: Add support for qcom, msm8996-smmu-v2 clocks Vivek Gautam 2017-07-06 9:37 ` [PATCH V4 6/6] iommu/arm-smmu: Add support for qcom,msm8996-smmu-v2 clocks Vivek Gautam [not found] ` <1499333825-7658-7-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-07-10 3:40 ` Rob Herring 2017-07-10 3:40 ` Rob Herring 2017-07-10 3:40 ` Rob Herring 2017-07-10 6:42 ` Vivek Gautam 2017-07-10 6:42 ` Vivek Gautam 2017-07-10 6:42 ` Vivek Gautam 2017-07-10 6:42 ` Vivek Gautam
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=fd13c041-7dd8-140b-1d02-36ff2bc3a633@arm.com \ --to=robin.murphy@arm.com \ --cc=iommu@lists.linux-foundation.org \ --cc=joro@8bytes.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=m.szyprowski@samsung.com \ --cc=mark.rutland@arm.com \ --cc=robdclark@gmail.com \ --cc=robh+dt@kernel.org \ --cc=sboyd@codeaurora.org \ --cc=sricharan@codeaurora.org \ --cc=stanimir.varbanov@linaro.org \ --cc=vivek.gautam@codeaurora.org \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.