| Message ID | 20191218215530.2280-42-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: Refactor vCPU creation | expand |
On Wed, Dec 18, 2019 at 01:55:26PM -0800, Sean Christopherson wrote: > Fold init() into create() now that the two are called back-to-back by > common KVM code (kvm_vcpu_init() calls kvm_arch_vcpu_init() as its last > action, and kvm_vm_ioctl_create_vcpu() calls kvm_arch_vcpu_create() > immediately thereafter). Rinse and repeat for kvm_arch_vcpu_uninit() > and kvm_arch_vcpu_destroy(). This paves the way for removing > kvm_arch_vcpu_{un}init() entirely. > > Note, calling kvmppc_mmu_destroy() if kvmppc_core_vcpu_create() fails > may or may not be necessary. Move it along with the more obvious call > to kvmppc_subarch_vcpu_uninit() so as not to inadvertantly introduce a > functional change and/or bug. > > No functional change intended. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> This doesn't compile. I get: CC [M] arch/powerpc/kvm/powerpc.o /home/paulus/kernel/kvm/arch/powerpc/kvm/powerpc.c: In function ‘kvm_arch_vcpu_create’: /home/paulus/kernel/kvm/arch/powerpc/kvm/powerpc.c:733:34: error: ‘kvmppc_decrementer_wakeup’ undeclared (first use in this function) vcpu->arch.dec_timer.function = kvmppc_decrementer_wakeup; ^ /home/paulus/kernel/kvm/arch/powerpc/kvm/powerpc.c:733:34: note: each undeclared identifier is reported only once for each function it appears in /home/paulus/kernel/kvm/arch/powerpc/kvm/powerpc.c: At top level: /home/paulus/kernel/kvm/arch/powerpc/kvm/powerpc.c:794:29: warning: ‘kvmppc_decrementer_wakeup’ defined but not used [-Wunused-function] static enum hrtimer_restart kvmppc_decrementer_wakeup(struct hrtimer *timer) ^ make[3]: *** [/home/paulus/kernel/kvm/scripts/Makefile.build:266: arch/powerpc/kvm/powerpc.o] Error 1 The problem is that kvmppc_decrementer_wakeup() is a static function defined in this file (arch/powerpc/kvm/powerpc.c) after kvm_arch_vcpu_create() but before kvm_arch_vcpu_init(). You need a forward static declaration of kvmppc_decrementer_wakeup() before kvm_arch_vcpu_create(), or else move one or other function. Paul.
On 20/01/20 04:46, Paul Mackerras wrote: > On Wed, Dec 18, 2019 at 01:55:26PM -0800, Sean Christopherson wrote: >> Fold init() into create() now that the two are called back-to-back by >> common KVM code (kvm_vcpu_init() calls kvm_arch_vcpu_init() as its last >> action, and kvm_vm_ioctl_create_vcpu() calls kvm_arch_vcpu_create() >> immediately thereafter). Rinse and repeat for kvm_arch_vcpu_uninit() >> and kvm_arch_vcpu_destroy(). This paves the way for removing >> kvm_arch_vcpu_{un}init() entirely. >> >> Note, calling kvmppc_mmu_destroy() if kvmppc_core_vcpu_create() fails >> may or may not be necessary. Move it along with the more obvious call >> to kvmppc_subarch_vcpu_uninit() so as not to inadvertantly introduce a >> functional change and/or bug. >> >> No functional change intended. >> >> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > This doesn't compile. I get: > > CC [M] arch/powerpc/kvm/powerpc.o > /home/paulus/kernel/kvm/arch/powerpc/kvm/powerpc.c: In function ‘kvm_arch_vcpu_create’: > /home/paulus/kernel/kvm/arch/powerpc/kvm/powerpc.c:733:34: error: ‘kvmppc_decrementer_wakeup’ undeclared (first use in this function) > vcpu->arch.dec_timer.function = kvmppc_decrementer_wakeup; > ^ > /home/paulus/kernel/kvm/arch/powerpc/kvm/powerpc.c:733:34: note: each undeclared identifier is reported only once for each function it appears in > /home/paulus/kernel/kvm/arch/powerpc/kvm/powerpc.c: At top level: > /home/paulus/kernel/kvm/arch/powerpc/kvm/powerpc.c:794:29: warning: ‘kvmppc_decrementer_wakeup’ defined but not used [-Wunused-function] > static enum hrtimer_restart kvmppc_decrementer_wakeup(struct hrtimer *timer) > ^ > make[3]: *** [/home/paulus/kernel/kvm/scripts/Makefile.build:266: arch/powerpc/kvm/powerpc.o] Error 1 > > The problem is that kvmppc_decrementer_wakeup() is a static function > defined in this file (arch/powerpc/kvm/powerpc.c) after > kvm_arch_vcpu_create() but before kvm_arch_vcpu_init(). You need a > forward static declaration of kvmppc_decrementer_wakeup() before > kvm_arch_vcpu_create(), or else move one or other function. > > Paul. > Squashed: diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 91cf94d4191e..4fbf8690b8c5 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -725,6 +725,16 @@ int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id) return 0; } +static enum hrtimer_restart kvmppc_decrementer_wakeup(struct hrtimer *timer) +{ + struct kvm_vcpu *vcpu; + + vcpu = container_of(timer, struct kvm_vcpu, arch.dec_timer); + kvmppc_decrementer_func(vcpu); + + return HRTIMER_NORESTART; +} + int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) { int err; @@ -791,16 +801,6 @@ int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu) return kvmppc_core_pending_dec(vcpu); } -static enum hrtimer_restart kvmppc_decrementer_wakeup(struct hrtimer *timer) -{ - struct kvm_vcpu *vcpu; - - vcpu = container_of(timer, struct kvm_vcpu, arch.dec_timer); - kvmppc_decrementer_func(vcpu); - - return HRTIMER_NORESTART; -} - int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) { return 0; Paolo
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index fce1b4776e55..91cf94d4191e 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -729,13 +729,29 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) { int err; - err = kvmppc_core_vcpu_create(vcpu); + hrtimer_init(&vcpu->arch.dec_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS); + vcpu->arch.dec_timer.function = kvmppc_decrementer_wakeup; + vcpu->arch.dec_expires = get_tb(); + +#ifdef CONFIG_KVM_EXIT_TIMING + mutex_init(&vcpu->arch.exit_timing_lock); +#endif + err = kvmppc_subarch_vcpu_init(vcpu); if (err) return err; + err = kvmppc_core_vcpu_create(vcpu); + if (err) + goto out_vcpu_uninit; + vcpu->arch.wqp = &vcpu->wq; kvmppc_create_vcpu_debugfs(vcpu, vcpu->vcpu_id); return 0; + +out_vcpu_uninit: + kvmppc_mmu_destroy(vcpu); + kvmppc_subarch_vcpu_uninit(vcpu); + return err; } void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu) @@ -765,6 +781,9 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) } kvmppc_core_vcpu_free(vcpu); + + kvmppc_mmu_destroy(vcpu); + kvmppc_subarch_vcpu_uninit(vcpu); } int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu) @@ -784,23 +803,12 @@ static enum hrtimer_restart kvmppc_decrementer_wakeup(struct hrtimer *timer) int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) { - int ret; - - hrtimer_init(&vcpu->arch.dec_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS); - vcpu->arch.dec_timer.function = kvmppc_decrementer_wakeup; - vcpu->arch.dec_expires = get_tb(); - -#ifdef CONFIG_KVM_EXIT_TIMING - mutex_init(&vcpu->arch.exit_timing_lock); -#endif - ret = kvmppc_subarch_vcpu_init(vcpu); - return ret; + return 0; } void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) { - kvmppc_mmu_destroy(vcpu); - kvmppc_subarch_vcpu_uninit(vcpu); + } void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
Fold init() into create() now that the two are called back-to-back by common KVM code (kvm_vcpu_init() calls kvm_arch_vcpu_init() as its last action, and kvm_vm_ioctl_create_vcpu() calls kvm_arch_vcpu_create() immediately thereafter). Rinse and repeat for kvm_arch_vcpu_uninit() and kvm_arch_vcpu_destroy(). This paves the way for removing kvm_arch_vcpu_{un}init() entirely. Note, calling kvmppc_mmu_destroy() if kvmppc_core_vcpu_create() fails may or may not be necessary. Move it along with the more obvious call to kvmppc_subarch_vcpu_uninit() so as not to inadvertantly introduce a functional change and/or bug. No functional change intended. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/powerpc/kvm/powerpc.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-)