Re: [PATCH v6 14/15] KVM: ARM: Power State Coordination Interface implementation

2013-01-21 Thread Gleb Natapov 
On Sun, Jan 20, 2013 at 06:35:51PM -0500, Christoffer Dall wrote:
 On Thu, Jan 17, 2013 at 10:55 AM, Marc Zyngier marc.zyng...@arm.com wrote:
  On 16/01/13 17:59, Christoffer Dall wrote:
  From: Marc Zyngier marc.zyng...@arm.com
 
  Implement the PSCI specification (ARM DEN 0022A) to control
  virtual CPUs being powered on or off.
 
  PSCI/KVM is detected using the KVM_CAP_ARM_PSCI capability.
 
  A virtual CPU can now be initialized in a powered off state,
  using the KVM_ARM_VCPU_POWER_OFF feature flag.
 
  The guest can use either SMC or HVC to execute a PSCI function.
 
  Reviewed-by: Will Deacon will.dea...@arm.com
  Signed-off-by: Marc Zyngier marc.zyng...@arm.com
  Signed-off-by: Christoffer Dall c.d...@virtualopensystems.com
 
  A few bits went wrong when you reworked this patch. See below.
 
  ---
   Documentation/virtual/kvm/api.txt  |4 +
   arch/arm/include/asm/kvm_emulate.h |5 ++
   arch/arm/include/asm/kvm_host.h|5 +-
   arch/arm/include/asm/kvm_psci.h|   23 
   arch/arm/include/uapi/asm/kvm.h|   16 +
   arch/arm/kvm/Makefile  |2 -
   arch/arm/kvm/arm.c |   28 +-
   arch/arm/kvm/psci.c|  105 
  
   include/uapi/linux/kvm.h   |1
   9 files changed, 184 insertions(+), 5 deletions(-)
   create mode 100644 arch/arm/include/asm/kvm_psci.h
   create mode 100644 arch/arm/kvm/psci.c
 
  diff --git a/Documentation/virtual/kvm/api.txt 
  b/Documentation/virtual/kvm/api.txt
  index 38066a7a..c25439a 100644
  --- a/Documentation/virtual/kvm/api.txt
  +++ b/Documentation/virtual/kvm/api.txt
  @@ -2185,6 +2185,10 @@ return ENOEXEC for that vcpu.
   Note that because some registers reflect machine topology, all vcpus
   should be created before this ioctl is invoked.
 
  +Possible features:
  + - KVM_ARM_VCPU_POWER_OFF: Starts the CPU in a power-off state.
  +   Depends on KVM_CAP_ARM_PSCI.
  +
 
   4.78 KVM_GET_REG_LIST
 
  diff --git a/arch/arm/include/asm/kvm_emulate.h 
  b/arch/arm/include/asm/kvm_emulate.h
  index 4c1a073..ba07de9 100644
  --- a/arch/arm/include/asm/kvm_emulate.h
  +++ b/arch/arm/include/asm/kvm_emulate.h
  @@ -32,6 +32,11 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu);
   void kvm_inject_dabt(struct kvm_vcpu *vcpu, unsigned long addr);
   void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
 
  +static inline bool vcpu_mode_is_32bit(struct kvm_vcpu *vcpu)
  +{
  + return 1;
  +}
  +
   static inline u32 *vcpu_pc(struct kvm_vcpu *vcpu)
   {
return (u32 *)vcpu-arch.regs.usr_regs.ARM_pc;
  diff --git a/arch/arm/include/asm/kvm_host.h 
  b/arch/arm/include/asm/kvm_host.h
  index e65fc96..c9ba918 100644
  --- a/arch/arm/include/asm/kvm_host.h
  +++ b/arch/arm/include/asm/kvm_host.h
  @@ -30,7 +30,7 @@
   #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
   #define KVM_HAVE_ONE_REG
 
  -#define KVM_VCPU_MAX_FEATURES 0
  +#define KVM_VCPU_MAX_FEATURES 1
 
   /* We don't currently support large pages. */
   #define KVM_HPAGE_GFN_SHIFT(x)   0
  @@ -100,6 +100,9 @@ struct kvm_vcpu_arch {
int last_pcpu;
cpumask_t require_dcache_flush;
 
  + /* Don't run the guest: see copy_current_insn() */
 
  Now that you made this field PSCI-specific, how about rewording the comment?
 
 
 yeah, that would work. And actually it's kind of broken, because if we
 sent a paused VCPU a signal and that VCPU is never woken up using the
 PSCI call we'll busy spin in the kernel, which is sad :(
 
 This should eventually be moved to the vcpu requests infrastructure,
 but I'll send some preparatory patches to the kvm first, so let's fix
 it in the right way later on (requires reworking the run loop quite a
 bit), and for now simply fix the pause clause, see patch below.
 
  + bool pause;
  +
/* IO related fields */
struct kvm_decode mmio_decode;
 
  diff --git a/arch/arm/include/asm/kvm_psci.h 
  b/arch/arm/include/asm/kvm_psci.h
  new file mode 100644
  index 000..9a83d98
  --- /dev/null
  +++ b/arch/arm/include/asm/kvm_psci.h
  @@ -0,0 +1,23 @@
  +/*
  + * Copyright (C) 2012 - ARM Ltd
  + * Author: Marc Zyngier marc.zyng...@arm.com
  + *
  + * This program is free software; you can redistribute it and/or modify
  + * it under the terms of the GNU General Public License version 2 as
  + * published by the Free Software Foundation.
  + *
  + * This program is distributed in the hope that it will be useful,
  + * but WITHOUT ANY WARRANTY; without even the implied warranty of
  + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  + * GNU General Public License for more details.
  + *
  + * You should have received a copy of the GNU General Public License
  + * along with this program.  If not, see http://www.gnu.org/licenses/.
  + */
  +
  +#ifndef __ARM_KVM_PSCI_H__
  +#define __ARM_KVM_PSCI_H__
  +
  +bool kvm_psci_call(struct kvm_vcpu *vcpu);
  +
  +#endif /* __ARM_KVM_PSCI_H__ */
  diff --git

Re: [PATCH v6 14/15] KVM: ARM: Power State Coordination Interface implementation

2013-01-20 Thread Christoffer Dall 
On Thu, Jan 17, 2013 at 10:55 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 16/01/13 17:59, Christoffer Dall wrote:
 From: Marc Zyngier marc.zyng...@arm.com

 Implement the PSCI specification (ARM DEN 0022A) to control
 virtual CPUs being powered on or off.

 PSCI/KVM is detected using the KVM_CAP_ARM_PSCI capability.

 A virtual CPU can now be initialized in a powered off state,
 using the KVM_ARM_VCPU_POWER_OFF feature flag.

 The guest can use either SMC or HVC to execute a PSCI function.

 Reviewed-by: Will Deacon will.dea...@arm.com
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 Signed-off-by: Christoffer Dall c.d...@virtualopensystems.com

 A few bits went wrong when you reworked this patch. See below.

 ---
  Documentation/virtual/kvm/api.txt  |4 +
  arch/arm/include/asm/kvm_emulate.h |5 ++
  arch/arm/include/asm/kvm_host.h|5 +-
  arch/arm/include/asm/kvm_psci.h|   23 
  arch/arm/include/uapi/asm/kvm.h|   16 +
  arch/arm/kvm/Makefile  |2 -
  arch/arm/kvm/arm.c |   28 +-
  arch/arm/kvm/psci.c|  105 
 
  include/uapi/linux/kvm.h   |1
  9 files changed, 184 insertions(+), 5 deletions(-)
  create mode 100644 arch/arm/include/asm/kvm_psci.h
  create mode 100644 arch/arm/kvm/psci.c

 diff --git a/Documentation/virtual/kvm/api.txt 
 b/Documentation/virtual/kvm/api.txt
 index 38066a7a..c25439a 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -2185,6 +2185,10 @@ return ENOEXEC for that vcpu.
  Note that because some registers reflect machine topology, all vcpus
  should be created before this ioctl is invoked.

 +Possible features:
 + - KVM_ARM_VCPU_POWER_OFF: Starts the CPU in a power-off state.
 +   Depends on KVM_CAP_ARM_PSCI.
 +

  4.78 KVM_GET_REG_LIST

 diff --git a/arch/arm/include/asm/kvm_emulate.h 
 b/arch/arm/include/asm/kvm_emulate.h
 index 4c1a073..ba07de9 100644
 --- a/arch/arm/include/asm/kvm_emulate.h
 +++ b/arch/arm/include/asm/kvm_emulate.h
 @@ -32,6 +32,11 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu);
  void kvm_inject_dabt(struct kvm_vcpu *vcpu, unsigned long addr);
  void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);

 +static inline bool vcpu_mode_is_32bit(struct kvm_vcpu *vcpu)
 +{
 + return 1;
 +}
 +
  static inline u32 *vcpu_pc(struct kvm_vcpu *vcpu)
  {
   return (u32 *)vcpu-arch.regs.usr_regs.ARM_pc;
 diff --git a/arch/arm/include/asm/kvm_host.h 
 b/arch/arm/include/asm/kvm_host.h
 index e65fc96..c9ba918 100644
 --- a/arch/arm/include/asm/kvm_host.h
 +++ b/arch/arm/include/asm/kvm_host.h
 @@ -30,7 +30,7 @@
  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
  #define KVM_HAVE_ONE_REG

 -#define KVM_VCPU_MAX_FEATURES 0
 +#define KVM_VCPU_MAX_FEATURES 1

  /* We don't currently support large pages. */
  #define KVM_HPAGE_GFN_SHIFT(x)   0
 @@ -100,6 +100,9 @@ struct kvm_vcpu_arch {
   int last_pcpu;
   cpumask_t require_dcache_flush;

 + /* Don't run the guest: see copy_current_insn() */

 Now that you made this field PSCI-specific, how about rewording the comment?


yeah, that would work. And actually it's kind of broken, because if we
sent a paused VCPU a signal and that VCPU is never woken up using the
PSCI call we'll busy spin in the kernel, which is sad :(

This should eventually be moved to the vcpu requests infrastructure,
but I'll send some preparatory patches to the kvm first, so let's fix
it in the right way later on (requires reworking the run loop quite a
bit), and for now simply fix the pause clause, see patch below.

 + bool pause;
 +
   /* IO related fields */
   struct kvm_decode mmio_decode;

 diff --git a/arch/arm/include/asm/kvm_psci.h 
 b/arch/arm/include/asm/kvm_psci.h
 new file mode 100644
 index 000..9a83d98
 --- /dev/null
 +++ b/arch/arm/include/asm/kvm_psci.h
 @@ -0,0 +1,23 @@
 +/*
 + * Copyright (C) 2012 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#ifndef __ARM_KVM_PSCI_H__
 +#define __ARM_KVM_PSCI_H__
 +
 +bool kvm_psci_call(struct kvm_vcpu *vcpu);
 +
 +#endif /* __ARM_KVM_PSCI_H__ */
 diff --git a/arch/arm/include/uapi/asm/kvm.h 
 b/arch/arm/include/uapi/asm/kvm.h
 index bbb6b23..3303ff5 100644
 --- a/arch/arm/include/uapi/asm/kvm.h
 +++ b/arch/arm/include/uapi/asm/kvm.h
 @@ -65,6 +65,8 @@

Re: [PATCH v6 14/15] KVM: ARM: Power State Coordination Interface implementation

2013-01-17 Thread Marc Zyngier 
On 16/01/13 17:59, Christoffer Dall wrote:
 From: Marc Zyngier marc.zyng...@arm.com
 
 Implement the PSCI specification (ARM DEN 0022A) to control
 virtual CPUs being powered on or off.
 
 PSCI/KVM is detected using the KVM_CAP_ARM_PSCI capability.
 
 A virtual CPU can now be initialized in a powered off state,
 using the KVM_ARM_VCPU_POWER_OFF feature flag.
 
 The guest can use either SMC or HVC to execute a PSCI function.
 
 Reviewed-by: Will Deacon will.dea...@arm.com
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 Signed-off-by: Christoffer Dall c.d...@virtualopensystems.com

A few bits went wrong when you reworked this patch. See below.

 ---
  Documentation/virtual/kvm/api.txt  |4 +
  arch/arm/include/asm/kvm_emulate.h |5 ++
  arch/arm/include/asm/kvm_host.h|5 +-
  arch/arm/include/asm/kvm_psci.h|   23 
  arch/arm/include/uapi/asm/kvm.h|   16 +
  arch/arm/kvm/Makefile  |2 -
  arch/arm/kvm/arm.c |   28 +-
  arch/arm/kvm/psci.c|  105 
 
  include/uapi/linux/kvm.h   |1 
  9 files changed, 184 insertions(+), 5 deletions(-)
  create mode 100644 arch/arm/include/asm/kvm_psci.h
  create mode 100644 arch/arm/kvm/psci.c
 
 diff --git a/Documentation/virtual/kvm/api.txt 
 b/Documentation/virtual/kvm/api.txt
 index 38066a7a..c25439a 100644
 --- a/Documentation/virtual/kvm/api.txt
 +++ b/Documentation/virtual/kvm/api.txt
 @@ -2185,6 +2185,10 @@ return ENOEXEC for that vcpu.
  Note that because some registers reflect machine topology, all vcpus
  should be created before this ioctl is invoked.
  
 +Possible features:
 + - KVM_ARM_VCPU_POWER_OFF: Starts the CPU in a power-off state.
 +   Depends on KVM_CAP_ARM_PSCI.
 +
  
  4.78 KVM_GET_REG_LIST
  
 diff --git a/arch/arm/include/asm/kvm_emulate.h 
 b/arch/arm/include/asm/kvm_emulate.h
 index 4c1a073..ba07de9 100644
 --- a/arch/arm/include/asm/kvm_emulate.h
 +++ b/arch/arm/include/asm/kvm_emulate.h
 @@ -32,6 +32,11 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu);
  void kvm_inject_dabt(struct kvm_vcpu *vcpu, unsigned long addr);
  void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
  
 +static inline bool vcpu_mode_is_32bit(struct kvm_vcpu *vcpu)
 +{
 + return 1;
 +}
 +
  static inline u32 *vcpu_pc(struct kvm_vcpu *vcpu)
  {
   return (u32 *)vcpu-arch.regs.usr_regs.ARM_pc;
 diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
 index e65fc96..c9ba918 100644
 --- a/arch/arm/include/asm/kvm_host.h
 +++ b/arch/arm/include/asm/kvm_host.h
 @@ -30,7 +30,7 @@
  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
  #define KVM_HAVE_ONE_REG
  
 -#define KVM_VCPU_MAX_FEATURES 0
 +#define KVM_VCPU_MAX_FEATURES 1
  
  /* We don't currently support large pages. */
  #define KVM_HPAGE_GFN_SHIFT(x)   0
 @@ -100,6 +100,9 @@ struct kvm_vcpu_arch {
   int last_pcpu;
   cpumask_t require_dcache_flush;
  
 + /* Don't run the guest: see copy_current_insn() */

Now that you made this field PSCI-specific, how about rewording the comment?

 + bool pause;
 +
   /* IO related fields */
   struct kvm_decode mmio_decode;
  
 diff --git a/arch/arm/include/asm/kvm_psci.h b/arch/arm/include/asm/kvm_psci.h
 new file mode 100644
 index 000..9a83d98
 --- /dev/null
 +++ b/arch/arm/include/asm/kvm_psci.h
 @@ -0,0 +1,23 @@
 +/*
 + * Copyright (C) 2012 - ARM Ltd
 + * Author: Marc Zyngier marc.zyng...@arm.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#ifndef __ARM_KVM_PSCI_H__
 +#define __ARM_KVM_PSCI_H__
 +
 +bool kvm_psci_call(struct kvm_vcpu *vcpu);
 +
 +#endif /* __ARM_KVM_PSCI_H__ */
 diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
 index bbb6b23..3303ff5 100644
 --- a/arch/arm/include/uapi/asm/kvm.h
 +++ b/arch/arm/include/uapi/asm/kvm.h
 @@ -65,6 +65,8 @@ struct kvm_regs {
  #define KVM_ARM_TARGET_CORTEX_A150
  #define KVM_ARM_NUM_TARGETS  1
  
 +#define KVM_ARM_VCPU_POWER_OFF   0 /* CPU is started in OFF 
 state */
 +
  struct kvm_vcpu_init {
   __u32 target;
   __u32 features[7];
 @@ -145,4 +147,18 @@ struct kvm_arch_memory_slot {
  /* Highest supported SPI, from VGIC_NR_IRQS */
  #define KVM_ARM_IRQ_GIC_MAX  127
  
 +/* PSCI interface */
 +#define KVM_PSCI_FN_BASE 0x95c1ba5e
 +#define KVM_PSCI_FN(n)