[PATCH]3/5 Using kvm_arch prefix to define functions, and replace

[PATCH]3/5 Using kvm_arch prefix to define functions, and replace

[Zhang, Xiantao]

>From cba4340cef2217343c540ca5de9b67cc8826b63f Mon Sep 17 00:00:00 2001
From: Zhang Xiantao <xiantao.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date: Thu, 8 Nov 2007 13:07:23 +0800
Subject: [PATCH] Using kvm_arch prefix to define functions, and replace
kvm_x86_ops callback.
Signed-off-by: Zhang Xiantao <xiantao.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/kvm/kvm.h      |   15 +++++++++++++++
 drivers/kvm/kvm_main.c |   26 +++++++++++++-------------
 drivers/kvm/x86.c      |   48
++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 13 deletions(-)

diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index 091f9b7..4b2421a 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -649,6 +649,21 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu,
struct kvm_run *kvm_run);
 
 __init void kvm_arch_init(void);
 
+
+void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu);
+void kvm_arch_vcpu_decache(struct kvm_vcpu *vcpu);
+void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
+void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
+struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int
id);
+
+int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu);
+void kvm_arch_hardware_enable(void *garbage);
+void kvm_arch_hardware_disable(void *garbage);
+int kvm_arch_hardware_setup(void);
+void kvm_arch_hardware_unsetup(void);
+void kvm_arch_check_processor_compat(void *rtn);
+
+
 static inline void kvm_guest_enter(void)
 {
 	account_system_vtime(current);
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index f1cf8f0..da7fb22 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -240,7 +240,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
 			kvm_unload_vcpu_mmu(kvm->vcpus[i]);
 	for (i = 0; i < KVM_MAX_VCPUS; ++i) {
 		if (kvm->vcpus[i]) {
-			kvm_x86_ops->vcpu_free(kvm->vcpus[i]);
+			kvm_arch_vcpu_free(kvm->vcpus[i]);
 			kvm->vcpus[i] = NULL;
 		}
 	}
@@ -890,7 +890,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm,
int n)
 	if (!valid_vcpu(n))
 		return -EINVAL;
 
-	vcpu = kvm_x86_ops->vcpu_create(kvm, n);
+	vcpu = kvm_arch_vcpu_create(kvm, n);
 	if (IS_ERR(vcpu))
 		return PTR_ERR(vcpu);
 
@@ -900,7 +900,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm,
int n)
 	BUG_ON((unsigned long)&vcpu->host_fx_image & 0xF);
 
 	vcpu_load(vcpu);
-	r = kvm_x86_ops->vcpu_reset(vcpu);
+	r = kvm_arch_vcpu_reset(vcpu);
 	if (r == 0)
 		r = kvm_mmu_setup(vcpu);
 	vcpu_put(vcpu);
@@ -933,7 +933,7 @@ mmu_unload:
 	vcpu_put(vcpu);
 
 free_vcpu:
-	kvm_x86_ops->vcpu_free(vcpu);
+	kvm_arch_vcpu_free(vcpu);
 	return r;
 }
 
@@ -1297,7 +1297,7 @@ static void decache_vcpus_on_cpu(int cpu)
 			 */
 			if (mutex_trylock(&vcpu->mutex)) {
 				if (vcpu->cpu == cpu) {
-					kvm_x86_ops->vcpu_decache(vcpu);
+					kvm_arch_vcpu_decache(vcpu);
 					vcpu->cpu = -1;
 				}
 				mutex_unlock(&vcpu->mutex);
@@ -1313,7 +1313,7 @@ static void hardware_enable(void *junk)
 	if (cpu_isset(cpu, cpus_hardware_enabled))
 		return;
 	cpu_set(cpu, cpus_hardware_enabled);
-	kvm_x86_ops->hardware_enable(NULL);
+	kvm_arch_hardware_enable(NULL);
 }
 
 static void hardware_disable(void *junk)
@@ -1324,7 +1324,7 @@ static void hardware_disable(void *junk)
 		return;
 	cpu_clear(cpu, cpus_hardware_enabled);
 	decache_vcpus_on_cpu(cpu);
-	kvm_x86_ops->hardware_disable(NULL);
+	kvm_arch_hardware_disable(NULL);
 }
 
 static int kvm_cpu_hotplug(struct notifier_block *notifier, unsigned
long val,
@@ -1492,7 +1492,7 @@ static void kvm_sched_in(struct preempt_notifier
*pn, int cpu)
 {
 	struct kvm_vcpu *vcpu = preempt_notifier_to_vcpu(pn);
 
-	kvm_x86_ops->vcpu_load(vcpu, cpu);
+	kvm_arch_vcpu_load(vcpu, cpu);
 }
 
 static void kvm_sched_out(struct preempt_notifier *pn,
@@ -1500,7 +1500,7 @@ static void kvm_sched_out(struct preempt_notifier
*pn,
 {
 	struct kvm_vcpu *vcpu = preempt_notifier_to_vcpu(pn);
 
-	kvm_x86_ops->vcpu_put(vcpu);
+	kvm_arch_vcpu_put(vcpu);
 }
 
 int kvm_init_x86(struct kvm_x86_ops *ops, unsigned int vcpu_size,
@@ -1525,13 +1525,13 @@ int kvm_init_x86(struct kvm_x86_ops *ops,
unsigned int vcpu_size,
 
 	kvm_x86_ops = ops;
 
-	r = kvm_x86_ops->hardware_setup();
+	r = kvm_arch_hardware_setup();
 	if (r < 0)
 		goto out;
 
 	for_each_online_cpu(cpu) {
 		smp_call_function_single(cpu,
-
kvm_x86_ops->check_processor_compatibility,
+				kvm_arch_check_processor_compat,
 				&r, 0, 1);
 		if (r < 0)
 			goto out_free_0;
@@ -1586,7 +1586,7 @@ out_free_2:
 out_free_1:
 	on_each_cpu(hardware_disable, NULL, 0, 1);
 out_free_0:
-	kvm_x86_ops->hardware_unsetup();
+	kvm_arch_hardware_unsetup();
 out:
 	kvm_x86_ops = NULL;
 	return r;
@@ -1602,7 +1602,7 @@ void kvm_exit_x86(void)
 	unregister_reboot_notifier(&kvm_reboot_notifier);
 	unregister_cpu_notifier(&kvm_cpu_notifier);
 	on_each_cpu(hardware_disable, NULL, 0, 1);
-	kvm_x86_ops->hardware_unsetup();
+	kvm_arch_hardware_unsetup();
 	kvm_x86_ops = NULL;
 }
 EXPORT_SYMBOL_GPL(kvm_exit_x86);
diff --git a/drivers/kvm/x86.c b/drivers/kvm/x86.c
index e26e46a..8aea240 100644
--- a/drivers/kvm/x86.c
+++ b/drivers/kvm/x86.c
@@ -2327,3 +2327,50 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
 	fx_restore(&vcpu->host_fx_image);
 }
 EXPORT_SYMBOL_GPL(kvm_put_guest_fpu);
+
+void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
+{
+	kvm_x86_ops->vcpu_free(vcpu);
+}
+
+void kvm_arch_vcpu_decache(struct kvm_vcpu *vcpu)
+{
+	kvm_x86_ops->vcpu_decache(vcpu);
+}
+
+struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
+						unsigned int id)
+{
+	return kvm_x86_ops->vcpu_create(kvm, id);
+}
+
+int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
+{
+	return kvm_x86_ops->vcpu_reset(vcpu);
+}
+
+void kvm_arch_hardware_enable(void *garbage)
+{
+	kvm_x86_ops->hardware_enable(garbage);
+}
+
+void kvm_arch_hardware_disable(void *garbage)
+{
+	kvm_x86_ops->hardware_disable(garbage);
+}
+
+int kvm_arch_hardware_setup(void)
+{
+	return kvm_x86_ops->hardware_setup();
+}
+
+void kvm_arch_hardware_unsetup(void)
+{
+	kvm_x86_ops->hardware_unsetup();
+}
+
+void kvm_arch_check_processor_compat(void *rtn)
+{
+	kvm_x86_ops->check_processor_compatibility(rtn);
+}
+
-- 
1.5.1.2

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH]3/5 Using kvm_arch prefix to define functions, and replace

[Carsten Otte]

Zhang, Xiantao wrote:
> +void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu);
> +void kvm_arch_vcpu_decache(struct kvm_vcpu *vcpu);
> +void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
> +void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
> +struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int
> id);
> +
> +int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu);
> +void kvm_arch_hardware_enable(void *garbage);
> +void kvm_arch_hardware_disable(void *garbage);
> +int kvm_arch_hardware_setup(void);
> +void kvm_arch_hardware_unsetup(void);
> +void kvm_arch_check_processor_compat(void *rtn);
I don't like the generic introduction of all x86_ops wrappers into the 
arch callbacks. I would rather prefer to work out a different split 
between common and arch specifics - at least in the following cases:
- unloading the mmu needs to be moved out of kvm_free_vcpus into the 
arch part, because we don't have a shaddow mmu on s390
- decache_vcpus_on_cpu should be arch-dependent alltogether, rather 
than having a per cpu callback. We've got nothing to decache, so the 
entire thing is a nop for us.
- vcpu_reset works very different for our architecture, we'd need an 
initial processor status word. I'd prefer to keep the existence of 
this callback arch dependent.
- hardware enable/disable/setup/unsetup/check_processor_compat does not
make any sense for us: all CPUs that have been sold since the 1970s have
proper hardware virtualization, and there's nothing to enable - it just
works.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH]3/5 Using kvm_arch prefix to definefunctions, and replace

[Zhang, Xiantao]

Carsten Otte wrote:
> Zhang, Xiantao wrote:
>> +void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu);
>> +void kvm_arch_vcpu_decache(struct kvm_vcpu *vcpu);
>> +void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
>> +void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
>> +struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int
>> id); +
>> +int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu);
>> +void kvm_arch_hardware_enable(void *garbage);
>> +void kvm_arch_hardware_disable(void *garbage);
>> +int kvm_arch_hardware_setup(void);
>> +void kvm_arch_hardware_unsetup(void);
>> +void kvm_arch_check_processor_compat(void *rtn);
> I don't like the generic introduction of all x86_ops wrappers into the
> arch callbacks. I would rather prefer to work out a different split
> between common and arch specifics - at least in the following cases:
> - unloading the mmu needs to be moved out of kvm_free_vcpus into the
> arch part, because we don't have a shaddow mmu on s390

I think we have an old thread related to mmu code in kvm_main.c. In
IA64, we haven't shadow mmu as well, but we still need mmu interfaces,
such as mmu load, reload, unload to notify arch-specific VMM module
about the memory changes. 

> - decache_vcpus_on_cpu should be arch-dependent alltogether, rather
> than having a per cpu callback. We've got nothing to decache, so the
> entire thing is a nop for us.

Actually, IA64 doesn't require it. As you said, decache_vcpus_on_cpu is
only needed by x86.

> - vcpu_reset works very different for our architecture, we'd need an
> initial processor status word. I'd prefer to keep the existence of
> this callback arch dependent.

Yes, I think kvm_arch_vcpu_reset in this patch should meet your
requirement.

> - hardware enable/disable/setup/unsetup/check_processor_compat does
> not make any sense for us: all CPUs that have been sold since the
> 1970s have proper hardware virtualization, and there's nothing to
> enable - it just works.
OK,  Maybe we can move them to arch-specific.  

Avi, How about your ideas?

Thanks 
Xiantao

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH]3/5 Using kvm_arch prefix to definefunctions, and replace

[Avi Kivity]

Zhang, Xiantao wrote:
>
>> - hardware enable/disable/setup/unsetup/check_processor_compat does
>> not make any sense for us: all CPUs that have been sold since the
>> 1970s have proper hardware virtualization, and there's nothing to
>> enable - it just works.
>>     
> OK,  Maybe we can move them to arch-specific.  
>
> Avi, How about your ideas?

I'm actually least qualified to comment since I only know about x86.  
Since this is all preparations before actual code is seen, it's hard to 
see where things are flowing.

I don't mind if a few mistakes are made along the way that are corrected 
later, as long as x86 continues to build and run fine.  It's okay to 
start working on this problem with a 10-pound hammer and move to atomic 
force microscopy once things are merged.

So, I will be very much inclined to accept anything that has gained the 
consensus of the arch maintainers.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH]3/5 Using kvm_arch prefix to define functions, and replace

[Hollis Blanchard]

On Thu, 2007-11-08 at 14:49 +0100, Carsten Otte wrote:
> Zhang, Xiantao wrote:
> > +void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu);
> > +void kvm_arch_vcpu_decache(struct kvm_vcpu *vcpu);
> > +void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
> > +void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
> > +struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int
> > id);
> > +
> > +int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu);
> > +void kvm_arch_hardware_enable(void *garbage);
> > +void kvm_arch_hardware_disable(void *garbage);
> > +int kvm_arch_hardware_setup(void);
> > +void kvm_arch_hardware_unsetup(void);
> > +void kvm_arch_check_processor_compat(void *rtn);
> I don't like the generic introduction of all x86_ops wrappers into the 
> arch callbacks. I would rather prefer to work out a different split 
> between common and arch specifics - at least in the following cases:
> - unloading the mmu needs to be moved out of kvm_free_vcpus into the 
> arch part, because we don't have a shaddow mmu on s390
> - decache_vcpus_on_cpu should be arch-dependent alltogether, rather 
> than having a per cpu callback. We've got nothing to decache, so the 
> entire thing is a nop for us.
> - vcpu_reset works very different for our architecture, we'd need an 
> initial processor status word. I'd prefer to keep the existence of 
> this callback arch dependent.
> - hardware enable/disable/setup/unsetup/check_processor_compat does not
> make any sense for us: all CPUs that have been sold since the 1970s have
> proper hardware virtualization, and there's nothing to enable - it just
> works.

Sounds fine to me: you're just proposing to move the abstraction one
level higher in some places.

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH]3/5 Using kvm_arch prefix to define functions, and replace

[Carsten Otte]

Hollis Blanchard wrote:
> On Thu, 2007-11-08 at 14:49 +0100, Carsten Otte wrote:
>> Zhang, Xiantao wrote:
>>> +void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu);
>>> +void kvm_arch_vcpu_decache(struct kvm_vcpu *vcpu);
>>> +void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
>>> +void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
>>> +struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int
>>> id);
>>> +
>>> +int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu);
>>> +void kvm_arch_hardware_enable(void *garbage);
>>> +void kvm_arch_hardware_disable(void *garbage);
>>> +int kvm_arch_hardware_setup(void);
>>> +void kvm_arch_hardware_unsetup(void);
>>> +void kvm_arch_check_processor_compat(void *rtn);
>> I don't like the generic introduction of all x86_ops wrappers into the 
>> arch callbacks. I would rather prefer to work out a different split 
>> between common and arch specifics - at least in the following cases:
>> - unloading the mmu needs to be moved out of kvm_free_vcpus into the 
>> arch part, because we don't have a shaddow mmu on s390
>> - decache_vcpus_on_cpu should be arch-dependent alltogether, rather 
>> than having a per cpu callback. We've got nothing to decache, so the 
>> entire thing is a nop for us.
>> - vcpu_reset works very different for our architecture, we'd need an 
>> initial processor status word. I'd prefer to keep the existence of 
>> this callback arch dependent.
>> - hardware enable/disable/setup/unsetup/check_processor_compat does not
>> make any sense for us: all CPUs that have been sold since the 1970s have
>> proper hardware virtualization, and there's nothing to enable - it just
>> works.
> 
> Sounds fine to me: you're just proposing to move the abstraction one
> level higher in some places.
That's right, I'd like to drag the bar a little where the common code 
does something just to call a callback that is nop for us.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH]3/5 Using kvm_arch prefix to define functions, and replace

[Avi Kivity]

Carsten Otte wrote:
> Hollis Blanchard wrote:
>   
>> On Thu, 2007-11-08 at 14:49 +0100, Carsten Otte wrote:
>>     
>>> Zhang, Xiantao wrote:
>>>       
>>>> +void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu);
>>>> +void kvm_arch_vcpu_decache(struct kvm_vcpu *vcpu);
>>>> +void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
>>>> +void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
>>>> +struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int
>>>> id);
>>>> +
>>>> +int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu);
>>>> +void kvm_arch_hardware_enable(void *garbage);
>>>> +void kvm_arch_hardware_disable(void *garbage);
>>>> +int kvm_arch_hardware_setup(void);
>>>> +void kvm_arch_hardware_unsetup(void);
>>>> +void kvm_arch_check_processor_compat(void *rtn);
>>>>         
>>> I don't like the generic introduction of all x86_ops wrappers into the 
>>> arch callbacks. I would rather prefer to work out a different split 
>>> between common and arch specifics - at least in the following cases:
>>> - unloading the mmu needs to be moved out of kvm_free_vcpus into the 
>>> arch part, because we don't have a shaddow mmu on s390
>>> - decache_vcpus_on_cpu should be arch-dependent alltogether, rather 
>>> than having a per cpu callback. We've got nothing to decache, so the 
>>> entire thing is a nop for us.
>>> - vcpu_reset works very different for our architecture, we'd need an 
>>> initial processor status word. I'd prefer to keep the existence of 
>>> this callback arch dependent.
>>> - hardware enable/disable/setup/unsetup/check_processor_compat does not
>>> make any sense for us: all CPUs that have been sold since the 1970s have
>>> proper hardware virtualization, and there's nothing to enable - it just
>>> works.
>>>       
>> Sounds fine to me: you're just proposing to move the abstraction one
>> level higher in some places.
>>     
> That's right, I'd like to drag the bar a little where the common code 
> does something just to call a callback that is nop for us.
>   

For the present discussion, I agree, but in general we should be 
prepared to accept some no-op callouts.


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH]3/5 Using kvm_arch prefix to define functions, and replace

[Carsten Otte]

Avi Kivity wrote:
> For the present discussion, I agree, but in general we should be 
> prepared to accept some no-op callouts.

Oh sure, I don't mind those. We'll have plenty of them, where other 
architectures will need to take action and s390 won't. It's just that 
in the current location, the common code would do

common_func() {
	loop {
		arch_callback();
	}
}
And I suggested to make the whole thing a callback.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH]3/5 Using kvm_arch prefix to define functions, and replace

[Avi Kivity]

Carsten Otte wrote:
> Avi Kivity wrote:
>   
>> For the present discussion, I agree, but in general we should be 
>> prepared to accept some no-op callouts.
>>     
>
> Oh sure, I don't mind those. We'll have plenty of them, where other 
> architectures will need to take action and s390 won't. It's just that 
> in the current location, the common code would do
>
> common_func() {
> 	loop {
> 		arch_callback();
> 	}
> }
> And I suggested to make the whole thing a callback.
>   

Ah, agreed.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH]3/5 Using kvm_arch prefix to define functions, and replace

[Hollis Blanchard]

On Thu, 2007-11-08 at 15:21 +0800, Zhang, Xiantao wrote:
> @@ -890,7 +890,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm
> *kvm,
> int n)
>         if (!valid_vcpu(n))
>                 return -EINVAL;
>  
> -       vcpu = kvm_x86_ops->vcpu_create(kvm, n);
> +       vcpu = kvm_arch_vcpu_create(kvm, n);
>         if (IS_ERR(vcpu))
>                 return PTR_ERR(vcpu);
>  
> @@ -900,7 +900,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm
> *kvm,
> int n)
>         BUG_ON((unsigned long)&vcpu->host_fx_image & 0xF);
>  
>         vcpu_load(vcpu);
> -       r = kvm_x86_ops->vcpu_reset(vcpu);
> +       r = kvm_arch_vcpu_reset(vcpu);
>         if (r == 0)
>                 r = kvm_mmu_setup(vcpu);
>         vcpu_put(vcpu);
> @@ -933,7 +933,7 @@ mmu_unload:
>         vcpu_put(vcpu);
>  
>  free_vcpu:
> -       kvm_x86_ops->vcpu_free(vcpu);
> +       kvm_arch_vcpu_free(vcpu);
>         return r;
>  }

Have a look at the patch I posted on Wednesday: "[PATCH 2 of 2] RFC:
Create kvm_arch_vcpu_create()". kvm_arch_vcpu_create() will actually
encompass more logic from kvm_vm_ioctl_create_vcpu(), and once you do
that, kvm_arch_vcpu_reset() doesn't need to exist (which will also make
Carsten happy).

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH]3/5 Using kvm_arch prefix to define functions, andreplace

[Zhang, Xiantao]

Hollis Blanchard wrote:

>> -       kvm_x86_ops->vcpu_free(vcpu);
>> +       kvm_arch_vcpu_free(vcpu);
>>         return r;
>>  }
> 
> Have a look at the patch I posted on Wednesday: "[PATCH 2 of 2] RFC:
> Create kvm_arch_vcpu_create()". kvm_arch_vcpu_create() will actually
> encompass more logic from kvm_vm_ioctl_create_vcpu(), and once you do
> that, kvm_arch_vcpu_reset() doesn't need to exist (which will also
> make Carsten happy).

Hi, Hollis
   Thanks for your response. If we need to move out the code related to
mmu, sure to keep more logic in kvm_arch_create_vcpu. But I don't know
the decision about the mmu part putting in common or arch. As Avi said
before, maybe we need to keep mmu interfaces in common for future
possible memory hotplug of guest and so on.  
Best Wishes
Xiantao

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/