From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH 13/13] tools/libxl: Add 'vtsc_khz' option to
 set guest TSC rate
Date: Tue, 29 Sep 2015 11:07:17 +0100
Message-ID: <1443521237.16718.32.camel@citrix.com>
References: <1443424438-13404-1-git-send-email-haozhong.zhang@intel.com>
	<1443424438-13404-14-git-send-email-haozhong.zhang@intel.com>
	<20150928141925.GH13821@zion.uk.xensource.com>
	<20150929004023.GB3583@hzzhang-OptiPlex-9020.sh.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <20150929004023.GB3583@hzzhang-OptiPlex-9020.sh.intel.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Haozhong Zhang <haozhong.zhang@intel.com>, Wei Liu <wei.liu2@citrix.com>
Cc: Kevin Tian <kevin.tian@intel.com>, Keir Fraser <keir@xen.org>, Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Jun Nakajima <jun.nakajima@intel.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, xen-devel@lists.xen.org, Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>, Jan Beulich <jbeulich@suse.com>, Boris Ostrovsky <boris.ostrovsky@oracle.com>
List-Id: xen-devel@lists.xenproject.org

On Tue, 2015-09-29 at 08:40 +0800, Haozhong Zhang wrote:
> > > @@ -1462,6 +1462,28 @@ static void parse_config_data(const char *config_source,
> > >          }
> > >      }
> > >  
> > > +    /* "vtsc_khz" option works only if "tsc_mode" option is
> > > +     * "default". In this case, if "vtsc_khz" option is set to 0, we
> > > +     * will reset it to the host TSC rate. In all other cases, we just
> > > +     * ignore any given value and always set it to 0.
> > > +     */
> > > +    if (!xlu_cfg_get_long(config, "vtsc_khz", &l, 0))
> > > +        b_info->vtsc_khz = l;
> > > +    if (b_info->tsc_mode == LIBXL_TSC_MODE_DEFAULT) {
> > > +        if (b_info->vtsc_khz == 0) {
> > > +            libxl_physinfo physinfo;
> > > +            if (!libxl_get_physinfo(ctx, &physinfo))
> > > +                b_info->vtsc_khz = physinfo.cpu_khz;
> > > +            else
> > > +                fprintf(stderr, "WARNING: cannot get host TSC rate.\n");
> > > +        }
> > 
> > And this hunk (the decision making bit) should be in libxl, not xl.
> > 
> > Consider there are other toolstack that uses libxl, say libvirt.
> > 
> 
> Good to know this.
> 
> I'm going to move it to libxl__arch_domain_create() where
> b_info->vtsc_khz is used.

libxl__domain_build_info_setdefault would be more usual, I think.

Rather than calling get_physinfo in order to give vtsc_khz a specific value
instead of zero can we not leave it as zero and just not call
 xc_domain_set_tsc_info() in that case and let the hypervisor default to
using the host rate?

Then the check in libxl just becomes "is vtsc_khz non-zero and is tsc_mode
not DEFAULT".

Don't forget to switch from fprintf to the proper log macros.

Ian.