Re: [RFC 3/5] dt-bindings: arm-smmu: Add reserved-msi-region

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

On 12/03/18 07:19, Jitendra Bhivare wrote:
> On Tue, Mar 6, 2018 at 5:17 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>>
>>> iPROC SoC has a special device window to treat GICv3 ITS MSI.
>>
>>
>> Ugh, really? After preferably printing out 100 copies of the SBSA, binding
>> them all together and dropping the lot onto the hardware designers from a
>> great height, could you clarify what exactly the special behaviour is here?
>>
>> Robin.
>>
> The special device window needs to programmed so that writes to
> specified address
> region helps for specific ordering of traffic prior to it.

OK, I know of PCIe root complexes having address matching to give the 
MSI write the appropriate AXI memory type attributes when the SMMU is 
bypassed, but ordering is a new one. I guess you have some glue logic on 
the root complex master interface which injects an ACE barrier 
transaction in front of any write to the ITS doorbell address?
>>> Current code maps MSI to IOVA space. Add SMMU node property to use
>>> direct mappings for MSI region.
>>>
>>> This property is read and reserved when arm_smmu_get_resv_regions
>>> gets called.

Either way, this should be a generic, not SMMU-specific, property - 
there are other platforms that would also make use of it to describe a 
similar hardware situation (which we currently only support via ACPI). 
The big question is where to put it: hardware-wise, the property of 
"MSIs won't work properly if the doorbell is remapped to a different 
address" belongs firmly to the root complex, not the IOMMU, while the 
address itself is already a property of the MSI controller. The IOMMU is 
just the innocent guy in the middle who has to discover and respect the 
constraints.

I'd like to think we could just have some flag to say "you can't remap 
my MSIs!" then go and chase the msi-parent/msi-map phandle to the MSI 
controller to get the address from its reg property, which is more or 
less the equivalent of what the current ACPI workaround does - see 
commit 8b4282e6b8e2 ("ACPI/IORT: Add msi address regions reservation 
helper") in linux-next - but I can already think of ways in which that 
might not work. For one, there's not necessarily any guarantee that the 
MSI controller's programming interface and doorbell are in the same 
place, so we probably do need to describe the specific MSI address(es) 
that the root complex cares about, from the root complex end :/

Robin.

>>>
>>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> ---
>>>    Documentation/devicetree/bindings/iommu/arm,smmu.txt | 12 ++++++++++++
>>>    1 file changed, 12 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> index 8a6ffce..13fa2b9 100644
>>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> @@ -71,6 +71,15 @@ conditions.
>>>                      or using stream matching with #iommu-cells = <2>, and
>>>                      may be ignored if present in such cases.
>>>    +- reserved-msi-region: MSI region to be reserved with specific prot in
>>> IOVA
>>> +                 space for direct mapping. The region is specified in
>>> tuple
>>> +                 of (busno,prot,bus_addr,size).
>>> +
>>> +- #region-address-cells: specifies number of cells needed to encode
>>> bus_addr
>>> +
>>> +- #region-size-cells: specifies number of cells needed to encode size
>>> +
>>> +
>>>    ** Deprecated properties:
>>>      - mmu-masters (deprecated in favour of the generic "iommus" binding) :
>>> @@ -95,6 +104,9 @@ conditions.
>>>                                 <0 36 4>,
>>>                                 <0 37 4>;
>>>                    #iommu-cells = <1>;
>>> +               #region-address-cells = <1>;
>>> +               #region-size-cells = <1>;
>>> +               reserved-msi-region = <0x0 0x1a 0x63c3000 0x8000>;
>>>            };
>>>              /* device with two stream IDs, 0 and 7 */
>>>
>>