Re: [PATCH v2] KVM: Factor out kvm_vcpu_kick to arch-generic code

2012-02-08 Thread Alexander Graf

On 06.02.2012, at 19:25, Marcelo Tosatti wrote:

 On Tue, Jan 24, 2012 at 11:27:39PM -0500, Christoffer Dall wrote:
 The kvm_vcpu_kick function performs roughly the same funcitonality on
 most all architectures, so we shouldn't have separate copies.
 
 PowerPC keeps a pointer to interchanging waitqueues on the vcpu_arch
 structure and to accomodate this special need a
 __KVM_HAVE_ARCH_VCPU_GET_WQ define and accompanying function
 kvm_arch_vcpu_wq have been defined. For all other architectures this
 is a generic inline that just returns vcpu-wq;
 
 This patch applies to Linus' tree on the Linux 3.3-rc1 tag.
 
 Changes since v1:
 - Abstact CPU mode check into arch-specific function
 - Remove redundant vcpu-cpu assignment
 
 Signed-off-by: Christoffer Dall c.d...@virtualopensystems.com
 ---
 arch/ia64/include/asm/kvm_host.h|1 +
 arch/ia64/kvm/kvm-ia64.c|   16 
 arch/powerpc/include/asm/kvm_host.h |6 ++
 arch/powerpc/kvm/powerpc.c  |   18 +++---
 arch/s390/kvm/kvm-s390.c|8 
 arch/x86/kvm/x86.c  |   17 ++---
 include/linux/kvm_host.h|9 +
 virt/kvm/kvm_main.c |   23 +++
 8 files changed, 56 insertions(+), 42 deletions(-)
 
 diff --git a/arch/ia64/include/asm/kvm_host.h 
 b/arch/ia64/include/asm/kvm_host.h
 index 2689ee5..06a5e91 100644
 --- a/arch/ia64/include/asm/kvm_host.h
 +++ b/arch/ia64/include/asm/kvm_host.h
 @@ -365,6 +365,7 @@ struct thash_cb {
 };
 
 struct kvm_vcpu_stat {
 +u32 halt_wakeup;
 };
 
 struct kvm_vcpu_arch {
 diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
 index 43f4c92..428eb0b 100644
 --- a/arch/ia64/kvm/kvm-ia64.c
 +++ b/arch/ia64/kvm/kvm-ia64.c
 @@ -1402,7 +1402,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
  if (cpu != vcpu-cpu) {
 -vcpu-cpu = cpu;
  if (vcpu-arch.ht_active)
  kvm_migrate_hlt_timer(vcpu);
  }
 @@ -1851,21 +1850,6 @@ void kvm_arch_hardware_unsetup(void)
 {
 }
 
 -void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
 -{
 -int me;
 -int cpu = vcpu-cpu;
 -
 -if (waitqueue_active(vcpu-wq))
 -wake_up_interruptible(vcpu-wq);
 -
 -me = get_cpu();
 -if (cpu != me  (unsigned) cpu  nr_cpu_ids  cpu_online(cpu))
 -if (!test_and_set_bit(KVM_REQ_KICK, vcpu-requests))
 -smp_send_reschedule(cpu);
 -put_cpu();
 -}
 -
 int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
 {
  return __apic_accept_irq(vcpu, irq-vector);
 diff --git a/arch/powerpc/include/asm/kvm_host.h 
 b/arch/powerpc/include/asm/kvm_host.h
 index bf8af5d..b687444 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -438,4 +438,10 @@ struct kvm_vcpu_arch {
 #define KVMPPC_VCPU_BUSY_IN_HOST 1
 #define KVMPPC_VCPU_RUNNABLE 2
 
 +#define __KVM_HAVE_ARCH_VCPU_GET_WQ 1
 +static inline wait_queue_head *kvm_arch_vcpu_wq(struct kvm_vcpu *vcpu)
 +{
 +return vcpu-arch.wqp;
 +}
 +
 #endif /* __POWERPC_KVM_HOST_H__ */
 diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
 index 607fbdf..dd0c929 100644
 --- a/arch/powerpc/kvm/powerpc.c
 +++ b/arch/powerpc/kvm/powerpc.c
 @@ -42,6 +42,11 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
 !!(v-arch.pending_exceptions);
 }
 
 +int kvm_arch_vcpu_in_guest_mode(struct kvm_vcpu *vcpu)
 +{
 +return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
 +}
 
 As mentioned previously, i fail to see where powerpc kvm sets
 vcpu-mode?

Yeah, we don't. Christoffer, would you like to write up a small patch to set it 
or should I do it?

Alex

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


Re: [PATCH v2] KVM: Factor out kvm_vcpu_kick to arch-generic code

2012-02-08 Thread Christoffer Dall
I'll give it one more shot at fixing it up.

On Wed, Feb 8, 2012 at 8:52 AM, Alexander Graf ag...@suse.de wrote:

 On 06.02.2012, at 19:25, Marcelo Tosatti wrote:

 On Tue, Jan 24, 2012 at 11:27:39PM -0500, Christoffer Dall wrote:
 The kvm_vcpu_kick function performs roughly the same funcitonality on
 most all architectures, so we shouldn't have separate copies.

 PowerPC keeps a pointer to interchanging waitqueues on the vcpu_arch
 structure and to accomodate this special need a
 __KVM_HAVE_ARCH_VCPU_GET_WQ define and accompanying function
 kvm_arch_vcpu_wq have been defined. For all other architectures this
 is a generic inline that just returns vcpu-wq;

 This patch applies to Linus' tree on the Linux 3.3-rc1 tag.

 Changes since v1:
 - Abstact CPU mode check into arch-specific function
 - Remove redundant vcpu-cpu assignment

 Signed-off-by: Christoffer Dall c.d...@virtualopensystems.com
 ---
 arch/ia64/include/asm/kvm_host.h    |    1 +
 arch/ia64/kvm/kvm-ia64.c            |   16 
 arch/powerpc/include/asm/kvm_host.h |    6 ++
 arch/powerpc/kvm/powerpc.c          |   18 +++---
 arch/s390/kvm/kvm-s390.c            |    8 
 arch/x86/kvm/x86.c                  |   17 ++---
 include/linux/kvm_host.h            |    9 +
 virt/kvm/kvm_main.c                 |   23 +++
 8 files changed, 56 insertions(+), 42 deletions(-)

 diff --git a/arch/ia64/include/asm/kvm_host.h 
 b/arch/ia64/include/asm/kvm_host.h
 index 2689ee5..06a5e91 100644
 --- a/arch/ia64/include/asm/kvm_host.h
 +++ b/arch/ia64/include/asm/kvm_host.h
 @@ -365,6 +365,7 @@ struct thash_cb {
 };

 struct kvm_vcpu_stat {
 +    u32 halt_wakeup;
 };

 struct kvm_vcpu_arch {
 diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
 index 43f4c92..428eb0b 100644
 --- a/arch/ia64/kvm/kvm-ia64.c
 +++ b/arch/ia64/kvm/kvm-ia64.c
 @@ -1402,7 +1402,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
      if (cpu != vcpu-cpu) {
 -            vcpu-cpu = cpu;
              if (vcpu-arch.ht_active)
                      kvm_migrate_hlt_timer(vcpu);
      }
 @@ -1851,21 +1850,6 @@ void kvm_arch_hardware_unsetup(void)
 {
 }

 -void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
 -{
 -    int me;
 -    int cpu = vcpu-cpu;
 -
 -    if (waitqueue_active(vcpu-wq))
 -            wake_up_interruptible(vcpu-wq);
 -
 -    me = get_cpu();
 -    if (cpu != me  (unsigned) cpu  nr_cpu_ids  cpu_online(cpu))
 -            if (!test_and_set_bit(KVM_REQ_KICK, vcpu-requests))
 -                    smp_send_reschedule(cpu);
 -    put_cpu();
 -}
 -
 int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
 {
      return __apic_accept_irq(vcpu, irq-vector);
 diff --git a/arch/powerpc/include/asm/kvm_host.h 
 b/arch/powerpc/include/asm/kvm_host.h
 index bf8af5d..b687444 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -438,4 +438,10 @@ struct kvm_vcpu_arch {
 #define KVMPPC_VCPU_BUSY_IN_HOST     1
 #define KVMPPC_VCPU_RUNNABLE         2

 +#define __KVM_HAVE_ARCH_VCPU_GET_WQ 1
 +static inline wait_queue_head *kvm_arch_vcpu_wq(struct kvm_vcpu *vcpu)
 +{
 +    return vcpu-arch.wqp;
 +}
 +
 #endif /* __POWERPC_KVM_HOST_H__ */
 diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
 index 607fbdf..dd0c929 100644
 --- a/arch/powerpc/kvm/powerpc.c
 +++ b/arch/powerpc/kvm/powerpc.c
 @@ -42,6 +42,11 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
             !!(v-arch.pending_exceptions);
 }

 +int kvm_arch_vcpu_in_guest_mode(struct kvm_vcpu *vcpu)
 +{
 +    return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
 +}

 As mentioned previously, i fail to see where powerpc kvm sets
 vcpu-mode?

 Yeah, we don't. Christoffer, would you like to write up a small patch to set 
 it or should I do it?

 Alex

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


Re: [PATCH v2] KVM: Factor out kvm_vcpu_kick to arch-generic code

2012-02-06 Thread Marcelo Tosatti
On Tue, Jan 24, 2012 at 11:27:39PM -0500, Christoffer Dall wrote:
 The kvm_vcpu_kick function performs roughly the same funcitonality on
 most all architectures, so we shouldn't have separate copies.
 
 PowerPC keeps a pointer to interchanging waitqueues on the vcpu_arch
 structure and to accomodate this special need a
 __KVM_HAVE_ARCH_VCPU_GET_WQ define and accompanying function
 kvm_arch_vcpu_wq have been defined. For all other architectures this
 is a generic inline that just returns vcpu-wq;
 
 This patch applies to Linus' tree on the Linux 3.3-rc1 tag.
 
 Changes since v1:
  - Abstact CPU mode check into arch-specific function
  - Remove redundant vcpu-cpu assignment
 
 Signed-off-by: Christoffer Dall c.d...@virtualopensystems.com
 ---
  arch/ia64/include/asm/kvm_host.h|1 +
  arch/ia64/kvm/kvm-ia64.c|   16 
  arch/powerpc/include/asm/kvm_host.h |6 ++
  arch/powerpc/kvm/powerpc.c  |   18 +++---
  arch/s390/kvm/kvm-s390.c|8 
  arch/x86/kvm/x86.c  |   17 ++---
  include/linux/kvm_host.h|9 +
  virt/kvm/kvm_main.c |   23 +++
  8 files changed, 56 insertions(+), 42 deletions(-)
 
 diff --git a/arch/ia64/include/asm/kvm_host.h 
 b/arch/ia64/include/asm/kvm_host.h
 index 2689ee5..06a5e91 100644
 --- a/arch/ia64/include/asm/kvm_host.h
 +++ b/arch/ia64/include/asm/kvm_host.h
 @@ -365,6 +365,7 @@ struct thash_cb {
  };
  
  struct kvm_vcpu_stat {
 + u32 halt_wakeup;
  };
  
  struct kvm_vcpu_arch {
 diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
 index 43f4c92..428eb0b 100644
 --- a/arch/ia64/kvm/kvm-ia64.c
 +++ b/arch/ia64/kvm/kvm-ia64.c
 @@ -1402,7 +1402,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
  void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
  {
   if (cpu != vcpu-cpu) {
 - vcpu-cpu = cpu;
   if (vcpu-arch.ht_active)
   kvm_migrate_hlt_timer(vcpu);
   }
 @@ -1851,21 +1850,6 @@ void kvm_arch_hardware_unsetup(void)
  {
  }
  
 -void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
 -{
 - int me;
 - int cpu = vcpu-cpu;
 -
 - if (waitqueue_active(vcpu-wq))
 - wake_up_interruptible(vcpu-wq);
 -
 - me = get_cpu();
 - if (cpu != me  (unsigned) cpu  nr_cpu_ids  cpu_online(cpu))
 - if (!test_and_set_bit(KVM_REQ_KICK, vcpu-requests))
 - smp_send_reschedule(cpu);
 - put_cpu();
 -}
 -
  int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq)
  {
   return __apic_accept_irq(vcpu, irq-vector);
 diff --git a/arch/powerpc/include/asm/kvm_host.h 
 b/arch/powerpc/include/asm/kvm_host.h
 index bf8af5d..b687444 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -438,4 +438,10 @@ struct kvm_vcpu_arch {
  #define KVMPPC_VCPU_BUSY_IN_HOST 1
  #define KVMPPC_VCPU_RUNNABLE 2
  
 +#define __KVM_HAVE_ARCH_VCPU_GET_WQ 1
 +static inline wait_queue_head *kvm_arch_vcpu_wq(struct kvm_vcpu *vcpu)
 +{
 + return vcpu-arch.wqp;
 +}
 +
  #endif /* __POWERPC_KVM_HOST_H__ */
 diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
 index 607fbdf..dd0c929 100644
 --- a/arch/powerpc/kvm/powerpc.c
 +++ b/arch/powerpc/kvm/powerpc.c
 @@ -42,6 +42,11 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
  !!(v-arch.pending_exceptions);
  }
  
 +int kvm_arch_vcpu_in_guest_mode(struct kvm_vcpu *vcpu)
 +{
 + return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
 +}

As mentioned previously, i fail to see where powerpc kvm sets
vcpu-mode?

 +
  int kvmppc_kvm_pv(struct kvm_vcpu *vcpu)
  {
   int nr = kvmppc_get_gpr(vcpu, 11);
 @@ -311,10 +316,7 @@ static void kvmppc_decrementer_func(unsigned long data)
  
   kvmppc_core_queue_dec(vcpu);
  
 - if (waitqueue_active(vcpu-arch.wqp)) {
 - wake_up_interruptible(vcpu-arch.wqp);
 - vcpu-stat.halt_wakeup++;
 - }
 + kvm_vcpu_kick(vcpu);
  }
  
  /*
 @@ -363,7 +365,6 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
   mtspr(SPRN_VRSAVE, vcpu-arch.vrsave);
  #endif
   kvmppc_core_vcpu_load(vcpu, cpu);
 - vcpu-cpu = smp_processor_id();
  }
  
  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 @@ -572,12 +573,7 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, 
 struct kvm_interrupt *irq)
  
   kvmppc_core_queue_external(vcpu, irq);
  
 - if (waitqueue_active(vcpu-arch.wqp)) {
 - wake_up_interruptible(vcpu-arch.wqp);
 - vcpu-stat.halt_wakeup++;
 - } else if (vcpu-cpu != -1) {
 - smp_send_reschedule(vcpu-cpu);
 - }
 + kvm_vcpu_kick(vcpu);
  
   return 0;
  }
 diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
 index d1c44573..5e2217f 100644
 --- a/arch/s390/kvm/kvm-s390.c
 +++ b/arch/s390/kvm/kvm-s390.c
 @@ -380,6 +380,14 @@ int 

Re: [PATCH v2] KVM: Factor out kvm_vcpu_kick to arch-generic code

2012-02-06 Thread Jan Kiszka
On 2012-02-06 19:25, Marcelo Tosatti wrote:
 diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
 index c38efd7..a1761ff 100644
 --- a/arch/x86/kvm/x86.c
 +++ b/arch/x86/kvm/x86.c
 @@ -2252,7 +2252,6 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
  kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
  if (vcpu-cpu != cpu)
  kvm_migrate_timers(vcpu);
 -vcpu-cpu = cpu;
  }
 
 This is wrong, kvm_sched_in fails to see vcpu-cpu properly. Please
 keep vcpu-cpu assignment in arch code.

True, but then kvm_sched_in is a better place for this assignment (as
it's central), no?

Jan



signature.asc
Description: OpenPGP digital signature


Re: [PATCH v2] KVM: Factor out kvm_vcpu_kick to arch-generic code

2012-02-06 Thread Marcelo Tosatti
On Mon, Feb 06, 2012 at 09:06:04PM +0100, Jan Kiszka wrote:
 On 2012-02-06 19:25, Marcelo Tosatti wrote:
  diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
  index c38efd7..a1761ff 100644
  --- a/arch/x86/kvm/x86.c
  +++ b/arch/x86/kvm/x86.c
  @@ -2252,7 +2252,6 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int 
  cpu)
 kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
 if (vcpu-cpu != cpu)
 kvm_migrate_timers(vcpu);
  -  vcpu-cpu = cpu;
 }
  
  This is wrong, kvm_sched_in fails to see vcpu-cpu properly. Please
  keep vcpu-cpu assignment in arch code.
 
 True, but then kvm_sched_in is a better place for this assignment (as
 it's central), no?
 
 Jan

Not necessarily. Arch code might want to have:

kvm_arch_vcpu_load()
{
cpu = smp_processor_id();

if (vcpu-cpu != cpu) {
localize vcpu to cpu
vcpu-cpu = cpu
}

call functions which use vcpu-cpu
}

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