Re: [patch-4.16] arm/smmuv1,v2: Protect smmu master list with a lock

[Michal Orzel <michal.orzel@xxxxxxx>]

Hello,

On 01.11.2021 21:51, Stefano Stabellini wrote:
> On Mon, 1 Nov 2021, Ian Jackson wrote:
>> Stefano Stabellini writes ("Re: [patch-4.16] arm/smmuv1,v2: Protect smmu 
>> master list with a lock"):
>>> In regards to this specific patch and also the conversation about 4.16
>>> or 4.17: I think it would be fine to take this patch in 4.16 in its
>>> current form. Although it is not required because PCI passthrough is
>>> not going to be complete in 4.16 anyway, I like that this patch makes
>>> the code consistent in terms of protection of rbtree accesses.  With
>>> this patch the arm_smmu_master rbtree is consistently protected from
>>> concurrent accesses. Without this patch, it is sometimes protected and
>>> sometimes not, which is not great.
>>
>> It sounds like this is a possible latent bug, or at least a bad state
>> of the code that might lead to the introduction of bad bugs later.
>>
>> So I think I understand the upside.
>>
>>> So I think that is something that could be good to have in 4.16. But
>>> like you said, the patch is not strictly required so it is fine either
>>> way.
>>
>> Can you set out the downside for me too ?  What are the risks ?  How
>> are the affected code paths used in 4.16 ?
>>
>> A good way to think about this is: if taking this patch for 4.16
>> causes problems, what would that look like ?
> 
> The patch affects the SMMU code paths that are currently in-use for
> non-PCI devices which are currently supported. A bug in this patch could
> cause a failure to setup the SMMU for one or more devices. I would
> imagine that it would manifest probably as either an error or an hang
> (given that it is adding spin locks) early at boot when the SMMU is
> configured.
> 
> The validation of this patch would mostly happen by review: it is the
> kind of patch that changes some "return -1" into "goto err".
> 

In order not to leave this patch high and dry:
I can see that Stefano and Bertrand are in favor of this patch and
Julien is rather against. Therefore I wanted to ask what are we doing with this 
patch.
Do we want it for 4.16?

Cheers,
Michal