| Message ID | 20210323095849.37858-5-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libs/guest: new CPUID/MSR interface | expand |
On 23.03.2021 10:58, Roger Pau Monne wrote: > Such helper is based on the existing functions to fetch a CPUID and > MSR policies, but uses the xc_cpu_policy_t type to return the data to > the caller. > > Note some helper functions are introduced, those are split from > xc_cpu_policy_get_system because they will be used by other functions > also. > > No user of the interface introduced on the patch. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> with just one minor remark below: > --- a/tools/include/xenctrl.h > +++ b/tools/include/xenctrl.h > @@ -2596,6 +2596,10 @@ typedef struct cpu_policy *xc_cpu_policy_t; > xc_cpu_policy_t xc_cpu_policy_init(void); > void xc_cpu_policy_destroy(xc_cpu_policy_t policy); > > +/* Retrieve a system policy, or get/set a domains policy. */ > +int xc_cpu_policy_get_system(xc_interface *xch, unsigned int idx, > + xc_cpu_policy_t policy); > + > 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 ade5281c178..3710fb63839 100644 > --- a/tools/libs/guest/xg_cpuid_x86.c > +++ b/tools/libs/guest/xg_cpuid_x86.c > @@ -687,3 +687,93 @@ void xc_cpu_policy_destroy(xc_cpu_policy_t policy) > free(policy->msr); > free(policy); > } > + > +static int allocate_buffers(xc_interface *xch, > + unsigned int *nr_leaves, xen_cpuid_leaf_t **leaves, > + unsigned int *nr_msrs, xen_msr_entry_t **msrs) > +{ > + int rc; > + > + *leaves = NULL; > + *msrs = NULL; > + > + rc = xc_cpu_policy_get_size(xch, nr_leaves, nr_msrs); > + if ( rc ) > + { > + PERROR("Failed to obtain policy info size"); > + return -errno; > + } > + > + *leaves = calloc(*nr_leaves, sizeof(**leaves)); > + *msrs = calloc(*nr_msrs, sizeof(**msrs)); > + if ( !*leaves || !*msrs ) > + { > + PERROR("Failed to allocate resources"); > + free(*leaves); > + free(*msrs); > + return -ENOMEM; > + } > + > + return 0; > +} > + > +static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t policy, > + unsigned int nr_leaves, > + const xen_cpuid_leaf_t *leaves, > + unsigned int nr_msrs, const xen_msr_entry_t *msrs) > +{ > + uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1; > + int rc; > + > + rc = x86_cpuid_copy_from_buffer(policy->cpuid, leaves, nr_leaves, > + &err_leaf, &err_subleaf); > + if ( rc ) > + { > + ERROR("Failed to deserialise CPUID (err leaf %#x, subleaf %#x) (%d = %s)", > + err_leaf, err_subleaf, -rc, strerror(-rc)); > + return rc; > + } > + > + rc = x86_msr_copy_from_buffer(policy->msr, msrs, nr_msrs, &err_msr); > + if ( rc ) > + { > + ERROR("Failed to deserialise MSR (err MSR %#x) (%d = %s)", > + err_msr, -rc, strerror(-rc)); > + return rc; > + } > + > + return 0; > +} > + > +int xc_cpu_policy_get_system(xc_interface *xch, unsigned int idx, > + xc_cpu_policy_t policy) > +{ > + unsigned int nr_leaves, nr_msrs; > + xen_cpuid_leaf_t *leaves = NULL; > + xen_msr_entry_t *msrs = NULL; > + int rc; > + > + rc = allocate_buffers(xch, &nr_leaves, &leaves, &nr_msrs, &msrs); > + if ( rc ) > + { > + errno = -rc; > + return -1; > + } > + > + rc = xc_get_system_cpu_policy(xch, idx, &nr_leaves, leaves, &nr_msrs, msrs); > + if ( rc ) > + { > + PERROR("Failed to obtain %u policy", idx); > + rc = -1; > + goto out; > + } > + > + rc = deserialize_policy(xch, policy, nr_leaves, leaves, nr_msrs, msrs); > + if ( rc ) > + errno = -rc; Perhaps also set rc to -1 here for consistency? Jan
On 23/03/2021 09:58, Roger Pau Monne wrote: > Such helper is based on the existing functions to fetch a CPUID and > MSR policies, but uses the xc_cpu_policy_t type to return the data to > the caller. > > Note some helper functions are introduced, those are split from > xc_cpu_policy_get_system because they will be used by other functions > also. > > No user of the interface introduced on the patch. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > tools/include/xenctrl.h | 4 ++ > tools/libs/guest/xg_cpuid_x86.c | 90 +++++++++++++++++++++++++++++++++ > 2 files changed, 94 insertions(+) > > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h > index ffb3024bfeb..fc8e4b28781 100644 > --- a/tools/include/xenctrl.h > +++ b/tools/include/xenctrl.h > @@ -2596,6 +2596,10 @@ typedef struct cpu_policy *xc_cpu_policy_t; > xc_cpu_policy_t xc_cpu_policy_init(void); > void xc_cpu_policy_destroy(xc_cpu_policy_t policy); > > +/* Retrieve a system policy, or get/set a domains policy. */ > +int xc_cpu_policy_get_system(xc_interface *xch, unsigned int idx, > + xc_cpu_policy_t policy); I'd recommend "policy_idx" as the parameter name. > +static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t policy, > + unsigned int nr_leaves, > + const xen_cpuid_leaf_t *leaves, > + unsigned int nr_msrs, const xen_msr_entry_t *msrs) > +{ > + uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1; > + int rc; > + > + rc = x86_cpuid_copy_from_buffer(policy->cpuid, leaves, nr_leaves, > + &err_leaf, &err_subleaf); > + if ( rc ) > + { > + ERROR("Failed to deserialise CPUID (err leaf %#x, subleaf %#x) (%d = %s)", > + err_leaf, err_subleaf, -rc, strerror(-rc)); > + return rc; > + } > + > + rc = x86_msr_copy_from_buffer(policy->msr, msrs, nr_msrs, &err_msr); > + if ( rc ) > + { > + ERROR("Failed to deserialise MSR (err MSR %#x) (%d = %s)", > + err_msr, -rc, strerror(-rc)); We possibly want to split out helpers to render the error information. For MSRs, it ought to be something like if ( msr == -1 ) // general -ENOMEM etc else // MSR-specific error. render index and value. I think we can probably even depend on MSR-specific errors always being -EINVAL. ~Andrew
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h index ffb3024bfeb..fc8e4b28781 100644 --- a/tools/include/xenctrl.h +++ b/tools/include/xenctrl.h @@ -2596,6 +2596,10 @@ typedef struct cpu_policy *xc_cpu_policy_t; xc_cpu_policy_t xc_cpu_policy_init(void); void xc_cpu_policy_destroy(xc_cpu_policy_t policy); +/* Retrieve a system policy, or get/set a domains policy. */ +int xc_cpu_policy_get_system(xc_interface *xch, unsigned int idx, + xc_cpu_policy_t policy); + 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 ade5281c178..3710fb63839 100644 --- a/tools/libs/guest/xg_cpuid_x86.c +++ b/tools/libs/guest/xg_cpuid_x86.c @@ -687,3 +687,93 @@ void xc_cpu_policy_destroy(xc_cpu_policy_t policy) free(policy->msr); free(policy); } + +static int allocate_buffers(xc_interface *xch, + unsigned int *nr_leaves, xen_cpuid_leaf_t **leaves, + unsigned int *nr_msrs, xen_msr_entry_t **msrs) +{ + int rc; + + *leaves = NULL; + *msrs = NULL; + + rc = xc_cpu_policy_get_size(xch, nr_leaves, nr_msrs); + if ( rc ) + { + PERROR("Failed to obtain policy info size"); + return -errno; + } + + *leaves = calloc(*nr_leaves, sizeof(**leaves)); + *msrs = calloc(*nr_msrs, sizeof(**msrs)); + if ( !*leaves || !*msrs ) + { + PERROR("Failed to allocate resources"); + free(*leaves); + free(*msrs); + return -ENOMEM; + } + + return 0; +} + +static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t policy, + unsigned int nr_leaves, + const xen_cpuid_leaf_t *leaves, + unsigned int nr_msrs, const xen_msr_entry_t *msrs) +{ + uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1; + int rc; + + rc = x86_cpuid_copy_from_buffer(policy->cpuid, leaves, nr_leaves, + &err_leaf, &err_subleaf); + if ( rc ) + { + ERROR("Failed to deserialise CPUID (err leaf %#x, subleaf %#x) (%d = %s)", + err_leaf, err_subleaf, -rc, strerror(-rc)); + return rc; + } + + rc = x86_msr_copy_from_buffer(policy->msr, msrs, nr_msrs, &err_msr); + if ( rc ) + { + ERROR("Failed to deserialise MSR (err MSR %#x) (%d = %s)", + err_msr, -rc, strerror(-rc)); + return rc; + } + + return 0; +} + +int xc_cpu_policy_get_system(xc_interface *xch, unsigned int idx, + xc_cpu_policy_t policy) +{ + unsigned int nr_leaves, nr_msrs; + xen_cpuid_leaf_t *leaves = NULL; + xen_msr_entry_t *msrs = NULL; + int rc; + + rc = allocate_buffers(xch, &nr_leaves, &leaves, &nr_msrs, &msrs); + if ( rc ) + { + errno = -rc; + return -1; + } + + rc = xc_get_system_cpu_policy(xch, idx, &nr_leaves, leaves, &nr_msrs, msrs); + if ( rc ) + { + PERROR("Failed to obtain %u policy", idx); + rc = -1; + goto out; + } + + rc = deserialize_policy(xch, policy, nr_leaves, leaves, nr_msrs, msrs); + if ( rc ) + errno = -rc; + + out: + free(leaves); + free(msrs); + return rc; +}
Such helper is based on the existing functions to fetch a CPUID and MSR policies, but uses the xc_cpu_policy_t type to return the data to the caller. Note some helper functions are introduced, those are split from xc_cpu_policy_get_system because they will be used by other functions also. No user of the interface introduced on the patch. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- tools/include/xenctrl.h | 4 ++ tools/libs/guest/xg_cpuid_x86.c | 90 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+)