[v8,1/7] x86/domctl: generalize the restore of vMCE parameters

Commit Message

Haozhong Zhang July 10, 2017, 2:52 a.m. UTC

vMCE parameters in struct xen_domctl_ext_vcpucontext were extended in
the past, and is likely to be extended in the future. When migrating a
PV domain from old Xen, XEN_DOMCTL_set_ext_vcpucontext should handle
the differences.

Instead of adding ad-hoc handling code at each extension, we introduce
an array to record sizes of the current and all past versions of vMCE
parameters, and search for the largest one that does not expire the
size of passed-in parameters to determine vMCE parameters that will be
restored. If vMCE parameters are extended in the future, we only need
to adapt the array to reflect the extension.

Signed-off-by: Haozhong Zhang <[e-mail]>
---
Cc: Jan Beulich <[e-mail]>
Cc: Andrew Cooper <[e-mail]>

Changes in v8:
 * Rename valid_vmce_size[] tp valid_sizes[].
 * Use offsetof() + sizeof() in valid_sizes[] and macroize it.
 * Remove element 0 from valid_sizes[].
 * int i --> unsigned int i
 * Leave a blank line before the ending return.
---
 xen/arch/x86/domctl.c | 54 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 37 insertions(+), 17 deletions(-)

Comments

Jan Beulich July 11, 2017, 8:18 p.m. UTC | #1

>>> Haozhong Zhang <[e-mail]> 07/10/17 4:53 AM >>>
>--- a/xen/arch/x86/domctl.c
>+++ b/xen/arch/x86/domctl.c
>@@ -302,6 +302,42 @@ static int update_domain_cpuid_info(struct domain *d,
     >return 0;
 >}
 >
>+static int vcpu_set_vmce(struct vcpu *v,
>+                         const struct xen_domctl_ext_vcpucontext *evc)
>+{
>+    /*
>+     * Sizes of vMCE parameters used by the current and past versions
>+     * of Xen in descending order. If vMCE parameters are extended,
>+     * remember to add the old size to this array by VMCE_SIZE().
>+     */
>+#define VMCE_SIZE(param) \

I dislike macro (or function) parameter to be named "param" or alike. Please
use names that say what they stand for, e.g. "field" here.

>+    (offsetof(typeof(evc->vmce), param) + sizeof(evc->vmce.param))
>+
>+    static const unsigned int valid_sizes[] = {
>+        sizeof(evc->vmce),
>+        VMCE_SIZE(caps),
>+    };
>+#undef VMCE_SIZE
>+
>+    struct hvm_vmce_vcpu vmce = { };
>+    unsigned int evc_vmce_size = evc->size - offsetof(typeof(*evc), mcg_cap);

I'd prefer for this to be put in a min(..., sizeof(evc->vmce)) to cope with
possible future additions of members after the vmce one.

Jan

Patch

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 7fa58b49af..125537b96d 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -302,6 +302,42 @@  static int update_domain_cpuid_info(struct domain *d,
     return 0;
 }
 
+static int vcpu_set_vmce(struct vcpu *v,
+                         const struct xen_domctl_ext_vcpucontext *evc)
+{
+    /*
+     * Sizes of vMCE parameters used by the current and past versions
+     * of Xen in descending order. If vMCE parameters are extended,
+     * remember to add the old size to this array by VMCE_SIZE().
+     */
+#define VMCE_SIZE(param) \
+    (offsetof(typeof(evc->vmce), param) + sizeof(evc->vmce.param))
+
+    static const unsigned int valid_sizes[] = {
+        sizeof(evc->vmce),
+        VMCE_SIZE(caps),
+    };
+#undef VMCE_SIZE
+
+    struct hvm_vmce_vcpu vmce = { };
+    unsigned int evc_vmce_size = evc->size - offsetof(typeof(*evc), mcg_cap);
+    unsigned int i = 0;
+
+    BUILD_BUG_ON(offsetof(typeof(*evc), mcg_cap) !=
+                 offsetof(typeof(*evc), vmce.caps));
+    BUILD_BUG_ON(sizeof(evc->mcg_cap) != sizeof(evc->vmce.caps));
+
+    while ( i < ARRAY_SIZE(valid_sizes) && evc_vmce_size < valid_sizes[i] )
+        ++i;
+
+    if ( i == ARRAY_SIZE(valid_sizes) )
+        return 0;
+
+    memcpy(&vmce, &evc->vmce, valid_sizes[i]);
+
+    return vmce_restore_vcpu(v, &vmce);
+}
+
 void arch_get_domain_info(const struct domain *d,
                           struct xen_domctl_getdomaininfo *info)
 {
@@ -912,23 +948,7 @@  long arch_do_domctl(
             else
                 domain_pause(d);
 
-            BUILD_BUG_ON(offsetof(struct xen_domctl_ext_vcpucontext,
-                                  mcg_cap) !=
-                         offsetof(struct xen_domctl_ext_vcpucontext,
-                                  vmce.caps));
-            BUILD_BUG_ON(sizeof(evc->mcg_cap) != sizeof(evc->vmce.caps));
-            if ( evc->size >= offsetof(typeof(*evc), vmce) +
-                              sizeof(evc->vmce) )
-                ret = vmce_restore_vcpu(v, &evc->vmce);
-            else if ( evc->size >= offsetof(typeof(*evc), mcg_cap) +
-                                   sizeof(evc->mcg_cap) )
-            {
-                struct hvm_vmce_vcpu vmce = { .caps = evc->mcg_cap };
-
-                ret = vmce_restore_vcpu(v, &vmce);
-            }
-            else
-                ret = 0;
+            ret = vcpu_set_vmce(v, evc);
 
             domain_unpause(d);
         }