| Message ID | 20170707035314.15659-2-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 07.07.17 at 05:53, <haozhong.zhang@intel.com> wrote: > --- a/xen/arch/x86/domctl.c > +++ b/xen/arch/x86/domctl.c > @@ -302,6 +302,39 @@ 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 in this array. > + */ > + static const unsigned int valid_vmce_size[] = { valid_sizes[] ? > + sizeof(evc->vmce), > + sizeof(evc->mcg_cap), The first sizeof() is fine, but I think all subsequent entries should be offsetof() + sizeof(). Looking ahead into patch 2, you're indeed coding it the wrong way there (just think about the validity of the expression used when another field gets added), which to me hints that you've fallen into your own trap. It may then be worthwhile to macro-ize this so that the relevant field name needs to only be handed once to the macro. > + 0, I think you can easily get away without this sentinel. Alternatively there's no point in using ARRAY_SIZE() further down. > + }; > + struct hvm_vmce_vcpu vmce = { }; > + unsigned int vmce_size = evc->size - offsetof(typeof(*evc), mcg_cap); > + int i = 0; unsigned int > + BUILD_BUG_ON(offsetof(struct xen_domctl_ext_vcpucontext, mcg_cap) != > + offsetof(struct xen_domctl_ext_vcpucontext, vmce.caps)); Please consistently use typeof(*evc) now that you move (and hence touch) this code anyway. > + BUILD_BUG_ON(sizeof(evc->mcg_cap) != sizeof(evc->vmce.caps)); > + > + while ( i < ARRAY_SIZE(valid_vmce_size) - 1 && > + vmce_size < valid_vmce_size[i] ) > + ++i; > + vmce_size = valid_vmce_size[i]; > + > + if ( !vmce_size ) > + return 0; > + > + memcpy(&vmce, &evc->vmce, vmce_size); > + return vmce_restore_vcpu(v, &vmce); Blank line before final return statement please. Jan
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 7fa58b49af..5cd2af76bb 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -302,6 +302,39 @@ 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 in this array. + */ + static const unsigned int valid_vmce_size[] = { + sizeof(evc->vmce), + sizeof(evc->mcg_cap), + 0, + }; + struct hvm_vmce_vcpu vmce = { }; + unsigned int vmce_size = evc->size - offsetof(typeof(*evc), mcg_cap); + int i = 0; + + 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)); + + while ( i < ARRAY_SIZE(valid_vmce_size) - 1 && + vmce_size < valid_vmce_size[i] ) + ++i; + vmce_size = valid_vmce_size[i]; + + if ( !vmce_size ) + return 0; + + memcpy(&vmce, &evc->vmce, vmce_size); + return vmce_restore_vcpu(v, &vmce); +} + void arch_get_domain_info(const struct domain *d, struct xen_domctl_getdomaininfo *info) { @@ -912,23 +945,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); }
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 <haozhong.zhang@intel.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/domctl.c | 51 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 17 deletions(-)