| Message ID | 20210430155211.3709-9-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libs/guest: new CPUID/MSR interface | expand |
On 30.04.2021 17:52, Roger Pau Monne wrote: > @@ -1086,3 +1075,42 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch, > > return rc; > } > + > +int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy, > + bool hvm) I'm concerned of the naming, and in particular the two very different meanings of "compatible" for xc_cpu_policy_calc_compatible() and this new one. I'm afraid I don't have a good suggestion though, short of making the name even longer and inserting "backwards". Jan > +{ > + xc_cpu_policy_t host; > + int rc; > + > + host = xc_cpu_policy_init(); > + if ( !host ) > + { > + errno = ENOMEM; > + return -1; > + } > + > + rc = xc_cpu_policy_get_system(xch, XEN_SYSCTL_cpu_policy_host, host); > + if ( rc ) > + { > + ERROR("Failed to get host policy"); > + goto out; > + } > + > + /* > + * Account for features which have been disabled by default since Xen 4.13, > + * so migrated-in VM's don't risk seeing features disappearing. > + */ > + policy->cpuid.basic.rdrand = host->cpuid.basic.rdrand; > + > + if ( hvm ) > + policy->cpuid.feat.mpx = host->cpuid.feat.mpx; > + > + /* Clamp maximum leaves to the ones supported on 4.12. */ > + policy->cpuid.basic.max_leaf = min(policy->cpuid.basic.max_leaf, 0xdu); > + policy->cpuid.feat.max_subleaf = 0; > + policy->cpuid.extd.max_leaf = min(policy->cpuid.extd.max_leaf, 0x1cu); > + > + out: > + xc_cpu_policy_destroy(host); > + return rc; > +} >
On Mon, May 03, 2021 at 01:09:41PM +0200, Jan Beulich wrote: > On 30.04.2021 17:52, Roger Pau Monne wrote: > > @@ -1086,3 +1075,42 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch, > > > > return rc; > > } > > + > > +int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy, > > + bool hvm) > > I'm concerned of the naming, and in particular the two very different > meanings of "compatible" for xc_cpu_policy_calc_compatible() and this > new one. I'm afraid I don't have a good suggestion though, short of > making the name even longer and inserting "backwards". Would xc_cpu_policy_make_compat_412 be acceptable? That's the more concise one I can think of. Thanks, Roger.
On 04.05.2021 17:34, Roger Pau Monné wrote: > On Mon, May 03, 2021 at 01:09:41PM +0200, Jan Beulich wrote: >> On 30.04.2021 17:52, Roger Pau Monne wrote: >>> @@ -1086,3 +1075,42 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch, >>> >>> return rc; >>> } >>> + >>> +int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy, >>> + bool hvm) >> >> I'm concerned of the naming, and in particular the two very different >> meanings of "compatible" for xc_cpu_policy_calc_compatible() and this >> new one. I'm afraid I don't have a good suggestion though, short of >> making the name even longer and inserting "backwards". > > Would xc_cpu_policy_make_compat_412 be acceptable? > > That's the more concise one I can think of. Hmm, maybe (perhaps with an underscore inserted between 4 and 12). Yet (sorry) a comment in the function says "since Xen 4.13", which includes changes that have happened later. Therefore it's not really clear to me whether the function really _only_ deals with the 4.12 / 4.13 boundary. Jan
On Wed, May 05, 2021 at 09:42:09AM +0200, Jan Beulich wrote: > On 04.05.2021 17:34, Roger Pau Monné wrote: > > On Mon, May 03, 2021 at 01:09:41PM +0200, Jan Beulich wrote: > >> On 30.04.2021 17:52, Roger Pau Monne wrote: > >>> @@ -1086,3 +1075,42 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch, > >>> > >>> return rc; > >>> } > >>> + > >>> +int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy, > >>> + bool hvm) > >> > >> I'm concerned of the naming, and in particular the two very different > >> meanings of "compatible" for xc_cpu_policy_calc_compatible() and this > >> new one. I'm afraid I don't have a good suggestion though, short of > >> making the name even longer and inserting "backwards". > > > > Would xc_cpu_policy_make_compat_412 be acceptable? > > > > That's the more concise one I can think of. > > Hmm, maybe (perhaps with an underscore inserted between 4 and 12). Yet > (sorry) a comment in the function says "since Xen 4.13", which includes > changes that have happened later. Therefore it's not really clear to me > whether the function really _only_ deals with the 4.12 / 4.13 boundary. It should deal with any changes in the default cpuid policy that happened after (and not including) Xen 4.12. So resulting policy is compatible with the behaviour that Xen 4.12 had. Any changes made in Xen 4.13 and further versions should be accounted for here. Thanks, Roger.
On 06.05.2021 12:23, Roger Pau Monné wrote: > On Wed, May 05, 2021 at 09:42:09AM +0200, Jan Beulich wrote: >> On 04.05.2021 17:34, Roger Pau Monné wrote: >>> On Mon, May 03, 2021 at 01:09:41PM +0200, Jan Beulich wrote: >>>> On 30.04.2021 17:52, Roger Pau Monne wrote: >>>>> @@ -1086,3 +1075,42 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch, >>>>> >>>>> return rc; >>>>> } >>>>> + >>>>> +int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy, >>>>> + bool hvm) >>>> >>>> I'm concerned of the naming, and in particular the two very different >>>> meanings of "compatible" for xc_cpu_policy_calc_compatible() and this >>>> new one. I'm afraid I don't have a good suggestion though, short of >>>> making the name even longer and inserting "backwards". >>> >>> Would xc_cpu_policy_make_compat_412 be acceptable? >>> >>> That's the more concise one I can think of. >> >> Hmm, maybe (perhaps with an underscore inserted between 4 and 12). Yet >> (sorry) a comment in the function says "since Xen 4.13", which includes >> changes that have happened later. Therefore it's not really clear to me >> whether the function really _only_ deals with the 4.12 / 4.13 boundary. > > It should deal with any changes in the default cpuid policy that > happened after (and not including) Xen 4.12. So resulting policy is > compatible with the behaviour that Xen 4.12 had. Any changes made in > Xen 4.13 and further versions should be accounted for here. I see. Jan
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h index c41d794683c..89a73fd6823 100644 --- a/tools/include/xenctrl.h +++ b/tools/include/xenctrl.h @@ -2627,6 +2627,10 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch, const xc_cpu_policy_t p2, xc_cpu_policy_t out); +/* Make a policy compatible with pre-4.14 Xen versions. */ +int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy, + bool hvm); + int xc_get_cpu_levelling_caps(xc_interface *xch, uint32_t *caps); int xc_get_cpu_featureset(xc_interface *xch, uint32_t index, uint32_t *nr_features, uint32_t *featureset); diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c index be2056469aa..855d252e067 100644 --- a/tools/libs/guest/xg_cpuid_x86.c +++ b/tools/libs/guest/xg_cpuid_x86.c @@ -446,6 +446,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore, unsigned int i, nr_leaves, nr_msrs; xen_cpuid_leaf_t *leaves = NULL; struct cpuid_policy *p = NULL; + struct xc_cpu_policy policy = { }; uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1; uint32_t host_featureset[FEATURESET_NR_ENTRIES] = {}; uint32_t len = ARRAY_SIZE(host_featureset); @@ -510,21 +511,9 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore, if ( restore ) { - /* - * Account for feature which have been disabled by default since Xen 4.13, - * so migrated-in VM's don't risk seeing features disappearing. - */ - p->basic.rdrand = test_bit(X86_FEATURE_RDRAND, host_featureset); - - if ( di.hvm ) - { - p->feat.mpx = test_bit(X86_FEATURE_MPX, host_featureset); - } - - /* Clamp maximum leaves to the ones supported on 4.12. */ - p->basic.max_leaf = min(p->basic.max_leaf, 0xdu); - p->feat.max_subleaf = 0; - p->extd.max_leaf = min(p->extd.max_leaf, 0x1cu); + policy.cpuid = *p; + xc_cpu_policy_make_compatible(xch, &policy, di.hvm); + *p = policy.cpuid; } if ( featureset ) @@ -1086,3 +1075,42 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch, return rc; } + +int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy, + bool hvm) +{ + xc_cpu_policy_t host; + int rc; + + host = xc_cpu_policy_init(); + if ( !host ) + { + errno = ENOMEM; + return -1; + } + + rc = xc_cpu_policy_get_system(xch, XEN_SYSCTL_cpu_policy_host, host); + if ( rc ) + { + ERROR("Failed to get host policy"); + goto out; + } + + /* + * Account for features which have been disabled by default since Xen 4.13, + * so migrated-in VM's don't risk seeing features disappearing. + */ + policy->cpuid.basic.rdrand = host->cpuid.basic.rdrand; + + if ( hvm ) + policy->cpuid.feat.mpx = host->cpuid.feat.mpx; + + /* Clamp maximum leaves to the ones supported on 4.12. */ + policy->cpuid.basic.max_leaf = min(policy->cpuid.basic.max_leaf, 0xdu); + policy->cpuid.feat.max_subleaf = 0; + policy->cpuid.extd.max_leaf = min(policy->cpuid.extd.max_leaf, 0x1cu); + + out: + xc_cpu_policy_destroy(host); + return rc; +}
Older Xen versions used to expose some CPUID bits which are no longer exposed by default. In order to keep a compatible behavior with guests migrated from versions of Xen that don't encode the CPUID data on the migration stream introduce a function that sets the same bits as older Xen versions. This is pulled out from xc_cpuid_apply_policy which already has this logic present. No functional change intended. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v1: - Move comments and explicitly mention pre-4.14 Xen. --- tools/include/xenctrl.h | 4 +++ tools/libs/guest/xg_cpuid_x86.c | 58 ++++++++++++++++++++++++--------- 2 files changed, 47 insertions(+), 15 deletions(-)