On 23/03/2021 09:58, Roger Pau Monne wrote:
> Introduce an opaque type that is used to store the CPUID and MSRs
> policies of a domain. Such type uses the existing cpu_policy structure
> to store the data, but doesn't expose the type to the users of the
> xenguest library.
>
> Introduce an allocation (init) and freeing function (destroy) to
> manage the type.
>
> Note the type is not yet used anywhere.
>
> Signed-off-by: Roger Pau Monné <roger....@citrix.com>
> ---
> tools/include/xenctrl.h | 6 ++++++
> tools/libs/guest/xg_cpuid_x86.c | 28 ++++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> index e91ff92b9b1..ffb3024bfeb 100644
> --- a/tools/include/xenctrl.h
> +++ b/tools/include/xenctrl.h
> @@ -2590,6 +2590,12 @@ int xc_psr_get_domain_data(xc_interface *xch, uint32_t
> domid,
> int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
> xc_psr_feat_type type, xc_psr_hw_info *hw_info);
>
> +typedef struct cpu_policy *xc_cpu_policy_t;
> +
> +/* Create and free a xc_cpu_policy object. */
> +xc_cpu_policy_t xc_cpu_policy_init(void);
> +void xc_cpu_policy_destroy(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 9846f81e1f1..ade5281c178 100644
> --- a/tools/libs/guest/xg_cpuid_x86.c
> +++ b/tools/libs/guest/xg_cpuid_x86.c
> @@ -659,3 +659,31 @@ out:
>
> return rc;
> }
> +
> +xc_cpu_policy_t xc_cpu_policy_init(void)
> +{
> + xc_cpu_policy_t policy = calloc(1, sizeof(*policy));
> +
> + if ( !policy )
> + return NULL;
> +
> + policy->cpuid = calloc(1, sizeof(*policy->cpuid));
> + policy->msr = calloc(1, sizeof(*policy->msr));
> + if ( !policy->cpuid || !policy->msr )
> + {
> + xc_cpu_policy_destroy(policy);
> + return NULL;
> + }
> +
> + return policy;
> +}
> +
> +void xc_cpu_policy_destroy(xc_cpu_policy_t policy)
> +{
> + if ( !policy )
> + return;
> +
> + free(policy->cpuid);
> + free(policy->msr);
> + free(policy);
> +}
Looking at the series as a whole, we have a fair quantity of complexity
from short-lived dynamic allocations.
I suspect that the code would be rather better if we had
struct xc_cpu_policy {
struct cpuid_policy cpuid;
struct msr_policy msr;
xen_cpuid_leaf_t leaves[CPUID_MAX_SERIALISED_LEAVES];
xen_msr_entry_t msrs[MSR_MAX_SERIALISED_ENTRIES];
/* Names perhaps subject to improvement */
};
and just made one memory allocation.
This is userspace after all, and we're taking about <4k at the moment.
All operations with Xen need to bounce through the leaves/msrs encoding
(so we're using the space a minimum of twice for any logical operation
at the higher level), and several userspace-only operations use them too.
~Andrew
