Re: [kvm-devel] [PATCH]3/5 Using kvm_arch prefix to define functions, and replace
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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH]3/5 Using kvm_arch prefix to define functions, and replace
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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH]3/5 Using kvm_arch prefix to define functions, and replace
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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH]3/5 Using kvm_arch prefix to define functions, and replace
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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH]3/5 Using kvm_arch prefix to define functions, and replace
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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH]3/5 Using kvm_arch prefix to define functions, and replace
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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH]3/5 Using kvm_arch prefix to define functions, and replace
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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] [PATCH]3/5 Using kvm_arch prefix to define functions, and replace
From cba4340cef2217343c540ca5de9b67cc8826b63f Mon Sep 17 00:00:00 2001 From: Zhang Xiantao [EMAIL PROTECTED] 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 [EMAIL PROTECTED] --- 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