[PATCH v4 01/28] x86/xstate: re-size save area when CPUID policy changes

[Jan Beulich <jbeulich@xxxxxxxx>]

vCPU-s get maximum size areas allocated initially. Hidden (and in
particular default-off) features may allow for a smaller size area to
suffice.

Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
v4: Re-base.
v2: Use 1ul instead of 1ull. Re-base.
---
This could be further shrunk if we used XSAVEC / if we really used
XSAVES, as then we don't need to also cover the holes. But since we
currently use neither of the two in reality, this would require more
work than just adding the alternative size calculation here.

Seeing that both vcpu_init_fpu() and cpu_policy_updated() are called
from arch_vcpu_create(), I'm not sure we really need this two-stage
approach - the slightly longer period of time during which
v->arch.xsave_area would remain NULL doesn't look all that problematic.
But since xstate_alloc_save_area() gets called for idle vCPU-s, it has
to stay anyway in some form, so the extra code churn may not be worth
it.

Instead of cpu_policy_xcr0_max(), cpu_policy_xstates() may be the
interface to use here. But it remains to be determined whether the
xcr0_accum field is meant to be inclusive of XSS (in which case it would
better be renamed) or exclusive. Right now there's no difference as we
don't support any XSS-controlled features.

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -290,13 +290,17 @@ void update_guest_memory_policy(struct v
  * Called during vcpu construction, and each time the toolstack changes the
  * CPU policy configuration for the domain.
  */
-static void cpu_policy_updated(struct vcpu *v)
+static int __must_check cpu_policy_updated(struct vcpu *v)
 {
-    if ( is_hvm_vcpu(v) )
+    int rc = xstate_update_save_area(v);
+
+    if ( !rc && is_hvm_vcpu(v) )
         hvm_cpuid_policy_changed(v);
+
+    return rc;
 }
 
-void domain_cpu_policy_changed(struct domain *d)
+int domain_cpu_policy_changed(struct domain *d)
 {
     const struct cpu_policy *p = d->arch.cpu_policy;
     struct vcpu *v;
@@ -454,17 +458,22 @@ void domain_cpu_policy_changed(struct do
 
     /* Nested doesn't have the necessary processing, yet. */
     if ( nestedhvm_enabled(d) && (p->feat.user_msr || p->feat.msr_imm) )
-        return /*-EINVAL*/;
+        return -EINVAL;
 
     for_each_vcpu ( d, v )
     {
-        cpu_policy_updated(v);
+        int rc = cpu_policy_updated(v);
+
+        if ( rc )
+            return rc;
 
         /* If PMU version is zero then the guest doesn't have VPMU */
         if ( boot_cpu_data.vendor == X86_VENDOR_INTEL &&
              p->basic.pmu_version == 0 )
             vpmu_destroy(v);
     }
+
+    return 0;
 }
 
 unsigned int arch_domain_struct_memflags(void)
@@ -562,7 +571,7 @@ int arch_vcpu_create(struct vcpu *v)
     {
         vpmu_initialise(v);
 
-        cpu_policy_updated(v);
+        rc = cpu_policy_updated(v);
     }
 
     return rc;
@@ -936,11 +945,11 @@ int arch_domain_create(struct domain *d,
      */
     d->arch.x87_fip_width = cpu_has_fpu_sel ? 0 : 8;
 
-    domain_cpu_policy_changed(d);
-
     d->arch.msr_relaxed = config->arch.misc_flags & XEN_X86_MSR_RELAXED;
 
-    return 0;
+    rc = domain_cpu_policy_changed(d);
+    if ( !rc )
+        return 0;
 
  fail:
     d->is_dying = DOMDYING_dead;
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -87,7 +87,7 @@ static int update_domain_cpu_policy(stru
     recalculate_cpuid_policy(d);
 
     /* Recalculate relevant dom/vcpu state now the policy has changed. */
-    domain_cpu_policy_changed(d);
+    ret = domain_cpu_policy_changed(d);
 
  out:
     /* Free whichever struct is not installed in struct domain. */
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -692,7 +692,7 @@ struct guest_memory_policy
 void update_guest_memory_policy(struct vcpu *v,
                                 struct guest_memory_policy *policy);
 
-void domain_cpu_policy_changed(struct domain *d);
+int __must_check domain_cpu_policy_changed(struct domain *d);
 
 bool update_secondary_system_time(struct vcpu *v,
                                   struct vcpu_time_info *u);
--- a/xen/arch/x86/include/asm/xstate.h
+++ b/xen/arch/x86/include/asm/xstate.h
@@ -109,6 +109,7 @@ void compress_xsave_states(struct vcpu *
 /* extended state init and cleanup functions */
 void xstate_free_save_area(struct vcpu *v);
 int xstate_alloc_save_area(struct vcpu *v);
+int xstate_update_save_area(struct vcpu *v);
 void xstate_init(struct cpuinfo_x86 *c);
 unsigned int xstate_uncompressed_size(uint64_t xcr0);
 unsigned int xstate_compressed_size(uint64_t xstates);
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -575,6 +575,40 @@ int xstate_alloc_save_area(struct vcpu *
     return 0;
 }
 
+int xstate_update_save_area(struct vcpu *v)
+{
+    unsigned int i, size, old;
+    struct xsave_struct *save_area;
+    uint64_t xcr0_max = cpu_policy_xcr0_max(v->domain->arch.cpuid);
+
+    ASSERT(!is_idle_vcpu(v));
+
+    if ( !cpu_has_xsave )
+        return 0;
+
+    if ( v->arch.xcr0_accum & ~xcr0_max )
+        return -EBUSY;
+
+    for ( size = old = XSTATE_AREA_MIN_SIZE, i = 2; i < xstate_features; ++i )
+    {
+        if ( xcr0_max & (1ul << i) )
+            size = max(size, xstate_offsets[i] + xstate_sizes[i]);
+        if ( v->arch.xcr0_accum & (1ul << i) )
+            old = max(old, xstate_offsets[i] + xstate_sizes[i]);
+    }
+
+    save_area = _xvrealloc(v->arch.xsave_area, size, __alignof(*save_area));
+    if ( !save_area )
+        return -ENOMEM;
+
+    ASSERT(old <= size);
+    memset((void *)save_area + old, 0, size - old);
+
+    v->arch.xsave_area = save_area;
+
+    return 0;
+}
+
 void xstate_free_save_area(struct vcpu *v)
 {
     XVFREE(v->arch.xsave_area);