From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 5/5] x86/cpuid: Fix handling of xsave dynamic leaves
Date: Tue, 4 May 2021 14:56:09 +0200 [thread overview]
Message-ID: <5e6511ca-83bd-8a43-202e-949b4d19b1ab@suse.com> (raw)
In-Reply-To: <20210503153938.14109-6-andrew.cooper3@citrix.com>
On 03.05.2021 17:39, Andrew Cooper wrote:
> If max leaf is greater than 0xd but xsave not available to the guest, then the
> current XSAVE size should not be filled in. This is a latent bug for now as
> the guest max leaf is 0xd, but will become problematic in the future.
>
> The comment concerning XSS state is wrong. VT-x doesn't manage host/guest
> state automatically, but there is provision for "host only" bits to be set, so
> the implications are still accurate.
>
> Introduce {xstate,hw}_compressed_size() helpers to mirror the uncompressed
> ones.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
albeit with a remark:
> +unsigned int xstate_compressed_size(uint64_t xstates)
> +{
> + unsigned int i, size = XSTATE_AREA_MIN_SIZE;
> +
> + xstates &= ~XSTATE_FP_SSE;
> + for_each_set_bit ( i, &xstates, 63 )
> + {
> + if ( test_bit(i, &xstate_align) )
> + size = ROUNDUP(size, 64);
> +
> + size += xstate_sizes[i];
> + }
> +
> + /* In debug builds, cross-check our calculation with hardware. */
> + if ( IS_ENABLED(CONFIG_DEBUG) )
> + {
> + unsigned int hwsize;
> +
> + xstates |= XSTATE_FP_SSE;
> + hwsize = hw_compressed_size(xstates);
> +
> + if ( size != hwsize )
> + printk_once(XENLOG_ERR "%s(%#"PRIx64") size %#x != hwsize %#x\n",
> + __func__, xstates, size, hwsize);
> + size = hwsize;
To be honest, already on the earlier patch I was wondering whether
it does any good to override size here: That'll lead to different
behavior on debug vs release builds. If the log message is not
paid attention to, we'd then end up with longer term breakage.
Jan
next prev parent reply other threads:[~2021-05-04 12:56 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-03 15:39 [PATCH 0/5] x86/xstate: Fixes to size calculations Andrew Cooper
2021-05-03 15:39 ` [PATCH 1/5] x86/xstate: Elide redundant writes in set_xcr0() Andrew Cooper
2021-05-04 11:51 ` Jan Beulich
2021-05-03 15:39 ` [PATCH 2/5] x86/xstate: Rename _xstate_ctxt_size() to hw_uncompressed_size() Andrew Cooper
2021-05-04 11:53 ` Jan Beulich
2021-05-03 15:39 ` [PATCH 3/5] x86/xstate: Rework xstate_ctxt_size() as xstate_uncompressed_size() Andrew Cooper
2021-05-03 18:17 ` Andrew Cooper
2021-05-04 12:08 ` Jan Beulich
2021-05-04 12:15 ` Andrew Cooper
2021-05-04 12:20 ` Jan Beulich
2021-05-04 12:22 ` Andrew Cooper
2021-05-04 12:45 ` Jan Beulich
2021-05-03 15:39 ` [PATCH 4/5] x86/cpuid: Simplify recalculate_xstate() Andrew Cooper
2021-05-04 12:43 ` Jan Beulich
2021-05-04 13:58 ` Andrew Cooper
2021-05-05 8:19 ` Jan Beulich
2021-05-05 14:53 ` Andrew Cooper
2021-05-05 15:14 ` Jan Beulich
2021-05-03 15:39 ` [PATCH 5/5] x86/cpuid: Fix handling of xsave dynamic leaves Andrew Cooper
2021-05-04 12:56 ` Jan Beulich [this message]
2021-05-04 14:17 ` Andrew Cooper
2021-05-05 8:33 ` Jan Beulich
2021-05-05 16:59 ` Andrew Cooper
2021-05-06 6:17 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5e6511ca-83bd-8a43-202e-949b4d19b1ab@suse.com \
--to=jbeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=roger.pau@citrix.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).