On 06/02/2020 09:28, Jan Beulich wrote: > On 05.02.2020 17:50, Andrew Cooper wrote: >> --- a/tools/libxc/xc_sr_restore_x86_hvm.c >> +++ b/tools/libxc/xc_sr_restore_x86_hvm.c >> @@ -72,6 +72,16 @@ static int handle_hvm_params(struct xc_sr_context *ctx, >> case HVM_PARAM_BUFIOREQ_PFN: >> xc_clear_domain_page(xch, ctx->domid, entry->value); >> break; >> + >> + case HVM_PARAM_PAE_ENABLED: >> + /* >> + * This HVM_PARAM only ever existed a non-standard calling ABI >> for >> + * xc_cpuid_apply_policy(). It has now been updated to use a >> + * regular calling convention, making the param obsolete. >> + * >> + * Discard if we find it in an old migration stream. >> + */ >> + continue; > Having also looked at the previous patch (the only one in this series > relevant to the adjustments done here afaict)
Correct. > I wonder whether simply > ignoring it (i.e. not even warning anywhere when out of sync with > whatever info->u.hvm.pae gets populated from) is a good approach. We can't (easily) cross check at all, because info->u.hvm.pae is in a separate process (as far as the xl/libxl toolstack goes). On cross checking, you'll find that migration in from pre Xen 4.6 doesn't actually have the data. If you look back at the 4.5 legacy migration code, you'll observe that this param is restored but never saved. In hindsight, we probably shouldn't have fixed that in migration v2, but we did. Upstream was actually fine, because libxl sets HVM_PARAM_PAE_ENABLED after the migration stream completes, and overwrites whatever was where. XenServer did not, and we noticed as a consequence of Xen 4.5 actually cross-checked CPUID settings on a mov to %cr4 emulation. > But of course I may be easily missing aspects here that make this quite > fine. It really is obsolete and needs forgetting, not checking. > >> --- a/xen/include/public/hvm/params.h >> +++ b/xen/include/public/hvm/params.h >> @@ -86,7 +86,7 @@ >> #define HVM_PARAM_STORE_PFN 1 >> #define HVM_PARAM_STORE_EVTCHN 2 >> >> -#define HVM_PARAM_PAE_ENABLED 4 >> +#define HVM_PARAM_PAE_ENABLED 4 /* Obsolete. Do not use. */ > I think this should be moved up in the deprecated section. There isn't a deprecated section. The params are currently sorted numerically. Playing "which param is this integer?" with an unsorted params.h is an experience I wish never to repeat again. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
