On Thu, Jan 05, 2017 at 12:08:57PM +0000, Mark Rutland wrote: > 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: > > > 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? It's not a policy, it's a hardware capability. There are some non-probeable reasons why stalling mode is unsafe or unusable: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/474530.html Some of these are specific to the SMMU implementation (e.g. whether or not the SS bit can remain set without reasserting the IRQ) and some are specific to the integration (e.g. whether or not stalling an endpoint can deadlock the SoC). The key point is that, without support from both the implementation and the integration, stalls are unusable. > > > 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? See above. The "stalling works" property (arm,smmu-enable-stall) would indicate that both the implementation *and* the integration are such that stalling is usable for demand paging. I suspect there are endpoints that can't deal with stalls (e.g. they might timeout and signal a RAS event), but in that case their respective device drivers should ensure that any DMA buffers are pinned and/or register a fault handler to request termination of the faulting transaction. Will -- 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
