Re: [PATCH] x86/FPU: make vcpu_reset_fpu() build again with old gcc

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

On Mon Dec 9, 2024 at 3:13 PM GMT, Jan Beulich wrote:
> Fields of anonymous structs/unions may not be part of an initializer for
> rather old gcc.

Can you add the specific version for tracking purposes?

>
> Fixes: 49a068471d77 ("x86/fpu: Rework fpu_setup_fpu() uses to split it in 
> two")
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/arch/x86/i387.c
> +++ b/xen/arch/x86/i387.c
> @@ -306,13 +306,13 @@ void vcpu_reset_fpu(struct vcpu *v)
>  {
>      v->fpu_initialised = false;
>      *v->arch.xsave_area = (struct xsave_struct) {
> -        .fpu_sse = {
> -            .mxcsr = MXCSR_DEFAULT,
> -            .fcw = FCW_RESET,
> -            .ftw = FXSAVE_FTW_RESET,
> -        },
>          .xsave_hdr.xstate_bv = X86_XCR0_X87,
>      };
> +
> +    /* Old gcc doesn't permit these to be part of the initializer. */
> +    v->arch.xsave_area->fpu_sse.mxcsr = MXCSR_DEFAULT;
> +    v->arch.xsave_area->fpu_sse.fcw = FCW_RESET;
> +    v->arch.xsave_area->fpu_sse.ftw = FXSAVE_FTW_RESET;

That's not quite the same though. A more apt equivalence would be to memset the
area to zero ahead of the assignments. Otherwise rubble will be left behind.

>  }
>  
>  void vcpu_setup_fpu(struct vcpu *v, const void *data)

Out of context and not triggering the GCC bug, but vcpu_setup_fpu() should
probably share the same initialization style as vcpu_reset_fpu(), imo.

Cheers,
Alejandro