Hi Robin,

On 14/09/2016 14:53, Robin Murphy wrote:
> On 14/09/16 13:32, Auger Eric wrote:
>> Hi,
>> On 14/09/2016 12:35, Robin Murphy wrote:
>>> On 14/09/16 09:41, Auger Eric wrote:
>>>> Hi,
>>>>
>>>> On 12/09/2016 18:13, Robin Murphy wrote:
>>>>> Hi all,
>>>>>
>>>>> To any more confusing fixups and crazily numbered extra patches, here's
>>>>> a quick v7 with everything rebased into the right order. The significant
>>>>> change this time is to implement iommu_fwspec properly from the start,
>>>>> which ends up being far simpler and more robust than faffing about
>>>>> introducing it somewhere 'less intrusive' to move toward core code later.
>>>>>
>>>>> New branch in the logical place:
>>>>>
>>>>> git://linux-arm.org/linux-rm iommu/generic-v7
>>>>
>>>> For information, as discussed privately with Robin I experience some
>>>> regressions with the former and now deprecated dt description.
>>>>
>>>> on my AMD Overdrive board and my old dt description I now only see a
>>>> single group:
>>>>
>>>> /sys/kernel/iommu_groups/
>>>> /sys/kernel/iommu_groups/0
>>>> /sys/kernel/iommu_groups/0/devices
>>>> /sys/kernel/iommu_groups/0/devices/e0700000.xgmac
>>>>
>>>> whereas I formerly see
>>>>
>>>> /sys/kernel/iommu_groups/
>>>> /sys/kernel/iommu_groups/3
>>>> /sys/kernel/iommu_groups/3/devices
>>>> /sys/kernel/iommu_groups/3/devices/0000:00:00.0
>>>> /sys/kernel/iommu_groups/1
>>>> /sys/kernel/iommu_groups/1/devices
>>>> /sys/kernel/iommu_groups/1/devices/e0700000.xgmac
>>>> /sys/kernel/iommu_groups/4
>>>> /sys/kernel/iommu_groups/4/devices
>>>> /sys/kernel/iommu_groups/4/devices/0000:00:02.2
>>>> /sys/kernel/iommu_groups/4/devices/0000:01:00.1
>>>> /sys/kernel/iommu_groups/4/devices/0000:00:02.0
>>>> /sys/kernel/iommu_groups/4/devices/0000:01:00.0
>>>> /sys/kernel/iommu_groups/2
>>>> /sys/kernel/iommu_groups/2/devices
>>>> /sys/kernel/iommu_groups/2/devices/e0900000.xgmac
>>>> /sys/kernel/iommu_groups/0
>>>> /sys/kernel/iommu_groups/0/devices
>>>> /sys/kernel/iommu_groups/0/devices/f0000000.pcie
>>>>
>>>> This is the group topology without ACS override. Applying the non
>>>> upstreamed "pci: Enable overrides for missing ACS capabilities" I used
>>>> to see separate groups for each PCIe components. Now I don't see any
>>>> difference with and without ACS override.
>>>
>>> OK, having reproduced on my Juno, the problem looks to be that
>>> of_for_each_phandle() leaves err set to -ENOENT after successfully
>>> walking a phandle list, which makes __find_legacy_master_phandle()
>>> always bail out after the first SMMU.
>>>
>>> Can you confirm that the following diff fixes things for you?
>>
>> Well it improves but there are still differences in the group topology.
>> The PFs now are in group 0.
>>
>> root@trusty:~# lspci -nk
>> 00:00.0 0600: 1022:1a00
>>         Subsystem: 1022:1a00
>> 00:02.0 0600: 1022:1a01
>> 00:02.2 0604: 1022:1a02
>>         Kernel driver in use: pcieport
>> 01:00.0 0200: 8086:1521 (rev 01)
>>         Subsystem: 8086:0002
>>         Kernel driver in use: igb
>> 01:00.1 0200: 8086:1521 (rev 01)
>>         Subsystem: 8086:0002
>>         Kernel driver in use: igb
>>
>>
>> with your series + fix:
>> /sys/kernel/iommu_groups/
>> /sys/kernel/iommu_groups/3
>> /sys/kernel/iommu_groups/3/devices
>> /sys/kernel/iommu_groups/3/devices/0000:00:00.0
>> /sys/kernel/iommu_groups/1
>> /sys/kernel/iommu_groups/1/devices
>> /sys/kernel/iommu_groups/1/devices/e0700000.xgmac
>> /sys/kernel/iommu_groups/4
>> /sys/kernel/iommu_groups/4/devices
>> /sys/kernel/iommu_groups/4/devices/0000:00:02.2
>> /sys/kernel/iommu_groups/4/devices/0000:00:02.0
>> /sys/kernel/iommu_groups/2
>> /sys/kernel/iommu_groups/2/devices
>> /sys/kernel/iommu_groups/2/devices/e0900000.xgmac
>> /sys/kernel/iommu_groups/0
>> /sys/kernel/iommu_groups/0/devices
>> /sys/kernel/iommu_groups/0/devices/0000:01:00.1
>> /sys/kernel/iommu_groups/0/devices/f0000000.pcie
>> /sys/kernel/iommu_groups/0/devices/0000:01:00.0
>>
>> Before (4.8-rc5):
>>
>> /sys/kernel/iommu_groups/
>> /sys/kernel/iommu_groups/3
>> /sys/kernel/iommu_groups/3/devices
>> /sys/kernel/iommu_groups/3/devices/0000:00:00.0
>> /sys/kernel/iommu_groups/1
>> /sys/kernel/iommu_groups/1/devices
>> /sys/kernel/iommu_groups/1/devices/e0700000.xgmac
>> /sys/kernel/iommu_groups/4
>> /sys/kernel/iommu_groups/4/devices
>> /sys/kernel/iommu_groups/4/devices/0000:00:02.2
>> /sys/kernel/iommu_groups/4/devices/0000:01:00.1
>> /sys/kernel/iommu_groups/4/devices/0000:00:02.0
>> /sys/kernel/iommu_groups/4/devices/0000:01:00.0
>> /sys/kernel/iommu_groups/2
>> /sys/kernel/iommu_groups/2/devices
>> /sys/kernel/iommu_groups/2/devices/e0900000.xgmac
>> /sys/kernel/iommu_groups/0
>> /sys/kernel/iommu_groups/0/devices
>> /sys/kernel/iommu_groups/0/devices/f0000000.pcie
> 
> Your DT claims that f0000000.pcie (i.e. the "platform device" side of
> the host controller) owns the IDs 0x100 0x101 0x102 0x103 0x200 0x201
> 0x202 0x203 0x300 0x301 0x302 0x303 0x400 0x401 0x402 0x403. Thus when
> new devices (the PCI PFs) come along *also* claiming those IDs (via the
> RID-to-SID assumption), we now detect the aliasing and assign them to
> the existing group.
> 
> The only way that DT worked without SMR conflicts before is that
> f0000000.pcie wasn't actually getting attached to a domain, thus wasn't
> getting SMRs allocated (ISTR you _did_ get conflicts back when we first
> tried to switch on default domains; that was probably the reason).

Thanks for your explanations. meanwhile I converted the overdrive dt to
the new syntax and it works properly.

However I noticed there are MSI frame iommu mapping although the
device's domain is the default one. There is no check currently in
21/22. I guess this isn't needed?

Thanks

Eric
> 
> Robin.
> 
>>
>> Thanks
>>
>> Eric
>>
>>>
>>> Robin
>>>
>>> --->8---
>>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>>> index fa892d25004d..ac4aab97c93a 100644
>>> --- a/drivers/iommu/arm-smmu.c
>>> +++ b/drivers/iommu/arm-smmu.c
>>> @@ -477,7 +477,7 @@ static int __find_legacy_master_phandle(struct
>>> device *dev, void *data)
>>>                     return 1;
>>>             }
>>>     it->node = np;
>>> -   return err;
>>> +   return err == -ENOENT ? 0 : err;
>>>  }
>>>
>>>  static struct platform_driver arm_smmu_driver;
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-ker...@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>
>>
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to