From: Wei Liu <wei.liu2@citrix.com>
Subject: Re: [PATCH] tools/libxc: Improve efficiency of
 xc_cpuid_apply_policy()
Date: Tue, 6 Oct 2015 10:14:57 +0100
Message-ID: <20151006091457.GP29124@zion.uk.xensource.com>
References: <1444050737-8437-1-git-send-email-andrew.cooper3@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
In-Reply-To: <1444050737-8437-1-git-send-email-andrew.cooper3@citrix.com>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, Xen-devel <xen-devel@lists.xen.org>

On Mon, Oct 05, 2015 at 02:12:17PM +0100, Andrew Cooper wrote:
> Having the internals of xc_cpuid_policy() make hypercalls to collect domain
> information causes xc_cpuid_apply_policy() to be very inefficient.
> 
> Re-order operations to collect all information at once at the outermost layer,
> and pass a structure in to all cpuid policy generation functions.
> 
> This removes several hypercalls (4 from HVM, 3 from PV) for each of the
> up-to 108 leaves processed.
> 
> No change in the eventual policy provided, although all the information
> gathering how has (or has correct) error checking.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
>  tools/libxc/xc_cpuid_x86.c |  245 +++++++++++++++++++++++++-------------------
>  1 file changed, 141 insertions(+), 104 deletions(-)
> 
> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
> index e146a3e..715f30c 100644
> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -33,6 +33,27 @@
>  #define DEF_MAX_INTELEXT  0x80000008u
>  #define DEF_MAX_AMDEXT    0x8000001cu
>  
> +struct cpuid_domain_info
> +{
> +    enum
> +    {
> +        VENDOR_UNKNOWN,
> +        VENDOR_INTEL,
> +        VENDOR_AMD,
> +    } vendor;
> +
> +    bool hvm;
> +    bool pvh;
> +    uint64_t xfeature_mask;
> +
> +    /* PV-only information. */
> +    bool pv64;
> +
> +    /* HVM-only infomation. */
> +    bool pae;
> +    bool nestedhvm;

I would have used union for these fields. But I don't want to bikeshed
too much on this. Definitely not a blocker for this patch. :-)