On Thu, Jan 05, 2017 at 11:55:29AM +0000, Will Deacon wrote: > On Tue, Jan 03, 2017 at 04:30:54PM -0500, Rob Clark wrote: > > TODO maybe we want two options, one to enable stalling, and 2nd to punt > > handling to wq? I haven't needed to use mm APIs from fault handler yet > > (although it is something that I think we'll want some day). Perhaps > > stalling support is limited to just letting driver dump some extra > > debugging information otherwise. Threaded handling probably only useful > > with stalling, but inverse may not always be true. > > I'd actually like to see this stuck on a worker thread, because I think > that's more generally useful and I don't want to have a situation where > sometimes the IOMMU fault notifier is run in IRQ context and sometimes it's > not. > > > > > Signed-off-by: Rob Clark <robdclark@xxxxxxxxx> > > --- > > .../devicetree/bindings/iommu/arm,smmu.txt | 3 ++ > > drivers/iommu/arm-smmu.c | 42 ++++++++++++++++++---- > > 2 files changed, 39 insertions(+), 6 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt > > index ef465b0..5f405a6 100644 > > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt > > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt > > @@ -68,6 +68,9 @@ conditions. > > aliases of secure registers have to be used during > > SMMU configuration. > > > > +- arm,smmu-enable-stall : Enable stall mode to stall memory transactions > > + and resume after fault is handled The wording here seems to describe a policy rather than a property. Can you elaborate on when/why this is required/preferred/valid? > > static irqreturn_t arm_smmu_global_fault(int irq, void *dev) > > @@ -824,6 +852,8 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, > > > > /* SCTLR */ > > reg = SCTLR_CFIE | SCTLR_CFRE | SCTLR_AFE | SCTLR_TRE | SCTLR_M; > > + if (smmu->options & ARM_SMMU_OPT_ENABLE_STALL) > > + reg |= SCTLR_CFCFG; > > I wonder if this should also be predicated on the compatible string, so > that the "arm,smmu-enable-stall" property is ignored (with a warning) if > the compatible string isn't specific enough to identify an implementation > with the required SS behaviour? On the other hand, it feels pretty > redundant and a single "stalling works" property is all we need. Can you elaborate on what "stalling works" entails? Is that just the SS bit behaviour? are there integration or endpoint-specific things that we need to care about? Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
