Re: [PATCH v4: kvm 1/4] Code motion. Separate timer intialization into an indepedent function.

2009-09-30 Thread Avi Kivity

On 09/29/2009 11:38 PM, Zachary Amsden wrote:

Signed-off-by: Zachary Amsdenzams...@redhat.com
   



Looks good.

Is anything preventing us from unifying the constant_tsc and !same 
paths?  We could just do a quick check in the notifier, see the tsc 
frequency hasn't changed, and return.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4: kvm 1/4] Code motion. Separate timer intialization into an indepedent function.

2009-09-30 Thread Zachary Amsden

On 09/29/2009 10:45 PM, Avi Kivity wrote:

On 09/29/2009 11:38 PM, Zachary Amsden wrote:

Signed-off-by: Zachary Amsdenzams...@redhat.com



Looks good.

Is anything preventing us from unifying the constant_tsc and !same 
paths?  We could just do a quick check in the notifier, see the tsc 
frequency hasn't changed, and return.


Actually, yes.  On constant_tsc processors, the processor frequency may 
still change, however the TSC frequency does not change with it.


I actually have both of these kinds of processors (freq changes with 
constant TSC and freq changes with variable TSC) so I was able to test 
both of these cases.


Zach
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4: kvm 1/4] Code motion. Separate timer intialization into an indepedent function.

2009-09-30 Thread Avi Kivity

On 09/30/2009 05:51 PM, Zachary Amsden wrote:
Is anything preventing us from unifying the constant_tsc and !same 
paths?  We could just do a quick check in the notifier, see the tsc 
frequency hasn't changed, and return.



Actually, yes.  On constant_tsc processors, the processor frequency 
may still change, however the TSC frequency does not change with it.


I actually have both of these kinds of processors (freq changes with 
constant TSC and freq changes with variable TSC) so I was able to test 
both of these cases.


If the API allows us to query the tsc frequency, it would simply return 
the same values in all cases, which we'd ignore.


--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4: kvm 1/4] Code motion. Separate timer intialization into an indepedent function.

2009-09-30 Thread Zachary Amsden

On 09/30/2009 05:56 AM, Avi Kivity wrote:

On 09/30/2009 05:51 PM, Zachary Amsden wrote:

If the API allows us to query the tsc frequency, it would simply 
return the same values in all cases, which we'd ignore.


The API only allows querying the processor frequency.  In the 
constant_tsc case, the highest processor frequency is likely going to be 
the actual TSC frequency, but I don't think it's a guarantee; 
theoretically, it could be faster on normal hardware ... or slower on 
overclocked hardware with an externally clocked TSC.


Zach
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4: kvm 1/4] Code motion. Separate timer intialization into an indepedent function.

2009-09-30 Thread Zachary Amsden

On 09/30/2009 06:11 AM, Avi Kivity wrote:

On 09/30/2009 06:06 PM, Zachary Amsden wrote:

On 09/30/2009 05:56 AM, Avi Kivity wrote:

On 09/30/2009 05:51 PM, Zachary Amsden wrote:

If the API allows us to query the tsc frequency, it would simply 
return the same values in all cases, which we'd ignore.


The API only allows querying the processor frequency.  In the 
constant_tsc case, the highest processor frequency is likely going to 
be the actual TSC frequency, but I don't think it's a guarantee; 
theoretically, it could be faster on normal hardware ... or slower on 
overclocked hardware with an externally clocked TSC.


Well we could add a new API then (or a new tscfreq notifier).  Those 
conditionals don't belong in client code.


It's possible... but it's also possible to run without cpufreq enabled, 
which won't work properly unless the cpufreq code is aware of the 
measured tsc_khz...  this could be a little ugly architecture wise given 
the big melting pot of generic code and vendor / arch specific code here.


Since we're already very hardware dependent and one of the few clients 
who care, it seems okay to leave it as is for now.


Zach
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4: kvm 1/4] Code motion. Separate timer intialization into an indepedent function.

2009-09-29 Thread Zachary Amsden
Signed-off-by: Zachary Amsden zams...@redhat.com
---
 arch/x86/kvm/x86.c |   23 +++
 1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fedac9d..15d2ace 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3116,9 +3116,22 @@ static struct notifier_block 
kvmclock_cpufreq_notifier_block = {
 .notifier_call  = kvmclock_cpufreq_notifier
 };
 
+static void kvm_timer_init(void)
+{
+   int cpu;
+
+   for_each_possible_cpu(cpu)
+   per_cpu(cpu_tsc_khz, cpu) = tsc_khz;
+   if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC)) {
+   tsc_khz_ref = tsc_khz;
+   cpufreq_register_notifier(kvmclock_cpufreq_notifier_block,
+ CPUFREQ_TRANSITION_NOTIFIER);
+   }
+}
+
 int kvm_arch_init(void *opaque)
 {
-   int r, cpu;
+   int r;
struct kvm_x86_ops *ops = (struct kvm_x86_ops *)opaque;
 
if (kvm_x86_ops) {
@@ -3150,13 +3163,7 @@ int kvm_arch_init(void *opaque)
kvm_mmu_set_mask_ptes(PT_USER_MASK, PT_ACCESSED_MASK,
PT_DIRTY_MASK, PT64_NX_MASK, 0);
 
-   for_each_possible_cpu(cpu)
-   per_cpu(cpu_tsc_khz, cpu) = tsc_khz;
-   if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC)) {
-   tsc_khz_ref = tsc_khz;
-   cpufreq_register_notifier(kvmclock_cpufreq_notifier_block,
- CPUFREQ_TRANSITION_NOTIFIER);
-   }
+   kvm_timer_init();
 
return 0;
 
-- 
1.6.4.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html