[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 03/21] libs/guest: introduce xc_cpu_policy_t


  • To: Roger Pau Monne <roger.pau@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Wed, 31 Mar 2021 21:10:13 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=29YlNYnM2DdZ+kxuHHV6vjXlNq+Y15PLPSQmAkLDxyU=; b=bSPXZfILyXkj1zcpWDintGqsplLd6WiELjy21OYPbTfPZUSp8exKZa64XH3tZQljA5lq35o6WU/1aAfp9/gasWPBEyqPCI13HwHuDm3rhSUd0Ed5je5zUHi/fJWRyj2aNv0M13BXXUT/u74kPHjL5VixdrlKLW+PSM8XSddYr7blIAtUsfm4GIw00OmW5pClE5UJiPz35Q/vligntd+vky5GymlQqqbHo/5WV3g1FNw2pCxoSN3WJlx9u9QotJxJtq27S6QKu20f78Ry0QryxfSoLqs4z4eQ0jjk/BEnI9pt3zH033J6jPxDTzauYgEo1aMagdmVpDjVIFPoA7xc+w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QiPb9oxl2lUR/VJgyMiBEk5Jjl7p7zYlDLVpMHe8rAVLaooIpqFlDIbssL1+C4XyKmSmXd/cQDaATVXcogSxInHv/JZTnhldDskS3khwtg8HDEyRfAnJPWC8W6NdOPw46mUh7nYq2ooHIfhMz0WxrqTRBhTNAf8zlNBZgah/VLDtaxQ3CSOlg0Ok1I0LAuj/onEYiyWPj/sqwez/27TTEiE4gf2rBY8edog3bR/YokRK5CzI8CrsEm4pTkDWFBPX+li2t/lyqa3uD1vS0E7Vgq33VLYzCaa3t6pAlGHFIbtQEoVOhub/Rfwb0btJmyyUC6++R6njiknhATpgPtkuMg==
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Wed, 31 Mar 2021 20:10:40 +0000
  • Ironport-hdrordr: A9a23:HXv3P6hHyfTw/MerL5dyPtqS73BQXwh13DAbvn1ZSRFFG/Gwv9 yynfgdyB//gCsQXnZlotybJKycWxrnmKJdy4N5B9efdSPhv3alK5wn0Jv6z1TbaknD38N+9Y MlSahxD9XsEUN35PyR3CCUG8stqePpzImGnuHbpk0CcShPS4VNqzh0ERyaFEoefngiObMcGI CH7sRK4xqMEE5nDfiTPXUOU+jdq9CjrvuPDSIuPBI79BKIyQqh9b+SKXOl9y0DWDBCy6pKyx mmryXF4MyY0s2T+1vn+EL4q79Xn9bgzdUrPr3wtuElbg/CpyztSIBoW7iptC04rue1+D8R4a XxiiZlBetfwTf8eXy0vAvM1mDboUkTwk6n83C0qz/CptH0Xz0zAcYpv/MmTjLpr3AOkfs59Y Aj5RP/i7NnSSnusQ642v3zEzZtrUawqWpKq59ps1VvFbEwRZUUkZYS5ypuYfE9NRO/0q8LOs 90AvrR4f5HGGnqFUzxjy1UzNugUm9bJGb+fmEy/sic0z1hlHtk1UcvxMsGgnca9J4mIqM0n9 j5Dg==
  • Ironport-sdr: ZlJ77bSq1IiLsy8msoCCjKzWWs8hQwUU9oFk247V2EVEWoa1fXWh3bnptF3jUeJ4lTh/RKoinP Ux9cfPwEhJV8c8p2JMT38kL26QMEqaxD2w2qM9IKIiknaRcDsYhiVWT+AwsGvnupstpEKIjm1Z 2cOMnO5u0qMLNR4yS6su/Im0JOQE56EeGjjD4Jsh5Xd9YIhmkpmTFV3xqBYH76DjLpjhedUDmP aMmXsK64dMEbqVhdcyV+2FHrG/R9pqSKEmEuC9uDKDBi11FwNJsHcfmU0pww+VUBgtm4m+fzoS mIM=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.pau@xxxxxxxxxx>
> ---
>  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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.