| Message ID | 20210323095849.37858-10-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libs/guest: new CPUID/MSR interface | expand |
On 23/03/2021 09:58, Roger Pau Monne wrote: > diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c > index 48351f1c4c6..a1e1bf10d5c 100644 > --- a/tools/libs/guest/xg_cpuid_x86.c > +++ b/tools/libs/guest/xg_cpuid_x86.c > @@ -883,3 +883,45 @@ int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t p, > errno = 0; > return 0; > } > + > +int xc_cpu_policy_get_cpuid(xc_interface *xch, const xc_cpu_policy_t policy, > + uint32_t leaf, uint32_t subleaf, > + xen_cpuid_leaf_t *out) > +{ > + unsigned int nr_leaves, nr_msrs, i; > + xen_cpuid_leaf_t *leaves; > + int rc = xc_cpu_policy_get_size(xch, &nr_leaves, &nr_msrs); > + > + if ( rc ) > + { > + PERROR("Failed to obtain policy info size"); > + return -1; > + } > + > + leaves = calloc(nr_leaves, sizeof(*leaves)); > + if ( !leaves ) > + { > + PERROR("Failed to allocate resources"); > + errno = ENOMEM; > + return -1; > + } > + > + rc = xc_cpu_policy_serialise(xch, policy, leaves, &nr_leaves, NULL, 0); > + if ( rc ) > + goto out; > + > + for ( i = 0; i < nr_leaves; i++ ) > + if ( leaves[i].leaf == leaf && leaves[i].subleaf == subleaf ) > + { > + *out = leaves[i]; > + goto out; > + } Please adapt find_leaf(), probably by dropping xc_xend_cpuid and passing in leaf/subleaf parameters. Serialised leaves are sorted and there are plenty of them, so a log search is better. How frequent is this call going to be for the same policy? With the arrays embedded in a policy, they're still around, and serialise is an expensive operation. I wonder if it makes sense to try and keep both forms in sync, so we can avoid redundant calls like this? ~Andrew
On Thu, Apr 01, 2021 at 03:47:20PM +0100, Andrew Cooper wrote: > On 23/03/2021 09:58, Roger Pau Monne wrote: > > diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c > > index 48351f1c4c6..a1e1bf10d5c 100644 > > --- a/tools/libs/guest/xg_cpuid_x86.c > > +++ b/tools/libs/guest/xg_cpuid_x86.c > > @@ -883,3 +883,45 @@ int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t p, > > errno = 0; > > return 0; > > } > > + > > +int xc_cpu_policy_get_cpuid(xc_interface *xch, const xc_cpu_policy_t policy, > > + uint32_t leaf, uint32_t subleaf, > > + xen_cpuid_leaf_t *out) > > +{ > > + unsigned int nr_leaves, nr_msrs, i; > > + xen_cpuid_leaf_t *leaves; > > + int rc = xc_cpu_policy_get_size(xch, &nr_leaves, &nr_msrs); > > + > > + if ( rc ) > > + { > > + PERROR("Failed to obtain policy info size"); > > + return -1; > > + } > > + > > + leaves = calloc(nr_leaves, sizeof(*leaves)); > > + if ( !leaves ) > > + { > > + PERROR("Failed to allocate resources"); > > + errno = ENOMEM; > > + return -1; > > + } > > + > > + rc = xc_cpu_policy_serialise(xch, policy, leaves, &nr_leaves, NULL, 0); > > + if ( rc ) > > + goto out; > > + > > + for ( i = 0; i < nr_leaves; i++ ) > > + if ( leaves[i].leaf == leaf && leaves[i].subleaf == subleaf ) > > + { > > + *out = leaves[i]; > > + goto out; > > + } > > Please adapt find_leaf(), probably by dropping xc_xend_cpuid and passing > in leaf/subleaf parameters. > > Serialised leaves are sorted and there are plenty of them, so a log > search is better. > > How frequent is this call going to be for the same policy? With the > arrays embedded in a policy, they're still around, and serialise is an > expensive operation. > > I wonder if it makes sense to try and keep both forms in sync, so we can > avoid redundant calls like this? I think we would then need to call xc_cpu_policy_serialise whenever we update the CPUID policy in order to re-generate the full serialized version, as I expect changes to a specific leaf can affect others leaves, and thus the helpers should not try to update the serialized version manually. Hence I'm not sure it's worth to try to keep both versions in sync, as it seems easier to load the policy, modify the policy object and then serialize back only when needed to upload to Xen or to export. Thanks, Roger.
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h index d82c99b2f0d..983e4c11d93 100644 --- a/tools/include/xenctrl.h +++ b/tools/include/xenctrl.h @@ -2608,6 +2608,9 @@ int xc_cpu_policy_set_domain(xc_interface *xch, uint32_t domid, int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t policy, xen_cpuid_leaf_t *leaves, uint32_t *nr_leaves, xen_msr_entry_t *msrs, uint32_t *nr_msrs); +int xc_cpu_policy_get_cpuid(xc_interface *xch, const xc_cpu_policy_t policy, + uint32_t leaf, uint32_t subleaf, + xen_cpuid_leaf_t *out); int xc_get_cpu_levelling_caps(xc_interface *xch, uint32_t *caps); int xc_get_cpu_featureset(xc_interface *xch, uint32_t index, diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c index 48351f1c4c6..a1e1bf10d5c 100644 --- a/tools/libs/guest/xg_cpuid_x86.c +++ b/tools/libs/guest/xg_cpuid_x86.c @@ -883,3 +883,45 @@ int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t p, errno = 0; return 0; } + +int xc_cpu_policy_get_cpuid(xc_interface *xch, const xc_cpu_policy_t policy, + uint32_t leaf, uint32_t subleaf, + xen_cpuid_leaf_t *out) +{ + unsigned int nr_leaves, nr_msrs, i; + xen_cpuid_leaf_t *leaves; + int rc = xc_cpu_policy_get_size(xch, &nr_leaves, &nr_msrs); + + if ( rc ) + { + PERROR("Failed to obtain policy info size"); + return -1; + } + + leaves = calloc(nr_leaves, sizeof(*leaves)); + if ( !leaves ) + { + PERROR("Failed to allocate resources"); + errno = ENOMEM; + return -1; + } + + rc = xc_cpu_policy_serialise(xch, policy, leaves, &nr_leaves, NULL, 0); + if ( rc ) + goto out; + + for ( i = 0; i < nr_leaves; i++ ) + if ( leaves[i].leaf == leaf && leaves[i].subleaf == subleaf ) + { + *out = leaves[i]; + goto out; + } + + /* Unable to find a matching leaf. */ + errno = ENOENT; + rc = -1; + + out: + free(leaves); + return rc; +}
Introduce an interface that returns a specific leaf/subleaf from a cpu policy in xen_cpuid_leaf_t format. This is useful to callers can peek data from the opaque xc_cpu_policy_t type. No caller of the interface introduced on this patch. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- tools/include/xenctrl.h | 3 +++ tools/libs/guest/xg_cpuid_x86.c | 42 +++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+)