Re: [Xen-devel] [PATCH V7 2/3] x86/xsaves: fix two remained issues

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.03.16 at 10:57, <shuai.ruan@xxxxxxxxxxxxxxx> wrote:

Considering this isn't the last patch in the series, the subject isn't
really nice (apart from being mis-spelled). If you e.g. replaced
"remained" by "miscellaneous", I wouldn't insist on splitting.

> 1. get_xsave_addr() will only be called when
> xsave_area_compressed(xsave) is true. So drop the
> conditional expression.
> 
> 2. expand_xsave_states() will memset the area when
> get NULL from get_xsave_addr().

Reported-by: ...

> Signed-off-by: Shuai Ruan <shuai.ruan@xxxxxxxxx>
> 
>  xen/arch/x86/xstate.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
> index 8c652bc..f4ea54d 100644
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -164,12 +164,8 @@ static void *get_xsave_addr(struct xsave_struct *xsave,
>                              const uint16_t *comp_offsets,
>                              unsigned int xfeature_idx)
>  {
> -    if ( !((1ul << xfeature_idx) & xsave->xsave_hdr.xstate_bv) )
> -        return NULL;
> -
> -    return (void *)xsave + (xsave_area_compressed(xsave) ?
> -                            comp_offsets[xfeature_idx] :
> -                            xstate_offsets[xfeature_idx]);
> +    return (1ul << xfeature_idx) & xsave->xsave_hdr.xstate_bv ?
> +           (void *)xsave + comp_offsets[xfeature_idx] : NULL;
>  }

I would really have expected an ASSERT() to get added as
(documenting) replacement.

Jan


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