Re: [PATCH v3 03/10] x86/fpu/xstate: Correct xfeatures cache in guest pseudo fpu container

From: Dave Hansen
Date: Fri Mar 07 2025 - 12:48:13 EST


On 3/7/25 08:41, Chao Gao wrote:
> The xfeatures field in struct fpu_guest is designed to track the enabled
> xfeatures for guest FPUs. However, during allocation in
> fpu_alloc_guest_fpstate(), gfpu->xfeatures is initialized to
> fpu_user_cfg.default_features, while the corresponding
> fpstate->xfeatures is set to fpu_kernel_cfg.default_features
>
> Correct the mismatch to avoid confusion.
>
> Note this mismatch does not cause any functional issues. The
> gfpu->xfeatures is checked in fpu_enable_guest_xfd_features() to
> verify if XFD features are already enabled:
>
> xfeatures &= ~guest_fpu->xfeatures;
> if (!xfeatures)
> return 0;
>
> It gets updated in fpstate_realloc() after enabling some XFD features:
>
> guest_fpu->xfeatures |= xfeatures;
>
> So, backport is not needed.

I don't have any great suggestions for improving this, but I just don't
seem to find this changelog compelling. I can't put my finger on it, though.

I think I'd find it more convincing if you argued what the *CORRECT*
value is and why rather than just arguing for consistency with a random
value. I also don't get the pivot over the XFD for explaining why it is
harmless. XFD isn't even used in most cases, so I'd find a justification
separate from XFD more compelling.