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

Re: [Xen-devel] [PATCH v5 1/6] x86/hvm: Collect information of TSC scaling ratio



On 02/24/2016 08:46 AM, Haozhong Zhang wrote:
Sorry, forgot sending the last reply to Boris.

On 02/24/16 14:00, Haozhong Zhang wrote:
On 02/23/16 08:37, Jan Beulich wrote:
On 23.02.16 at 15:16, <boris.ostrovsky@xxxxxxxxxx> wrote:
On 02/23/2016 09:10 AM, Jan Beulich wrote:
On 23.02.16 at 15:00, <boris.ostrovsky@xxxxxxxxxx> wrote:
On 02/22/2016 09:04 PM, Haozhong Zhang wrote:

+ if ( cpu_has_tsc_ratio )
+        svm_function_table.tsc_scaling.ratio_frac_bits = 32;
+
+#define hvm_tsc_scaling_supported \
+    (!!hvm_funcs.tsc_scaling.ratio_frac_bits)
+
What is the difference (in usage) between cpu_has_tsc_ratio and
hvm_tsc_scaling_supported? Isn't the first imply the second (and if yes
then what's the reason for having the latter)?
Iiuc cpu_has_tsc_ratio is AMD/SVM specific, while
hvm_tsc_scaling_supported is meant to be vendor independent.
Yes, it's to be vendor independent. Earlier versions of this patch
series set a field tsc_scaling_supported in hvm_function_table if
cpu_has_vmx_tsc_scaling or cpu_has_tsc_ratio. Jan suggested we could get
the same information if some of other fields are initialized
conditionally, and no extra field (tsc_scaling_supported) would be
needed any more.

Ah, OK. Can we then

#define hvm_tsc_scaling_supported (cpu_has_vmx_tsc_scaling ||
cpu_has_tsc_ratio)
Why would we? The above is doing precisely (but implicitly) that,
just with only one memory access instead of two.

Boris, does the current one look fine for you, as it does the same thing
as your suggested one?

Yes, this is fine. IMO using cpu_has_* would be slightly more logical but as Jan pointed it is also potentially more costly.

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>

-boris


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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