Re: [UNIKRAFT PATCH 2/2] arch/arm: Provide ukarch_spinwait noop for ARM

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 14.08.20 22:14, Julien Grall wrote:
> Hi,
>
> On 23/07/2020 09:15, Alexander Jung wrote:
> > ARM does not support a pause-type instruction for busy spin wait
> > operations.  This commit provides the same alias to be used in
> > instances where it is required but defaults to a blank operation.
> On Arm, the closest instruction would be "yield".
>
> > Signed-off-by: Alexander Jung <alexander.jung@xxxxxxxxx>
> > ---
> >   arch/arm/arm/include/uk/asm/lcpu.h   | 5 +++++
> >   arch/arm/arm64/include/uk/asm/lcpu.h | 5 +++++
> >   2 files changed, 10 insertions(+)
> >
> > diff --git a/arch/arm/arm/include/uk/asm/lcpu.h 
> > b/arch/arm/arm/include/uk/asm/lcpu.h
> > index cdeffc3..f38ffb9 100644
> > --- a/arch/arm/arm/include/uk/asm/lcpu.h
> > +++ b/arch/arm/arm/include/uk/asm/lcpu.h
> > @@ -57,3 +57,8 @@ struct __regs {
> >   #ifndef wmb
> >   #define wmb() __asm__("dsb" : : : "memory")
> >   #endif
> > +
> > +static inline void ukarch_spinwait(void)
> > +{
> > +    /* Intelligent busy wait not supported on arm. */
> This at least need to contain a barrier() to force the compiler to 
> reload the memory after the wait. Otherwise, it may optimize waiting 
> loop to infinite loop.
> Unfortunately, I don't have a setup with Unikraft. But I am happy to 
> provide a patch if someone else can test it.
Hey Alex, Costin,

can one of you provide a follow-up patch that includes a barrier as 
Julien suggested? I saw that this patch made it upstream already.
Thanks,

Simon

> > +}
> > diff --git a/arch/arm/arm64/include/uk/asm/lcpu.h 
> > b/arch/arm/arm64/include/uk/asm/lcpu.h
> > index 45d1b23..51e17ad 100644
> > --- a/arch/arm/arm64/include/uk/asm/lcpu.h
> > +++ b/arch/arm/arm64/include/uk/asm/lcpu.h
> > @@ -146,4 +146,9 @@ static inline unsigned long ukarch_read_sp(void)
> >       return sp;
> >   }
> > +static inline void ukarch_spinwait(void)
> > +{
> > +    /* Intelligent busy wait not supported on arm64. */
> Likewise here.
>
> > +}
> > +
> >   #endif /* __ASSEMBLY__ */
> Cheers,