[PATCH] crypto: arm64/sha256 - fix build when CONFIG_PREEMPT_VOLUNTARY=y

[Ard Biesheuvel]

On Thu, 8 May 2025 at 18:05, Eric Biggers <ebiggers at kernel.org> wrote:
>
> On Thu, May 08, 2025 at 01:12:28PM +0200, Ard Biesheuvel wrote:
> > On Wed, 7 May 2025 at 19:09, Eric Biggers <ebiggers at kernel.org> wrote:
> > >
> > > From: Eric Biggers <ebiggers at google.com>
> > >
> > > Fix the build of sha256-ce.S when CONFIG_PREEMPT_VOLUNTARY=y by passing
> > > the correct label to the cond_yield macro.  Also adjust the code to
> > > execute only one branch instruction when CONFIG_PREEMPT_VOLUNTARY=n.
> > >
> > > Fixes: 6e36be511d28 ("crypto: arm64/sha256 - implement library instead of shash")
> > > Reported-by: kernel test robot <lkp at intel.com>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202505071811.yYpLUbav-lkp@intel.com/
> > > Signed-off-by: Eric Biggers <ebiggers at google.com>
> > > ---
> > >  arch/arm64/lib/crypto/sha256-ce.S | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm64/lib/crypto/sha256-ce.S b/arch/arm64/lib/crypto/sha256-ce.S
> > > index a8461d6dad634..f3e21c6d87d2e 100644
> > > --- a/arch/arm64/lib/crypto/sha256-ce.S
> > > +++ b/arch/arm64/lib/crypto/sha256-ce.S
> > > @@ -121,14 +121,15 @@ CPU_LE(   rev32           v19.16b, v19.16b        )
> > >
> > >         /* update state */
> > >         add             dgav.4s, dgav.4s, dg0v.4s
> > >         add             dgbv.4s, dgbv.4s, dg1v.4s
> > >
> > > +       /* return early if voluntary preemption is needed */
> > > +       cond_yield      1f, x5, x6
> > > +
> >
> > This will yield needlessly when the condition hits during the final iteration.
> >
> > >         /* handled all input blocks? */
> > > -       cbz             x2, 1f
> > > -       cond_yield      3f, x5, x6
> > > -       b               0b
> > > +       cbnz            x2, 0b
>
> cond_yield doesn't actually yield, though.  It just checks whether yielding is
> needed.  So the behavior is the same: on the last iteration this function
> returns 0 (i.e. 0 blocks remaining), regardless of whether it gets to the end by
> jumping there due to TSK_TI_PREEMPT being set or by falling through after seeing
> nblocks==0.  We could keep the nblocks==0 check first, but the cond_yield check
> is lightweight and it's probably better to avoid the extra branch instruction on
> every other iteration.
>

Ah yes, you're right.

Hopefully we'll soon be able to get rid of it entirely.