Re: [RFC 1/3] iommu/arm-smmu: Add support to opt-in to stalling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 12, 2017 at 10:17 AM, Will Deacon <will.deacon@xxxxxxx> wrote:
> On Wed, Jan 11, 2017 at 03:59:30PM -0500, Rob Clark wrote:
>> On Wed, Jan 11, 2017 at 4:36 AM, Will Deacon <will.deacon@xxxxxxx> wrote:
>> > On Tue, Jan 10, 2017 at 02:20:13PM -0500, Rob Clark wrote:
>> >> On Tue, Jan 10, 2017 at 12:52 PM, Will Deacon <will.deacon@xxxxxxx> wrote:
>> >> > On Fri, Jan 06, 2017 at 11:26:49AM -0500, Rob Clark wrote:
>> >> >> Hmm, well we install the fault handler on the iommu_domain..  perhaps
>> >> >> maybe a combo of dts property (or deciding based on more specific
>> >> >> compat string), plus extra param passed in to
>> >> >> iommu_set_fault_hander().  The dts property or compat string to
>> >> >> indicate whether the iommu (and how it is wired up) can handle stalls,
>> >> >> and enable_stall param when fault handler is registered to indicate
>> >> >> whether the device itself can cope.. if either can't do stalling, then
>> >> >> don't set CFCFG.
>> >> >
>> >> > I thought about this some more, and I think you're right. Having
>> >> > iommu_set_fault_handler take a flags parameter indicating that, for example,
>> >> > the fault handler can deal with paging, is all we need to implement the
>> >> > per-master opt-in functionality for stalling faults. There's no real
>> >> > requirement to standardise a generic firmware property for that (but
>> >> > we still need *something* that says stalling is usable on the SMMU --
>> >> > perhaps just the compatible string is ok).
>> >>
>> >> btw, it occurred to me that maybe it should be flags param to
>> >> iommu_attach_device() (just in case fault handler not installed?)
>> >> otoh stalling without a fault handler is silly, but I guess we need it
>> >> to infer whether stalling can be supported by other devices on same
>> >> iommu.. tbh I'm on a bit shaky ground when it comes to multiple
>> >> devices per iommu since the SoC's I'm familiar with do it the other
>> >> way around.  But I guess you have thought more about the multi-device
>> >> case, so figured I should suggest it..
>> >
>> > I don't think it works at attach time, because the stalling property belongs
>> > to the domain, rather than the individual devices within it. Similarly, I
>> > don't think we should allow this property to be toggled once devices have
>> > been attached.
>> >
>>
>> hmm, I was more thinking of cases where drivers for particular devices
>> need some work (ie. like potentially disabling hw hang detect during
>> faults).. I guess we could have three levels, that all have to be true
>> in order to enable stall: smmu, domain (pass flags in to
>> iommu_domain_alloc()??), and device (iommu_attach_device())?
>
> Hooking iommu_set_fault_handler, as you originally suggested, is the best
> way to set the flag on the domain. I think we just need to enforce that
> iommu_set_fault_handler is called prior to attaching devices to a domain,
> so that the IOMMU driver can configure the domain appropriately on the
> first attach.

Hi Will, just (finally) revisiting this..

So I started working on a patch to add can_stall to
iommu_set_fault_handler() (fortunately not many callers).  And then
adding an iommu_domain_resume(domain, resume/terminate).  (Ie.
iommu_domain_resume() would be called by the IOMMU user either
directly from fault handler callback, or indirectly from a thread
context..  that seemed a bit less clunky than passing a callback to
the callback..)

But is there any good way to iterate all the domains associated w/ the
arm_smmu_device?  Unfortunately we don't pass in the device ptr to
iommu_domain_alloc() no I'm not entirely sure at what point we know
whether all the associated domains can stall..

BR,
-R
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux