Skip to content

Commit

Permalink
x86/xstate: Switch back to for_each_set_bit()
Browse files Browse the repository at this point in the history
In all 3 examples, we're iterating over a scaler.  They have an upper bound of
63 which is to exclude the top bit; the COMPRESSED bit.

recalculate_xstate() calculates xstates directly and doesn't set the
COMPRESSED bit.  Both xstate_{un,}compressed_size() take architectural
register values, neither of which permit the COMPRESSED bit either.

xstate_uncompressed_size() has an ASSERT() covering this properly; add a
equivelent ASSERT() to xstate_compressed_size() too.

This alone produces:

  add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-161 (-161)
  Function                                     old     new   delta
  compress_xsave_states                         66      58      -8
  xstate_uncompressed_size                     119      71     -48
  xstate_compressed_size                       124      76     -48
  recalculate_xstate                           347     290     -57

where xstate_{un,}compressed_size() have practically halved in size despite
being small before.

The change in compress_xsave_states() is unexpected.  The function is almost
entirely dead code, and within what remains there's a smaller stack frame.  I
suspect it's leftovers that the optimiser couldn't fully discard.

Signed-off-by: Andrew Cooper <[email protected]>
Reviewed-by: Jan Beulich <[email protected]>
  • Loading branch information
andyhhp committed Aug 23, 2024
1 parent 2387c46 commit 20885a3
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 6 deletions.
4 changes: 2 additions & 2 deletions xen/arch/x86/cpu-policy.c
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ static void sanitise_featureset(uint32_t *fs)
static void recalculate_xstate(struct cpu_policy *p)
{
uint64_t xstates = XSTATE_FP_SSE;
unsigned int i, ecx_mask = 0, Da1 = p->xstate.Da1;
unsigned int ecx_mask = 0, Da1 = p->xstate.Da1;

/*
* The Da1 leaf is the only piece of information preserved in the common
Expand Down Expand Up @@ -237,7 +237,7 @@ static void recalculate_xstate(struct cpu_policy *p)
/* Subleafs 2+ */
xstates &= ~XSTATE_FP_SSE;
BUILD_BUG_ON(ARRAY_SIZE(p->xstate.comp) < 63);
bitmap_for_each ( i, &xstates, 63 )
for_each_set_bit ( i, xstates )
{
/*
* Pass through size (eax) and offset (ebx) directly. Visbility of
Expand Down
10 changes: 6 additions & 4 deletions xen/arch/x86/xstate.c
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ static bool valid_xcr0(uint64_t xcr0)

unsigned int xstate_uncompressed_size(uint64_t xcr0)
{
unsigned int size = XSTATE_AREA_MIN_SIZE, i;
unsigned int size = XSTATE_AREA_MIN_SIZE;

/* Non-XCR0 states don't exist in an uncompressed image. */
ASSERT((xcr0 & ~X86_XCR0_STATES) == 0);
Expand All @@ -606,7 +606,7 @@ unsigned int xstate_uncompressed_size(uint64_t xcr0)
* with respect their index.
*/
xcr0 &= ~(X86_XCR0_SSE | X86_XCR0_X87);
bitmap_for_each ( i, &xcr0, 63 )
for_each_set_bit ( i, xcr0 )
{
const struct xstate_component *c = &raw_cpu_policy.xstate.comp[i];
unsigned int s = c->offset + c->size;
Expand All @@ -621,7 +621,9 @@ unsigned int xstate_uncompressed_size(uint64_t xcr0)

unsigned int xstate_compressed_size(uint64_t xstates)
{
unsigned int i, size = XSTATE_AREA_MIN_SIZE;
unsigned int size = XSTATE_AREA_MIN_SIZE;

ASSERT((xstates & ~(X86_XCR0_STATES | X86_XSS_STATES)) == 0);

if ( xstates == 0 )
return 0;
Expand All @@ -634,7 +636,7 @@ unsigned int xstate_compressed_size(uint64_t xstates)
* componenets require aligning to 64 first.
*/
xstates &= ~(X86_XCR0_SSE | X86_XCR0_X87);
bitmap_for_each ( i, &xstates, 63 )
for_each_set_bit ( i, xstates )
{
const struct xstate_component *c = &raw_cpu_policy.xstate.comp[i];

Expand Down

0 comments on commit 20885a3

Please sign in to comment.