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

[Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>]

On 05/01/17 16:07, Will Deacon wrote:
> On Thu, Jan 05, 2017 at 03:32:50PM +0000, Robin Murphy wrote:
>> On 05/01/17 14:47, Will Deacon wrote:
>>> On Thu, Jan 05, 2017 at 02:07:31PM +0000, Mark Rutland wrote:
>>>> Ok. It would be good to elaborate on what "stalling is useable" means in
>>>> the property description. i.e. what specificallty the implementation and
>>>> integration need to ensure.
>>>
>>> We can describe some of those guarantees in the property description, but
>>> it's difficult to enumerate them exhaustively. For example, you wouldn't
>>> want stalling to lead to data corruption, denial of service, or for the
>>> thing to catch fire, but having those as explicit requirements is a bit
>>> daft. It's also impossible to check that you thought of everything.
>>>
>>> Aside from renaming the option, I'm really after an opinion on whether
>>> it's better to have one property or combine it with the compatible
>>> string, because I can see benefits of both and don't much care either
>>> way.
>>
>> The SMMU implementation side of the decision (i.e. independence of IRQ
>> assertion vs. SS) seems like exactly the sort of stuff the compatible
>> string already has covered. The integration side I'm less confident can
>> be described this way at all - the "this device definitely won't go
>> wrong if stalled for an indefinite length of time" is inherently a
>> per-master thing, so a single property on the SMMU implying that for
>> every device connected to it seems a bit optimistic, and breaks down as
>> soon as you have one device in the system for which that isn't true (a
>> PCI root complex, say), even if that guy's traffic never crosses paths
>> with whichever few devices you actually care about using stalls with.
>>
>> I think this needs to be some kind of "arm,smmu-stall-safe" property
>> placed on individual master device nodes (mad idea: or even an extra
>> cell of flags in the IOMMU specifier) to encapsulate both that the given
>> device itself is OK with being stalled, and that it's integrated in such
>> a way that its stalled transactions cannot disrupt any *other* device
>> (e.g. it has a TBU all to itself). Then upon initialising a context bank
>> on a suitable SMMU implementation, we set CFCFG based on whatever device
>> is being attached to the corresponding domain, and refuse any subsequent
>> attempts to attach a non-stallable device to a stalling domain (and
>> possibly even vice-versa).
> 
> If we're going to add per-master properties, I'd *really* like them to be
> independent of the IOMMU in use. That is, we should be able to re-use this
> property as part of supporting SVM for platform devices in future.

I'd argue that they are still fairly separate things despite the
overlap: stalling is a specific ARM SMMU architecture thing (in both
architectures) which may be used for purposes unrelated to SVM;
conversely SVM implemented via PRI or similar mechanisms should be
pretty much oblivious to the transaction fault model.

> But I think we agree that we need:
> 
>   1. A compatible string for the SMMU that can be used to infer the SS
>      behaviour in the driver
> 
>   2. A property on the SMMU to say that it's been integrated in such a
>      way that stalling is safe (doesn't deadlock)

That's still got to be a per-master property, not a SMMU property, I
think. To illustrate:

  [A]         [B]   [C]
   |           |_____|
 __|______________|___
| TBU |        | TBU |
|_____|  SMMU  |_____|
|__|______________|__|
   |              |

Say A and B are instances of some device happy to be stalled, and C is a
PCIe RC, and each is attached to their own context bank - enabling
stalls for A is definitely fine. However even though B and C are using
different context banks, enabling stalls for B might deadlock C if it
results in more total outstanding transactions than the TBU's slave port
supports. Therefore A can happily claim to be stall-safe, but B cannot
due to its integration with respect to C.

And yes, I can point you at some existing hardware which really does
posess a topology like that.

>   3. A generic master property that says that the device can DMA to
>      unpinned memory

That sounds a bit *too* generic to me, given that there are multiple
incompatible ways that could be implemented. I'm not the biggest fan of
properties with heavily context-specific interpretations, especially
when there's more than a hint of software implementation details in the mix.

Robin.

> 
> Anything else?
> 
> Will
>