Re: [PATCH] x86/fpu: Set the corret permission value for perm.__state_perm

[Yang Zhong <yang.zhong@intel.com>]

On Tue, Nov 09, 2021 at 12:13:58AM +0800, Bae, Chang Seok wrote:
> On Nov 8, 2021, at 14:28, Zhong, Yang <yang.zhong@intel.com> wrote:
> >
> > The perm.__state_perm is default xsave set features, when we request
> > AMX permission from application, the requested value(bit18) should be
> > replaced with mask value(requested | permitted), which can keep default
> > features there.
> >
> > Without this change, the below prctl syscall:
> > (1). ARCH_GET_XCOMP_PERM, the bitmask=0x202e7
> > (2). set ARCH_REQ_XCOMP_PERM with XFEATURE_XTILEDATA
> > (3). ARCH_GET_XCOMP_PERM, the bitmask=0x40000, the correct value
> >     should be 0x602e7 here.
> 
> Oh, bad.
> 
> > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> > index d28829403ed0..fc1ab0116f4e 100644
> > --- a/arch/x86/kernel/fpu/xstate.c
> > +++ b/arch/x86/kernel/fpu/xstate.c
> > @@ -1626,7 +1626,7 @@ static int __xstate_request_perm(u64 permitted, u64 requested)
> >               return ret;
> >
> >       /* Pairs with the READ_ONCE() in xstate_get_group_perm() */
> > -     WRITE_ONCE(fpu->perm.__state_perm, requested);
> > +     WRITE_ONCE(fpu->perm.__state_perm, mask);
> 
> I think this has to update the group leader’s like:
>     WRITE_ONCE(current->group_leader->thread.fpu.perm.__state_perm, mask);
>

  I think this is no need to change with group leader here, the fpu pointer has defined as below: 
  
  struct fpu *fpu = &current->group_leader->thread.fpu;

  Please check Thomas's changes for KVM, the WRITE_ONCE(fpu->perm.__state_perm, mask) is more reasonable
  here.

  https://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git/commit/?h=x86/fpu-kvm&id=62016e4cc41716b33da890ad43145b646f9db51d

  I also verified this changes from host and guest side, all work well.

  Regards,

  Yang
   


> Thanks,
> Chang
> 
> 
>