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

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Julien,

> On 20 Apr 2021, at 1:39 pm, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi,
> 
> On 19/04/2021 17:21, Stefano Stabellini wrote:
>> On Mon, 19 Apr 2021, Rahul Singh wrote:
>>> Hi Julien,
>>> 
>>>> On 18 Apr 2021, at 6:48 pm, Julien Grall <julien@xxxxxxx> wrote:
>>>> 
>>>> 
>>>> 
>>>> On 16/04/2021 17:41, Rahul Singh wrote:
>>>>> Hi Julien
>>>> 
>>>> Hi Rahul,
>>>> 
>>>>>> 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.
>>>> 
>>>> Look good to me. Is this patch to be applied on top of Stefano's series? 
>>>> If not, is there going to be more clash?
>>>> 
>>> 
>>> As per Stefano's mail he already tested his patch series on top of this 
>>> patch. I think this patch has to merged before Stefano’s patch series
>>> Let Stefano also confirm that.
>>> 
>>> I think there will be no more clashes.
>> Yes, this patch is to be committed *before* my series and I have already
>> tested this patch alone and with my series on top. Both cases work fine.
> 
> Cool. Thanks for the confirmation. I have committed the patch with the new 
> commit message (although, I tweaked a little bit to use the abbreviated 
> version of the commit ID).
> 

Thanks! 

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