On 2019-01-28 19:49:59 [+0100], Borislav Petkov wrote:
> > --- a/arch/x86/kernel/fpu/xstate.c
> > +++ b/arch/x86/kernel/fpu/xstate.c
> > @@ -830,15 +830,15 @@ static void *__raw_xsave_addr(struct xregs_state
> > *xsave, int xfeature_nr)
…
> > -void *get_xsave_addr(struct xregs_state *xsave, int xstate_feature)
> > +void *get_xsave_addr(struct xregs_state *xsave, int xfeature_nr)
> > {
> > - int xfeature_nr;
> > + u64 xfeature_mask = 1ULL << xfeature_nr;
>
> You can paste directly BIT_ULL(xfeature_nr) where you need it in this
> function...
changed.
> > @@ -850,11 +850,11 @@ void *get_xsave_addr(struct xregs_state *xsave, int
> > xstate_feature)
> > * have not enabled. Remember that pcntxt_mask is
> > * what we write to the XCR0 register.
> > */
> > - WARN_ONCE(!(xfeatures_mask & xstate_feature),
> > + WARN_ONCE(!(xfeatures_mask & xfeature_mask),
>
> ... and turn this into:
>
> WARN_ONCE(!(xfeatures_mask & BIT_ULL(xfeature_nr))
>
> which is more readable than the AND of two variables which I had to
> re-focus my eyes to see the difference. :)
>
you mean with vs without the `s' ?
> Oh and this way, gcc generates better code by doing simply a BT
> directly:
>
> # arch/x86/kernel/fpu/xstate.c:852: WARN_ONCE(!(xfeatures_mask &
> BIT_ULL(xfeature_nr)),
> .loc 1 852 2 view .LVU258
> movq xfeatures_mask(%rip), %rax # xfeatures_mask, tmp124
> btq %rsi, %rax # xfeature_nr, tmp124
interesting. gcc should know that it can use btq or shift + and because
it has all the raw data.
Anyway, I replaced the two user of xfeature_mask with
BIT_ULL(xfeature_nr).
> Thx.
Sebastian
