On Thu, Mar 23, 2023, Mingwei Zhang wrote:
> > > static inline u64 kvm_get_filtered_xcr0(void)
> > > {
> > > - return kvm_caps.supported_xcr0 & xstate_get_guest_group_perm();
> > > + u64 supported_xcr0 = kvm_caps.supported_xcr0;
> > > +
> > > + BUILD_BUG_ON(XFEATURE_MASK_USER_DYNAMIC != XFEATURE_MASK_XTILE_DATA);
> > > +
> > > + if (supported_xcr0 & XFEATURE_MASK_USER_DYNAMIC) {
> > > + supported_xcr0 &= xstate_get_guest_group_perm();
> > > +
> > > + /*
> > > + * Treat XTILE_CFG as unsupported if the current process isn't
> > > + * allowed to use XTILE_DATA, as attempting to set XTILE_CFG in
> > > + * XCR0 without setting XTILE_DATA is architecturally illegal.
> > > + */
> > > + if (!(supported_xcr0 & XFEATURE_MASK_XTILE_DATA))
> > > + supported_xcr0 &= XFEATURE_MASK_XTILE_CFG;
> >
> > should be this? supported_xcr0 &= ~XFEATURE_MASK_XTILE_CFG;
> >
> >
> > > + }
> > > + return supported_xcr0;
> > > }
> Also, a minor opinion: shall we use permitted_xcr0 instead of
> supported_xcr0 to be consistent with other places? This way, it is
> clear that supported_xcr0 is (almost) never changing. permitted_xcr0,
> as its name suggested, will be subject to permission change.
Ya, works for me.
