On 29.04.2024 20:28, Andrew Cooper wrote:
> @@ -567,16 +567,51 @@ static unsigned int hw_uncompressed_size(uint64_t xcr0)
> return size;
> }
>
> -/* Fastpath for common xstate size requests, avoiding reloads of xcr0. */
> -unsigned int xstate_ctxt_size(u64 xcr0)
> +unsigned int xstate_uncompressed_size(uint64_t xcr0)
> {
> + unsigned int size = XSTATE_AREA_MIN_SIZE, i;
> +
> if ( xcr0 == xfeature_mask )
> return xsave_cntxt_size;
>
> if ( xcr0 == 0 ) /* TODO: clean up paths passing 0 in here. */
> return 0;
>
> - return hw_uncompressed_size(xcr0);
> + if ( xcr0 <= (X86_XCR0_SSE | X86_XCR0_FP) )
This is open-coded XSTATE_FP_SSE, which I wouldn't mind if ...
> + return size;
> +
> + /*
> + * For the non-legacy states, search all activate states and find the
> + * maximum offset+size. Some states (e.g. LWP, APX_F) are out-of-order
> + * with respect their index.
> + */
> + xcr0 &= ~XSTATE_FP_SSE;
... you didn't use that macro here (and once further down). IOW please
be consistent, no matter which way round.
> + for_each_set_bit ( i, &xcr0, 63 )
> + {
> + unsigned int s;
> +
> + ASSERT(xstate_offsets[i] && xstate_sizes[i]);
> +
> + s = xstate_offsets[i] && xstate_sizes[i];
You mean + here, don't you? Then:
Reviewed-by: Jan Beulich <[email protected]>
I'm also inclined to suggest making this the initializer of s.
Jan
