RE: [PATCH v4 1/2] x86/delay: Fix the wrong asm constraint in `delay_loop()`

David Laight
2022-03-01T09:54:16Z

From: Ammar Faizi
> Sent: 01 March 2022 09:46
>
> The asm constraint does not reflect that the asm statement can modify
> the value of @loops. But the asm statement in delay_loop() does change
> the @loops.
>
> If by any chance the compiler inlines this function, it may clobber
> random stuff (e.g. local variable, important temporary value in reg,
> etc.).
>
> Fortunately, delay_loop() is only called indirectly (so it can't
> inline), and then the register it clobbers is %rax (which is by the
> nature of the calling convention, it's a caller saved register), so it
> didn't yield any bug.

Both the function pointers in that code need killing.
They only have two options (each) so conditional branches
will almost certainly always have been better.

I also wonder how well the comment
The additional jump magic is needed to get the timing stable
on all the CPU' we have to worry about.
applies to any modern cpu!
The code is unchanged since (at least) 2.6.27.
(It might have been moved from another file.)

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)