Re: [XEN PATCH v1] xen/arm : Add support for SMMUv3 driver

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Julien,

> On 26 Oct 2020, at 19:05, Julien Grall <julien@xxxxxxx> wrote:
> 
> On 26/10/2020 12:10, Ash Wilding wrote:
>> Hi,
> 
> Hi Ash,
> 
>>> 1. atomic_set_release
>>> 2. atomic_fetch_andnot_relaxed
>>> 3. atomic_cond_read_relaxed
>>> 4. atomic_long_cond_read_relaxed
>>> 5. atomic_long_xor
>>> 6. atomic_set_release
>>> 7. atomic_cmpxchg_relaxed might be we can use atomic_cmpxchg that is
>>>    implemented in XEN need to check.
>>> 8. atomic_dec_return_release
>>> 9. atomic_fetch_inc_relaxed
>> If we're going to pull in Linux's implementations of the above atomics
>> helpers for SMMUv3, and given the majority of SMMUv3 systems are v8.1+
>> with LSE, perhaps this would be a good time to drop the current
>> atomic.h in Xen completely and pull in both Linux's LL/SC and LSE
>> helpers,
> 
> When I originally answered to the thread, I thought about suggesting 
> importing LSE. But I felt it was too much to ask in order to merge the SMMUv3 
> code.
> 
> However, I would love to have support for LSE in Xen as this would solve 
> other not yet fully closed security issue with LL/SC (see XSA-295 [2]).
> 
> Would Arm be willing to add support for LSE before merging the SMMUv3?

We are willing to work on LSE but I cannot commit on me and my team to start 
working on this subject before the end of the year.

So I think it would be good to integrate this version of SMMUv3 and we can then 
update it to the latest Linux one once LSE has been added.

I think it make sense in the meantime to discuss how this should be designed 
but it might be a good idea to make a specific discussion thread for that.

Cheers
Bertrand

> 
> As an alternative, it might also be possible to provide "dumb" implementation 
> for all the helpers even if they are stricter than necessary for the memory 
> ordering requirements.
> 
> then use a new Kconfig to toggle between them?
> 
> I would prefer to follow the same approach as Linux and allow Xen to select 
> at boot time which implementations to use. This would enable distro to 
> provide a single binary that boot on all Armv8 and still allow Xen to select 
> the best set of instructions.
> 
> Xen already provides a framework to switch between two sets of instructions 
> at boot. This was borrowed from Linux, so I don't expect a big hurdle to get 
> this supported.
> 
>> Back in 5d45ecabe3 [1] Jan mentioned we probably want to avoid relying
>> on gcc atomics helpers as we can't switch between LL/SC and LSE
>> atomics. 
> 
> I asked Jan to add this line in the commit message :). My concern was that 
> even if we provided a runtime switch (or sanity check for XSA-295), the GCC 
> helpers would not be able to take advantage (the code is not written by Xen 
> community).
> 
> Cheers,
> 
>> [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=5d45ecabe3
> 
> [2] https://xenbits.xen.org/xsa/advisory-295.html
> 
> 
> -- 
> Julien Grall