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

[Julien Grall <julien@xxxxxxx>]

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?

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