All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64
@ 2014-01-21 13:01 Anup Patel
  2014-01-21 13:01 ` [RFC PATCH 1/3] KVM: Add capability to advertise PSCI v0.2 support Anup Patel
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Anup Patel @ 2014-01-21 13:01 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, KVM ARM/ARM64 only provides in-kernel emulation of Power State
and Coordination Interface (PSCI) v0.1.

This patchset aims at providing newer PSCI v0.2 for KVM ARM/ARM64 VCPUs
such that it does not break current KVM ARM/ARM64 ABI. Also, the patchset
provides emulation of only few PSCI v0.2 functions such as PSCI_VERSION,
CPU_ON, and CPU_OFF. Emulation of other PSCI v0.2 functions will be added
later.

The user space tools (i.e. QEMU or KVMTOOL) will have to explicitly enable
KVM_ARM_VCPU_PSCI_0_2 feature using KVM_ARM_VCPU_INIT ioctl for providing
PSCI v0.2 to VCPUs. 

Anup Patel (3):
  KVM: Add capability to advertise PSCI v0.2 support
  ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
  KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature

 Documentation/virtual/kvm/api.txt |    2 +
 arch/arm/include/asm/kvm_host.h   |    2 +-
 arch/arm/include/uapi/asm/kvm.h   |   39 ++++++++++++++++--
 arch/arm/kvm/arm.c                |    6 ++-
 arch/arm/kvm/psci.c               |   79 ++++++++++++++++++++++++++++++-------
 arch/arm64/include/asm/kvm_host.h |    2 +-
 arch/arm64/include/uapi/asm/kvm.h |   39 ++++++++++++++++--
 include/uapi/linux/kvm.h          |    1 +
 8 files changed, 146 insertions(+), 24 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 1/3] KVM: Add capability to advertise PSCI v0.2 support
  2014-01-21 13:01 [RFC PATCH 0/3] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
@ 2014-01-21 13:01 ` Anup Patel
  2014-01-21 13:01 ` [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation Anup Patel
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2014-01-21 13:01 UTC (permalink / raw)
  To: linux-arm-kernel

User space (i.e. QEMU or KVMTOOL) should be able to check whether KVM
ARM/ARM64 supports in-kernel PSCI v0.2 emulation. For this purpose, we
define KVM_CAP_ARM_PSCI_0_2 in KVM user space interface header.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 include/uapi/linux/kvm.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 902f124..d64349e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -674,6 +674,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_ARM_EL1_32BIT 93
 #define KVM_CAP_SPAPR_MULTITCE 94
 #define KVM_CAP_EXT_EMUL_CPUID 95
+#define KVM_CAP_ARM_PSCI_0_2 96
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
  2014-01-21 13:01 [RFC PATCH 0/3] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
  2014-01-21 13:01 ` [RFC PATCH 1/3] KVM: Add capability to advertise PSCI v0.2 support Anup Patel
@ 2014-01-21 13:01 ` Anup Patel
  2014-01-28 11:27   ` Marc Zyngier
  2014-01-28 21:04   ` Christoffer Dall
  2014-01-21 13:01 ` [RFC PATCH 3/3] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature Anup Patel
  2014-01-28  4:20 ` [RFC PATCH 0/3] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
  3 siblings, 2 replies; 14+ messages in thread
From: Anup Patel @ 2014-01-21 13:01 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, the in-kernel PSCI emulation provides PSCI v0.1 interface to
VCPUs. This patch extends current in-kernel PSCI emulation to provide
PSCI v0.2 interface to VCPUs.

By default, ARM/ARM64 KVM will always provide PSCI v0.1 interface for
keeping the ABI backward-compatible.

To select PSCI v0.2 interface for VCPUs, the user space (i.e. QEMU or
KVMTOOL) will have to set KVM_ARM_VCPU_PSCI_0_2 feature when doing VCPU
init using KVM_ARM_VCPU_INIT ioctl.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 arch/arm/include/asm/kvm_host.h   |    2 +-
 arch/arm/include/uapi/asm/kvm.h   |   39 ++++++++++++++++--
 arch/arm/kvm/arm.c                |    6 ++-
 arch/arm/kvm/psci.c               |   79 ++++++++++++++++++++++++++++++-------
 arch/arm64/include/asm/kvm_host.h |    2 +-
 arch/arm64/include/uapi/asm/kvm.h |   39 ++++++++++++++++--
 6 files changed, 143 insertions(+), 24 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 8a6f6db..0239ac5 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -36,7 +36,7 @@
 #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
 #define KVM_HAVE_ONE_REG
 
-#define KVM_VCPU_MAX_FEATURES 1
+#define KVM_VCPU_MAX_FEATURES 2
 
 #include <kvm/arm_vgic.h>
 
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index c498b60..d9eb74c 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -83,6 +83,7 @@ struct kvm_regs {
 #define KVM_VGIC_V2_CPU_SIZE		0x2000
 
 #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
+#define KVM_ARM_VCPU_PSCI_0_2		1 /* CPU uses PSCI v0.2 */
 
 struct kvm_vcpu_init {
 	__u32 target;
@@ -164,7 +165,7 @@ struct kvm_arch_memory_slot {
 /* Highest supported SPI, from VGIC_NR_IRQS */
 #define KVM_ARM_IRQ_GIC_MAX		127
 
-/* PSCI interface */
+/* PSCI v0.1 interface */
 #define KVM_PSCI_FN_BASE		0x95c1ba5e
 #define KVM_PSCI_FN(n)			(KVM_PSCI_FN_BASE + (n))
 
@@ -173,9 +174,41 @@ struct kvm_arch_memory_slot {
 #define KVM_PSCI_FN_CPU_ON		KVM_PSCI_FN(2)
 #define KVM_PSCI_FN_MIGRATE		KVM_PSCI_FN(3)
 
+/* PSCI v0.2 interface */
+#define KVM_PSCI_0_2_FN_BASE		0x84000000
+#define KVM_PSCI_0_2_FN(n)		(KVM_PSCI_0_2_FN_BASE + (n))
+#define KVM_PSCI_0_2_FN64_BASE		0xC4000000
+#define KVM_PSCI_0_2_FN64(n)		(KVM_PSCI_0_2_FN64_BASE + (n))
+
+#define KVM_PSCI_0_2_FN_PSCI_VERSION	KVM_PSCI_0_2_FN(0)
+#define KVM_PSCI_0_2_FN_CPU_SUSPEND	KVM_PSCI_0_2_FN(1)
+#define KVM_PSCI_0_2_FN_CPU_OFF		KVM_PSCI_0_2_FN(2)
+#define KVM_PSCI_0_2_FN_CPU_ON		KVM_PSCI_0_2_FN(3)
+#define KVM_PSCI_0_2_FN_AFFINITY_INFO	KVM_PSCI_0_2_FN(4)
+#define KVM_PSCI_0_2_FN_MIGRATE		KVM_PSCI_0_2_FN(5)
+#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
+					KVM_PSCI_0_2_FN(6)
+#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
+					KVM_PSCI_0_2_FN(7)
+#define KVM_PSCI_0_2_FN_SYSTEM_OFF	KVM_PSCI_0_2_FN(8)
+#define KVM_PSCI_0_2_FN_SYSTEM_RESET	KVM_PSCI_0_2_FN(9)
+
+#define KVM_PSCI_0_2_FN64_CPU_SUSPEND	KVM_PSCI_0_2_FN64(1)
+#define KVM_PSCI_0_2_FN64_CPU_ON	KVM_PSCI_0_2_FN64(3)
+#define KVM_PSCI_0_2_FN64_AFFINITY_INFO	KVM_PSCI_0_2_FN64(4)
+#define KVM_PSCI_0_2_FN64_MIGRATE	KVM_PSCI_0_2_FN64(5)
+#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
+					KVM_PSCI_0_2_FN64(7)
+
+/* PSCI return values */
 #define KVM_PSCI_RET_SUCCESS		0
-#define KVM_PSCI_RET_NI			((unsigned long)-1)
-#define KVM_PSCI_RET_INVAL		((unsigned long)-2)
+#define KVM_PSCI_RET_NOT_SUPPORTED	((unsigned long)-1)
+#define KVM_PSCI_RET_INVALID_PARAMS	((unsigned long)-2)
 #define KVM_PSCI_RET_DENIED		((unsigned long)-3)
+#define KVM_PSCI_RET_ALREADY_ON		((unsigned long)-4)
+#define KVM_PSCI_RET_ON_PENDING		((unsigned long)-5)
+#define KVM_PSCI_RET_INTERNAL_FAILURE	((unsigned long)-6)
+#define KVM_PSCI_RET_NOT_PRESENT	((unsigned long)-7)
+#define KVM_PSCI_RET_DISABLED		((unsigned long)-8)
 
 #endif /* __ARM_KVM_H__ */
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 2a700e0..0b7817a 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -193,6 +193,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
 	case KVM_CAP_ONE_REG:
 	case KVM_CAP_ARM_PSCI:
+	case KVM_CAP_ARM_PSCI_0_2:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:
@@ -483,7 +484,10 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
 	 * PSCI code.
 	 */
 	if (test_and_clear_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features)) {
-		*vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
+		if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
+			*vcpu_reg(vcpu, 0) = KVM_PSCI_0_2_FN_CPU_OFF;
+		else
+			*vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
 		kvm_psci_call(vcpu);
 	}
 
diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index 0881bf1..ee044a3 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -55,13 +55,13 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	}
 
 	if (!vcpu)
-		return KVM_PSCI_RET_INVAL;
+		return KVM_PSCI_RET_INVALID_PARAMS;
 
 	target_pc = *vcpu_reg(source_vcpu, 2);
 
 	wq = kvm_arch_vcpu_wq(vcpu);
 	if (!waitqueue_active(wq))
-		return KVM_PSCI_RET_INVAL;
+		return KVM_PSCI_RET_INVALID_PARAMS;
 
 	kvm_reset_vcpu(vcpu);
 
@@ -84,17 +84,49 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	return KVM_PSCI_RET_SUCCESS;
 }
 
-/**
- * kvm_psci_call - handle PSCI call if r0 value is in range
- * @vcpu: Pointer to the VCPU struct
- *
- * Handle PSCI calls from guests through traps from HVC instructions.
- * The calling convention is similar to SMC calls to the secure world where
- * the function number is placed in r0 and this function returns true if the
- * function number specified in r0 is withing the PSCI range, and false
- * otherwise.
- */
-bool kvm_psci_call(struct kvm_vcpu *vcpu)
+static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
+{
+	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
+	unsigned long val;
+
+	switch (psci_fn) {
+	case KVM_PSCI_0_2_FN_PSCI_VERSION:
+		/*
+		 * Bits[31:16] = Major Version = 0
+		 * Bits[15:0] = Minor Version = 2
+		 */
+		val = 2;
+		break;
+	case KVM_PSCI_0_2_FN_CPU_OFF:
+		kvm_psci_vcpu_off(vcpu);
+		val = KVM_PSCI_RET_SUCCESS;
+		break;
+	case KVM_PSCI_0_2_FN_CPU_ON:
+	case KVM_PSCI_0_2_FN64_CPU_ON:
+		val = kvm_psci_vcpu_on(vcpu);
+		break;
+	case KVM_PSCI_0_2_FN_CPU_SUSPEND:
+	case KVM_PSCI_0_2_FN_AFFINITY_INFO:
+	case KVM_PSCI_0_2_FN_MIGRATE:
+	case KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
+	case KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
+	case KVM_PSCI_0_2_FN_SYSTEM_OFF:
+	case KVM_PSCI_0_2_FN_SYSTEM_RESET:
+	case KVM_PSCI_0_2_FN64_CPU_SUSPEND:
+	case KVM_PSCI_0_2_FN64_AFFINITY_INFO:
+	case KVM_PSCI_0_2_FN64_MIGRATE:
+	case KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
+		val = KVM_PSCI_RET_NOT_SUPPORTED;
+		break;
+	default:
+		return false;
+	}
+
+	*vcpu_reg(vcpu, 0) = val;
+	return true;
+}
+
+static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
 {
 	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
 	unsigned long val;
@@ -109,9 +141,8 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
 		break;
 	case KVM_PSCI_FN_CPU_SUSPEND:
 	case KVM_PSCI_FN_MIGRATE:
-		val = KVM_PSCI_RET_NI;
+		val = KVM_PSCI_RET_NOT_SUPPORTED;
 		break;
-
 	default:
 		return false;
 	}
@@ -119,3 +150,21 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
 	*vcpu_reg(vcpu, 0) = val;
 	return true;
 }
+
+/**
+ * kvm_psci_call - handle PSCI call if r0 value is in range
+ * @vcpu: Pointer to the VCPU struct
+ *
+ * Handle PSCI calls from guests through traps from HVC instructions.
+ * The calling convention is similar to SMC calls to the secure world where
+ * the function number is placed in r0 and this function returns true if the
+ * function number specified in r0 is withing the PSCI range, and false
+ * otherwise.
+ */
+bool kvm_psci_call(struct kvm_vcpu *vcpu)
+{
+	if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
+		return kvm_psci_0_2_call(vcpu);
+
+	return kvm_psci_0_1_call(vcpu);
+}
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 0a1d697..92242ce 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -39,7 +39,7 @@
 #include <kvm/arm_vgic.h>
 #include <kvm/arm_arch_timer.h>
 
-#define KVM_VCPU_MAX_FEATURES 2
+#define KVM_VCPU_MAX_FEATURES 3
 
 struct kvm_vcpu;
 int kvm_target_cpu(void);
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index d9f026b..0eb254d 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -77,6 +77,7 @@ struct kvm_regs {
 
 #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
 #define KVM_ARM_VCPU_EL1_32BIT		1 /* CPU running a 32bit VM */
+#define KVM_ARM_VCPU_PSCI_0_2		2 /* CPU uses PSCI v0.2 */
 
 struct kvm_vcpu_init {
 	__u32 target;
@@ -150,7 +151,7 @@ struct kvm_arch_memory_slot {
 /* Highest supported SPI, from VGIC_NR_IRQS */
 #define KVM_ARM_IRQ_GIC_MAX		127
 
-/* PSCI interface */
+/* PSCI v0.1 interface */
 #define KVM_PSCI_FN_BASE		0x95c1ba5e
 #define KVM_PSCI_FN(n)			(KVM_PSCI_FN_BASE + (n))
 
@@ -159,10 +160,42 @@ struct kvm_arch_memory_slot {
 #define KVM_PSCI_FN_CPU_ON		KVM_PSCI_FN(2)
 #define KVM_PSCI_FN_MIGRATE		KVM_PSCI_FN(3)
 
+/* PSCI v0.2 interface */
+#define KVM_PSCI_0_2_FN_BASE		0x84000000
+#define KVM_PSCI_0_2_FN(n)		(KVM_PSCI_0_2_FN_BASE + (n))
+#define KVM_PSCI_0_2_FN64_BASE		0xC4000000
+#define KVM_PSCI_0_2_FN64(n)		(KVM_PSCI_0_2_FN64_BASE + (n))
+
+#define KVM_PSCI_0_2_FN_PSCI_VERSION	KVM_PSCI_0_2_FN(0)
+#define KVM_PSCI_0_2_FN_CPU_SUSPEND	KVM_PSCI_0_2_FN(1)
+#define KVM_PSCI_0_2_FN_CPU_OFF		KVM_PSCI_0_2_FN(2)
+#define KVM_PSCI_0_2_FN_CPU_ON		KVM_PSCI_0_2_FN(3)
+#define KVM_PSCI_0_2_FN_AFFINITY_INFO	KVM_PSCI_0_2_FN(4)
+#define KVM_PSCI_0_2_FN_MIGRATE		KVM_PSCI_0_2_FN(5)
+#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
+					KVM_PSCI_0_2_FN(6)
+#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
+					KVM_PSCI_0_2_FN(7)
+#define KVM_PSCI_0_2_FN_SYSTEM_OFF	KVM_PSCI_0_2_FN(8)
+#define KVM_PSCI_0_2_FN_SYSTEM_RESET	KVM_PSCI_0_2_FN(9)
+
+#define KVM_PSCI_0_2_FN64_CPU_SUSPEND	KVM_PSCI_0_2_FN64(1)
+#define KVM_PSCI_0_2_FN64_CPU_ON	KVM_PSCI_0_2_FN64(3)
+#define KVM_PSCI_0_2_FN64_AFFINITY_INFO	KVM_PSCI_0_2_FN64(4)
+#define KVM_PSCI_0_2_FN64_MIGRATE	KVM_PSCI_0_2_FN64(5)
+#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
+					KVM_PSCI_0_2_FN64(7)
+
+/* PSCI return values */
 #define KVM_PSCI_RET_SUCCESS		0
-#define KVM_PSCI_RET_NI			((unsigned long)-1)
-#define KVM_PSCI_RET_INVAL		((unsigned long)-2)
+#define KVM_PSCI_RET_NOT_SUPPORTED	((unsigned long)-1)
+#define KVM_PSCI_RET_INVALID_PARAMS	((unsigned long)-2)
 #define KVM_PSCI_RET_DENIED		((unsigned long)-3)
+#define KVM_PSCI_RET_ALREADY_ON		((unsigned long)-4)
+#define KVM_PSCI_RET_ON_PENDING		((unsigned long)-5)
+#define KVM_PSCI_RET_INTERNAL_FAILURE	((unsigned long)-6)
+#define KVM_PSCI_RET_NOT_PRESENT	((unsigned long)-7)
+#define KVM_PSCI_RET_DISABLED		((unsigned long)-8)
 
 #endif
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 3/3] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature
  2014-01-21 13:01 [RFC PATCH 0/3] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
  2014-01-21 13:01 ` [RFC PATCH 1/3] KVM: Add capability to advertise PSCI v0.2 support Anup Patel
  2014-01-21 13:01 ` [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation Anup Patel
@ 2014-01-21 13:01 ` Anup Patel
  2014-01-28 21:05   ` Christoffer Dall
  2014-01-28  4:20 ` [RFC PATCH 0/3] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
  3 siblings, 1 reply; 14+ messages in thread
From: Anup Patel @ 2014-01-21 13:01 UTC (permalink / raw)
  To: linux-arm-kernel

We have in-kernel emulation of PSCI v0.2 in KVM ARM/ARM64. To provide
PSCI v0.2 interface to VCPUs, we have to enable KVM_ARM_VCPU_PSCI_0_2
feature when doing KVM_ARM_VCPU_INIT ioctl.

The patch updates documentation of KVM_ARM_VCPU_INIT ioctl to provide
info regarding KVM_ARM_VCPU_PSCI_0_2 feature.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 Documentation/virtual/kvm/api.txt |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index aad3244..a15fcdd 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2346,6 +2346,8 @@ Possible features:
 	  Depends on KVM_CAP_ARM_PSCI.
 	- KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
 	  Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
+	- KVM_ARM_VCPU_PSCI_0_2: Emulate PSCI v0.2 for CPU.
+	  Depends on KVM_CAP_ARM_PSCI_0_2.
 
 
 4.83 KVM_ARM_PREFERRED_TARGET
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 0/3] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64
  2014-01-21 13:01 [RFC PATCH 0/3] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
                   ` (2 preceding siblings ...)
  2014-01-21 13:01 ` [RFC PATCH 3/3] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature Anup Patel
@ 2014-01-28  4:20 ` Anup Patel
  3 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2014-01-28  4:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jan 21, 2014 at 6:31 PM, Anup Patel <anup.patel@linaro.org> wrote:
> Currently, KVM ARM/ARM64 only provides in-kernel emulation of Power State
> and Coordination Interface (PSCI) v0.1.
>
> This patchset aims at providing newer PSCI v0.2 for KVM ARM/ARM64 VCPUs
> such that it does not break current KVM ARM/ARM64 ABI. Also, the patchset
> provides emulation of only few PSCI v0.2 functions such as PSCI_VERSION,
> CPU_ON, and CPU_OFF. Emulation of other PSCI v0.2 functions will be added
> later.
>
> The user space tools (i.e. QEMU or KVMTOOL) will have to explicitly enable
> KVM_ARM_VCPU_PSCI_0_2 feature using KVM_ARM_VCPU_INIT ioctl for providing
> PSCI v0.2 to VCPUs.
>
> Anup Patel (3):
>   KVM: Add capability to advertise PSCI v0.2 support
>   ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
>   KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature
>
>  Documentation/virtual/kvm/api.txt |    2 +
>  arch/arm/include/asm/kvm_host.h   |    2 +-
>  arch/arm/include/uapi/asm/kvm.h   |   39 ++++++++++++++++--
>  arch/arm/kvm/arm.c                |    6 ++-
>  arch/arm/kvm/psci.c               |   79 ++++++++++++++++++++++++++++++-------
>  arch/arm64/include/asm/kvm_host.h |    2 +-
>  arch/arm64/include/uapi/asm/kvm.h |   39 ++++++++++++++++--
>  include/uapi/linux/kvm.h          |    1 +
>  8 files changed, 146 insertions(+), 24 deletions(-)
>
> --
> 1.7.9.5
>
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

Hi All,

Please provide feedback and comments on this patchset.

Regards,
Anup

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
  2014-01-21 13:01 ` [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation Anup Patel
@ 2014-01-28 11:27   ` Marc Zyngier
  2014-01-28 11:43     ` Anup Patel
  2014-01-28 21:04   ` Christoffer Dall
  1 sibling, 1 reply; 14+ messages in thread
From: Marc Zyngier @ 2014-01-28 11:27 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Anup,

On 21/01/14 13:01, Anup Patel wrote:
> Currently, the in-kernel PSCI emulation provides PSCI v0.1 interface to
> VCPUs. This patch extends current in-kernel PSCI emulation to provide
> PSCI v0.2 interface to VCPUs.
> 
> By default, ARM/ARM64 KVM will always provide PSCI v0.1 interface for
> keeping the ABI backward-compatible.
> 
> To select PSCI v0.2 interface for VCPUs, the user space (i.e. QEMU or
> KVMTOOL) will have to set KVM_ARM_VCPU_PSCI_0_2 feature when doing VCPU
> init using KVM_ARM_VCPU_INIT ioctl.
> 
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> ---
>  arch/arm/include/asm/kvm_host.h   |    2 +-
>  arch/arm/include/uapi/asm/kvm.h   |   39 ++++++++++++++++--
>  arch/arm/kvm/arm.c                |    6 ++-
>  arch/arm/kvm/psci.c               |   79 ++++++++++++++++++++++++++++++-------
>  arch/arm64/include/asm/kvm_host.h |    2 +-
>  arch/arm64/include/uapi/asm/kvm.h |   39 ++++++++++++++++--
>  6 files changed, 143 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 8a6f6db..0239ac5 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -36,7 +36,7 @@
>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>  #define KVM_HAVE_ONE_REG
> 
> -#define KVM_VCPU_MAX_FEATURES 1
> +#define KVM_VCPU_MAX_FEATURES 2
> 
>  #include <kvm/arm_vgic.h>
> 
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index c498b60..d9eb74c 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -83,6 +83,7 @@ struct kvm_regs {
>  #define KVM_VGIC_V2_CPU_SIZE           0x2000
> 
>  #define KVM_ARM_VCPU_POWER_OFF         0 /* CPU is started in OFF state */
> +#define KVM_ARM_VCPU_PSCI_0_2          1 /* CPU uses PSCI v0.2 */
> 
>  struct kvm_vcpu_init {
>         __u32 target;
> @@ -164,7 +165,7 @@ struct kvm_arch_memory_slot {
>  /* Highest supported SPI, from VGIC_NR_IRQS */
>  #define KVM_ARM_IRQ_GIC_MAX            127
> 
> -/* PSCI interface */
> +/* PSCI v0.1 interface */
>  #define KVM_PSCI_FN_BASE               0x95c1ba5e
>  #define KVM_PSCI_FN(n)                 (KVM_PSCI_FN_BASE + (n))
> 
> @@ -173,9 +174,41 @@ struct kvm_arch_memory_slot {
>  #define KVM_PSCI_FN_CPU_ON             KVM_PSCI_FN(2)
>  #define KVM_PSCI_FN_MIGRATE            KVM_PSCI_FN(3)
> 
> +/* PSCI v0.2 interface */
> +#define KVM_PSCI_0_2_FN_BASE           0x84000000
> +#define KVM_PSCI_0_2_FN(n)             (KVM_PSCI_0_2_FN_BASE + (n))
> +#define KVM_PSCI_0_2_FN64_BASE         0xC4000000
> +#define KVM_PSCI_0_2_FN64(n)           (KVM_PSCI_0_2_FN64_BASE + (n))
> +
> +#define KVM_PSCI_0_2_FN_PSCI_VERSION   KVM_PSCI_0_2_FN(0)
> +#define KVM_PSCI_0_2_FN_CPU_SUSPEND    KVM_PSCI_0_2_FN(1)
> +#define KVM_PSCI_0_2_FN_CPU_OFF                KVM_PSCI_0_2_FN(2)
> +#define KVM_PSCI_0_2_FN_CPU_ON         KVM_PSCI_0_2_FN(3)
> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO  KVM_PSCI_0_2_FN(4)
> +#define KVM_PSCI_0_2_FN_MIGRATE                KVM_PSCI_0_2_FN(5)
> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
> +                                       KVM_PSCI_0_2_FN(6)
> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
> +                                       KVM_PSCI_0_2_FN(7)
> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF     KVM_PSCI_0_2_FN(8)
> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET   KVM_PSCI_0_2_FN(9)
> +
> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND  KVM_PSCI_0_2_FN64(1)
> +#define KVM_PSCI_0_2_FN64_CPU_ON       KVM_PSCI_0_2_FN64(3)
> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO        KVM_PSCI_0_2_FN64(4)
> +#define KVM_PSCI_0_2_FN64_MIGRATE      KVM_PSCI_0_2_FN64(5)
> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
> +                                       KVM_PSCI_0_2_FN64(7)
> +
> +/* PSCI return values */
>  #define KVM_PSCI_RET_SUCCESS           0
> -#define KVM_PSCI_RET_NI                        ((unsigned long)-1)
> -#define KVM_PSCI_RET_INVAL             ((unsigned long)-2)

This is a massive no-no. It is already part of the userspace API, and we
cannot break it (neither arm nor arm64). These #defines have to stay
forever.

> +#define KVM_PSCI_RET_NOT_SUPPORTED     ((unsigned long)-1)
> +#define KVM_PSCI_RET_INVALID_PARAMS    ((unsigned long)-2)

As such, you can drop these two defines, and use the original ones in
your new code, which will reduce the churn.

>  #define KVM_PSCI_RET_DENIED            ((unsigned long)-3)
> +#define KVM_PSCI_RET_ALREADY_ON                ((unsigned long)-4)
> +#define KVM_PSCI_RET_ON_PENDING                ((unsigned long)-5)
> +#define KVM_PSCI_RET_INTERNAL_FAILURE  ((unsigned long)-6)
> +#define KVM_PSCI_RET_NOT_PRESENT       ((unsigned long)-7)
> +#define KVM_PSCI_RET_DISABLED          ((unsigned long)-8)
> 
>  #endif /* __ARM_KVM_H__ */
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 2a700e0..0b7817a 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -193,6 +193,7 @@ int kvm_dev_ioctl_check_extension(long ext)
>         case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
>         case KVM_CAP_ONE_REG:
>         case KVM_CAP_ARM_PSCI:
> +       case KVM_CAP_ARM_PSCI_0_2:
>                 r = 1;
>                 break;
>         case KVM_CAP_COALESCED_MMIO:
> @@ -483,7 +484,10 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
>          * PSCI code.
>          */
>         if (test_and_clear_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features)) {
> -               *vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
> +               if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))

Can you make this a function? Something like:

int get_psci_version(struct kvm_vcpu *vcpu)
{
	if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
		return PSCI_VERSION_0_2;

	return PSCI_VERSION_0_1;
}

probably located in psci.c. This way, when PSCI version 3.14159 comes
along, we'll have some mechanism in place, and we're free to change the
detection method without impacting the rest of the code.

> +                       *vcpu_reg(vcpu, 0) = KVM_PSCI_0_2_FN_CPU_OFF;
> +               else
> +                       *vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
>                 kvm_psci_call(vcpu);
>         }
> 
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 0881bf1..ee044a3 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -55,13 +55,13 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>         }
> 
>         if (!vcpu)
> -               return KVM_PSCI_RET_INVAL;
> +               return KVM_PSCI_RET_INVALID_PARAMS;
> 
>         target_pc = *vcpu_reg(source_vcpu, 2);
> 
>         wq = kvm_arch_vcpu_wq(vcpu);
>         if (!waitqueue_active(wq))
> -               return KVM_PSCI_RET_INVAL;
> +               return KVM_PSCI_RET_INVALID_PARAMS;
> 
>         kvm_reset_vcpu(vcpu);
> 
> @@ -84,17 +84,49 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>         return KVM_PSCI_RET_SUCCESS;
>  }
> 
> -/**
> - * kvm_psci_call - handle PSCI call if r0 value is in range
> - * @vcpu: Pointer to the VCPU struct
> - *
> - * Handle PSCI calls from guests through traps from HVC instructions.
> - * The calling convention is similar to SMC calls to the secure world where
> - * the function number is placed in r0 and this function returns true if the
> - * function number specified in r0 is withing the PSCI range, and false
> - * otherwise.
> - */
> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
> +static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> +{
> +       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
> +       unsigned long val;
> +
> +       switch (psci_fn) {
> +       case KVM_PSCI_0_2_FN_PSCI_VERSION:
> +               /*
> +                * Bits[31:16] = Major Version = 0
> +                * Bits[15:0] = Minor Version = 2
> +                */
> +               val = 2;
> +               break;
> +       case KVM_PSCI_0_2_FN_CPU_OFF:
> +               kvm_psci_vcpu_off(vcpu);
> +               val = KVM_PSCI_RET_SUCCESS;
> +               break;
> +       case KVM_PSCI_0_2_FN_CPU_ON:
> +       case KVM_PSCI_0_2_FN64_CPU_ON:
> +               val = kvm_psci_vcpu_on(vcpu);
> +               break;
> +       case KVM_PSCI_0_2_FN_CPU_SUSPEND:
> +       case KVM_PSCI_0_2_FN_AFFINITY_INFO:
> +       case KVM_PSCI_0_2_FN_MIGRATE:
> +       case KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
> +       case KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
> +       case KVM_PSCI_0_2_FN_SYSTEM_OFF:
> +       case KVM_PSCI_0_2_FN_SYSTEM_RESET:
> +       case KVM_PSCI_0_2_FN64_CPU_SUSPEND:
> +       case KVM_PSCI_0_2_FN64_AFFINITY_INFO:
> +       case KVM_PSCI_0_2_FN64_MIGRATE:
> +       case KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
> +               val = KVM_PSCI_RET_NOT_SUPPORTED;
> +               break;
> +       default:
> +               return false;
> +       }
> +
> +       *vcpu_reg(vcpu, 0) = val;
> +       return true;
> +}
> +
> +static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>  {
>         unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>         unsigned long val;
> @@ -109,9 +141,8 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>                 break;
>         case KVM_PSCI_FN_CPU_SUSPEND:
>         case KVM_PSCI_FN_MIGRATE:
> -               val = KVM_PSCI_RET_NI;
> +               val = KVM_PSCI_RET_NOT_SUPPORTED;
>                 break;
> -
>         default:
>                 return false;
>         }
> @@ -119,3 +150,21 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>         *vcpu_reg(vcpu, 0) = val;
>         return true;
>  }
> +
> +/**
> + * kvm_psci_call - handle PSCI call if r0 value is in range
> + * @vcpu: Pointer to the VCPU struct
> + *
> + * Handle PSCI calls from guests through traps from HVC instructions.
> + * The calling convention is similar to SMC calls to the secure world where
> + * the function number is placed in r0 and this function returns true if the
> + * function number specified in r0 is withing the PSCI range, and false
> + * otherwise.
> + */
> +bool kvm_psci_call(struct kvm_vcpu *vcpu)
> +{
> +       if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
> +               return kvm_psci_0_2_call(vcpu);
> +
> +       return kvm_psci_0_1_call(vcpu);
> +}
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 0a1d697..92242ce 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -39,7 +39,7 @@
>  #include <kvm/arm_vgic.h>
>  #include <kvm/arm_arch_timer.h>
> 
> -#define KVM_VCPU_MAX_FEATURES 2
> +#define KVM_VCPU_MAX_FEATURES 3
> 
>  struct kvm_vcpu;
>  int kvm_target_cpu(void);
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index d9f026b..0eb254d 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -77,6 +77,7 @@ struct kvm_regs {
> 
>  #define KVM_ARM_VCPU_POWER_OFF         0 /* CPU is started in OFF state */
>  #define KVM_ARM_VCPU_EL1_32BIT         1 /* CPU running a 32bit VM */
> +#define KVM_ARM_VCPU_PSCI_0_2          2 /* CPU uses PSCI v0.2 */
> 
>  struct kvm_vcpu_init {
>         __u32 target;
> @@ -150,7 +151,7 @@ struct kvm_arch_memory_slot {
>  /* Highest supported SPI, from VGIC_NR_IRQS */
>  #define KVM_ARM_IRQ_GIC_MAX            127
> 
> -/* PSCI interface */
> +/* PSCI v0.1 interface */
>  #define KVM_PSCI_FN_BASE               0x95c1ba5e
>  #define KVM_PSCI_FN(n)                 (KVM_PSCI_FN_BASE + (n))
> 
> @@ -159,10 +160,42 @@ struct kvm_arch_memory_slot {
>  #define KVM_PSCI_FN_CPU_ON             KVM_PSCI_FN(2)
>  #define KVM_PSCI_FN_MIGRATE            KVM_PSCI_FN(3)
> 
> +/* PSCI v0.2 interface */
> +#define KVM_PSCI_0_2_FN_BASE           0x84000000
> +#define KVM_PSCI_0_2_FN(n)             (KVM_PSCI_0_2_FN_BASE + (n))
> +#define KVM_PSCI_0_2_FN64_BASE         0xC4000000
> +#define KVM_PSCI_0_2_FN64(n)           (KVM_PSCI_0_2_FN64_BASE + (n))
> +
> +#define KVM_PSCI_0_2_FN_PSCI_VERSION   KVM_PSCI_0_2_FN(0)
> +#define KVM_PSCI_0_2_FN_CPU_SUSPEND    KVM_PSCI_0_2_FN(1)
> +#define KVM_PSCI_0_2_FN_CPU_OFF                KVM_PSCI_0_2_FN(2)
> +#define KVM_PSCI_0_2_FN_CPU_ON         KVM_PSCI_0_2_FN(3)
> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO  KVM_PSCI_0_2_FN(4)
> +#define KVM_PSCI_0_2_FN_MIGRATE                KVM_PSCI_0_2_FN(5)
> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
> +                                       KVM_PSCI_0_2_FN(6)
> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
> +                                       KVM_PSCI_0_2_FN(7)
> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF     KVM_PSCI_0_2_FN(8)
> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET   KVM_PSCI_0_2_FN(9)
> +
> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND  KVM_PSCI_0_2_FN64(1)
> +#define KVM_PSCI_0_2_FN64_CPU_ON       KVM_PSCI_0_2_FN64(3)
> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO        KVM_PSCI_0_2_FN64(4)
> +#define KVM_PSCI_0_2_FN64_MIGRATE      KVM_PSCI_0_2_FN64(5)
> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
> +                                       KVM_PSCI_0_2_FN64(7)
> +
> +/* PSCI return values */
>  #define KVM_PSCI_RET_SUCCESS           0
> -#define KVM_PSCI_RET_NI                        ((unsigned long)-1)
> -#define KVM_PSCI_RET_INVAL             ((unsigned long)-2)
> +#define KVM_PSCI_RET_NOT_SUPPORTED     ((unsigned long)-1)
> +#define KVM_PSCI_RET_INVALID_PARAMS    ((unsigned long)-2)
>  #define KVM_PSCI_RET_DENIED            ((unsigned long)-3)
> +#define KVM_PSCI_RET_ALREADY_ON                ((unsigned long)-4)
> +#define KVM_PSCI_RET_ON_PENDING                ((unsigned long)-5)
> +#define KVM_PSCI_RET_INTERNAL_FAILURE  ((unsigned long)-6)
> +#define KVM_PSCI_RET_NOT_PRESENT       ((unsigned long)-7)
> +#define KVM_PSCI_RET_DISABLED          ((unsigned long)-8)
> 
>  #endif
> 
> --
> 1.7.9.5

With the above remarks fixed, I think we'll have a good base for future
developments. Looking forward to the next version.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
  2014-01-28 11:27   ` Marc Zyngier
@ 2014-01-28 11:43     ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2014-01-28 11:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jan 28, 2014 at 4:57 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Hi Anup,
>
> On 21/01/14 13:01, Anup Patel wrote:
>> Currently, the in-kernel PSCI emulation provides PSCI v0.1 interface to
>> VCPUs. This patch extends current in-kernel PSCI emulation to provide
>> PSCI v0.2 interface to VCPUs.
>>
>> By default, ARM/ARM64 KVM will always provide PSCI v0.1 interface for
>> keeping the ABI backward-compatible.
>>
>> To select PSCI v0.2 interface for VCPUs, the user space (i.e. QEMU or
>> KVMTOOL) will have to set KVM_ARM_VCPU_PSCI_0_2 feature when doing VCPU
>> init using KVM_ARM_VCPU_INIT ioctl.
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> ---
>>  arch/arm/include/asm/kvm_host.h   |    2 +-
>>  arch/arm/include/uapi/asm/kvm.h   |   39 ++++++++++++++++--
>>  arch/arm/kvm/arm.c                |    6 ++-
>>  arch/arm/kvm/psci.c               |   79 ++++++++++++++++++++++++++++++-------
>>  arch/arm64/include/asm/kvm_host.h |    2 +-
>>  arch/arm64/include/uapi/asm/kvm.h |   39 ++++++++++++++++--
>>  6 files changed, 143 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index 8a6f6db..0239ac5 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -36,7 +36,7 @@
>>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>>  #define KVM_HAVE_ONE_REG
>>
>> -#define KVM_VCPU_MAX_FEATURES 1
>> +#define KVM_VCPU_MAX_FEATURES 2
>>
>>  #include <kvm/arm_vgic.h>
>>
>> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
>> index c498b60..d9eb74c 100644
>> --- a/arch/arm/include/uapi/asm/kvm.h
>> +++ b/arch/arm/include/uapi/asm/kvm.h
>> @@ -83,6 +83,7 @@ struct kvm_regs {
>>  #define KVM_VGIC_V2_CPU_SIZE           0x2000
>>
>>  #define KVM_ARM_VCPU_POWER_OFF         0 /* CPU is started in OFF state */
>> +#define KVM_ARM_VCPU_PSCI_0_2          1 /* CPU uses PSCI v0.2 */
>>
>>  struct kvm_vcpu_init {
>>         __u32 target;
>> @@ -164,7 +165,7 @@ struct kvm_arch_memory_slot {
>>  /* Highest supported SPI, from VGIC_NR_IRQS */
>>  #define KVM_ARM_IRQ_GIC_MAX            127
>>
>> -/* PSCI interface */
>> +/* PSCI v0.1 interface */
>>  #define KVM_PSCI_FN_BASE               0x95c1ba5e
>>  #define KVM_PSCI_FN(n)                 (KVM_PSCI_FN_BASE + (n))
>>
>> @@ -173,9 +174,41 @@ struct kvm_arch_memory_slot {
>>  #define KVM_PSCI_FN_CPU_ON             KVM_PSCI_FN(2)
>>  #define KVM_PSCI_FN_MIGRATE            KVM_PSCI_FN(3)
>>
>> +/* PSCI v0.2 interface */
>> +#define KVM_PSCI_0_2_FN_BASE           0x84000000
>> +#define KVM_PSCI_0_2_FN(n)             (KVM_PSCI_0_2_FN_BASE + (n))
>> +#define KVM_PSCI_0_2_FN64_BASE         0xC4000000
>> +#define KVM_PSCI_0_2_FN64(n)           (KVM_PSCI_0_2_FN64_BASE + (n))
>> +
>> +#define KVM_PSCI_0_2_FN_PSCI_VERSION   KVM_PSCI_0_2_FN(0)
>> +#define KVM_PSCI_0_2_FN_CPU_SUSPEND    KVM_PSCI_0_2_FN(1)
>> +#define KVM_PSCI_0_2_FN_CPU_OFF                KVM_PSCI_0_2_FN(2)
>> +#define KVM_PSCI_0_2_FN_CPU_ON         KVM_PSCI_0_2_FN(3)
>> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO  KVM_PSCI_0_2_FN(4)
>> +#define KVM_PSCI_0_2_FN_MIGRATE                KVM_PSCI_0_2_FN(5)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
>> +                                       KVM_PSCI_0_2_FN(6)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
>> +                                       KVM_PSCI_0_2_FN(7)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF     KVM_PSCI_0_2_FN(8)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET   KVM_PSCI_0_2_FN(9)
>> +
>> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND  KVM_PSCI_0_2_FN64(1)
>> +#define KVM_PSCI_0_2_FN64_CPU_ON       KVM_PSCI_0_2_FN64(3)
>> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO        KVM_PSCI_0_2_FN64(4)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE      KVM_PSCI_0_2_FN64(5)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
>> +                                       KVM_PSCI_0_2_FN64(7)
>> +
>> +/* PSCI return values */
>>  #define KVM_PSCI_RET_SUCCESS           0
>> -#define KVM_PSCI_RET_NI                        ((unsigned long)-1)
>> -#define KVM_PSCI_RET_INVAL             ((unsigned long)-2)
>
> This is a massive no-no. It is already part of the userspace API, and we
> cannot break it (neither arm nor arm64). These #defines have to stay
> forever.
>
>> +#define KVM_PSCI_RET_NOT_SUPPORTED     ((unsigned long)-1)
>> +#define KVM_PSCI_RET_INVALID_PARAMS    ((unsigned long)-2)
>
> As such, you can drop these two defines, and use the original ones in
> your new code, which will reduce the churn.

Sure, I will use the original defines. I was just trying to name all
return values
as described in spec.

>
>>  #define KVM_PSCI_RET_DENIED            ((unsigned long)-3)
>> +#define KVM_PSCI_RET_ALREADY_ON                ((unsigned long)-4)
>> +#define KVM_PSCI_RET_ON_PENDING                ((unsigned long)-5)
>> +#define KVM_PSCI_RET_INTERNAL_FAILURE  ((unsigned long)-6)
>> +#define KVM_PSCI_RET_NOT_PRESENT       ((unsigned long)-7)
>> +#define KVM_PSCI_RET_DISABLED          ((unsigned long)-8)
>>
>>  #endif /* __ARM_KVM_H__ */
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index 2a700e0..0b7817a 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -193,6 +193,7 @@ int kvm_dev_ioctl_check_extension(long ext)
>>         case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
>>         case KVM_CAP_ONE_REG:
>>         case KVM_CAP_ARM_PSCI:
>> +       case KVM_CAP_ARM_PSCI_0_2:
>>                 r = 1;
>>                 break;
>>         case KVM_CAP_COALESCED_MMIO:
>> @@ -483,7 +484,10 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
>>          * PSCI code.
>>          */
>>         if (test_and_clear_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features)) {
>> -               *vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
>> +               if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>
> Can you make this a function? Something like:
>
> int get_psci_version(struct kvm_vcpu *vcpu)
> {
>         if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>                 return PSCI_VERSION_0_2;
>
>         return PSCI_VERSION_0_1;
> }
>
> probably located in psci.c. This way, when PSCI version 3.14159 comes
> along, we'll have some mechanism in place, and we're free to change the
> detection method without impacting the rest of the code.

Yes, I was not sure about this change.

I will implement "int psci_version(struct kvm_vcpu *vcpu)" in psci.c and use
this wherever applicable (as-per your suggestion).

>
>> +                       *vcpu_reg(vcpu, 0) = KVM_PSCI_0_2_FN_CPU_OFF;
>> +               else
>> +                       *vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
>>                 kvm_psci_call(vcpu);
>>         }
>>
>> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
>> index 0881bf1..ee044a3 100644
>> --- a/arch/arm/kvm/psci.c
>> +++ b/arch/arm/kvm/psci.c
>> @@ -55,13 +55,13 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>>         }
>>
>>         if (!vcpu)
>> -               return KVM_PSCI_RET_INVAL;
>> +               return KVM_PSCI_RET_INVALID_PARAMS;
>>
>>         target_pc = *vcpu_reg(source_vcpu, 2);
>>
>>         wq = kvm_arch_vcpu_wq(vcpu);
>>         if (!waitqueue_active(wq))
>> -               return KVM_PSCI_RET_INVAL;
>> +               return KVM_PSCI_RET_INVALID_PARAMS;
>>
>>         kvm_reset_vcpu(vcpu);
>>
>> @@ -84,17 +84,49 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>>         return KVM_PSCI_RET_SUCCESS;
>>  }
>>
>> -/**
>> - * kvm_psci_call - handle PSCI call if r0 value is in range
>> - * @vcpu: Pointer to the VCPU struct
>> - *
>> - * Handle PSCI calls from guests through traps from HVC instructions.
>> - * The calling convention is similar to SMC calls to the secure world where
>> - * the function number is placed in r0 and this function returns true if the
>> - * function number specified in r0 is withing the PSCI range, and false
>> - * otherwise.
>> - */
>> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> +static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>> +{
>> +       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>> +       unsigned long val;
>> +
>> +       switch (psci_fn) {
>> +       case KVM_PSCI_0_2_FN_PSCI_VERSION:
>> +               /*
>> +                * Bits[31:16] = Major Version = 0
>> +                * Bits[15:0] = Minor Version = 2
>> +                */
>> +               val = 2;
>> +               break;
>> +       case KVM_PSCI_0_2_FN_CPU_OFF:
>> +               kvm_psci_vcpu_off(vcpu);
>> +               val = KVM_PSCI_RET_SUCCESS;
>> +               break;
>> +       case KVM_PSCI_0_2_FN_CPU_ON:
>> +       case KVM_PSCI_0_2_FN64_CPU_ON:
>> +               val = kvm_psci_vcpu_on(vcpu);
>> +               break;
>> +       case KVM_PSCI_0_2_FN_CPU_SUSPEND:
>> +       case KVM_PSCI_0_2_FN_AFFINITY_INFO:
>> +       case KVM_PSCI_0_2_FN_MIGRATE:
>> +       case KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
>> +       case KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
>> +       case KVM_PSCI_0_2_FN_SYSTEM_OFF:
>> +       case KVM_PSCI_0_2_FN_SYSTEM_RESET:
>> +       case KVM_PSCI_0_2_FN64_CPU_SUSPEND:
>> +       case KVM_PSCI_0_2_FN64_AFFINITY_INFO:
>> +       case KVM_PSCI_0_2_FN64_MIGRATE:
>> +       case KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
>> +               val = KVM_PSCI_RET_NOT_SUPPORTED;
>> +               break;
>> +       default:
>> +               return false;
>> +       }
>> +
>> +       *vcpu_reg(vcpu, 0) = val;
>> +       return true;
>> +}
>> +
>> +static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>>  {
>>         unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>         unsigned long val;
>> @@ -109,9 +141,8 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>>                 break;
>>         case KVM_PSCI_FN_CPU_SUSPEND:
>>         case KVM_PSCI_FN_MIGRATE:
>> -               val = KVM_PSCI_RET_NI;
>> +               val = KVM_PSCI_RET_NOT_SUPPORTED;
>>                 break;
>> -
>>         default:
>>                 return false;
>>         }
>> @@ -119,3 +150,21 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>>         *vcpu_reg(vcpu, 0) = val;
>>         return true;
>>  }
>> +
>> +/**
>> + * kvm_psci_call - handle PSCI call if r0 value is in range
>> + * @vcpu: Pointer to the VCPU struct
>> + *
>> + * Handle PSCI calls from guests through traps from HVC instructions.
>> + * The calling convention is similar to SMC calls to the secure world where
>> + * the function number is placed in r0 and this function returns true if the
>> + * function number specified in r0 is withing the PSCI range, and false
>> + * otherwise.
>> + */
>> +bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> +{
>> +       if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>> +               return kvm_psci_0_2_call(vcpu);
>> +
>> +       return kvm_psci_0_1_call(vcpu);
>> +}
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 0a1d697..92242ce 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -39,7 +39,7 @@
>>  #include <kvm/arm_vgic.h>
>>  #include <kvm/arm_arch_timer.h>
>>
>> -#define KVM_VCPU_MAX_FEATURES 2
>> +#define KVM_VCPU_MAX_FEATURES 3
>>
>>  struct kvm_vcpu;
>>  int kvm_target_cpu(void);
>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>> index d9f026b..0eb254d 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -77,6 +77,7 @@ struct kvm_regs {
>>
>>  #define KVM_ARM_VCPU_POWER_OFF         0 /* CPU is started in OFF state */
>>  #define KVM_ARM_VCPU_EL1_32BIT         1 /* CPU running a 32bit VM */
>> +#define KVM_ARM_VCPU_PSCI_0_2          2 /* CPU uses PSCI v0.2 */
>>
>>  struct kvm_vcpu_init {
>>         __u32 target;
>> @@ -150,7 +151,7 @@ struct kvm_arch_memory_slot {
>>  /* Highest supported SPI, from VGIC_NR_IRQS */
>>  #define KVM_ARM_IRQ_GIC_MAX            127
>>
>> -/* PSCI interface */
>> +/* PSCI v0.1 interface */
>>  #define KVM_PSCI_FN_BASE               0x95c1ba5e
>>  #define KVM_PSCI_FN(n)                 (KVM_PSCI_FN_BASE + (n))
>>
>> @@ -159,10 +160,42 @@ struct kvm_arch_memory_slot {
>>  #define KVM_PSCI_FN_CPU_ON             KVM_PSCI_FN(2)
>>  #define KVM_PSCI_FN_MIGRATE            KVM_PSCI_FN(3)
>>
>> +/* PSCI v0.2 interface */
>> +#define KVM_PSCI_0_2_FN_BASE           0x84000000
>> +#define KVM_PSCI_0_2_FN(n)             (KVM_PSCI_0_2_FN_BASE + (n))
>> +#define KVM_PSCI_0_2_FN64_BASE         0xC4000000
>> +#define KVM_PSCI_0_2_FN64(n)           (KVM_PSCI_0_2_FN64_BASE + (n))
>> +
>> +#define KVM_PSCI_0_2_FN_PSCI_VERSION   KVM_PSCI_0_2_FN(0)
>> +#define KVM_PSCI_0_2_FN_CPU_SUSPEND    KVM_PSCI_0_2_FN(1)
>> +#define KVM_PSCI_0_2_FN_CPU_OFF                KVM_PSCI_0_2_FN(2)
>> +#define KVM_PSCI_0_2_FN_CPU_ON         KVM_PSCI_0_2_FN(3)
>> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO  KVM_PSCI_0_2_FN(4)
>> +#define KVM_PSCI_0_2_FN_MIGRATE                KVM_PSCI_0_2_FN(5)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
>> +                                       KVM_PSCI_0_2_FN(6)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
>> +                                       KVM_PSCI_0_2_FN(7)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF     KVM_PSCI_0_2_FN(8)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET   KVM_PSCI_0_2_FN(9)
>> +
>> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND  KVM_PSCI_0_2_FN64(1)
>> +#define KVM_PSCI_0_2_FN64_CPU_ON       KVM_PSCI_0_2_FN64(3)
>> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO        KVM_PSCI_0_2_FN64(4)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE      KVM_PSCI_0_2_FN64(5)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
>> +                                       KVM_PSCI_0_2_FN64(7)
>> +
>> +/* PSCI return values */
>>  #define KVM_PSCI_RET_SUCCESS           0
>> -#define KVM_PSCI_RET_NI                        ((unsigned long)-1)
>> -#define KVM_PSCI_RET_INVAL             ((unsigned long)-2)
>> +#define KVM_PSCI_RET_NOT_SUPPORTED     ((unsigned long)-1)
>> +#define KVM_PSCI_RET_INVALID_PARAMS    ((unsigned long)-2)
>>  #define KVM_PSCI_RET_DENIED            ((unsigned long)-3)
>> +#define KVM_PSCI_RET_ALREADY_ON                ((unsigned long)-4)
>> +#define KVM_PSCI_RET_ON_PENDING                ((unsigned long)-5)
>> +#define KVM_PSCI_RET_INTERNAL_FAILURE  ((unsigned long)-6)
>> +#define KVM_PSCI_RET_NOT_PRESENT       ((unsigned long)-7)
>> +#define KVM_PSCI_RET_DISABLED          ((unsigned long)-8)
>>
>>  #endif
>>
>> --
>> 1.7.9.5
>
> With the above remarks fixed, I think we'll have a good base for future
> developments. Looking forward to the next version.

I will revise and send-out next version soon.

>
> Thanks,
>
>         M.
> --
> Jazz is not dead. It just smells funny...
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

Thanks,
Anup

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
  2014-01-21 13:01 ` [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation Anup Patel
  2014-01-28 11:27   ` Marc Zyngier
@ 2014-01-28 21:04   ` Christoffer Dall
  2014-01-29  4:52     ` Anup Patel
  2014-01-30  5:28     ` Anup Patel
  1 sibling, 2 replies; 14+ messages in thread
From: Christoffer Dall @ 2014-01-28 21:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jan 21, 2014 at 06:31:40PM +0530, Anup Patel wrote:
> Currently, the in-kernel PSCI emulation provides PSCI v0.1 interface to
> VCPUs. This patch extends current in-kernel PSCI emulation to provide
> PSCI v0.2 interface to VCPUs.
> 
> By default, ARM/ARM64 KVM will always provide PSCI v0.1 interface for
> keeping the ABI backward-compatible.
> 
> To select PSCI v0.2 interface for VCPUs, the user space (i.e. QEMU or
> KVMTOOL) will have to set KVM_ARM_VCPU_PSCI_0_2 feature when doing VCPU
> init using KVM_ARM_VCPU_INIT ioctl.
> 
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> ---
>  arch/arm/include/asm/kvm_host.h   |    2 +-
>  arch/arm/include/uapi/asm/kvm.h   |   39 ++++++++++++++++--
>  arch/arm/kvm/arm.c                |    6 ++-
>  arch/arm/kvm/psci.c               |   79 ++++++++++++++++++++++++++++++-------
>  arch/arm64/include/asm/kvm_host.h |    2 +-
>  arch/arm64/include/uapi/asm/kvm.h |   39 ++++++++++++++++--
>  6 files changed, 143 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 8a6f6db..0239ac5 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -36,7 +36,7 @@
>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>  #define KVM_HAVE_ONE_REG
>  
> -#define KVM_VCPU_MAX_FEATURES 1
> +#define KVM_VCPU_MAX_FEATURES 2
>  
>  #include <kvm/arm_vgic.h>
>  
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index c498b60..d9eb74c 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -83,6 +83,7 @@ struct kvm_regs {
>  #define KVM_VGIC_V2_CPU_SIZE		0x2000
>  
>  #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
> +#define KVM_ARM_VCPU_PSCI_0_2		1 /* CPU uses PSCI v0.2 */
>  
>  struct kvm_vcpu_init {
>  	__u32 target;
> @@ -164,7 +165,7 @@ struct kvm_arch_memory_slot {
>  /* Highest supported SPI, from VGIC_NR_IRQS */
>  #define KVM_ARM_IRQ_GIC_MAX		127
>  
> -/* PSCI interface */
> +/* PSCI v0.1 interface */
>  #define KVM_PSCI_FN_BASE		0x95c1ba5e
>  #define KVM_PSCI_FN(n)			(KVM_PSCI_FN_BASE + (n))
>  
> @@ -173,9 +174,41 @@ struct kvm_arch_memory_slot {
>  #define KVM_PSCI_FN_CPU_ON		KVM_PSCI_FN(2)
>  #define KVM_PSCI_FN_MIGRATE		KVM_PSCI_FN(3)
>  
> +/* PSCI v0.2 interface */
> +#define KVM_PSCI_0_2_FN_BASE		0x84000000
> +#define KVM_PSCI_0_2_FN(n)		(KVM_PSCI_0_2_FN_BASE + (n))
> +#define KVM_PSCI_0_2_FN64_BASE		0xC4000000
> +#define KVM_PSCI_0_2_FN64(n)		(KVM_PSCI_0_2_FN64_BASE + (n))
> +
> +#define KVM_PSCI_0_2_FN_PSCI_VERSION	KVM_PSCI_0_2_FN(0)
> +#define KVM_PSCI_0_2_FN_CPU_SUSPEND	KVM_PSCI_0_2_FN(1)
> +#define KVM_PSCI_0_2_FN_CPU_OFF		KVM_PSCI_0_2_FN(2)
> +#define KVM_PSCI_0_2_FN_CPU_ON		KVM_PSCI_0_2_FN(3)
> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO	KVM_PSCI_0_2_FN(4)
> +#define KVM_PSCI_0_2_FN_MIGRATE		KVM_PSCI_0_2_FN(5)
> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
> +					KVM_PSCI_0_2_FN(6)
> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
> +					KVM_PSCI_0_2_FN(7)
> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF	KVM_PSCI_0_2_FN(8)
> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET	KVM_PSCI_0_2_FN(9)
> +
> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND	KVM_PSCI_0_2_FN64(1)
> +#define KVM_PSCI_0_2_FN64_CPU_ON	KVM_PSCI_0_2_FN64(3)
> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO	KVM_PSCI_0_2_FN64(4)
> +#define KVM_PSCI_0_2_FN64_MIGRATE	KVM_PSCI_0_2_FN64(5)
> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
> +					KVM_PSCI_0_2_FN64(7)
> +
> +/* PSCI return values */
>  #define KVM_PSCI_RET_SUCCESS		0
> -#define KVM_PSCI_RET_NI			((unsigned long)-1)
> -#define KVM_PSCI_RET_INVAL		((unsigned long)-2)
> +#define KVM_PSCI_RET_NOT_SUPPORTED	((unsigned long)-1)
> +#define KVM_PSCI_RET_INVALID_PARAMS	((unsigned long)-2)
>  #define KVM_PSCI_RET_DENIED		((unsigned long)-3)
> +#define KVM_PSCI_RET_ALREADY_ON		((unsigned long)-4)
> +#define KVM_PSCI_RET_ON_PENDING		((unsigned long)-5)
> +#define KVM_PSCI_RET_INTERNAL_FAILURE	((unsigned long)-6)
> +#define KVM_PSCI_RET_NOT_PRESENT	((unsigned long)-7)
> +#define KVM_PSCI_RET_DISABLED		((unsigned long)-8)
>  
>  #endif /* __ARM_KVM_H__ */
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 2a700e0..0b7817a 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -193,6 +193,7 @@ int kvm_dev_ioctl_check_extension(long ext)
>  	case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
>  	case KVM_CAP_ONE_REG:
>  	case KVM_CAP_ARM_PSCI:
> +	case KVM_CAP_ARM_PSCI_0_2:
>  		r = 1;
>  		break;
>  	case KVM_CAP_COALESCED_MMIO:
> @@ -483,7 +484,10 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
>  	 * PSCI code.
>  	 */
>  	if (test_and_clear_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features)) {
> -		*vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
> +		if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
> +			*vcpu_reg(vcpu, 0) = KVM_PSCI_0_2_FN_CPU_OFF;
> +		else
> +			*vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
>  		kvm_psci_call(vcpu);

Which tree does this patch apply to?  It looks like you'll get a
conflict with:
478a823 arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive

>  	}
>  
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 0881bf1..ee044a3 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -55,13 +55,13 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>  	}
>  
>  	if (!vcpu)
> -		return KVM_PSCI_RET_INVAL;
> +		return KVM_PSCI_RET_INVALID_PARAMS;
>  
>  	target_pc = *vcpu_reg(source_vcpu, 2);
>  
>  	wq = kvm_arch_vcpu_wq(vcpu);
>  	if (!waitqueue_active(wq))
> -		return KVM_PSCI_RET_INVAL;
> +		return KVM_PSCI_RET_INVALID_PARAMS;
>  
>  	kvm_reset_vcpu(vcpu);
>  
> @@ -84,17 +84,49 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>  	return KVM_PSCI_RET_SUCCESS;
>  }
>  
> -/**
> - * kvm_psci_call - handle PSCI call if r0 value is in range
> - * @vcpu: Pointer to the VCPU struct
> - *
> - * Handle PSCI calls from guests through traps from HVC instructions.
> - * The calling convention is similar to SMC calls to the secure world where
> - * the function number is placed in r0 and this function returns true if the
> - * function number specified in r0 is withing the PSCI range, and false
> - * otherwise.
> - */
> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
> +static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> +{
> +	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
> +	unsigned long val;
> +
> +	switch (psci_fn) {
> +	case KVM_PSCI_0_2_FN_PSCI_VERSION:
> +		/*
> +		 * Bits[31:16] = Major Version = 0
> +		 * Bits[15:0] = Minor Version = 2
> +		 */
> +		val = 2;
> +		break;
> +	case KVM_PSCI_0_2_FN_CPU_OFF:
> +		kvm_psci_vcpu_off(vcpu);
> +		val = KVM_PSCI_RET_SUCCESS;
> +		break;
> +	case KVM_PSCI_0_2_FN_CPU_ON:
> +	case KVM_PSCI_0_2_FN64_CPU_ON:
> +		val = kvm_psci_vcpu_on(vcpu);
> +		break;
> +	case KVM_PSCI_0_2_FN_CPU_SUSPEND:
> +	case KVM_PSCI_0_2_FN_AFFINITY_INFO:
> +	case KVM_PSCI_0_2_FN_MIGRATE:
> +	case KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
> +	case KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
> +	case KVM_PSCI_0_2_FN_SYSTEM_OFF:
> +	case KVM_PSCI_0_2_FN_SYSTEM_RESET:
> +	case KVM_PSCI_0_2_FN64_CPU_SUSPEND:
> +	case KVM_PSCI_0_2_FN64_AFFINITY_INFO:
> +	case KVM_PSCI_0_2_FN64_MIGRATE:
> +	case KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
> +		val = KVM_PSCI_RET_NOT_SUPPORTED;
> +		break;
> +	default:
> +		return false;
> +	}
> +
> +	*vcpu_reg(vcpu, 0) = val;
> +	return true;
> +}
> +
> +static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>  	unsigned long val;
> @@ -109,9 +141,8 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>  		break;
>  	case KVM_PSCI_FN_CPU_SUSPEND:
>  	case KVM_PSCI_FN_MIGRATE:
> -		val = KVM_PSCI_RET_NI;
> +		val = KVM_PSCI_RET_NOT_SUPPORTED;
>  		break;
> -
>  	default:
>  		return false;
>  	}
> @@ -119,3 +150,21 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>  	*vcpu_reg(vcpu, 0) = val;
>  	return true;
>  }
> +
> +/**
> + * kvm_psci_call - handle PSCI call if r0 value is in range
> + * @vcpu: Pointer to the VCPU struct
> + *
> + * Handle PSCI calls from guests through traps from HVC instructions.
> + * The calling convention is similar to SMC calls to the secure world where
> + * the function number is placed in r0 and this function returns true if the
> + * function number specified in r0 is withing the PSCI range, and false
> + * otherwise.
> + */
> +bool kvm_psci_call(struct kvm_vcpu *vcpu)
> +{
> +	if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
> +		return kvm_psci_0_2_call(vcpu);
> +
> +	return kvm_psci_0_1_call(vcpu);
> +}

Why don't we just try one after the other?  Do they conflict in some
way?

I assume PSCI calls are never going to be in the critical path and calls
into PSCI are pretty much expected to be slow as a dog anyhow, so if we
could avoid the extra churn in user space code and potential user
confusion (providing PSCI 0.2 kernel but too old user space tool for
example), I think that would be preferred.

> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 0a1d697..92242ce 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -39,7 +39,7 @@
>  #include <kvm/arm_vgic.h>
>  #include <kvm/arm_arch_timer.h>
>  
> -#define KVM_VCPU_MAX_FEATURES 2
> +#define KVM_VCPU_MAX_FEATURES 3
>  
>  struct kvm_vcpu;
>  int kvm_target_cpu(void);
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index d9f026b..0eb254d 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -77,6 +77,7 @@ struct kvm_regs {
>  
>  #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
>  #define KVM_ARM_VCPU_EL1_32BIT		1 /* CPU running a 32bit VM */
> +#define KVM_ARM_VCPU_PSCI_0_2		2 /* CPU uses PSCI v0.2 */
>  
>  struct kvm_vcpu_init {
>  	__u32 target;
> @@ -150,7 +151,7 @@ struct kvm_arch_memory_slot {
>  /* Highest supported SPI, from VGIC_NR_IRQS */
>  #define KVM_ARM_IRQ_GIC_MAX		127
>  
> -/* PSCI interface */
> +/* PSCI v0.1 interface */
>  #define KVM_PSCI_FN_BASE		0x95c1ba5e
>  #define KVM_PSCI_FN(n)			(KVM_PSCI_FN_BASE + (n))
>  
> @@ -159,10 +160,42 @@ struct kvm_arch_memory_slot {
>  #define KVM_PSCI_FN_CPU_ON		KVM_PSCI_FN(2)
>  #define KVM_PSCI_FN_MIGRATE		KVM_PSCI_FN(3)
>  
> +/* PSCI v0.2 interface */
> +#define KVM_PSCI_0_2_FN_BASE		0x84000000
> +#define KVM_PSCI_0_2_FN(n)		(KVM_PSCI_0_2_FN_BASE + (n))
> +#define KVM_PSCI_0_2_FN64_BASE		0xC4000000
> +#define KVM_PSCI_0_2_FN64(n)		(KVM_PSCI_0_2_FN64_BASE + (n))
> +
> +#define KVM_PSCI_0_2_FN_PSCI_VERSION	KVM_PSCI_0_2_FN(0)
> +#define KVM_PSCI_0_2_FN_CPU_SUSPEND	KVM_PSCI_0_2_FN(1)
> +#define KVM_PSCI_0_2_FN_CPU_OFF		KVM_PSCI_0_2_FN(2)
> +#define KVM_PSCI_0_2_FN_CPU_ON		KVM_PSCI_0_2_FN(3)
> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO	KVM_PSCI_0_2_FN(4)
> +#define KVM_PSCI_0_2_FN_MIGRATE		KVM_PSCI_0_2_FN(5)
> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
> +					KVM_PSCI_0_2_FN(6)
> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
> +					KVM_PSCI_0_2_FN(7)
> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF	KVM_PSCI_0_2_FN(8)
> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET	KVM_PSCI_0_2_FN(9)
> +
> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND	KVM_PSCI_0_2_FN64(1)
> +#define KVM_PSCI_0_2_FN64_CPU_ON	KVM_PSCI_0_2_FN64(3)
> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO	KVM_PSCI_0_2_FN64(4)
> +#define KVM_PSCI_0_2_FN64_MIGRATE	KVM_PSCI_0_2_FN64(5)
> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
> +					KVM_PSCI_0_2_FN64(7)
> +
> +/* PSCI return values */
>  #define KVM_PSCI_RET_SUCCESS		0
> -#define KVM_PSCI_RET_NI			((unsigned long)-1)
> -#define KVM_PSCI_RET_INVAL		((unsigned long)-2)
> +#define KVM_PSCI_RET_NOT_SUPPORTED	((unsigned long)-1)
> +#define KVM_PSCI_RET_INVALID_PARAMS	((unsigned long)-2)
>  #define KVM_PSCI_RET_DENIED		((unsigned long)-3)
> +#define KVM_PSCI_RET_ALREADY_ON		((unsigned long)-4)
> +#define KVM_PSCI_RET_ON_PENDING		((unsigned long)-5)
> +#define KVM_PSCI_RET_INTERNAL_FAILURE	((unsigned long)-6)
> +#define KVM_PSCI_RET_NOT_PRESENT	((unsigned long)-7)
> +#define KVM_PSCI_RET_DISABLED		((unsigned long)-8)
>  
>  #endif
>  
> -- 
> 1.7.9.5
> 

Thanks,
-- 
Christoffer

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 3/3] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature
  2014-01-21 13:01 ` [RFC PATCH 3/3] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature Anup Patel
@ 2014-01-28 21:05   ` Christoffer Dall
  2014-01-29  4:30     ` Anup Patel
  0 siblings, 1 reply; 14+ messages in thread
From: Christoffer Dall @ 2014-01-28 21:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jan 21, 2014 at 06:31:41PM +0530, Anup Patel wrote:
> We have in-kernel emulation of PSCI v0.2 in KVM ARM/ARM64. To provide
> PSCI v0.2 interface to VCPUs, we have to enable KVM_ARM_VCPU_PSCI_0_2
> feature when doing KVM_ARM_VCPU_INIT ioctl.
> 
> The patch updates documentation of KVM_ARM_VCPU_INIT ioctl to provide
> info regarding KVM_ARM_VCPU_PSCI_0_2 feature.
> 
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> ---
>  Documentation/virtual/kvm/api.txt |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index aad3244..a15fcdd 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2346,6 +2346,8 @@ Possible features:
>  	  Depends on KVM_CAP_ARM_PSCI.
>  	- KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
>  	  Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
> +	- KVM_ARM_VCPU_PSCI_0_2: Emulate PSCI v0.2 for CPU.

nit: s/for CPU/for the CPU/

> +	  Depends on KVM_CAP_ARM_PSCI_0_2.
>  
>  
>  4.83 KVM_ARM_PREFERRED_TARGET
> -- 
> 1.7.9.5
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 3/3] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature
  2014-01-28 21:05   ` Christoffer Dall
@ 2014-01-29  4:30     ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2014-01-29  4:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jan 29, 2014 at 2:35 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Tue, Jan 21, 2014 at 06:31:41PM +0530, Anup Patel wrote:
>> We have in-kernel emulation of PSCI v0.2 in KVM ARM/ARM64. To provide
>> PSCI v0.2 interface to VCPUs, we have to enable KVM_ARM_VCPU_PSCI_0_2
>> feature when doing KVM_ARM_VCPU_INIT ioctl.
>>
>> The patch updates documentation of KVM_ARM_VCPU_INIT ioctl to provide
>> info regarding KVM_ARM_VCPU_PSCI_0_2 feature.
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> ---
>>  Documentation/virtual/kvm/api.txt |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index aad3244..a15fcdd 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -2346,6 +2346,8 @@ Possible features:
>>         Depends on KVM_CAP_ARM_PSCI.
>>       - KVM_ARM_VCPU_EL1_32BIT: Starts the CPU in a 32bit mode.
>>         Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
>> +     - KVM_ARM_VCPU_PSCI_0_2: Emulate PSCI v0.2 for CPU.
>
> nit: s/for CPU/for the CPU/

OK, I'll update this.

>
>> +       Depends on KVM_CAP_ARM_PSCI_0_2.
>>
>>
>>  4.83 KVM_ARM_PREFERRED_TARGET
>> --
>> 1.7.9.5
>>
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

--
Anup

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
  2014-01-28 21:04   ` Christoffer Dall
@ 2014-01-29  4:52     ` Anup Patel
  2014-01-29 15:50       ` Christoffer Dall
  2014-01-30  5:28     ` Anup Patel
  1 sibling, 1 reply; 14+ messages in thread
From: Anup Patel @ 2014-01-29  4:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jan 29, 2014 at 2:34 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Tue, Jan 21, 2014 at 06:31:40PM +0530, Anup Patel wrote:
>> Currently, the in-kernel PSCI emulation provides PSCI v0.1 interface to
>> VCPUs. This patch extends current in-kernel PSCI emulation to provide
>> PSCI v0.2 interface to VCPUs.
>>
>> By default, ARM/ARM64 KVM will always provide PSCI v0.1 interface for
>> keeping the ABI backward-compatible.
>>
>> To select PSCI v0.2 interface for VCPUs, the user space (i.e. QEMU or
>> KVMTOOL) will have to set KVM_ARM_VCPU_PSCI_0_2 feature when doing VCPU
>> init using KVM_ARM_VCPU_INIT ioctl.
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> ---
>>  arch/arm/include/asm/kvm_host.h   |    2 +-
>>  arch/arm/include/uapi/asm/kvm.h   |   39 ++++++++++++++++--
>>  arch/arm/kvm/arm.c                |    6 ++-
>>  arch/arm/kvm/psci.c               |   79 ++++++++++++++++++++++++++++++-------
>>  arch/arm64/include/asm/kvm_host.h |    2 +-
>>  arch/arm64/include/uapi/asm/kvm.h |   39 ++++++++++++++++--
>>  6 files changed, 143 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index 8a6f6db..0239ac5 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -36,7 +36,7 @@
>>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>>  #define KVM_HAVE_ONE_REG
>>
>> -#define KVM_VCPU_MAX_FEATURES 1
>> +#define KVM_VCPU_MAX_FEATURES 2
>>
>>  #include <kvm/arm_vgic.h>
>>
>> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
>> index c498b60..d9eb74c 100644
>> --- a/arch/arm/include/uapi/asm/kvm.h
>> +++ b/arch/arm/include/uapi/asm/kvm.h
>> @@ -83,6 +83,7 @@ struct kvm_regs {
>>  #define KVM_VGIC_V2_CPU_SIZE         0x2000
>>
>>  #define KVM_ARM_VCPU_POWER_OFF               0 /* CPU is started in OFF state */
>> +#define KVM_ARM_VCPU_PSCI_0_2                1 /* CPU uses PSCI v0.2 */
>>
>>  struct kvm_vcpu_init {
>>       __u32 target;
>> @@ -164,7 +165,7 @@ struct kvm_arch_memory_slot {
>>  /* Highest supported SPI, from VGIC_NR_IRQS */
>>  #define KVM_ARM_IRQ_GIC_MAX          127
>>
>> -/* PSCI interface */
>> +/* PSCI v0.1 interface */
>>  #define KVM_PSCI_FN_BASE             0x95c1ba5e
>>  #define KVM_PSCI_FN(n)                       (KVM_PSCI_FN_BASE + (n))
>>
>> @@ -173,9 +174,41 @@ struct kvm_arch_memory_slot {
>>  #define KVM_PSCI_FN_CPU_ON           KVM_PSCI_FN(2)
>>  #define KVM_PSCI_FN_MIGRATE          KVM_PSCI_FN(3)
>>
>> +/* PSCI v0.2 interface */
>> +#define KVM_PSCI_0_2_FN_BASE         0x84000000
>> +#define KVM_PSCI_0_2_FN(n)           (KVM_PSCI_0_2_FN_BASE + (n))
>> +#define KVM_PSCI_0_2_FN64_BASE               0xC4000000
>> +#define KVM_PSCI_0_2_FN64(n)         (KVM_PSCI_0_2_FN64_BASE + (n))
>> +
>> +#define KVM_PSCI_0_2_FN_PSCI_VERSION KVM_PSCI_0_2_FN(0)
>> +#define KVM_PSCI_0_2_FN_CPU_SUSPEND  KVM_PSCI_0_2_FN(1)
>> +#define KVM_PSCI_0_2_FN_CPU_OFF              KVM_PSCI_0_2_FN(2)
>> +#define KVM_PSCI_0_2_FN_CPU_ON               KVM_PSCI_0_2_FN(3)
>> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO        KVM_PSCI_0_2_FN(4)
>> +#define KVM_PSCI_0_2_FN_MIGRATE              KVM_PSCI_0_2_FN(5)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
>> +                                     KVM_PSCI_0_2_FN(6)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
>> +                                     KVM_PSCI_0_2_FN(7)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF   KVM_PSCI_0_2_FN(8)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET KVM_PSCI_0_2_FN(9)
>> +
>> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND        KVM_PSCI_0_2_FN64(1)
>> +#define KVM_PSCI_0_2_FN64_CPU_ON     KVM_PSCI_0_2_FN64(3)
>> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO      KVM_PSCI_0_2_FN64(4)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE    KVM_PSCI_0_2_FN64(5)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
>> +                                     KVM_PSCI_0_2_FN64(7)
>> +
>> +/* PSCI return values */
>>  #define KVM_PSCI_RET_SUCCESS         0
>> -#define KVM_PSCI_RET_NI                      ((unsigned long)-1)
>> -#define KVM_PSCI_RET_INVAL           ((unsigned long)-2)
>> +#define KVM_PSCI_RET_NOT_SUPPORTED   ((unsigned long)-1)
>> +#define KVM_PSCI_RET_INVALID_PARAMS  ((unsigned long)-2)
>>  #define KVM_PSCI_RET_DENIED          ((unsigned long)-3)
>> +#define KVM_PSCI_RET_ALREADY_ON              ((unsigned long)-4)
>> +#define KVM_PSCI_RET_ON_PENDING              ((unsigned long)-5)
>> +#define KVM_PSCI_RET_INTERNAL_FAILURE        ((unsigned long)-6)
>> +#define KVM_PSCI_RET_NOT_PRESENT     ((unsigned long)-7)
>> +#define KVM_PSCI_RET_DISABLED                ((unsigned long)-8)
>>
>>  #endif /* __ARM_KVM_H__ */
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index 2a700e0..0b7817a 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -193,6 +193,7 @@ int kvm_dev_ioctl_check_extension(long ext)
>>       case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
>>       case KVM_CAP_ONE_REG:
>>       case KVM_CAP_ARM_PSCI:
>> +     case KVM_CAP_ARM_PSCI_0_2:
>>               r = 1;
>>               break;
>>       case KVM_CAP_COALESCED_MMIO:
>> @@ -483,7 +484,10 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
>>        * PSCI code.
>>        */
>>       if (test_and_clear_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features)) {
>> -             *vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
>> +             if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>> +                     *vcpu_reg(vcpu, 0) = KVM_PSCI_0_2_FN_CPU_OFF;
>> +             else
>> +                     *vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
>>               kvm_psci_call(vcpu);
>
> Which tree does this patch apply to?  It looks like you'll get a
> conflict with:
> 478a823 arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive

This patchset applies on v3.13 tag of Torvalds tree.

I generally base my patches on latest stable/rc tag of Torvalds tree
so that I can provide KVM patches to folks interested in trying KVM
on X-Gene with latest Linux stable/rc.

I will make sure that revised patchset applies on top of
478a823 arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive

>
>>       }
>>
>> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
>> index 0881bf1..ee044a3 100644
>> --- a/arch/arm/kvm/psci.c
>> +++ b/arch/arm/kvm/psci.c
>> @@ -55,13 +55,13 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>>       }
>>
>>       if (!vcpu)
>> -             return KVM_PSCI_RET_INVAL;
>> +             return KVM_PSCI_RET_INVALID_PARAMS;
>>
>>       target_pc = *vcpu_reg(source_vcpu, 2);
>>
>>       wq = kvm_arch_vcpu_wq(vcpu);
>>       if (!waitqueue_active(wq))
>> -             return KVM_PSCI_RET_INVAL;
>> +             return KVM_PSCI_RET_INVALID_PARAMS;
>>
>>       kvm_reset_vcpu(vcpu);
>>
>> @@ -84,17 +84,49 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>>       return KVM_PSCI_RET_SUCCESS;
>>  }
>>
>> -/**
>> - * kvm_psci_call - handle PSCI call if r0 value is in range
>> - * @vcpu: Pointer to the VCPU struct
>> - *
>> - * Handle PSCI calls from guests through traps from HVC instructions.
>> - * The calling convention is similar to SMC calls to the secure world where
>> - * the function number is placed in r0 and this function returns true if the
>> - * function number specified in r0 is withing the PSCI range, and false
>> - * otherwise.
>> - */
>> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> +static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>> +{
>> +     unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>> +     unsigned long val;
>> +
>> +     switch (psci_fn) {
>> +     case KVM_PSCI_0_2_FN_PSCI_VERSION:
>> +             /*
>> +              * Bits[31:16] = Major Version = 0
>> +              * Bits[15:0] = Minor Version = 2
>> +              */
>> +             val = 2;
>> +             break;
>> +     case KVM_PSCI_0_2_FN_CPU_OFF:
>> +             kvm_psci_vcpu_off(vcpu);
>> +             val = KVM_PSCI_RET_SUCCESS;
>> +             break;
>> +     case KVM_PSCI_0_2_FN_CPU_ON:
>> +     case KVM_PSCI_0_2_FN64_CPU_ON:
>> +             val = kvm_psci_vcpu_on(vcpu);
>> +             break;
>> +     case KVM_PSCI_0_2_FN_CPU_SUSPEND:
>> +     case KVM_PSCI_0_2_FN_AFFINITY_INFO:
>> +     case KVM_PSCI_0_2_FN_MIGRATE:
>> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
>> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
>> +     case KVM_PSCI_0_2_FN_SYSTEM_OFF:
>> +     case KVM_PSCI_0_2_FN_SYSTEM_RESET:
>> +     case KVM_PSCI_0_2_FN64_CPU_SUSPEND:
>> +     case KVM_PSCI_0_2_FN64_AFFINITY_INFO:
>> +     case KVM_PSCI_0_2_FN64_MIGRATE:
>> +     case KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
>> +             val = KVM_PSCI_RET_NOT_SUPPORTED;
>> +             break;
>> +     default:
>> +             return false;
>> +     }
>> +
>> +     *vcpu_reg(vcpu, 0) = val;
>> +     return true;
>> +}
>> +
>> +static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>>  {
>>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>       unsigned long val;
>> @@ -109,9 +141,8 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>>               break;
>>       case KVM_PSCI_FN_CPU_SUSPEND:
>>       case KVM_PSCI_FN_MIGRATE:
>> -             val = KVM_PSCI_RET_NI;
>> +             val = KVM_PSCI_RET_NOT_SUPPORTED;
>>               break;
>> -
>>       default:
>>               return false;
>>       }
>> @@ -119,3 +150,21 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>>       *vcpu_reg(vcpu, 0) = val;
>>       return true;
>>  }
>> +
>> +/**
>> + * kvm_psci_call - handle PSCI call if r0 value is in range
>> + * @vcpu: Pointer to the VCPU struct
>> + *
>> + * Handle PSCI calls from guests through traps from HVC instructions.
>> + * The calling convention is similar to SMC calls to the secure world where
>> + * the function number is placed in r0 and this function returns true if the
>> + * function number specified in r0 is withing the PSCI range, and false
>> + * otherwise.
>> + */
>> +bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> +{
>> +     if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>> +             return kvm_psci_0_2_call(vcpu);
>> +
>> +     return kvm_psci_0_1_call(vcpu);
>> +}
>
> Why don't we just try one after the other?  Do they conflict in some
> way?

Atleast the functions IDs are totally different in v0.2 and v0.1

Also, in v0.2 we have separate function IDs for 32bit and 64bit
VCPU calling same PSCI function.

>
> I assume PSCI calls are never going to be in the critical path and calls
> into PSCI are pretty much expected to be slow as a dog anyhow, so if we
> could avoid the extra churn in user space code and potential user
> confusion (providing PSCI 0.2 kernel but too old user space tool for
> example), I think that would be preferred.

Yes, PSCI calls will not be in critical path except few functions such as
PSCI CPU_SUSPEND and CPU_ON.

For example,
On real HW, people are very much interested in time taken to resume a
HW CPU from suspended state because this affects responsiveness of
a system.

>
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 0a1d697..92242ce 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -39,7 +39,7 @@
>>  #include <kvm/arm_vgic.h>
>>  #include <kvm/arm_arch_timer.h>
>>
>> -#define KVM_VCPU_MAX_FEATURES 2
>> +#define KVM_VCPU_MAX_FEATURES 3
>>
>>  struct kvm_vcpu;
>>  int kvm_target_cpu(void);
>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>> index d9f026b..0eb254d 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -77,6 +77,7 @@ struct kvm_regs {
>>
>>  #define KVM_ARM_VCPU_POWER_OFF               0 /* CPU is started in OFF state */
>>  #define KVM_ARM_VCPU_EL1_32BIT               1 /* CPU running a 32bit VM */
>> +#define KVM_ARM_VCPU_PSCI_0_2                2 /* CPU uses PSCI v0.2 */
>>
>>  struct kvm_vcpu_init {
>>       __u32 target;
>> @@ -150,7 +151,7 @@ struct kvm_arch_memory_slot {
>>  /* Highest supported SPI, from VGIC_NR_IRQS */
>>  #define KVM_ARM_IRQ_GIC_MAX          127
>>
>> -/* PSCI interface */
>> +/* PSCI v0.1 interface */
>>  #define KVM_PSCI_FN_BASE             0x95c1ba5e
>>  #define KVM_PSCI_FN(n)                       (KVM_PSCI_FN_BASE + (n))
>>
>> @@ -159,10 +160,42 @@ struct kvm_arch_memory_slot {
>>  #define KVM_PSCI_FN_CPU_ON           KVM_PSCI_FN(2)
>>  #define KVM_PSCI_FN_MIGRATE          KVM_PSCI_FN(3)
>>
>> +/* PSCI v0.2 interface */
>> +#define KVM_PSCI_0_2_FN_BASE         0x84000000
>> +#define KVM_PSCI_0_2_FN(n)           (KVM_PSCI_0_2_FN_BASE + (n))
>> +#define KVM_PSCI_0_2_FN64_BASE               0xC4000000
>> +#define KVM_PSCI_0_2_FN64(n)         (KVM_PSCI_0_2_FN64_BASE + (n))
>> +
>> +#define KVM_PSCI_0_2_FN_PSCI_VERSION KVM_PSCI_0_2_FN(0)
>> +#define KVM_PSCI_0_2_FN_CPU_SUSPEND  KVM_PSCI_0_2_FN(1)
>> +#define KVM_PSCI_0_2_FN_CPU_OFF              KVM_PSCI_0_2_FN(2)
>> +#define KVM_PSCI_0_2_FN_CPU_ON               KVM_PSCI_0_2_FN(3)
>> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO        KVM_PSCI_0_2_FN(4)
>> +#define KVM_PSCI_0_2_FN_MIGRATE              KVM_PSCI_0_2_FN(5)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
>> +                                     KVM_PSCI_0_2_FN(6)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
>> +                                     KVM_PSCI_0_2_FN(7)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF   KVM_PSCI_0_2_FN(8)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET KVM_PSCI_0_2_FN(9)
>> +
>> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND        KVM_PSCI_0_2_FN64(1)
>> +#define KVM_PSCI_0_2_FN64_CPU_ON     KVM_PSCI_0_2_FN64(3)
>> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO      KVM_PSCI_0_2_FN64(4)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE    KVM_PSCI_0_2_FN64(5)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
>> +                                     KVM_PSCI_0_2_FN64(7)
>> +
>> +/* PSCI return values */
>>  #define KVM_PSCI_RET_SUCCESS         0
>> -#define KVM_PSCI_RET_NI                      ((unsigned long)-1)
>> -#define KVM_PSCI_RET_INVAL           ((unsigned long)-2)
>> +#define KVM_PSCI_RET_NOT_SUPPORTED   ((unsigned long)-1)
>> +#define KVM_PSCI_RET_INVALID_PARAMS  ((unsigned long)-2)
>>  #define KVM_PSCI_RET_DENIED          ((unsigned long)-3)
>> +#define KVM_PSCI_RET_ALREADY_ON              ((unsigned long)-4)
>> +#define KVM_PSCI_RET_ON_PENDING              ((unsigned long)-5)
>> +#define KVM_PSCI_RET_INTERNAL_FAILURE        ((unsigned long)-6)
>> +#define KVM_PSCI_RET_NOT_PRESENT     ((unsigned long)-7)
>> +#define KVM_PSCI_RET_DISABLED                ((unsigned long)-8)
>>
>>  #endif
>>
>> --
>> 1.7.9.5
>>
>
> Thanks,
> --
> Christoffer
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

--
Anup

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
  2014-01-29  4:52     ` Anup Patel
@ 2014-01-29 15:50       ` Christoffer Dall
  2014-01-30  4:09         ` Anup Patel
  0 siblings, 1 reply; 14+ messages in thread
From: Christoffer Dall @ 2014-01-29 15:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jan 29, 2014 at 10:22:47AM +0530, Anup Patel wrote:
> On Wed, Jan 29, 2014 at 2:34 AM, Christoffer Dall
> <christoffer.dall@linaro.org> wrote:
> > On Tue, Jan 21, 2014 at 06:31:40PM +0530, Anup Patel wrote:

[...]

> >
> > Which tree does this patch apply to?  It looks like you'll get a
> > conflict with:
> > 478a823 arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive
> 
> This patchset applies on v3.13 tag of Torvalds tree.

That would not contain anything in kvm/next or kvm-arm-next.

> 
> I generally base my patches on latest stable/rc tag of Torvalds tree
> so that I can provide KVM patches to folks interested in trying KVM
> on X-Gene with latest Linux stable/rc.

If you want to make it slightly easier for me or Marc to apply your
patches in general I would recommend basing them off kvm/next or
kvm-arm-next, but it's no big deal.

In this case all you need to consider is already in linus/master.

> 
> I will make sure that revised patchset applies on top of
> 478a823 arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive
> 
> >
> >>       }
> >>
> >> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> >> index 0881bf1..ee044a3 100644
> >> --- a/arch/arm/kvm/psci.c
> >> +++ b/arch/arm/kvm/psci.c
> >> @@ -55,13 +55,13 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
> >>       }
> >>
> >>       if (!vcpu)
> >> -             return KVM_PSCI_RET_INVAL;
> >> +             return KVM_PSCI_RET_INVALID_PARAMS;
> >>
> >>       target_pc = *vcpu_reg(source_vcpu, 2);
> >>
> >>       wq = kvm_arch_vcpu_wq(vcpu);
> >>       if (!waitqueue_active(wq))
> >> -             return KVM_PSCI_RET_INVAL;
> >> +             return KVM_PSCI_RET_INVALID_PARAMS;
> >>
> >>       kvm_reset_vcpu(vcpu);
> >>
> >> @@ -84,17 +84,49 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
> >>       return KVM_PSCI_RET_SUCCESS;
> >>  }
> >>
> >> -/**
> >> - * kvm_psci_call - handle PSCI call if r0 value is in range
> >> - * @vcpu: Pointer to the VCPU struct
> >> - *
> >> - * Handle PSCI calls from guests through traps from HVC instructions.
> >> - * The calling convention is similar to SMC calls to the secure world where
> >> - * the function number is placed in r0 and this function returns true if the
> >> - * function number specified in r0 is withing the PSCI range, and false
> >> - * otherwise.
> >> - */
> >> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
> >> +static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> >> +{
> >> +     unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
> >> +     unsigned long val;
> >> +
> >> +     switch (psci_fn) {
> >> +     case KVM_PSCI_0_2_FN_PSCI_VERSION:
> >> +             /*
> >> +              * Bits[31:16] = Major Version = 0
> >> +              * Bits[15:0] = Minor Version = 2
> >> +              */
> >> +             val = 2;
> >> +             break;
> >> +     case KVM_PSCI_0_2_FN_CPU_OFF:
> >> +             kvm_psci_vcpu_off(vcpu);
> >> +             val = KVM_PSCI_RET_SUCCESS;
> >> +             break;
> >> +     case KVM_PSCI_0_2_FN_CPU_ON:
> >> +     case KVM_PSCI_0_2_FN64_CPU_ON:
> >> +             val = kvm_psci_vcpu_on(vcpu);
> >> +             break;
> >> +     case KVM_PSCI_0_2_FN_CPU_SUSPEND:
> >> +     case KVM_PSCI_0_2_FN_AFFINITY_INFO:
> >> +     case KVM_PSCI_0_2_FN_MIGRATE:
> >> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
> >> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
> >> +     case KVM_PSCI_0_2_FN_SYSTEM_OFF:
> >> +     case KVM_PSCI_0_2_FN_SYSTEM_RESET:
> >> +     case KVM_PSCI_0_2_FN64_CPU_SUSPEND:
> >> +     case KVM_PSCI_0_2_FN64_AFFINITY_INFO:
> >> +     case KVM_PSCI_0_2_FN64_MIGRATE:
> >> +     case KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
> >> +             val = KVM_PSCI_RET_NOT_SUPPORTED;
> >> +             break;
> >> +     default:
> >> +             return false;
> >> +     }
> >> +
> >> +     *vcpu_reg(vcpu, 0) = val;
> >> +     return true;
> >> +}
> >> +
> >> +static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
> >>  {
> >>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
> >>       unsigned long val;
> >> @@ -109,9 +141,8 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
> >>               break;
> >>       case KVM_PSCI_FN_CPU_SUSPEND:
> >>       case KVM_PSCI_FN_MIGRATE:
> >> -             val = KVM_PSCI_RET_NI;
> >> +             val = KVM_PSCI_RET_NOT_SUPPORTED;
> >>               break;
> >> -
> >>       default:
> >>               return false;
> >>       }
> >> @@ -119,3 +150,21 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
> >>       *vcpu_reg(vcpu, 0) = val;
> >>       return true;
> >>  }
> >> +
> >> +/**
> >> + * kvm_psci_call - handle PSCI call if r0 value is in range
> >> + * @vcpu: Pointer to the VCPU struct
> >> + *
> >> + * Handle PSCI calls from guests through traps from HVC instructions.
> >> + * The calling convention is similar to SMC calls to the secure world where
> >> + * the function number is placed in r0 and this function returns true if the
> >> + * function number specified in r0 is withing the PSCI range, and false
> >> + * otherwise.
> >> + */
> >> +bool kvm_psci_call(struct kvm_vcpu *vcpu)
> >> +{
> >> +     if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
> >> +             return kvm_psci_0_2_call(vcpu);
> >> +
> >> +     return kvm_psci_0_1_call(vcpu);
> >> +}
> >
> > Why don't we just try one after the other?  Do they conflict in some
> > way?
> 
> Atleast the functions IDs are totally different in v0.2 and v0.1
> 
> Also, in v0.2 we have separate function IDs for 32bit and 64bit
> VCPU calling same PSCI function.
> 

So we could just do:

{
	ret = kvm_psci_0_2_call(vcpu);
	if (ret)
		return ret;

	ret = kvm_psci_0_1_call(vcpu);
	if (ret)
		return ret;

	return false;
}

and be rid of the vcpu feature, or?  I thought this was Marc's point in
the last KVM/ARM call?

> >
> > I assume PSCI calls are never going to be in the critical path and calls
> > into PSCI are pretty much expected to be slow as a dog anyhow, so if we
> > could avoid the extra churn in user space code and potential user
> > confusion (providing PSCI 0.2 kernel but too old user space tool for
> > example), I think that would be preferred.
> 
> Yes, PSCI calls will not be in critical path except few functions such as
> PSCI CPU_SUSPEND and CPU_ON.
> 
> For example,
> On real HW, people are very much interested in time taken to resume a
> HW CPU from suspended state because this affects responsiveness of
> a system.
> 

In which case time taken to wake up from suspended state in a VM will
for sure not be dominated by an extra call to a psci function id
checking function.

Thanks,
-Christoffer

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
  2014-01-29 15:50       ` Christoffer Dall
@ 2014-01-30  4:09         ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2014-01-30  4:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jan 29, 2014 at 9:20 PM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Wed, Jan 29, 2014 at 10:22:47AM +0530, Anup Patel wrote:
>> On Wed, Jan 29, 2014 at 2:34 AM, Christoffer Dall
>> <christoffer.dall@linaro.org> wrote:
>> > On Tue, Jan 21, 2014 at 06:31:40PM +0530, Anup Patel wrote:
>
> [...]
>
>> >
>> > Which tree does this patch apply to?  It looks like you'll get a
>> > conflict with:
>> > 478a823 arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive
>>
>> This patchset applies on v3.13 tag of Torvalds tree.
>
> That would not contain anything in kvm/next or kvm-arm-next.
>
>>
>> I generally base my patches on latest stable/rc tag of Torvalds tree
>> so that I can provide KVM patches to folks interested in trying KVM
>> on X-Gene with latest Linux stable/rc.
>
> If you want to make it slightly easier for me or Marc to apply your
> patches in general I would recommend basing them off kvm/next or
> kvm-arm-next, but it's no big deal.
>
> In this case all you need to consider is already in linus/master.
>
>>
>> I will make sure that revised patchset applies on top of
>> 478a823 arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive
>>
>> >
>> >>       }
>> >>
>> >> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
>> >> index 0881bf1..ee044a3 100644
>> >> --- a/arch/arm/kvm/psci.c
>> >> +++ b/arch/arm/kvm/psci.c
>> >> @@ -55,13 +55,13 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>> >>       }
>> >>
>> >>       if (!vcpu)
>> >> -             return KVM_PSCI_RET_INVAL;
>> >> +             return KVM_PSCI_RET_INVALID_PARAMS;
>> >>
>> >>       target_pc = *vcpu_reg(source_vcpu, 2);
>> >>
>> >>       wq = kvm_arch_vcpu_wq(vcpu);
>> >>       if (!waitqueue_active(wq))
>> >> -             return KVM_PSCI_RET_INVAL;
>> >> +             return KVM_PSCI_RET_INVALID_PARAMS;
>> >>
>> >>       kvm_reset_vcpu(vcpu);
>> >>
>> >> @@ -84,17 +84,49 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>> >>       return KVM_PSCI_RET_SUCCESS;
>> >>  }
>> >>
>> >> -/**
>> >> - * kvm_psci_call - handle PSCI call if r0 value is in range
>> >> - * @vcpu: Pointer to the VCPU struct
>> >> - *
>> >> - * Handle PSCI calls from guests through traps from HVC instructions.
>> >> - * The calling convention is similar to SMC calls to the secure world where
>> >> - * the function number is placed in r0 and this function returns true if the
>> >> - * function number specified in r0 is withing the PSCI range, and false
>> >> - * otherwise.
>> >> - */
>> >> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> >> +static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>> >> +{
>> >> +     unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>> >> +     unsigned long val;
>> >> +
>> >> +     switch (psci_fn) {
>> >> +     case KVM_PSCI_0_2_FN_PSCI_VERSION:
>> >> +             /*
>> >> +              * Bits[31:16] = Major Version = 0
>> >> +              * Bits[15:0] = Minor Version = 2
>> >> +              */
>> >> +             val = 2;
>> >> +             break;
>> >> +     case KVM_PSCI_0_2_FN_CPU_OFF:
>> >> +             kvm_psci_vcpu_off(vcpu);
>> >> +             val = KVM_PSCI_RET_SUCCESS;
>> >> +             break;
>> >> +     case KVM_PSCI_0_2_FN_CPU_ON:
>> >> +     case KVM_PSCI_0_2_FN64_CPU_ON:
>> >> +             val = kvm_psci_vcpu_on(vcpu);
>> >> +             break;
>> >> +     case KVM_PSCI_0_2_FN_CPU_SUSPEND:
>> >> +     case KVM_PSCI_0_2_FN_AFFINITY_INFO:
>> >> +     case KVM_PSCI_0_2_FN_MIGRATE:
>> >> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
>> >> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
>> >> +     case KVM_PSCI_0_2_FN_SYSTEM_OFF:
>> >> +     case KVM_PSCI_0_2_FN_SYSTEM_RESET:
>> >> +     case KVM_PSCI_0_2_FN64_CPU_SUSPEND:
>> >> +     case KVM_PSCI_0_2_FN64_AFFINITY_INFO:
>> >> +     case KVM_PSCI_0_2_FN64_MIGRATE:
>> >> +     case KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
>> >> +             val = KVM_PSCI_RET_NOT_SUPPORTED;
>> >> +             break;
>> >> +     default:
>> >> +             return false;
>> >> +     }
>> >> +
>> >> +     *vcpu_reg(vcpu, 0) = val;
>> >> +     return true;
>> >> +}
>> >> +
>> >> +static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>> >>  {
>> >>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>> >>       unsigned long val;
>> >> @@ -109,9 +141,8 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> >>               break;
>> >>       case KVM_PSCI_FN_CPU_SUSPEND:
>> >>       case KVM_PSCI_FN_MIGRATE:
>> >> -             val = KVM_PSCI_RET_NI;
>> >> +             val = KVM_PSCI_RET_NOT_SUPPORTED;
>> >>               break;
>> >> -
>> >>       default:
>> >>               return false;
>> >>       }
>> >> @@ -119,3 +150,21 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> >>       *vcpu_reg(vcpu, 0) = val;
>> >>       return true;
>> >>  }
>> >> +
>> >> +/**
>> >> + * kvm_psci_call - handle PSCI call if r0 value is in range
>> >> + * @vcpu: Pointer to the VCPU struct
>> >> + *
>> >> + * Handle PSCI calls from guests through traps from HVC instructions.
>> >> + * The calling convention is similar to SMC calls to the secure world where
>> >> + * the function number is placed in r0 and this function returns true if the
>> >> + * function number specified in r0 is withing the PSCI range, and false
>> >> + * otherwise.
>> >> + */
>> >> +bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> >> +{
>> >> +     if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>> >> +             return kvm_psci_0_2_call(vcpu);
>> >> +
>> >> +     return kvm_psci_0_1_call(vcpu);
>> >> +}
>> >
>> > Why don't we just try one after the other?  Do they conflict in some
>> > way?
>>
>> Atleast the functions IDs are totally different in v0.2 and v0.1
>>
>> Also, in v0.2 we have separate function IDs for 32bit and 64bit
>> VCPU calling same PSCI function.
>>
>
> So we could just do:
>
> {
>         ret = kvm_psci_0_2_call(vcpu);
>         if (ret)
>                 return ret;
>
>         ret = kvm_psci_0_1_call(vcpu);
>         if (ret)
>                 return ret;
>
>         return false;
> }
>
> and be rid of the vcpu feature, or?  I thought this was Marc's point in
> the last KVM/ARM call?

OK, I will update kvm_psci_call() as per this.

>
>> >
>> > I assume PSCI calls are never going to be in the critical path and calls
>> > into PSCI are pretty much expected to be slow as a dog anyhow, so if we
>> > could avoid the extra churn in user space code and potential user
>> > confusion (providing PSCI 0.2 kernel but too old user space tool for
>> > example), I think that would be preferred.
>>
>> Yes, PSCI calls will not be in critical path except few functions such as
>> PSCI CPU_SUSPEND and CPU_ON.
>>
>> For example,
>> On real HW, people are very much interested in time taken to resume a
>> HW CPU from suspended state because this affects responsiveness of
>> a system.
>>
>
> In which case time taken to wake up from suspended state in a VM will
> for sure not be dominated by an extra call to a psci function id
> checking function.
>
> Thanks,
> -Christoffer

--
Anup

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation
  2014-01-28 21:04   ` Christoffer Dall
  2014-01-29  4:52     ` Anup Patel
@ 2014-01-30  5:28     ` Anup Patel
  1 sibling, 0 replies; 14+ messages in thread
From: Anup Patel @ 2014-01-30  5:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Christoffer,

On Wed, Jan 29, 2014 at 2:34 AM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Tue, Jan 21, 2014 at 06:31:40PM +0530, Anup Patel wrote:
>> Currently, the in-kernel PSCI emulation provides PSCI v0.1 interface to
>> VCPUs. This patch extends current in-kernel PSCI emulation to provide
>> PSCI v0.2 interface to VCPUs.
>>
>> By default, ARM/ARM64 KVM will always provide PSCI v0.1 interface for
>> keeping the ABI backward-compatible.
>>
>> To select PSCI v0.2 interface for VCPUs, the user space (i.e. QEMU or
>> KVMTOOL) will have to set KVM_ARM_VCPU_PSCI_0_2 feature when doing VCPU
>> init using KVM_ARM_VCPU_INIT ioctl.
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> ---
>>  arch/arm/include/asm/kvm_host.h   |    2 +-
>>  arch/arm/include/uapi/asm/kvm.h   |   39 ++++++++++++++++--
>>  arch/arm/kvm/arm.c                |    6 ++-
>>  arch/arm/kvm/psci.c               |   79 ++++++++++++++++++++++++++++++-------
>>  arch/arm64/include/asm/kvm_host.h |    2 +-
>>  arch/arm64/include/uapi/asm/kvm.h |   39 ++++++++++++++++--
>>  6 files changed, 143 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index 8a6f6db..0239ac5 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -36,7 +36,7 @@
>>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>>  #define KVM_HAVE_ONE_REG
>>
>> -#define KVM_VCPU_MAX_FEATURES 1
>> +#define KVM_VCPU_MAX_FEATURES 2
>>
>>  #include <kvm/arm_vgic.h>
>>
>> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
>> index c498b60..d9eb74c 100644
>> --- a/arch/arm/include/uapi/asm/kvm.h
>> +++ b/arch/arm/include/uapi/asm/kvm.h
>> @@ -83,6 +83,7 @@ struct kvm_regs {
>>  #define KVM_VGIC_V2_CPU_SIZE         0x2000
>>
>>  #define KVM_ARM_VCPU_POWER_OFF               0 /* CPU is started in OFF state */
>> +#define KVM_ARM_VCPU_PSCI_0_2                1 /* CPU uses PSCI v0.2 */
>>
>>  struct kvm_vcpu_init {
>>       __u32 target;
>> @@ -164,7 +165,7 @@ struct kvm_arch_memory_slot {
>>  /* Highest supported SPI, from VGIC_NR_IRQS */
>>  #define KVM_ARM_IRQ_GIC_MAX          127
>>
>> -/* PSCI interface */
>> +/* PSCI v0.1 interface */
>>  #define KVM_PSCI_FN_BASE             0x95c1ba5e
>>  #define KVM_PSCI_FN(n)                       (KVM_PSCI_FN_BASE + (n))
>>
>> @@ -173,9 +174,41 @@ struct kvm_arch_memory_slot {
>>  #define KVM_PSCI_FN_CPU_ON           KVM_PSCI_FN(2)
>>  #define KVM_PSCI_FN_MIGRATE          KVM_PSCI_FN(3)
>>
>> +/* PSCI v0.2 interface */
>> +#define KVM_PSCI_0_2_FN_BASE         0x84000000
>> +#define KVM_PSCI_0_2_FN(n)           (KVM_PSCI_0_2_FN_BASE + (n))
>> +#define KVM_PSCI_0_2_FN64_BASE               0xC4000000
>> +#define KVM_PSCI_0_2_FN64(n)         (KVM_PSCI_0_2_FN64_BASE + (n))
>> +
>> +#define KVM_PSCI_0_2_FN_PSCI_VERSION KVM_PSCI_0_2_FN(0)
>> +#define KVM_PSCI_0_2_FN_CPU_SUSPEND  KVM_PSCI_0_2_FN(1)
>> +#define KVM_PSCI_0_2_FN_CPU_OFF              KVM_PSCI_0_2_FN(2)
>> +#define KVM_PSCI_0_2_FN_CPU_ON               KVM_PSCI_0_2_FN(3)
>> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO        KVM_PSCI_0_2_FN(4)
>> +#define KVM_PSCI_0_2_FN_MIGRATE              KVM_PSCI_0_2_FN(5)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
>> +                                     KVM_PSCI_0_2_FN(6)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
>> +                                     KVM_PSCI_0_2_FN(7)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF   KVM_PSCI_0_2_FN(8)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET KVM_PSCI_0_2_FN(9)
>> +
>> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND        KVM_PSCI_0_2_FN64(1)
>> +#define KVM_PSCI_0_2_FN64_CPU_ON     KVM_PSCI_0_2_FN64(3)
>> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO      KVM_PSCI_0_2_FN64(4)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE    KVM_PSCI_0_2_FN64(5)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
>> +                                     KVM_PSCI_0_2_FN64(7)
>> +
>> +/* PSCI return values */
>>  #define KVM_PSCI_RET_SUCCESS         0
>> -#define KVM_PSCI_RET_NI                      ((unsigned long)-1)
>> -#define KVM_PSCI_RET_INVAL           ((unsigned long)-2)
>> +#define KVM_PSCI_RET_NOT_SUPPORTED   ((unsigned long)-1)
>> +#define KVM_PSCI_RET_INVALID_PARAMS  ((unsigned long)-2)
>>  #define KVM_PSCI_RET_DENIED          ((unsigned long)-3)
>> +#define KVM_PSCI_RET_ALREADY_ON              ((unsigned long)-4)
>> +#define KVM_PSCI_RET_ON_PENDING              ((unsigned long)-5)
>> +#define KVM_PSCI_RET_INTERNAL_FAILURE        ((unsigned long)-6)
>> +#define KVM_PSCI_RET_NOT_PRESENT     ((unsigned long)-7)
>> +#define KVM_PSCI_RET_DISABLED                ((unsigned long)-8)
>>
>>  #endif /* __ARM_KVM_H__ */
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index 2a700e0..0b7817a 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -193,6 +193,7 @@ int kvm_dev_ioctl_check_extension(long ext)
>>       case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
>>       case KVM_CAP_ONE_REG:
>>       case KVM_CAP_ARM_PSCI:
>> +     case KVM_CAP_ARM_PSCI_0_2:
>>               r = 1;
>>               break;
>>       case KVM_CAP_COALESCED_MMIO:
>> @@ -483,7 +484,10 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
>>        * PSCI code.
>>        */
>>       if (test_and_clear_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features)) {
>> -             *vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
>> +             if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>> +                     *vcpu_reg(vcpu, 0) = KVM_PSCI_0_2_FN_CPU_OFF;
>> +             else
>> +                     *vcpu_reg(vcpu, 0) = KVM_PSCI_FN_CPU_OFF;
>>               kvm_psci_call(vcpu);
>
> Which tree does this patch apply to?  It looks like you'll get a
> conflict with:
> 478a823 arm: KVM: Don't return PSCI_INVAL if waitqueue is inactive
>
>>       }
>>
>> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
>> index 0881bf1..ee044a3 100644
>> --- a/arch/arm/kvm/psci.c
>> +++ b/arch/arm/kvm/psci.c
>> @@ -55,13 +55,13 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>>       }
>>
>>       if (!vcpu)
>> -             return KVM_PSCI_RET_INVAL;
>> +             return KVM_PSCI_RET_INVALID_PARAMS;
>>
>>       target_pc = *vcpu_reg(source_vcpu, 2);
>>
>>       wq = kvm_arch_vcpu_wq(vcpu);
>>       if (!waitqueue_active(wq))
>> -             return KVM_PSCI_RET_INVAL;
>> +             return KVM_PSCI_RET_INVALID_PARAMS;
>>
>>       kvm_reset_vcpu(vcpu);
>>
>> @@ -84,17 +84,49 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>>       return KVM_PSCI_RET_SUCCESS;
>>  }
>>
>> -/**
>> - * kvm_psci_call - handle PSCI call if r0 value is in range
>> - * @vcpu: Pointer to the VCPU struct
>> - *
>> - * Handle PSCI calls from guests through traps from HVC instructions.
>> - * The calling convention is similar to SMC calls to the secure world where
>> - * the function number is placed in r0 and this function returns true if the
>> - * function number specified in r0 is withing the PSCI range, and false
>> - * otherwise.
>> - */
>> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> +static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>> +{
>> +     unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>> +     unsigned long val;
>> +
>> +     switch (psci_fn) {
>> +     case KVM_PSCI_0_2_FN_PSCI_VERSION:
>> +             /*
>> +              * Bits[31:16] = Major Version = 0
>> +              * Bits[15:0] = Minor Version = 2
>> +              */
>> +             val = 2;
>> +             break;
>> +     case KVM_PSCI_0_2_FN_CPU_OFF:
>> +             kvm_psci_vcpu_off(vcpu);
>> +             val = KVM_PSCI_RET_SUCCESS;
>> +             break;
>> +     case KVM_PSCI_0_2_FN_CPU_ON:
>> +     case KVM_PSCI_0_2_FN64_CPU_ON:
>> +             val = kvm_psci_vcpu_on(vcpu);
>> +             break;
>> +     case KVM_PSCI_0_2_FN_CPU_SUSPEND:
>> +     case KVM_PSCI_0_2_FN_AFFINITY_INFO:
>> +     case KVM_PSCI_0_2_FN_MIGRATE:
>> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE:
>> +     case KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
>> +     case KVM_PSCI_0_2_FN_SYSTEM_OFF:
>> +     case KVM_PSCI_0_2_FN_SYSTEM_RESET:
>> +     case KVM_PSCI_0_2_FN64_CPU_SUSPEND:
>> +     case KVM_PSCI_0_2_FN64_AFFINITY_INFO:
>> +     case KVM_PSCI_0_2_FN64_MIGRATE:
>> +     case KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
>> +             val = KVM_PSCI_RET_NOT_SUPPORTED;
>> +             break;
>> +     default:
>> +             return false;
>> +     }
>> +
>> +     *vcpu_reg(vcpu, 0) = val;
>> +     return true;
>> +}
>> +
>> +static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>>  {
>>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>       unsigned long val;
>> @@ -109,9 +141,8 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>>               break;
>>       case KVM_PSCI_FN_CPU_SUSPEND:
>>       case KVM_PSCI_FN_MIGRATE:
>> -             val = KVM_PSCI_RET_NI;
>> +             val = KVM_PSCI_RET_NOT_SUPPORTED;
>>               break;
>> -
>>       default:
>>               return false;
>>       }
>> @@ -119,3 +150,21 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>>       *vcpu_reg(vcpu, 0) = val;
>>       return true;
>>  }
>> +
>> +/**
>> + * kvm_psci_call - handle PSCI call if r0 value is in range
>> + * @vcpu: Pointer to the VCPU struct
>> + *
>> + * Handle PSCI calls from guests through traps from HVC instructions.
>> + * The calling convention is similar to SMC calls to the secure world where
>> + * the function number is placed in r0 and this function returns true if the
>> + * function number specified in r0 is withing the PSCI range, and false
>> + * otherwise.
>> + */
>> +bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> +{
>> +     if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>> +             return kvm_psci_0_2_call(vcpu);
>> +
>> +     return kvm_psci_0_1_call(vcpu);
>> +}
>
> Why don't we just try one after the other?  Do they conflict in some
> way?
>
> I assume PSCI calls are never going to be in the critical path and calls
> into PSCI are pretty much expected to be slow as a dog anyhow, so if we
> could avoid the extra churn in user space code and potential user
> confusion (providing PSCI 0.2 kernel but too old user space tool for
> example), I think that would be preferred.

I gave more thought to this.

Currently, there is no conflict of function IDs between PSCI v0.1 and v0.2
but what if in future some new PSCI vX.Y comes-up with function IDs which
conflict with existing PSCI vA.B.

I think it is very likely that we will have more versions of PSCI and each
subsequent of version of PSCI will generally extend functions IDs defined
by PSCI v0.2

I will keep the KVM_ARM_VCPU_PSCI_0_2 feature for v2 patchset and
wait for more comments on this.

Regards,
Anup

>
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 0a1d697..92242ce 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -39,7 +39,7 @@
>>  #include <kvm/arm_vgic.h>
>>  #include <kvm/arm_arch_timer.h>
>>
>> -#define KVM_VCPU_MAX_FEATURES 2
>> +#define KVM_VCPU_MAX_FEATURES 3
>>
>>  struct kvm_vcpu;
>>  int kvm_target_cpu(void);
>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>> index d9f026b..0eb254d 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -77,6 +77,7 @@ struct kvm_regs {
>>
>>  #define KVM_ARM_VCPU_POWER_OFF               0 /* CPU is started in OFF state */
>>  #define KVM_ARM_VCPU_EL1_32BIT               1 /* CPU running a 32bit VM */
>> +#define KVM_ARM_VCPU_PSCI_0_2                2 /* CPU uses PSCI v0.2 */
>>
>>  struct kvm_vcpu_init {
>>       __u32 target;
>> @@ -150,7 +151,7 @@ struct kvm_arch_memory_slot {
>>  /* Highest supported SPI, from VGIC_NR_IRQS */
>>  #define KVM_ARM_IRQ_GIC_MAX          127
>>
>> -/* PSCI interface */
>> +/* PSCI v0.1 interface */
>>  #define KVM_PSCI_FN_BASE             0x95c1ba5e
>>  #define KVM_PSCI_FN(n)                       (KVM_PSCI_FN_BASE + (n))
>>
>> @@ -159,10 +160,42 @@ struct kvm_arch_memory_slot {
>>  #define KVM_PSCI_FN_CPU_ON           KVM_PSCI_FN(2)
>>  #define KVM_PSCI_FN_MIGRATE          KVM_PSCI_FN(3)
>>
>> +/* PSCI v0.2 interface */
>> +#define KVM_PSCI_0_2_FN_BASE         0x84000000
>> +#define KVM_PSCI_0_2_FN(n)           (KVM_PSCI_0_2_FN_BASE + (n))
>> +#define KVM_PSCI_0_2_FN64_BASE               0xC4000000
>> +#define KVM_PSCI_0_2_FN64(n)         (KVM_PSCI_0_2_FN64_BASE + (n))
>> +
>> +#define KVM_PSCI_0_2_FN_PSCI_VERSION KVM_PSCI_0_2_FN(0)
>> +#define KVM_PSCI_0_2_FN_CPU_SUSPEND  KVM_PSCI_0_2_FN(1)
>> +#define KVM_PSCI_0_2_FN_CPU_OFF              KVM_PSCI_0_2_FN(2)
>> +#define KVM_PSCI_0_2_FN_CPU_ON               KVM_PSCI_0_2_FN(3)
>> +#define KVM_PSCI_0_2_FN_AFFINITY_INFO        KVM_PSCI_0_2_FN(4)
>> +#define KVM_PSCI_0_2_FN_MIGRATE              KVM_PSCI_0_2_FN(5)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_TYPE \
>> +                                     KVM_PSCI_0_2_FN(6)
>> +#define KVM_PSCI_0_2_FN_MIGRATE_INFO_UP_CPU \
>> +                                     KVM_PSCI_0_2_FN(7)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_OFF   KVM_PSCI_0_2_FN(8)
>> +#define KVM_PSCI_0_2_FN_SYSTEM_RESET KVM_PSCI_0_2_FN(9)
>> +
>> +#define KVM_PSCI_0_2_FN64_CPU_SUSPEND        KVM_PSCI_0_2_FN64(1)
>> +#define KVM_PSCI_0_2_FN64_CPU_ON     KVM_PSCI_0_2_FN64(3)
>> +#define KVM_PSCI_0_2_FN64_AFFINITY_INFO      KVM_PSCI_0_2_FN64(4)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE    KVM_PSCI_0_2_FN64(5)
>> +#define KVM_PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU \
>> +                                     KVM_PSCI_0_2_FN64(7)
>> +
>> +/* PSCI return values */
>>  #define KVM_PSCI_RET_SUCCESS         0
>> -#define KVM_PSCI_RET_NI                      ((unsigned long)-1)
>> -#define KVM_PSCI_RET_INVAL           ((unsigned long)-2)
>> +#define KVM_PSCI_RET_NOT_SUPPORTED   ((unsigned long)-1)
>> +#define KVM_PSCI_RET_INVALID_PARAMS  ((unsigned long)-2)
>>  #define KVM_PSCI_RET_DENIED          ((unsigned long)-3)
>> +#define KVM_PSCI_RET_ALREADY_ON              ((unsigned long)-4)
>> +#define KVM_PSCI_RET_ON_PENDING              ((unsigned long)-5)
>> +#define KVM_PSCI_RET_INTERNAL_FAILURE        ((unsigned long)-6)
>> +#define KVM_PSCI_RET_NOT_PRESENT     ((unsigned long)-7)
>> +#define KVM_PSCI_RET_DISABLED                ((unsigned long)-8)
>>
>>  #endif
>>
>> --
>> 1.7.9.5
>>
>
> Thanks,
> --
> Christoffer
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-01-30  5:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-21 13:01 [RFC PATCH 0/3] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
2014-01-21 13:01 ` [RFC PATCH 1/3] KVM: Add capability to advertise PSCI v0.2 support Anup Patel
2014-01-21 13:01 ` [RFC PATCH 2/3] ARM/ARM64: KVM: Add support for PSCI v0.2 emulation Anup Patel
2014-01-28 11:27   ` Marc Zyngier
2014-01-28 11:43     ` Anup Patel
2014-01-28 21:04   ` Christoffer Dall
2014-01-29  4:52     ` Anup Patel
2014-01-29 15:50       ` Christoffer Dall
2014-01-30  4:09         ` Anup Patel
2014-01-30  5:28     ` Anup Patel
2014-01-21 13:01 ` [RFC PATCH 3/3] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature Anup Patel
2014-01-28 21:05   ` Christoffer Dall
2014-01-29  4:30     ` Anup Patel
2014-01-28  4:20 ` [RFC PATCH 0/3] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.