Re: [PATCH] xen/arm: smmuv1: Revert associating the group pointer with the S2CR

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Julein

> On 16 Apr 2021, at 4:23 pm, Julien Grall <julien@xxxxxxx> wrote:
> 
> 
> 
> On 16/04/2021 16:01, Rahul Singh wrote:
>> Hi Julien,
> 
> Hi Rahul,
> 
>>> On 16 Apr 2021, at 3:35 pm, Julien Grall <julien@xxxxxxx> wrote:
>>> 
>>> Hi,
>>> 
>>> On 16/04/2021 12:25, Rahul Singh wrote:
>>>> Revert the code that associates the group pointer with the S2CR as this
>>>> code causing an issue when the SMMU device has more than one master
>>>> device.
>>> 
>>> It is not clear to me why this change was first added. Are we missing any 
>>> feature when reverting it?
>> This feature was added when we backported the code from Linux to fix the 
>> stream match conflict issue
>> as part of commit "xen/arm: smmuv1: Intelligent SMR allocation”.
>> This is an extra feature added to allocate IOMMU group based on stream-id. 
>> If two device has the
>> same stream-id then we assign those devices to the same group. 
> 
> If we revert the patch, then it would not be possible to use the SMMU if two 
> devices use the same stream-id. Is that correct?

No. If we revert the patch we can use the SMMU if two devices use the same 
stream-id without any issue but each device will be in a separate group.This is 
same behaviour before the code is merged.
 
> 
>> This code was removed from Linux
>> later point in time when IOMMU group handling is done by Linux code not by a 
>> specific IOMMU driver.
> 
> Right.... But Linux still support that option. Is that correct?

Yes Linux support IOMMU groups handling at Linux code level and not by every 
IOMMU driver. 
That why I though when we add the support for IOMMU group at XEN level we can 
add this functionality.

> 
>> Therefore I think it is ok revert the code.
> 
> I am ok with the principle of (partially) reverting patch to unblock the 
> situation. But I have to admit, I don't quite understand why this is reverted 
> rather than fixed.

As I mention earlier I reverted this patch until we have proper support for 
IOMMU groups for ARM. Once we have IOMMU group support for ARM we can add this 
functionality.

Regards,
Rahul
> 
> Cheers,
> 
> -- 
> Julien Grall