Re: [Xen-devel] [V10 1/3] x86/xsaves: enable xsaves/xrstors/xsavec in xen

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.11.15 at 02:54, <shuai.ruan@xxxxxxxxxxxxxxx> wrote:
> @@ -91,7 +251,15 @@ void xsave(struct vcpu *v, uint64_t mask)
>          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 )
> +        if ( cpu_has_xsaves )
> +            asm volatile ( ".byte 0x48,0x0f,0xc7,0x2f"
> +                           : "=m" (*ptr)
> +                           : "a" (lmask), "d" (hmask), "D" (ptr) );
> +        else if ( cpu_has_xsavec )
> +            asm volatile ( ".byte 0x48,0x0f,0xc7,0x27"
> +                           : "=m" (*ptr)
> +                           : "a" (lmask), "d" (hmask), "D" (ptr) );
> +        else if ( cpu_has_xsaveopt )

I (only) now realize why I replied on the earlier version (wrongly)
assuming you hadn't switched to alternative patching: This code.
Why would you not do on the save side what you do on the
restore one?

> +/* Like alternative_io, but with fixup. */
> +#define alternative_io_fixup(oldinstr, newinstr, feature, fixup, output, 
> input...)   \
> +        asm volatile (ALTERNATIVE(oldinstr, newinstr, feature)       "\n"    
>                 \
> +             fixup : output : "i" (0), ## input)

Nothing here really requires "fixup" to be exception fixup code.
Macro names should, however, be chosen according to the
purpose of the macro, not according to the first use case of it.

That said, I don't think you even need this: The size of the
patchable region is determined by the labels (661 and 662 in
the header), and those labels will remain unaffected if
between them code emissions to other sections occur. By
having said that you shouldn't need to introduce XSTATE_FIXUP
I had actually tried to make you aware of this very fact.

Jan


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