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

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Julien

> On 16 Apr 2021, at 5:08 pm, Julien Grall <julien@xxxxxxx> wrote:
> 
> 
> 
> On 16/04/2021 17:05, Rahul Singh wrote:
>>> 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.
> 
> Ok. So there is no change in behavior. Good. Can you propose a commit message 
> clarifying that? 

Please have a look if it make sense.

xen/arm: smmuv1: Revert associating the group pointer with the S2CR

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 with same stream-id. This issue is introduced by the below commit:

“0435784cc75dcfef3b5f59c29deb1dbb84265ddb:xen/arm: smmuv1: Intelligent SMR 
allocation”
 
Reverting the code will not impact to use of SMMU if two devices use the
same stream-id but each device will be in a separate group. This is the same
behaviour before the code is merged.

> 
> OOI why was the code added if it doesn't make any difference?

This code is part of the code in Linux commit to fix stream match conflict 
issue when two devices have the same stream-id. See Linux commit message.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iommu/arm-smmu.c?h=linux-5.8.y&id=588888a7399db352d2b1a41c9d5b3bf0fd482390

When I tested the code on Juno board I didn’t  observe any issue so I thought 
it is ok to merge this code to have clean backport.

Regards,
Rahul

> 
> Anyway, thanks for the explanation!
> 
> Cheers,
> 
> -- 
> Julien Grall