Re: [Xen-devel] [PATCHv1 3/5] x86/fpu: Add a per-domain field to set the width of FIP/FDP

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.02.16 at 19:52, <david.vrabel@xxxxxxxxxx> wrote:
> The x86 architecture allows either: a) the 64-bit FIP/FDP registers to be
> restored (clearing FCS and FDS); or b) the 32-bit FIP/FDP and FCS/FDS
> registers to be restored (clearing the upper 32-bits).
> 
> Add a per-domain field to indicate which of these options a guest needs.
> The options are: 8, 4 or 0.  Where 0 indicates that the hypervisor should
> automatically guess the FIP width by checking the value of FIP/FDP when
> saving the state (this is the existing behaviour).
> 
> The FIP width is initially automatic but is set explicitly in the following
> cases:
> 
> - 32-bit PV guest: 4
> - 64-bit PV guest: 8

The latter is wrong: 64-bit OSes may, for the benefit of compat
mode processes, use 32-bit save/restore operations.

> @@ -261,28 +261,8 @@ void xsave(struct vcpu *v, uint64_t mask)
>                           "=m" (*ptr), \
>                           "a" (lmask), "d" (hmask), "D" (ptr))
>  
> -    if ( word_size <= 0 || !is_pv_32bit_vcpu(v) )
> +    if ( fip_width != 4 )
>      {
> -        typeof(ptr->fpu_sse.fip.sel) fcs = ptr->fpu_sse.fip.sel;
> -        typeof(ptr->fpu_sse.fdp.sel) fds = ptr->fpu_sse.fdp.sel;
> -
> -        if ( cpu_has_xsaveopt || cpu_has_xsaves )
> -        {
> -            /*
> -             * XSAVEOPT/XSAVES may not write the FPU portion even when the
> -             * respective mask bit is set. For the check further down to work
> -             * we hence need to put the save image back into the state that
> -             * it was in right after the previous XSAVEOPT.
> -             */
> -            if ( word_size > 0 &&
> -                 (ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] == 4 ||
> -                  ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] == 2) )
> -            {
> -                ptr->fpu_sse.fip.sel = 0;
> -                ptr->fpu_sse.fdp.sel = 0;
> -            }
> -        }
> -
>          XSAVE("0x48,");
>  
>          if ( !(mask & ptr->xsave_hdr.xstate_bv & XSTATE_FP) ||
> @@ -293,15 +273,14 @@ void xsave(struct vcpu *v, uint64_t mask)
>               (!(ptr->fpu_sse.fsw & 0x0080) &&
>                boot_cpu_data.x86_vendor == X86_VENDOR_AMD) )
>          {
> -            if ( (cpu_has_xsaveopt || cpu_has_xsaves) && word_size > 0 )
> -            {
> -                ptr->fpu_sse.fip.sel = fcs;
> -                ptr->fpu_sse.fdp.sel = fds;
> -            }
>              return;

I don't see how you can validly delete all of the above code without
any replacement. Can you explain the rationale behind this?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel