From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Zhang, Haozhong" <haozhong.zhang@intel.com>
Subject: Re: [RESEND PATCH v4 09/10] vmx: Add VMX RDTSC(P)
	scaling support
Date: Tue, 16 Feb 2016 15:59:07 +0800
Message-ID: <20160216075907.GA6519@hz-desktop.sh.intel.com>
References: <1453067939-9121-1-git-send-email-haozhong.zhang@intel.com>
	<1453067939-9121-10-git-send-email-haozhong.zhang@intel.com>
	<20160119025516.GA7885@hz-desktop.sh.intel.com>
	<56B4BA8902000078000CF19F@prv-mh.provo.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <56B4BA8902000078000CF19F@prv-mh.provo.novell.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: Jan Beulich <JBeulich@suse.com>
Cc: "Tian, Kevin" <kevin.tian@intel.com>, Keir Fraser <keir@xen.org>, Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>, Andrew Cooper <andrew.cooper3@citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>, "Nakajima, Jun" <jun.nakajima@intel.com>, Boris Ostrovsky <boris.ostrovsky@oracle.com>
List-Id: xen-devel@lists.xenproject.org

On 02/05/16 22:06, Jan Beulich wrote:
> >>> On 19.01.16 at 03:55, <haozhong.zhang@intel.com> wrote:
> > @@ -2107,6 +2115,14 @@ const struct hvm_function_table * __init start_vmx(void)
> >           && cpu_has_vmx_secondary_exec_control )
> >          vmx_function_table.pvh_supported = 1;
> >  
> > +    if ( cpu_has_vmx_tsc_scaling )
> > +    {
> > +        vmx_function_table.default_tsc_scaling_ratio = VMX_TSC_MULTIPLIER_DEFAULT;
> > +        vmx_function_table.max_tsc_scaling_ratio = VMX_TSC_MULTIPLIER_MAX;
> > +        vmx_function_table.tsc_scaling_ratio_frac_bits = 48;
> > +        vmx_function_table.setup_tsc_scaling = vmx_setup_tsc_scaling;
> > +    }
> 
> Same comments here as on the earlier patch - it indeed looks as if
> tsc_scaling_ratio_frac_bits would be the ideal field to dynamically
> initialize, as it being zero will not yield any bad behavior afaict.
>

Yes, I'll make changes similar to my reply under patch 4.

> Also please consider making all fields together a sub-structure
> of struct hvm_function_table, such that the above would become
> 
>         vmx_function_table.tsc_scaling.default_ratio = VMX_TSC_MULTIPLIER_DEFAULT;
>         vmx_function_table.tsc_scaling.max_ratio = VMX_TSC_MULTIPLIER_MAX;
>         vmx_function_table.tsc_scaling.ratio_frac_bits = 48;
>         vmx_function_table.tsc_scaling.setup = vmx_setup_tsc_scaling;
> 
> keeping everything nicely together.
>

OK, I'll put them in a sub-structure by the earlier patch that
introduces those fields.

> > @@ -258,6 +259,9 @@ extern u64 vmx_ept_vpid_cap;
> >  #define VMX_MISC_CR3_TARGET                     0x01ff0000
> >  #define VMX_MISC_VMWRITE_ALL                    0x20000000
> >  
> > +#define VMX_TSC_MULTIPLIER_DEFAULT              0x0001000000000000ULL
> 
> Considering this and the respective SVM value - do we really
> need the separate field in struct hvm_function_table? Both are
> 1ULL << tsc_scaling.ratio_frac_bits after all.
>

I'll remove VMX_TSC_MULTIPLIER_DEFAULT and DEFAULT_TSC_RATIO (for SVM),
and use ratio_frac_bits to initialize default_ratio.

Thanks,
Haozhong