diff mbox series

[v2,1/4] x86/hvm: Defer the size calculation in hvm_save_cpu_xsave_states()

Message ID 20240429182823.1130436-2-andrew.cooper3@citrix.com (mailing list archive)
State Superseded
Headers show
Series x86/xstate: Fixes to size calculations | expand

Commit Message

Andrew Cooper April 29, 2024, 6:28 p.m. UTC 
HVM_CPU_XSAVE_SIZE() may rewrite %xcr0 twice.  Defer the caluclation it until
after we've decided to write out an XSAVE record.

Note in hvm_load_cpu_xsave_states() that there were versions of Xen which
wrote out a useless XSAVE record.  This sadly limits out ability to tidy up
the existing infrastructure.  Also leave a note in xstate_ctxt_size() that 0
still needs tolerating for now.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

v2:
 * New
---
 xen/arch/x86/hvm/hvm.c | 10 ++++++++--
 xen/arch/x86/xstate.c  |  2 +-
 2 files changed, 9 insertions(+), 3 deletions(-)

Comments

Jan Beulich May 2, 2024, 12:08 p.m. UTC | #1 
On 29.04.2024 20:28, Andrew Cooper wrote:
> HVM_CPU_XSAVE_SIZE() may rewrite %xcr0 twice.  Defer the caluclation it until
> after we've decided to write out an XSAVE record.
> 
> Note in hvm_load_cpu_xsave_states() that there were versions of Xen which
> wrote out a useless XSAVE record.  This sadly limits out ability to tidy up
> the existing infrastructure.  Also leave a note in xstate_ctxt_size() that 0
> still needs tolerating for now.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with a nit that there are a few spelling/grammar issues in the text above.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0ce45b177cf4..9594e0a5c530 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1197,12 +1197,13 @@  static int cf_check hvm_save_cpu_xsave_states(
     struct vcpu *v, hvm_domain_context_t *h)
 {
     struct hvm_hw_cpu_xsave *ctxt;
-    unsigned int size = HVM_CPU_XSAVE_SIZE(v->arch.xcr0_accum);
+    unsigned int size;
     int err;
 
-    if ( !cpu_has_xsave || !xsave_enabled(v) )
+    if ( !xsave_enabled(v) )
         return 0;   /* do nothing */
 
+    size = HVM_CPU_XSAVE_SIZE(v->arch.xcr0_accum);
     err = _hvm_init_entry(h, CPU_XSAVE_CODE, v->vcpu_id, size);
     if ( err )
         return err;
@@ -1255,6 +1256,11 @@  static int cf_check hvm_load_cpu_xsave_states(
     if ( !cpu_has_xsave )
         return -EOPNOTSUPP;
 
+    /*
+     * Note: Xen prior to 4.12 would write out empty XSAVE records for VMs
+     * running on XSAVE-capable hardware but without XSAVE active.
+     */
+
     /* Customized checking for entry since our entry is of variable length */
     desc = (struct hvm_save_descriptor *)&h->data[h->cur];
     if ( sizeof (*desc) > h->size - h->cur)
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index cf94761d0542..99cedb4f5e24 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -573,7 +573,7 @@  unsigned int xstate_ctxt_size(u64 xcr0)
     if ( xcr0 == xfeature_mask )
         return xsave_cntxt_size;
 
-    if ( xcr0 == 0 )
+    if ( xcr0 == 0 ) /* TODO: clean up paths passing 0 in here. */
         return 0;
 
     return hw_uncompressed_size(xcr0);