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

On Thu, Jan 05, 2017 at 10:27:27AM -0500, Rob Clark wrote:

On Thu, Jan 5, 2017 at 6:55 AM, Will Deacon [off-list ref] wrote:

quoted

On Tue, Jan 03, 2017 at 04:30:54PM -0500, Rob Clark wrote:

quoted

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.

So I was talking a bit w/ Jordan on IRC yesterday..  and we also have
the GPU's hw hang-detect to contend with.  So I *suspect* that when we
get to the point of using this to do things like page in things from
swap and resume the faulting transaction, we probably want to get
called immediately from the IRQ handler so we can disable the hw
hang-detect.

Well, if you want to use an SMMU for paging, then the GPU driver would
need to request that explicitly when allocating its DMA buffers, to that
would be the time to either delay or disable the hang detection.

I'm not sure if the better solution then would be to have two fault
callbacks, one immediately from the IRQ and a later one from wq.  Or
let the driver handle the wq business and give it a way to tell the
IOMMU when to resume.

I kinda think we should punt on the worker thread for now until we are
ready to resume faulting transactions, because I guess a strong chance
that whatever way we do it now will be wrong ;-)

I guess what I'm after is for you to change the interrupt handlers to be
threaded, like they are for SMMUv3. I *think* you can do that with a NULL
thread_fn for now, and just call report_iommu_fault from the handler.
The return value of that could, in theory, be used to queued the paging
request and wake the paging thread in future.

quoted

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.

We could also drop the property and key the behavior on specific
compat strings I guess.  Having both seems a bit odd.  Anyways, I'll
defer to DT folks about what the cleaner approach is.

As Robin pointed out, we do need to be able to distinguish the integration
of the device from the device itself. For example, MMU-9000 might be capable
of stalling, but if it's bolted to a PCI RC, it's not safe to do so.

Will