Re: [Xen-devel] [PATCH V4 1/4] x86/xsaves: add basic definitions/helpers to support xsaves

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.08.15 at 12:54, <shuai.ruan@xxxxxxxxxxxxxxx> wrote:
> +void save_xsave_states(struct vcpu *v, void *dest ,unsigned int size)
> +{
> +    struct xsave_struct *xsave = v->arch.xsave_area;
> +    u64 xstate_bv = xsave->xsave_hdr.xstate_bv;
> +    u64 valid;
> +
> +    /*
> +     * Copy legacy XSAVE area, to avoid complications with CPUID
> +     * leaves 0 and 1 in the loop below.
> +     */
> +    memcpy(dest, xsave, FXSAVE_SIZE);
> +
> +    /* Set XSTATE_BV */
> +    *(u64 *)(dest + XSAVE_HDR_OFFSET) = xstate_bv;

Please try to avoid such (dangerous) casts.

> +    /*
> +     * Copy each region from the possibly compacted offset to the
> +     * non-compacted offset.
> +     */
> +    valid = xstate_bv & ~XSTATE_FP_SSE;
> +    while ( valid )
> +    {
> +        u64 feature = valid & -valid;
> +        int index = fls(feature) - 1;

find_first_set_bit() (or ffsl()) would make this more readable.

> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -506,6 +506,7 @@ struct arch_vcpu
>       */
>      struct xsave_struct *xsave_area;
>      uint64_t xcr0;
> +    uint64_t msr_ia32_xss;

Considering that you mean to not support this for PV guests, this
should go into struct hvm_vcpu. And I think naming it just xss
or at most msr_xss would be quite fine.

Jan


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