All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64
@ 2014-04-15  6:14 Anup Patel
  2014-04-15  6:14 ` [PATCH v9 01/12] KVM: Add capability to advertise PSCI v0.2 support Anup Patel
                   ` (11 more replies)
  0 siblings, 12 replies; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 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.

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.

Changlog:

V9:
 - Rename undefined PSCI_VER_xxx defines to PSCI_VERSION_xxx defines

V8:
 - Add #define for possible values of migrate type in uapi/linux/psci.h
 - Simplified psci_affinity_mask() in psci.c
 - Update comments in kvm_psci_vcpu_suspend() to indicate that for KVM
   wakeup events are interrupts.
 - Unconditionally update r0 (or x0) in kvm_psci_vcpu_on()

V7:
 - Make uapi/linux/psci.h inline with Ashwin's patch
   http://www.spinics.net/lists/arm-kernel/msg319090.html
 - Incorporate Rob's suggestions for uapi/linux/psci.h
 - Treat CPU_SUSPEND power-down request to be same as standby
   request. This further simplifies CPU_SUSPEND emulation.

V6:
 - Introduce uapi/linux/psci.h for sharing PSCI defines between
   ARM kernel, ARM64 kernel, KVM ARM/ARM64 and user space
 - Make CPU_SUSPEND emulation similar to WFI emulation

V5:
 - Have separate last patch to advertise KVM_CAP_ARM_PSCI_0_2
 - Use kvm_psci_version() in kvm_psci_vcpu_on()
 - Return ALREADY_ON for PSCI v0.2 CPU_ON if VCPU is not paused
 - Remove per-VCPU suspend context
 - As-per PSCI v0.2 spec, only current CPU can suspend itself

V4:
 - Implement all mandatory functions required by PSCI v0.2

V3:
 - Make KVM_ARM_VCPU_PSCI_0_2 feature experiementatl for now so that
   it fails for user space till all mandatory PSCI v0.2 functions are
   emulated by KVM ARM/ARM64
 - Have separate patch for making KVM_ARM_VCPU_PSCI_0_2 feature available
   to user space. This patch can be defferred for now

V2:
 - Don't rename PSCI return values KVM_PSCI_RET_NI and KVM_PSCI_RET_INVAL
 - Added kvm_psci_version() to get PSCI version available to VCPU
 - Fixed grammer in Documentation/virtual/kvm/api.txt

V1:
 - Initial RFC PATCH

Anup Patel (12):
  KVM: Add capability to advertise PSCI v0.2 support
  ARM/ARM64: KVM: Add common header for PSCI related defines
  ARM/ARM64: KVM: Add base for PSCI v0.2 emulation
  KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature
  ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible
  KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header
  ARM/ARM64: KVM: Emulate PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET
  ARM/ARM64: KVM: Emulate PSCI v0.2 AFFINITY_INFO
  ARM/ARM64: KVM: Emulate PSCI v0.2 MIGRATE_INFO_TYPE and related
    functions
  ARM/ARM64: KVM: Fix CPU_ON emulation for PSCI v0.2
  ARM/ARM64: KVM: Emulate PSCI v0.2 CPU_SUSPEND
  ARM/ARM64: KVM: Advertise KVM_CAP_ARM_PSCI_0_2 to user space

 Documentation/virtual/kvm/api.txt |   17 +++
 arch/arm/include/asm/kvm_host.h   |    2 +-
 arch/arm/include/asm/kvm_psci.h   |    6 +-
 arch/arm/include/uapi/asm/kvm.h   |   19 ++--
 arch/arm/kvm/arm.c                |    1 +
 arch/arm/kvm/handle_exit.c        |   10 +-
 arch/arm/kvm/psci.c               |  222 +++++++++++++++++++++++++++++++++----
 arch/arm64/include/asm/kvm_host.h |    2 +-
 arch/arm64/include/asm/kvm_psci.h |    6 +-
 arch/arm64/include/uapi/asm/kvm.h |   21 ++--
 arch/arm64/kvm/handle_exit.c      |   10 +-
 include/uapi/linux/Kbuild         |    1 +
 include/uapi/linux/kvm.h          |    9 ++
 include/uapi/linux/psci.h         |   78 +++++++++++++
 14 files changed, 356 insertions(+), 48 deletions(-)
 create mode 100644 include/uapi/linux/psci.h

-- 
1.7.9.5

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

* [PATCH v9 01/12] KVM: Add capability to advertise PSCI v0.2 support
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15  9:49   ` Marc Zyngier
  2014-04-15  6:14 ` [PATCH v9 02/12] ARM/ARM64: KVM: Add common header for PSCI related defines Anup Patel
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 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>
Acked-by: Christoffer Dall <christoffer.dall@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 a8f4ee5..01c5624 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -743,6 +743,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_IOAPIC_POLARITY_IGNORED 97
 #define KVM_CAP_ENABLE_CAP_VM 98
 #define KVM_CAP_S390_IRQCHIP 99
+#define KVM_CAP_ARM_PSCI_0_2 100
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
1.7.9.5

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

* [PATCH v9 02/12] ARM/ARM64: KVM: Add common header for PSCI related defines
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
  2014-04-15  6:14 ` [PATCH v9 01/12] KVM: Add capability to advertise PSCI v0.2 support Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15 10:06   ` Marc Zyngier
  2014-04-15  6:14 ` [PATCH v9 03/12] ARM/ARM64: KVM: Add base for PSCI v0.2 emulation Anup Patel
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 UTC (permalink / raw)
  To: linux-arm-kernel

We need a common place to share PSCI related defines among ARM kernel,
ARM64 kernel, KVM ARM/ARM64 PSCI emulation, and user space.

We introduce uapi/linux/psci.h for this purpose. This newly added
header will be first used by KVM ARM/ARM64 in-kernel PSCI emulation
and user space (i.e. QEMU or KVMTOOL).

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
---
 include/uapi/linux/Kbuild |    1 +
 include/uapi/linux/psci.h |   78 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)
 create mode 100644 include/uapi/linux/psci.h

diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 6929571..24e9033 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -317,6 +317,7 @@ header-y += ppp-ioctl.h
 header-y += ppp_defs.h
 header-y += pps.h
 header-y += prctl.h
+header-y += psci.h
 header-y += ptp_clock.h
 header-y += ptrace.h
 header-y += qnx4_fs.h
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
new file mode 100644
index 0000000..077b623
--- /dev/null
+++ b/include/uapi/linux/psci.h
@@ -0,0 +1,78 @@
+/*
+ * ARM Power State and Coordination Interface (PSCI) header
+ *
+ * This header holds common PSCI defines and macros shared
+ * by: ARM kernel, ARM64 kernel, KVM ARM/ARM64 and user space.
+ *
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Anup Patel <anup.patel@linaro.org>
+ */
+
+#ifndef _UAPI_LINUX_PSCI_H
+#define _UAPI_LINUX_PSCI_H
+
+/* PSCI v0.1 interface */
+#define PSCI_FN(base, n)			((base) + (n))
+
+#define PSCI_FN_CPU_SUSPEND(base)		PSCI_FN(base, 0)
+#define PSCI_FN_CPU_OFF(base)			PSCI_FN(base, 1)
+#define PSCI_FN_CPU_ON(base)			PSCI_FN(base, 2)
+#define PSCI_FN_MIGRATE(base)			PSCI_FN(base, 3)
+
+/* PSCI v0.2 interface */
+#define PSCI_0_2_FN_BASE			0x84000000
+#define PSCI_0_2_FN(n)				(PSCI_0_2_FN_BASE + (n))
+#define PSCI_0_2_64BIT				0x40000000
+#define PSCI_0_2_FN64_BASE			\
+					(PSCI_0_2_FN_BASE + PSCI_0_2_64BIT)
+#define PSCI_0_2_FN64(n)			(PSCI_0_2_FN64_BASE + (n))
+
+#define PSCI_0_2_FN_PSCI_VERSION		PSCI_0_2_FN(0)
+#define PSCI_0_2_FN_CPU_SUSPEND			PSCI_0_2_FN(1)
+#define PSCI_0_2_FN_CPU_OFF			PSCI_0_2_FN(2)
+#define PSCI_0_2_FN_CPU_ON			PSCI_0_2_FN(3)
+#define PSCI_0_2_FN_AFFINITY_INFO		PSCI_0_2_FN(4)
+#define PSCI_0_2_FN_MIGRATE			PSCI_0_2_FN(5)
+#define PSCI_0_2_FN_MIGRATE_INFO_TYPE		PSCI_0_2_FN(6)
+#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU		PSCI_0_2_FN(7)
+#define PSCI_0_2_FN_SYSTEM_OFF			PSCI_0_2_FN(8)
+#define PSCI_0_2_FN_SYSTEM_RESET		PSCI_0_2_FN(9)
+
+#define PSCI_0_2_FN64_CPU_SUSPEND		PSCI_0_2_FN64(1)
+#define PSCI_0_2_FN64_CPU_ON			PSCI_0_2_FN64(3)
+#define PSCI_0_2_FN64_AFFINITY_INFO		PSCI_0_2_FN64(4)
+#define PSCI_0_2_FN64_MIGRATE			PSCI_0_2_FN64(5)
+#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN64(7)
+
+#define PSCI_0_2_POWER_STATE_ID_MASK		0xffff
+#define PSCI_0_2_POWER_STATE_ID_SHIFT		0
+#define PSCI_0_2_POWER_STATE_TYPE_MASK		0x1
+#define PSCI_0_2_POWER_STATE_TYPE_SHIFT		16
+#define PSCI_0_2_POWER_STATE_AFFL_MASK		0x3
+#define PSCI_0_2_POWER_STATE_AFFL_SHIFT		24
+
+#define PSCI_0_2_TOS_UP_MIGRATE			0
+#define PSCI_0_2_TOS_UP_NO_MIGRATE		1
+#define PSCI_0_2_TOS_MP				2
+
+/* PSCI version decoding (independent of PSCI version) */
+#define PSCI_VERSION_MAJOR_MASK			0xffff0000
+#define PSCI_VERSION_MINOR_MASK			0x0000ffff
+#define PSCI_VERSION_MAJOR_SHIFT		16
+#define PSCI_VERSION_MAJOR(ver)			\
+		(((ver) & PSCI_VERSION_MAJOR_MASK) >> PSCI_VERSION_MAJOR_SHIFT)
+#define PSCI_VERSION_MINOR(ver)			\
+		((ver) & PSCI_VERSION_MINOR_MASK)
+
+/* PSCI return values (inclusive of all PSCI versions) */
+#define PSCI_RET_SUCCESS			0
+#define PSCI_RET_NOT_SUPPORTED			-1
+#define PSCI_RET_INVALID_PARAMS			-2
+#define PSCI_RET_DENIED				-3
+#define PSCI_RET_ALREADY_ON			-4
+#define PSCI_RET_ON_PENDING			-5
+#define PSCI_RET_INTERNAL_FAILURE		-6
+#define PSCI_RET_NOT_PRESENT			-7
+#define PSCI_RET_DISABLED			-8
+
+#endif /* _UAPI_LINUX_PSCI_H */
-- 
1.7.9.5

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

* [PATCH v9 03/12] ARM/ARM64: KVM: Add base for PSCI v0.2 emulation
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
  2014-04-15  6:14 ` [PATCH v9 01/12] KVM: Add capability to advertise PSCI v0.2 support Anup Patel
  2014-04-15  6:14 ` [PATCH v9 02/12] ARM/ARM64: KVM: Add common header for PSCI related defines Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15 10:19   ` Marc Zyngier
  2014-04-15  6:14 ` [PATCH v9 04/12] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature Anup Patel
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 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>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 arch/arm/include/asm/kvm_host.h   |    2 +-
 arch/arm/include/asm/kvm_psci.h   |    4 ++
 arch/arm/include/uapi/asm/kvm.h   |   19 ++++----
 arch/arm/kvm/psci.c               |   93 ++++++++++++++++++++++++++++++-------
 arch/arm64/include/asm/kvm_host.h |    2 +-
 arch/arm64/include/asm/kvm_psci.h |    4 ++
 arch/arm64/include/uapi/asm/kvm.h |   21 +++++----
 7 files changed, 108 insertions(+), 37 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 09af149..193ceaf 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/asm/kvm_psci.h b/arch/arm/include/asm/kvm_psci.h
index 9a83d98..4c0e3e1 100644
--- a/arch/arm/include/asm/kvm_psci.h
+++ b/arch/arm/include/asm/kvm_psci.h
@@ -18,6 +18,10 @@
 #ifndef __ARM_KVM_PSCI_H__
 #define __ARM_KVM_PSCI_H__
 
+#define KVM_ARM_PSCI_0_1	1
+#define KVM_ARM_PSCI_0_2	2
+
+int kvm_psci_version(struct kvm_vcpu *vcpu);
 bool kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM_KVM_PSCI_H__ */
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index ef0c878..6574ddf 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -20,6 +20,7 @@
 #define __ARM_KVM_H__
 
 #include <linux/types.h>
+#include <linux/psci.h>
 #include <asm/ptrace.h>
 
 #define __KVM_HAVE_GUEST_DEBUG
@@ -83,6 +84,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;
@@ -194,16 +196,15 @@ struct kvm_arch_memory_slot {
 
 /* PSCI interface */
 #define KVM_PSCI_FN_BASE		0x95c1ba5e
-#define KVM_PSCI_FN(n)			(KVM_PSCI_FN_BASE + (n))
 
-#define KVM_PSCI_FN_CPU_SUSPEND		KVM_PSCI_FN(0)
-#define KVM_PSCI_FN_CPU_OFF		KVM_PSCI_FN(1)
-#define KVM_PSCI_FN_CPU_ON		KVM_PSCI_FN(2)
-#define KVM_PSCI_FN_MIGRATE		KVM_PSCI_FN(3)
+#define KVM_PSCI_FN_CPU_SUSPEND		PSCI_FN(KVM_PSCI_FN_BASE, 0)
+#define KVM_PSCI_FN_CPU_OFF		PSCI_FN(KVM_PSCI_FN_BASE, 1)
+#define KVM_PSCI_FN_CPU_ON		PSCI_FN(KVM_PSCI_FN_BASE, 2)
+#define KVM_PSCI_FN_MIGRATE		PSCI_FN(KVM_PSCI_FN_BASE, 3)
 
-#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_DENIED		((unsigned long)-3)
+#define KVM_PSCI_RET_SUCCESS		PSCI_RET_SUCCESS
+#define KVM_PSCI_RET_NI			PSCI_RET_NOT_SUPPORTED
+#define KVM_PSCI_RET_INVAL		PSCI_RET_INVALID_PARAMS
+#define KVM_PSCI_RET_DENIED		PSCI_RET_DENIED
 
 #endif /* __ARM_KVM_H__ */
diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index 448f60e..8c42596c 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -59,7 +59,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	 * turned off.
 	 */
 	if (!vcpu || !vcpu->arch.pause)
-		return KVM_PSCI_RET_INVAL;
+		return PSCI_RET_INVALID_PARAMS;
 
 	target_pc = *vcpu_reg(source_vcpu, 2);
 
@@ -82,20 +82,60 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	wq = kvm_arch_vcpu_wq(vcpu);
 	wake_up_interruptible(wq);
 
-	return KVM_PSCI_RET_SUCCESS;
+	return 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)
+int kvm_psci_version(struct kvm_vcpu *vcpu)
+{
+	if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
+		return KVM_ARM_PSCI_0_2;
+
+	return KVM_ARM_PSCI_0_1;
+}
+
+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 PSCI_0_2_FN_PSCI_VERSION:
+		/*
+		 * Bits[31:16] = Major Version = 0
+		 * Bits[15:0] = Minor Version = 2
+		 */
+		val = 2;
+		break;
+	case PSCI_0_2_FN_CPU_OFF:
+		kvm_psci_vcpu_off(vcpu);
+		val = PSCI_RET_SUCCESS;
+		break;
+	case PSCI_0_2_FN_CPU_ON:
+	case PSCI_0_2_FN64_CPU_ON:
+		val = kvm_psci_vcpu_on(vcpu);
+		break;
+	case PSCI_0_2_FN_CPU_SUSPEND:
+	case PSCI_0_2_FN_AFFINITY_INFO:
+	case PSCI_0_2_FN_MIGRATE:
+	case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
+	case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
+	case PSCI_0_2_FN_SYSTEM_OFF:
+	case PSCI_0_2_FN_SYSTEM_RESET:
+	case PSCI_0_2_FN64_CPU_SUSPEND:
+	case PSCI_0_2_FN64_AFFINITY_INFO:
+	case PSCI_0_2_FN64_MIGRATE:
+	case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
+		val = 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;
@@ -103,16 +143,15 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
 	switch (psci_fn) {
 	case KVM_PSCI_FN_CPU_OFF:
 		kvm_psci_vcpu_off(vcpu);
-		val = KVM_PSCI_RET_SUCCESS;
+		val = PSCI_RET_SUCCESS;
 		break;
 	case KVM_PSCI_FN_CPU_ON:
 		val = kvm_psci_vcpu_on(vcpu);
 		break;
 	case KVM_PSCI_FN_CPU_SUSPEND:
 	case KVM_PSCI_FN_MIGRATE:
-		val = KVM_PSCI_RET_NI;
+		val = PSCI_RET_NOT_SUPPORTED;
 		break;
-
 	default:
 		return false;
 	}
@@ -120,3 +159,25 @@ 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)
+{
+	switch (kvm_psci_version(vcpu)) {
+	case KVM_ARM_PSCI_0_2:
+		return kvm_psci_0_2_call(vcpu);
+	case KVM_ARM_PSCI_0_1:
+		return kvm_psci_0_1_call(vcpu);
+	default:
+		return false;
+	};
+}
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/asm/kvm_psci.h b/arch/arm64/include/asm/kvm_psci.h
index e301a48..e25c658 100644
--- a/arch/arm64/include/asm/kvm_psci.h
+++ b/arch/arm64/include/asm/kvm_psci.h
@@ -18,6 +18,10 @@
 #ifndef __ARM64_KVM_PSCI_H__
 #define __ARM64_KVM_PSCI_H__
 
+#define KVM_ARM_PSCI_0_1	1
+#define KVM_ARM_PSCI_0_2	2
+
+int kvm_psci_version(struct kvm_vcpu *vcpu);
 bool kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM64_KVM_PSCI_H__ */
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index eaf54a3..9b67161 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -31,6 +31,7 @@
 #define KVM_NR_SPSR	5
 
 #ifndef __ASSEMBLY__
+#include <linux/psci.h>
 #include <asm/types.h>
 #include <asm/ptrace.h>
 
@@ -77,6 +78,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;
@@ -177,19 +179,18 @@ 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))
 
-#define KVM_PSCI_FN_CPU_SUSPEND		KVM_PSCI_FN(0)
-#define KVM_PSCI_FN_CPU_OFF		KVM_PSCI_FN(1)
-#define KVM_PSCI_FN_CPU_ON		KVM_PSCI_FN(2)
-#define KVM_PSCI_FN_MIGRATE		KVM_PSCI_FN(3)
+#define KVM_PSCI_FN_CPU_SUSPEND		PSCI_FN(KVM_PSCI_FN_BASE, 0)
+#define KVM_PSCI_FN_CPU_OFF		PSCI_FN(KVM_PSCI_FN_BASE, 1)
+#define KVM_PSCI_FN_CPU_ON		PSCI_FN(KVM_PSCI_FN_BASE, 2)
+#define KVM_PSCI_FN_MIGRATE		PSCI_FN(KVM_PSCI_FN_BASE, 3)
 
-#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_DENIED		((unsigned long)-3)
+#define KVM_PSCI_RET_SUCCESS		PSCI_RET_SUCCESS
+#define KVM_PSCI_RET_NI			PSCI_RET_NOT_SUPPORTED
+#define KVM_PSCI_RET_INVAL		PSCI_RET_INVALID_PARAMS
+#define KVM_PSCI_RET_DENIED		PSCI_RET_DENIED
 
 #endif
 
-- 
1.7.9.5

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

* [PATCH v9 04/12] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
                   ` (2 preceding siblings ...)
  2014-04-15  6:14 ` [PATCH v9 03/12] ARM/ARM64: KVM: Add base for PSCI v0.2 emulation Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15 10:20   ` Marc Zyngier
  2014-04-15  6:14 ` [PATCH v9 05/12] ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible Anup Patel
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 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>
Acked-by: Christoffer Dall <christoffer.dall@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 a9380ba5..6dc1db5 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2376,6 +2376,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 the 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] 33+ messages in thread

* [PATCH v9 05/12] ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
                   ` (3 preceding siblings ...)
  2014-04-15  6:14 ` [PATCH v9 04/12] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15 10:21   ` Marc Zyngier
  2014-04-15  6:14 ` [PATCH v9 06/12] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header Anup Patel
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, the kvm_psci_call() returns 'true' or 'false' based on whether
the PSCI function call was handled successfully or not. This does not help
us emulate system-level PSCI functions where the actual emulation work will
be done by user space (QEMU or KVMTOOL). Examples of such system-level PSCI
functions are: PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET.

This patch updates kvm_psci_call() to return three types of values:
1) > 0 (success)
2) = 0 (success but exit to user space)
3) < 0 (errors)

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 arch/arm/include/asm/kvm_psci.h   |    2 +-
 arch/arm/kvm/handle_exit.c        |   10 +++++++---
 arch/arm/kvm/psci.c               |   28 ++++++++++++++++------------
 arch/arm64/include/asm/kvm_psci.h |    2 +-
 arch/arm64/kvm/handle_exit.c      |   10 +++++++---
 5 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/arch/arm/include/asm/kvm_psci.h b/arch/arm/include/asm/kvm_psci.h
index 4c0e3e1..6bda945 100644
--- a/arch/arm/include/asm/kvm_psci.h
+++ b/arch/arm/include/asm/kvm_psci.h
@@ -22,6 +22,6 @@
 #define KVM_ARM_PSCI_0_2	2
 
 int kvm_psci_version(struct kvm_vcpu *vcpu);
-bool kvm_psci_call(struct kvm_vcpu *vcpu);
+int kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM_KVM_PSCI_H__ */
diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
index 0de91fc..1270095 100644
--- a/arch/arm/kvm/handle_exit.c
+++ b/arch/arm/kvm/handle_exit.c
@@ -38,14 +38,18 @@ static int handle_svc_hyp(struct kvm_vcpu *vcpu, struct kvm_run *run)
 
 static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
+	int ret;
+
 	trace_kvm_hvc(*vcpu_pc(vcpu), *vcpu_reg(vcpu, 0),
 		      kvm_vcpu_hvc_get_imm(vcpu));
 
-	if (kvm_psci_call(vcpu))
+	ret = kvm_psci_call(vcpu);
+	if (ret == -EINVAL) {
+		kvm_inject_undefined(vcpu);
 		return 1;
+	}
 
-	kvm_inject_undefined(vcpu);
-	return 1;
+	return ret;
 }
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index 8c42596c..14e6fa6 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -93,7 +93,7 @@ int kvm_psci_version(struct kvm_vcpu *vcpu)
 	return KVM_ARM_PSCI_0_1;
 }
 
-static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
+static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 {
 	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
 	unsigned long val;
@@ -128,14 +128,14 @@ static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 		val = PSCI_RET_NOT_SUPPORTED;
 		break;
 	default:
-		return false;
+		return -EINVAL;
 	}
 
 	*vcpu_reg(vcpu, 0) = val;
-	return true;
+	return 1;
 }
 
-static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
+static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
 {
 	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
 	unsigned long val;
@@ -153,11 +153,11 @@ static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
 		val = PSCI_RET_NOT_SUPPORTED;
 		break;
 	default:
-		return false;
+		return -EINVAL;
 	}
 
 	*vcpu_reg(vcpu, 0) = val;
-	return true;
+	return 1;
 }
 
 /**
@@ -165,12 +165,16 @@ static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
  * @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.
+ * The calling convention is similar to SMC calls to the secure world
+ * where the function number is placed in r0.
+ *
+ * This function returns: > 0 (success), 0 (success but exit to user
+ * space), and < 0 (errors)
+ *
+ * Errors:
+ * -EINVAL: Unrecognized PSCI function
  */
-bool kvm_psci_call(struct kvm_vcpu *vcpu)
+int kvm_psci_call(struct kvm_vcpu *vcpu)
 {
 	switch (kvm_psci_version(vcpu)) {
 	case KVM_ARM_PSCI_0_2:
@@ -178,6 +182,6 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
 	case KVM_ARM_PSCI_0_1:
 		return kvm_psci_0_1_call(vcpu);
 	default:
-		return false;
+		return -EINVAL;
 	};
 }
diff --git a/arch/arm64/include/asm/kvm_psci.h b/arch/arm64/include/asm/kvm_psci.h
index e25c658..bc39e55 100644
--- a/arch/arm64/include/asm/kvm_psci.h
+++ b/arch/arm64/include/asm/kvm_psci.h
@@ -22,6 +22,6 @@
 #define KVM_ARM_PSCI_0_2	2
 
 int kvm_psci_version(struct kvm_vcpu *vcpu);
-bool kvm_psci_call(struct kvm_vcpu *vcpu);
+int kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM64_KVM_PSCI_H__ */
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 7bc41ea..743a74d 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -30,11 +30,15 @@ typedef int (*exit_handle_fn)(struct kvm_vcpu *, struct kvm_run *);
 
 static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
-	if (kvm_psci_call(vcpu))
+	int ret;
+
+	ret = kvm_psci_call(vcpu);
+	if (ret == -EINVAL) {
+		kvm_inject_undefined(vcpu);
 		return 1;
+	}
 
-	kvm_inject_undefined(vcpu);
-	return 1;
+	return ret;
 }
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
-- 
1.7.9.5

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

* [PATCH v9 06/12] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
                   ` (4 preceding siblings ...)
  2014-04-15  6:14 ` [PATCH v9 05/12] ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15 10:25   ` Marc Zyngier
  2014-04-15  6:14 ` [PATCH v9 07/12] ARM/ARM64: KVM: Emulate PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET Anup Patel
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, we don't have an exit reason to notify user space about
a system-level event (for e.g. system reset or shutdown) triggered
by the VCPU. This patch adds exit reason KVM_EXIT_SYSTEM_EVENT for
this purpose. We can also inform user space about the 'type' and
architecture specific 'flags' of a system-level event using the
kvm_run structure.

This newly added KVM_EXIT_SYSTEM_EVENT will be used by KVM ARM/ARM64
in-kernel PSCI v0.2 support to reset/shutdown VMs.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 Documentation/virtual/kvm/api.txt |   15 +++++++++++++++
 include/uapi/linux/kvm.h          |    8 ++++++++
 2 files changed, 23 insertions(+)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 6dc1db5..c02d725 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2740,6 +2740,21 @@ It gets triggered whenever both KVM_CAP_PPC_EPR are enabled and an
 external interrupt has just been delivered into the guest. User space
 should put the acknowledged interrupt vector into the 'epr' field.
 
+		/* KVM_EXIT_SYSTEM_EVENT */
+		struct {
+#define KVM_SYSTEM_EVENT_SHUTDOWN       1
+#define KVM_SYSTEM_EVENT_RESET          2
+			__u32 type;
+			__u64 flags;
+		} system_event;
+
+If exit_reason is KVM_EXIT_SYSTEM_EVENT then the vcpu has triggered
+a system-level event using some architecture specific mechanism (hypercall
+or some special instruction). In case of ARM/ARM64, this is triggered using
+HVC instruction based PSCI call from the vcpu. The 'type' field describes
+the system-level event type. The 'flags' field describes architecture
+specific flags for the system-level event.
+
 		/* Fix the size of the union. */
 		char padding[256];
 	};
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 01c5624..e86c36a 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -171,6 +171,7 @@ struct kvm_pit_config {
 #define KVM_EXIT_WATCHDOG         21
 #define KVM_EXIT_S390_TSCH        22
 #define KVM_EXIT_EPR              23
+#define KVM_EXIT_SYSTEM_EVENT     24
 
 /* For KVM_EXIT_INTERNAL_ERROR */
 /* Emulate instruction failed. */
@@ -301,6 +302,13 @@ struct kvm_run {
 		struct {
 			__u32 epr;
 		} epr;
+		/* KVM_EXIT_SYSTEM_EVENT */
+		struct {
+#define KVM_SYSTEM_EVENT_SHUTDOWN       1
+#define KVM_SYSTEM_EVENT_RESET          2
+			__u32 type;
+			__u64 flags;
+		} system_event;
 		/* Fix the size of the union. */
 		char padding[256];
 	};
-- 
1.7.9.5

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

* [PATCH v9 07/12] ARM/ARM64: KVM: Emulate PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
                   ` (5 preceding siblings ...)
  2014-04-15  6:14 ` [PATCH v9 06/12] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15 10:34   ` Marc Zyngier
  2014-04-15  6:14 ` [PATCH v9 08/12] ARM/ARM64: KVM: Emulate PSCI v0.2 AFFINITY_INFO Anup Patel
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 UTC (permalink / raw)
  To: linux-arm-kernel

The PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET functions are system-level
functions hence cannot be fully emulated by in-kernel PSCI emulation code.

To tackle this, we forward PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET function
calls from vcpu to user space (i.e. QEMU or KVMTOOL) via kvm_run structure
using KVM_EXIT_SYSTEM_EVENT exit reasons.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 arch/arm/kvm/psci.c |   32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index 14e6fa6..b964aa4 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -85,6 +85,23 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	return PSCI_RET_SUCCESS;
 }
 
+static inline void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
+{
+	memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
+	vcpu->run->system_event.type = type;
+	vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
+}
+
+static void kvm_psci_system_off(struct kvm_vcpu *vcpu)
+{
+	kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_SHUTDOWN);
+}
+
+static void kvm_psci_system_reset(struct kvm_vcpu *vcpu)
+{
+	kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_RESET);
+}
+
 int kvm_psci_version(struct kvm_vcpu *vcpu)
 {
 	if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
@@ -95,6 +112,7 @@ int kvm_psci_version(struct kvm_vcpu *vcpu)
 
 static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 {
+	int ret = 1;
 	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
 	unsigned long val;
 
@@ -114,13 +132,21 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 	case PSCI_0_2_FN64_CPU_ON:
 		val = kvm_psci_vcpu_on(vcpu);
 		break;
+	case PSCI_0_2_FN_SYSTEM_OFF:
+		kvm_psci_system_off(vcpu);
+		val = PSCI_RET_SUCCESS;
+		ret = 0;
+		break;
+	case PSCI_0_2_FN_SYSTEM_RESET:
+		kvm_psci_system_reset(vcpu);
+		val = PSCI_RET_SUCCESS;
+		ret = 0;
+		break;
 	case PSCI_0_2_FN_CPU_SUSPEND:
 	case PSCI_0_2_FN_AFFINITY_INFO:
 	case PSCI_0_2_FN_MIGRATE:
 	case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
 	case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
-	case PSCI_0_2_FN_SYSTEM_OFF:
-	case PSCI_0_2_FN_SYSTEM_RESET:
 	case PSCI_0_2_FN64_CPU_SUSPEND:
 	case PSCI_0_2_FN64_AFFINITY_INFO:
 	case PSCI_0_2_FN64_MIGRATE:
@@ -132,7 +158,7 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 	}
 
 	*vcpu_reg(vcpu, 0) = val;
-	return 1;
+	return ret;
 }
 
 static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
-- 
1.7.9.5

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

* [PATCH v9 08/12] ARM/ARM64: KVM: Emulate PSCI v0.2 AFFINITY_INFO
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
                   ` (6 preceding siblings ...)
  2014-04-15  6:14 ` [PATCH v9 07/12] ARM/ARM64: KVM: Emulate PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15 10:55   ` Marc Zyngier
  2014-04-15  6:14 ` [PATCH v9 09/12] ARM/ARM64: KVM: Emulate PSCI v0.2 MIGRATE_INFO_TYPE and related functions Anup Patel
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds emulation of PSCI v0.2 AFFINITY_INFO function call
for KVM ARM/ARM64. This is a VCPU-level function call which will be
used to determine current state of given affinity level.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 arch/arm/kvm/psci.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index b964aa4..f6f9290 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -27,6 +27,16 @@
  * as described in ARM document number ARM DEN 0022A.
  */
 
+#define AFFINITY_MASK(level)	~((0x1UL << ((level) * 8)) - 1)
+
+static unsigned long psci_affinity_mask(unsigned long affinity_level)
+{
+	if (affinity_level <= 3)
+		return MPIDR_HWID_BITMASK & AFFINITY_MASK(affinity_level);
+
+	return 0;
+}
+
 static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
 {
 	vcpu->arch.pause = true;
@@ -85,6 +95,42 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	return PSCI_RET_SUCCESS;
 }
 
+static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
+{
+	int i;
+	unsigned long mpidr;
+	unsigned long target_affinity;
+	unsigned long target_affinity_mask;
+	unsigned long lowest_affinity_level;
+	struct kvm *kvm = vcpu->kvm;
+	struct kvm_vcpu *tmp;
+
+	target_affinity = *vcpu_reg(vcpu, 1);
+	lowest_affinity_level = *vcpu_reg(vcpu, 2);
+
+	/* Determine target affinity mask */
+	target_affinity_mask = psci_affinity_mask(lowest_affinity_level);
+	if (!target_affinity_mask)
+		return PSCI_RET_INVALID_PARAMS;
+
+	/* Ignore other bits of target affinity */
+	target_affinity &= target_affinity_mask;
+
+	/*
+	 * If one or more VCPU matching target affinity are running
+	 * then return 0 (ON) else return 1 (OFF)
+	 */
+	kvm_for_each_vcpu(i, tmp, kvm) {
+		mpidr = kvm_vcpu_get_mpidr(tmp);
+		if (((mpidr & target_affinity_mask) == target_affinity) &&
+		    !tmp->arch.pause) {
+			return 0;
+		}
+	}
+
+	return 1;
+}
+
 static inline void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
 {
 	memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
@@ -132,6 +178,10 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 	case PSCI_0_2_FN64_CPU_ON:
 		val = kvm_psci_vcpu_on(vcpu);
 		break;
+	case PSCI_0_2_FN_AFFINITY_INFO:
+	case PSCI_0_2_FN64_AFFINITY_INFO:
+		val = kvm_psci_vcpu_affinity_info(vcpu);
+		break;
 	case PSCI_0_2_FN_SYSTEM_OFF:
 		kvm_psci_system_off(vcpu);
 		val = PSCI_RET_SUCCESS;
@@ -143,12 +193,10 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 		ret = 0;
 		break;
 	case PSCI_0_2_FN_CPU_SUSPEND:
-	case PSCI_0_2_FN_AFFINITY_INFO:
 	case PSCI_0_2_FN_MIGRATE:
 	case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
 	case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
 	case PSCI_0_2_FN64_CPU_SUSPEND:
-	case PSCI_0_2_FN64_AFFINITY_INFO:
 	case PSCI_0_2_FN64_MIGRATE:
 	case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
 		val = PSCI_RET_NOT_SUPPORTED;
-- 
1.7.9.5

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

* [PATCH v9 09/12] ARM/ARM64: KVM: Emulate PSCI v0.2 MIGRATE_INFO_TYPE and related functions
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
                   ` (7 preceding siblings ...)
  2014-04-15  6:14 ` [PATCH v9 08/12] ARM/ARM64: KVM: Emulate PSCI v0.2 AFFINITY_INFO Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15 12:05   ` Marc Zyngier
  2014-04-15  6:14 ` [PATCH v9 10/12] ARM/ARM64: KVM: Fix CPU_ON emulation for PSCI v0.2 Anup Patel
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds emulation of PSCI v0.2 MIGRATE, MIGRATE_INFO_TYPE, and
MIGRATE_INFO_UP_CPU function calls for KVM ARM/ARM64.

KVM ARM/ARM64 being a hypervisor (and not a Trusted OS), we cannot provide
this functions hence we emulate these functions in following way:
1. MIGRATE - Returns "Not Supported"
2. MIGRATE_INFO_TYPE - Return 2 i.e. Trusted OS is not present
3. MIGRATE_INFO_UP_CPU - Returns "Not Supported"

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 arch/arm/kvm/psci.c |   21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index f6f9290..1af0016 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -182,6 +182,22 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 	case PSCI_0_2_FN64_AFFINITY_INFO:
 		val = kvm_psci_vcpu_affinity_info(vcpu);
 		break;
+	case PSCI_0_2_FN_MIGRATE:
+	case PSCI_0_2_FN64_MIGRATE:
+		val = PSCI_RET_NOT_SUPPORTED;
+		break;
+	case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
+		/*
+		 * Trusted OS is MP hence does not require migration
+	         * or
+		 * Trusted OS is not present
+		 */
+		val = PSCI_0_2_TOS_MP;
+		break;
+	case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
+	case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
+		val = PSCI_RET_NOT_SUPPORTED;
+		break;
 	case PSCI_0_2_FN_SYSTEM_OFF:
 		kvm_psci_system_off(vcpu);
 		val = PSCI_RET_SUCCESS;
@@ -193,12 +209,7 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 		ret = 0;
 		break;
 	case PSCI_0_2_FN_CPU_SUSPEND:
-	case PSCI_0_2_FN_MIGRATE:
-	case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
-	case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
 	case PSCI_0_2_FN64_CPU_SUSPEND:
-	case PSCI_0_2_FN64_MIGRATE:
-	case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
 		val = PSCI_RET_NOT_SUPPORTED;
 		break;
 	default:
-- 
1.7.9.5

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

* [PATCH v9 10/12] ARM/ARM64: KVM: Fix CPU_ON emulation for PSCI v0.2
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
                   ` (8 preceding siblings ...)
  2014-04-15  6:14 ` [PATCH v9 09/12] ARM/ARM64: KVM: Emulate PSCI v0.2 MIGRATE_INFO_TYPE and related functions Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15 12:10   ` Marc Zyngier
  2014-04-15  6:14 ` [PATCH v9 11/12] ARM/ARM64: KVM: Emulate PSCI v0.2 CPU_SUSPEND Anup Patel
  2014-04-15  6:14 ` [PATCH v9 12/12] ARM/ARM64: KVM: Advertise KVM_CAP_ARM_PSCI_0_2 to user space Anup Patel
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 UTC (permalink / raw)
  To: linux-arm-kernel

As-per PSCI v0.2, the source CPU provides physical address of
"entry point" and "context id" for starting a target CPU. Also,
if target CPU is already running then we should return ALREADY_ON.

Current emulation of CPU_ON function does not consider physical
address of "context id" and returns INVALID_PARAMETERS if target
CPU is already running.

This patch updates kvm_psci_vcpu_on() such that it works for both
PSCI v0.1 and PSCI v0.2.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 arch/arm/kvm/psci.c |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index 1af0016..f59f09d 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -48,6 +48,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	struct kvm_vcpu *vcpu = NULL, *tmp;
 	wait_queue_head_t *wq;
 	unsigned long cpu_id;
+	unsigned long context_id;
 	unsigned long mpidr;
 	phys_addr_t target_pc;
 	int i;
@@ -68,10 +69,17 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	 * Make sure the caller requested a valid CPU and that the CPU is
 	 * turned off.
 	 */
-	if (!vcpu || !vcpu->arch.pause)
+	if (!vcpu)
 		return PSCI_RET_INVALID_PARAMS;
+	if (!vcpu->arch.pause) {
+		if (kvm_psci_version(source_vcpu) != KVM_ARM_PSCI_0_1)
+			return PSCI_RET_ALREADY_ON;
+		else
+			return PSCI_RET_INVALID_PARAMS;
+	}
 
 	target_pc = *vcpu_reg(source_vcpu, 2);
+	context_id = *vcpu_reg(source_vcpu, 3);
 
 	kvm_reset_vcpu(vcpu);
 
@@ -86,6 +94,11 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 		kvm_vcpu_set_be(vcpu);
 
 	*vcpu_pc(vcpu) = target_pc;
+	/*
+	 * NOTE: We always update r0 (or x0) because for PSCI v0.1
+	 * the general puspose registers are undefined upon CPU_ON.
+	 */
+	*vcpu_reg(vcpu, 0) = context_id;
 	vcpu->arch.pause = false;
 	smp_mb();		/* Make sure the above is visible */
 
-- 
1.7.9.5

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

* [PATCH v9 11/12] ARM/ARM64: KVM: Emulate PSCI v0.2 CPU_SUSPEND
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
                   ` (9 preceding siblings ...)
  2014-04-15  6:14 ` [PATCH v9 10/12] ARM/ARM64: KVM: Fix CPU_ON emulation for PSCI v0.2 Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15 12:14   ` Marc Zyngier
  2014-04-15  6:14 ` [PATCH v9 12/12] ARM/ARM64: KVM: Advertise KVM_CAP_ARM_PSCI_0_2 to user space Anup Patel
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds emulation of PSCI v0.2 CPU_SUSPEND function call for
KVM ARM/ARM64. This is a CPU-level function call which can suspend
current CPU or current CPU cluster. We don't have VCPU clusters in
KVM so we only suspend the current VCPU.

The CPU_SUSPEND emulation is not tested much because currently there
is no CPUIDLE driver in Linux kernel that uses PSCI CPU_SUSPEND. The
PSCI CPU_SUSPEND implementation in ARM64 kernel was tested using a
Simple CPUIDLE driver which is not published due to unstable DT-bindings
for PSCI.
(For more info, http://lwn.net/Articles/574950/)

Even if we had stable DT-bindings for PSCI and CPUIDLE driver that
uses PSCI CPU_SUSPEND then still we need to define SUSPEND states
and WAKEUP events for KVM ARM/ARM64.

Due to above, we implement CPU_SUSPEND emulation similar to WFI
(Wait-for-interrupt) emulation and we also treat power-down request
to be same as stand-by request. This is consistent with section
5.4.1 and section 5.4.2 of PSCI v0.2 specification.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 arch/arm/kvm/psci.c |   29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index f59f09d..fdb347a 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -37,6 +37,27 @@ static unsigned long psci_affinity_mask(unsigned long affinity_level)
 	return 0;
 }
 
+static unsigned long kvm_psci_vcpu_suspend(struct kvm_vcpu *vcpu)
+{
+	/*
+	 * NOTE: Currently, we don't have any wakeup events defined
+	 * for KVM so for simplicity we make VCPU suspend emulation
+	 * to be same-as WFI (Wait-for-interrupt) emulation.
+	 *
+	 * This means for KVM the wakeup events are interrupts and
+	 * this is consistent with intended use of StateID as described
+	 * in section 5.4.1 of PSCI v0.2 specification (ARM DEN 0022A).
+	 *
+	 * Further, we also treat power-down request to be same as
+	 * stand-by request as-per section 5.4.2 clause 3 of PSCI v0.2
+	 * specification (ARM DEN 0022A). This means all suspend states
+	 * for KVM will preserve the register state.
+	 */
+	kvm_vcpu_block(vcpu);
+
+	return PSCI_RET_SUCCESS;
+}
+
 static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
 {
 	vcpu->arch.pause = true;
@@ -183,6 +204,10 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 		 */
 		val = 2;
 		break;
+	case PSCI_0_2_FN_CPU_SUSPEND:
+	case PSCI_0_2_FN64_CPU_SUSPEND:
+		val = kvm_psci_vcpu_suspend(vcpu);
+		break;
 	case PSCI_0_2_FN_CPU_OFF:
 		kvm_psci_vcpu_off(vcpu);
 		val = PSCI_RET_SUCCESS;
@@ -221,10 +246,6 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 		val = PSCI_RET_SUCCESS;
 		ret = 0;
 		break;
-	case PSCI_0_2_FN_CPU_SUSPEND:
-	case PSCI_0_2_FN64_CPU_SUSPEND:
-		val = PSCI_RET_NOT_SUPPORTED;
-		break;
 	default:
 		return -EINVAL;
 	}
-- 
1.7.9.5

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

* [PATCH v9 12/12] ARM/ARM64: KVM: Advertise KVM_CAP_ARM_PSCI_0_2 to user space
  2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
                   ` (10 preceding siblings ...)
  2014-04-15  6:14 ` [PATCH v9 11/12] ARM/ARM64: KVM: Emulate PSCI v0.2 CPU_SUSPEND Anup Patel
@ 2014-04-15  6:14 ` Anup Patel
  2014-04-15 12:16   ` Marc Zyngier
  11 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15  6:14 UTC (permalink / raw)
  To: linux-arm-kernel

We have PSCI v0.2 emulation available in KVM ARM/ARM64
hence advertise this to user space (i.e. QEMU or KVMTOOL)
via KVM_CHECK_EXTENSION ioctl.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 arch/arm/kvm/arm.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index f0e50a0..3c82b37 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -197,6 +197,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:
-- 
1.7.9.5

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

* [PATCH v9 01/12] KVM: Add capability to advertise PSCI v0.2 support
  2014-04-15  6:14 ` [PATCH v9 01/12] KVM: Add capability to advertise PSCI v0.2 support Anup Patel
@ 2014-04-15  9:49   ` Marc Zyngier
  0 siblings, 0 replies; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15  9:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:04 am BST, Anup Patel <anup.patel@linaro.org> wrote:
> 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>
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

> ---
>  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 a8f4ee5..01c5624 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -743,6 +743,7 @@ struct kvm_ppc_smmu_info {
>  #define KVM_CAP_IOAPIC_POLARITY_IGNORED 97
>  #define KVM_CAP_ENABLE_CAP_VM 98
>  #define KVM_CAP_S390_IRQCHIP 99
> +#define KVM_CAP_ARM_PSCI_0_2 100
>  
>  #ifdef KVM_CAP_IRQ_ROUTING

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

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

* [PATCH v9 02/12] ARM/ARM64: KVM: Add common header for PSCI related defines
  2014-04-15  6:14 ` [PATCH v9 02/12] ARM/ARM64: KVM: Add common header for PSCI related defines Anup Patel
@ 2014-04-15 10:06   ` Marc Zyngier
  2014-04-15 11:10     ` Anup Patel
  0 siblings, 1 reply; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 10:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:05 am BST, Anup Patel <anup.patel@linaro.org> wrote:
> We need a common place to share PSCI related defines among ARM kernel,
> ARM64 kernel, KVM ARM/ARM64 PSCI emulation, and user space.
>
> We introduce uapi/linux/psci.h for this purpose. This newly added
> header will be first used by KVM ARM/ARM64 in-kernel PSCI emulation
> and user space (i.e. QEMU or KVMTOOL).
>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
> ---
>  include/uapi/linux/Kbuild |    1 +
>  include/uapi/linux/psci.h |   78 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 79 insertions(+)
>  create mode 100644 include/uapi/linux/psci.h
>
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index 6929571..24e9033 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -317,6 +317,7 @@ header-y += ppp-ioctl.h
>  header-y += ppp_defs.h
>  header-y += pps.h
>  header-y += prctl.h
> +header-y += psci.h
>  header-y += ptp_clock.h
>  header-y += ptrace.h
>  header-y += qnx4_fs.h
> diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
> new file mode 100644
> index 0000000..077b623
> --- /dev/null
> +++ b/include/uapi/linux/psci.h
> @@ -0,0 +1,78 @@
> +/*
> + * ARM Power State and Coordination Interface (PSCI) header
> + *
> + * This header holds common PSCI defines and macros shared
> + * by: ARM kernel, ARM64 kernel, KVM ARM/ARM64 and user space.
> + *
> + * Copyright (C) 2014 Linaro Ltd.
> + * Author: Anup Patel <anup.patel@linaro.org>
> + */
> +
> +#ifndef _UAPI_LINUX_PSCI_H
> +#define _UAPI_LINUX_PSCI_H
> +
> +/* PSCI v0.1 interface */
> +#define PSCI_FN(base, n)			((base) + (n))
> +
> +#define PSCI_FN_CPU_SUSPEND(base)		PSCI_FN(base, 0)
> +#define PSCI_FN_CPU_OFF(base)			PSCI_FN(base, 1)
> +#define PSCI_FN_CPU_ON(base)			PSCI_FN(base, 2)
> +#define PSCI_FN_MIGRATE(base)			PSCI_FN(base, 3)

For PSCI v0.1, all the function numbers are independent (the spec didn't
specify any number...). You cannot assume contiguous numbering.

> +/* PSCI v0.2 interface */
> +#define PSCI_0_2_FN_BASE			0x84000000
> +#define PSCI_0_2_FN(n)				(PSCI_0_2_FN_BASE + (n))
> +#define PSCI_0_2_64BIT				0x40000000
> +#define PSCI_0_2_FN64_BASE			\
> +					(PSCI_0_2_FN_BASE + PSCI_0_2_64BIT)
> +#define PSCI_0_2_FN64(n)			(PSCI_0_2_FN64_BASE + (n))
> +
> +#define PSCI_0_2_FN_PSCI_VERSION		PSCI_0_2_FN(0)
> +#define PSCI_0_2_FN_CPU_SUSPEND			PSCI_0_2_FN(1)
> +#define PSCI_0_2_FN_CPU_OFF			PSCI_0_2_FN(2)
> +#define PSCI_0_2_FN_CPU_ON			PSCI_0_2_FN(3)
> +#define PSCI_0_2_FN_AFFINITY_INFO		PSCI_0_2_FN(4)
> +#define PSCI_0_2_FN_MIGRATE			PSCI_0_2_FN(5)
> +#define PSCI_0_2_FN_MIGRATE_INFO_TYPE		PSCI_0_2_FN(6)
> +#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU		PSCI_0_2_FN(7)
> +#define PSCI_0_2_FN_SYSTEM_OFF			PSCI_0_2_FN(8)
> +#define PSCI_0_2_FN_SYSTEM_RESET		PSCI_0_2_FN(9)
> +
> +#define PSCI_0_2_FN64_CPU_SUSPEND		PSCI_0_2_FN64(1)
> +#define PSCI_0_2_FN64_CPU_ON			PSCI_0_2_FN64(3)
> +#define PSCI_0_2_FN64_AFFINITY_INFO		PSCI_0_2_FN64(4)
> +#define PSCI_0_2_FN64_MIGRATE			PSCI_0_2_FN64(5)
> +#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN64(7)
> +
> +#define PSCI_0_2_POWER_STATE_ID_MASK		0xffff
> +#define PSCI_0_2_POWER_STATE_ID_SHIFT		0
> +#define PSCI_0_2_POWER_STATE_TYPE_MASK		0x1
> +#define PSCI_0_2_POWER_STATE_TYPE_SHIFT		16
> +#define PSCI_0_2_POWER_STATE_AFFL_MASK		0x3
> +#define PSCI_0_2_POWER_STATE_AFFL_SHIFT		24
> +
> +#define PSCI_0_2_TOS_UP_MIGRATE			0
> +#define PSCI_0_2_TOS_UP_NO_MIGRATE		1
> +#define PSCI_0_2_TOS_MP				2
> +
> +/* PSCI version decoding (independent of PSCI version) */
> +#define PSCI_VERSION_MAJOR_MASK			0xffff0000
> +#define PSCI_VERSION_MINOR_MASK			0x0000ffff
> +#define PSCI_VERSION_MAJOR_SHIFT		16

What about:

#define PSCI_VERSION_MINOR_MASK	((1U << PSCI_VERSION_MAJOR_SHIFT) - 1)
#define PSCI_VERSION_MAJOR_MASK	~PSCI_VERSION_MINOR_MASK

> +#define PSCI_VERSION_MAJOR(ver)			\
> +		(((ver) & PSCI_VERSION_MAJOR_MASK) >> PSCI_VERSION_MAJOR_SHIFT)
> +#define PSCI_VERSION_MINOR(ver)			\
> +		((ver) & PSCI_VERSION_MINOR_MASK)
> +
> +/* PSCI return values (inclusive of all PSCI versions) */
> +#define PSCI_RET_SUCCESS			0
> +#define PSCI_RET_NOT_SUPPORTED			-1
> +#define PSCI_RET_INVALID_PARAMS			-2
> +#define PSCI_RET_DENIED				-3
> +#define PSCI_RET_ALREADY_ON			-4
> +#define PSCI_RET_ON_PENDING			-5
> +#define PSCI_RET_INTERNAL_FAILURE		-6
> +#define PSCI_RET_NOT_PRESENT			-7
> +#define PSCI_RET_DISABLED			-8
> +
> +#endif /* _UAPI_LINUX_PSCI_H */

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

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

* [PATCH v9 03/12] ARM/ARM64: KVM: Add base for PSCI v0.2 emulation
  2014-04-15  6:14 ` [PATCH v9 03/12] ARM/ARM64: KVM: Add base for PSCI v0.2 emulation Anup Patel
@ 2014-04-15 10:19   ` Marc Zyngier
  0 siblings, 0 replies; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 10:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:06 am BST, Anup Patel <anup.patel@linaro.org> 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>
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

> ---
>  arch/arm/include/asm/kvm_host.h   |    2 +-
>  arch/arm/include/asm/kvm_psci.h   |    4 ++
>  arch/arm/include/uapi/asm/kvm.h   |   19 ++++----
>  arch/arm/kvm/psci.c               |   93 ++++++++++++++++++++++++++++++-------
>  arch/arm64/include/asm/kvm_host.h |    2 +-
>  arch/arm64/include/asm/kvm_psci.h |    4 ++
>  arch/arm64/include/uapi/asm/kvm.h |   21 +++++----
>  7 files changed, 108 insertions(+), 37 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 09af149..193ceaf 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/asm/kvm_psci.h b/arch/arm/include/asm/kvm_psci.h
> index 9a83d98..4c0e3e1 100644
> --- a/arch/arm/include/asm/kvm_psci.h
> +++ b/arch/arm/include/asm/kvm_psci.h
> @@ -18,6 +18,10 @@
>  #ifndef __ARM_KVM_PSCI_H__
>  #define __ARM_KVM_PSCI_H__
>  
> +#define KVM_ARM_PSCI_0_1	1
> +#define KVM_ARM_PSCI_0_2	2
> +
> +int kvm_psci_version(struct kvm_vcpu *vcpu);
>  bool kvm_psci_call(struct kvm_vcpu *vcpu);
>  
>  #endif /* __ARM_KVM_PSCI_H__ */
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index ef0c878..6574ddf 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -20,6 +20,7 @@
>  #define __ARM_KVM_H__
>  
>  #include <linux/types.h>
> +#include <linux/psci.h>
>  #include <asm/ptrace.h>
>  
>  #define __KVM_HAVE_GUEST_DEBUG
> @@ -83,6 +84,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;
> @@ -194,16 +196,15 @@ struct kvm_arch_memory_slot {
>  
>  /* PSCI interface */
>  #define KVM_PSCI_FN_BASE		0x95c1ba5e
> -#define KVM_PSCI_FN(n)			(KVM_PSCI_FN_BASE + (n))
>  
> -#define KVM_PSCI_FN_CPU_SUSPEND		KVM_PSCI_FN(0)
> -#define KVM_PSCI_FN_CPU_OFF		KVM_PSCI_FN(1)
> -#define KVM_PSCI_FN_CPU_ON		KVM_PSCI_FN(2)
> -#define KVM_PSCI_FN_MIGRATE		KVM_PSCI_FN(3)
> +#define KVM_PSCI_FN_CPU_SUSPEND		PSCI_FN(KVM_PSCI_FN_BASE, 0)
> +#define KVM_PSCI_FN_CPU_OFF		PSCI_FN(KVM_PSCI_FN_BASE, 1)
> +#define KVM_PSCI_FN_CPU_ON		PSCI_FN(KVM_PSCI_FN_BASE, 2)
> +#define KVM_PSCI_FN_MIGRATE		PSCI_FN(KVM_PSCI_FN_BASE, 3)
>  
> -#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_DENIED		((unsigned long)-3)
> +#define KVM_PSCI_RET_SUCCESS		PSCI_RET_SUCCESS
> +#define KVM_PSCI_RET_NI			PSCI_RET_NOT_SUPPORTED
> +#define KVM_PSCI_RET_INVAL		PSCI_RET_INVALID_PARAMS
> +#define KVM_PSCI_RET_DENIED		PSCI_RET_DENIED
>  
>  #endif /* __ARM_KVM_H__ */
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 448f60e..8c42596c 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -59,7 +59,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>  	 * turned off.
>  	 */
>  	if (!vcpu || !vcpu->arch.pause)
> -		return KVM_PSCI_RET_INVAL;
> +		return PSCI_RET_INVALID_PARAMS;
>  
>  	target_pc = *vcpu_reg(source_vcpu, 2);
>  
> @@ -82,20 +82,60 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>  	wq = kvm_arch_vcpu_wq(vcpu);
>  	wake_up_interruptible(wq);
>  
> -	return KVM_PSCI_RET_SUCCESS;
> +	return 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)
> +int kvm_psci_version(struct kvm_vcpu *vcpu)
> +{
> +	if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
> +		return KVM_ARM_PSCI_0_2;
> +
> +	return KVM_ARM_PSCI_0_1;
> +}
> +
> +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 PSCI_0_2_FN_PSCI_VERSION:
> +		/*
> +		 * Bits[31:16] = Major Version = 0
> +		 * Bits[15:0] = Minor Version = 2
> +		 */
> +		val = 2;
> +		break;
> +	case PSCI_0_2_FN_CPU_OFF:
> +		kvm_psci_vcpu_off(vcpu);
> +		val = PSCI_RET_SUCCESS;
> +		break;
> +	case PSCI_0_2_FN_CPU_ON:
> +	case PSCI_0_2_FN64_CPU_ON:
> +		val = kvm_psci_vcpu_on(vcpu);
> +		break;
> +	case PSCI_0_2_FN_CPU_SUSPEND:
> +	case PSCI_0_2_FN_AFFINITY_INFO:
> +	case PSCI_0_2_FN_MIGRATE:
> +	case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
> +	case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
> +	case PSCI_0_2_FN_SYSTEM_OFF:
> +	case PSCI_0_2_FN_SYSTEM_RESET:
> +	case PSCI_0_2_FN64_CPU_SUSPEND:
> +	case PSCI_0_2_FN64_AFFINITY_INFO:
> +	case PSCI_0_2_FN64_MIGRATE:
> +	case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
> +		val = 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;
> @@ -103,16 +143,15 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>  	switch (psci_fn) {
>  	case KVM_PSCI_FN_CPU_OFF:
>  		kvm_psci_vcpu_off(vcpu);
> -		val = KVM_PSCI_RET_SUCCESS;
> +		val = PSCI_RET_SUCCESS;
>  		break;
>  	case KVM_PSCI_FN_CPU_ON:
>  		val = kvm_psci_vcpu_on(vcpu);
>  		break;
>  	case KVM_PSCI_FN_CPU_SUSPEND:
>  	case KVM_PSCI_FN_MIGRATE:
> -		val = KVM_PSCI_RET_NI;
> +		val = PSCI_RET_NOT_SUPPORTED;
>  		break;
> -
>  	default:
>  		return false;
>  	}
> @@ -120,3 +159,25 @@ 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)
> +{
> +	switch (kvm_psci_version(vcpu)) {
> +	case KVM_ARM_PSCI_0_2:
> +		return kvm_psci_0_2_call(vcpu);
> +	case KVM_ARM_PSCI_0_1:
> +		return kvm_psci_0_1_call(vcpu);
> +	default:
> +		return false;
> +	};
> +}
> 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/asm/kvm_psci.h b/arch/arm64/include/asm/kvm_psci.h
> index e301a48..e25c658 100644
> --- a/arch/arm64/include/asm/kvm_psci.h
> +++ b/arch/arm64/include/asm/kvm_psci.h
> @@ -18,6 +18,10 @@
>  #ifndef __ARM64_KVM_PSCI_H__
>  #define __ARM64_KVM_PSCI_H__
>  
> +#define KVM_ARM_PSCI_0_1	1
> +#define KVM_ARM_PSCI_0_2	2
> +
> +int kvm_psci_version(struct kvm_vcpu *vcpu);
>  bool kvm_psci_call(struct kvm_vcpu *vcpu);
>  
>  #endif /* __ARM64_KVM_PSCI_H__ */
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index eaf54a3..9b67161 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -31,6 +31,7 @@
>  #define KVM_NR_SPSR	5
>  
>  #ifndef __ASSEMBLY__
> +#include <linux/psci.h>
>  #include <asm/types.h>
>  #include <asm/ptrace.h>
>  
> @@ -77,6 +78,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;
> @@ -177,19 +179,18 @@ 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))
>  
> -#define KVM_PSCI_FN_CPU_SUSPEND		KVM_PSCI_FN(0)
> -#define KVM_PSCI_FN_CPU_OFF		KVM_PSCI_FN(1)
> -#define KVM_PSCI_FN_CPU_ON		KVM_PSCI_FN(2)
> -#define KVM_PSCI_FN_MIGRATE		KVM_PSCI_FN(3)
> +#define KVM_PSCI_FN_CPU_SUSPEND		PSCI_FN(KVM_PSCI_FN_BASE, 0)
> +#define KVM_PSCI_FN_CPU_OFF		PSCI_FN(KVM_PSCI_FN_BASE, 1)
> +#define KVM_PSCI_FN_CPU_ON		PSCI_FN(KVM_PSCI_FN_BASE, 2)
> +#define KVM_PSCI_FN_MIGRATE		PSCI_FN(KVM_PSCI_FN_BASE, 3)
>  
> -#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_DENIED		((unsigned long)-3)
> +#define KVM_PSCI_RET_SUCCESS		PSCI_RET_SUCCESS
> +#define KVM_PSCI_RET_NI			PSCI_RET_NOT_SUPPORTED
> +#define KVM_PSCI_RET_INVAL		PSCI_RET_INVALID_PARAMS
> +#define KVM_PSCI_RET_DENIED		PSCI_RET_DENIED
>  
>  #endif

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

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

* [PATCH v9 04/12] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature
  2014-04-15  6:14 ` [PATCH v9 04/12] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature Anup Patel
@ 2014-04-15 10:20   ` Marc Zyngier
  0 siblings, 0 replies; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 10:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:07 am BST, Anup Patel <anup.patel@linaro.org> 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>
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

> ---
>  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 a9380ba5..6dc1db5 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2376,6 +2376,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 the CPU.
> +	  Depends on KVM_CAP_ARM_PSCI_0_2.
>  
>  
>  4.83 KVM_ARM_PREFERRED_TARGET

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

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

* [PATCH v9 05/12] ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible
  2014-04-15  6:14 ` [PATCH v9 05/12] ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible Anup Patel
@ 2014-04-15 10:21   ` Marc Zyngier
  2014-04-15 11:13     ` Anup Patel
  0 siblings, 1 reply; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 10:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:08 am BST, Anup Patel <anup.patel@linaro.org> wrote:
> Currently, the kvm_psci_call() returns 'true' or 'false' based on whether
> the PSCI function call was handled successfully or not. This does not help
> us emulate system-level PSCI functions where the actual emulation work will
> be done by user space (QEMU or KVMTOOL). Examples of such system-level PSCI
> functions are: PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET.
>
> This patch updates kvm_psci_call() to return three types of values:
> 1) > 0 (success)
> 2) = 0 (success but exit to user space)
> 3) < 0 (errors)
>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
>  arch/arm/include/asm/kvm_psci.h   |    2 +-
>  arch/arm/kvm/handle_exit.c        |   10 +++++++---
>  arch/arm/kvm/psci.c               |   28 ++++++++++++++++------------
>  arch/arm64/include/asm/kvm_psci.h |    2 +-
>  arch/arm64/kvm/handle_exit.c      |   10 +++++++---
>  5 files changed, 32 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_psci.h b/arch/arm/include/asm/kvm_psci.h
> index 4c0e3e1..6bda945 100644
> --- a/arch/arm/include/asm/kvm_psci.h
> +++ b/arch/arm/include/asm/kvm_psci.h
> @@ -22,6 +22,6 @@
>  #define KVM_ARM_PSCI_0_2	2
>  
>  int kvm_psci_version(struct kvm_vcpu *vcpu);
> -bool kvm_psci_call(struct kvm_vcpu *vcpu);
> +int kvm_psci_call(struct kvm_vcpu *vcpu);
>  
>  #endif /* __ARM_KVM_PSCI_H__ */
> diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
> index 0de91fc..1270095 100644
> --- a/arch/arm/kvm/handle_exit.c
> +++ b/arch/arm/kvm/handle_exit.c
> @@ -38,14 +38,18 @@ static int handle_svc_hyp(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  
>  static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  {
> +	int ret;
> +
>  	trace_kvm_hvc(*vcpu_pc(vcpu), *vcpu_reg(vcpu, 0),
>  		      kvm_vcpu_hvc_get_imm(vcpu));
>  
> -	if (kvm_psci_call(vcpu))
> +	ret = kvm_psci_call(vcpu);
> +	if (ret == -EINVAL) {

Please check for (ret < 0), so it actually matches with the comment (and
will same some debugging when we introduce another error code).

> +		kvm_inject_undefined(vcpu);
>  		return 1;
> +	}
>  
> -	kvm_inject_undefined(vcpu);
> -	return 1;
> +	return ret;
>  }
>  
>  static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 8c42596c..14e6fa6 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -93,7 +93,7 @@ int kvm_psci_version(struct kvm_vcpu *vcpu)
>  	return KVM_ARM_PSCI_0_1;
>  }
>  
> -static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> +static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>  	unsigned long val;
> @@ -128,14 +128,14 @@ static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  		val = PSCI_RET_NOT_SUPPORTED;
>  		break;
>  	default:
> -		return false;
> +		return -EINVAL;
>  	}
>  
>  	*vcpu_reg(vcpu, 0) = val;
> -	return true;
> +	return 1;
>  }
>  
> -static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
> +static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>  	unsigned long val;
> @@ -153,11 +153,11 @@ static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>  		val = PSCI_RET_NOT_SUPPORTED;
>  		break;
>  	default:
> -		return false;
> +		return -EINVAL;
>  	}
>  
>  	*vcpu_reg(vcpu, 0) = val;
> -	return true;
> +	return 1;
>  }
>  
>  /**
> @@ -165,12 +165,16 @@ static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>   * @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.
> + * The calling convention is similar to SMC calls to the secure world
> + * where the function number is placed in r0.
> + *
> + * This function returns: > 0 (success), 0 (success but exit to user
> + * space), and < 0 (errors)
> + *
> + * Errors:
> + * -EINVAL: Unrecognized PSCI function
>   */
> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
> +int kvm_psci_call(struct kvm_vcpu *vcpu)
>  {
>  	switch (kvm_psci_version(vcpu)) {
>  	case KVM_ARM_PSCI_0_2:
> @@ -178,6 +182,6 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>  	case KVM_ARM_PSCI_0_1:
>  		return kvm_psci_0_1_call(vcpu);
>  	default:
> -		return false;
> +		return -EINVAL;
>  	};
>  }
> diff --git a/arch/arm64/include/asm/kvm_psci.h b/arch/arm64/include/asm/kvm_psci.h
> index e25c658..bc39e55 100644
> --- a/arch/arm64/include/asm/kvm_psci.h
> +++ b/arch/arm64/include/asm/kvm_psci.h
> @@ -22,6 +22,6 @@
>  #define KVM_ARM_PSCI_0_2	2
>  
>  int kvm_psci_version(struct kvm_vcpu *vcpu);
> -bool kvm_psci_call(struct kvm_vcpu *vcpu);
> +int kvm_psci_call(struct kvm_vcpu *vcpu);
>  
>  #endif /* __ARM64_KVM_PSCI_H__ */
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 7bc41ea..743a74d 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -30,11 +30,15 @@ typedef int (*exit_handle_fn)(struct kvm_vcpu *, struct kvm_run *);
>  
>  static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  {
> -	if (kvm_psci_call(vcpu))
> +	int ret;
> +
> +	ret = kvm_psci_call(vcpu);
> +	if (ret == -EINVAL) {

Same here.

> +		kvm_inject_undefined(vcpu);
>  		return 1;
> +	}
>  
> -	kvm_inject_undefined(vcpu);
> -	return 1;
> +	return ret;
>  }
>  
>  static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)

Thanks,

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

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

* [PATCH v9 06/12] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header
  2014-04-15  6:14 ` [PATCH v9 06/12] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header Anup Patel
@ 2014-04-15 10:25   ` Marc Zyngier
  0 siblings, 0 replies; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:09 am BST, Anup Patel <anup.patel@linaro.org> wrote:
> Currently, we don't have an exit reason to notify user space about
> a system-level event (for e.g. system reset or shutdown) triggered
> by the VCPU. This patch adds exit reason KVM_EXIT_SYSTEM_EVENT for
> this purpose. We can also inform user space about the 'type' and
> architecture specific 'flags' of a system-level event using the
> kvm_run structure.
>
> This newly added KVM_EXIT_SYSTEM_EVENT will be used by KVM ARM/ARM64
> in-kernel PSCI v0.2 support to reset/shutdown VMs.
>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

This patch should be posted on the main KVM ML, and reviewed by the KVM
maintainers (shouldn't be controvertial though).

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

> ---
>  Documentation/virtual/kvm/api.txt |   15 +++++++++++++++
>  include/uapi/linux/kvm.h          |    8 ++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 6dc1db5..c02d725 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2740,6 +2740,21 @@ It gets triggered whenever both KVM_CAP_PPC_EPR are enabled and an
>  external interrupt has just been delivered into the guest. User space
>  should put the acknowledged interrupt vector into the 'epr' field.
>  
> +		/* KVM_EXIT_SYSTEM_EVENT */
> +		struct {
> +#define KVM_SYSTEM_EVENT_SHUTDOWN       1
> +#define KVM_SYSTEM_EVENT_RESET          2
> +			__u32 type;
> +			__u64 flags;
> +		} system_event;
> +
> +If exit_reason is KVM_EXIT_SYSTEM_EVENT then the vcpu has triggered
> +a system-level event using some architecture specific mechanism (hypercall
> +or some special instruction). In case of ARM/ARM64, this is triggered using
> +HVC instruction based PSCI call from the vcpu. The 'type' field describes
> +the system-level event type. The 'flags' field describes architecture
> +specific flags for the system-level event.
> +
>  		/* Fix the size of the union. */
>  		char padding[256];
>  	};
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 01c5624..e86c36a 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -171,6 +171,7 @@ struct kvm_pit_config {
>  #define KVM_EXIT_WATCHDOG         21
>  #define KVM_EXIT_S390_TSCH        22
>  #define KVM_EXIT_EPR              23
> +#define KVM_EXIT_SYSTEM_EVENT     24
>  
>  /* For KVM_EXIT_INTERNAL_ERROR */
>  /* Emulate instruction failed. */
> @@ -301,6 +302,13 @@ struct kvm_run {
>  		struct {
>  			__u32 epr;
>  		} epr;
> +		/* KVM_EXIT_SYSTEM_EVENT */
> +		struct {
> +#define KVM_SYSTEM_EVENT_SHUTDOWN       1
> +#define KVM_SYSTEM_EVENT_RESET          2
> +			__u32 type;
> +			__u64 flags;
> +		} system_event;
>  		/* Fix the size of the union. */
>  		char padding[256];
>  	};

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

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

* [PATCH v9 07/12] ARM/ARM64: KVM: Emulate PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET
  2014-04-15  6:14 ` [PATCH v9 07/12] ARM/ARM64: KVM: Emulate PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET Anup Patel
@ 2014-04-15 10:34   ` Marc Zyngier
  2014-04-15 11:26     ` Anup Patel
  0 siblings, 1 reply; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 10:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:10 am BST, Anup Patel <anup.patel@linaro.org> wrote:
> The PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET functions are system-level
> functions hence cannot be fully emulated by in-kernel PSCI emulation code.
>
> To tackle this, we forward PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET function
> calls from vcpu to user space (i.e. QEMU or KVMTOOL) via kvm_run structure
> using KVM_EXIT_SYSTEM_EVENT exit reasons.
>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
>  arch/arm/kvm/psci.c |   32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 14e6fa6..b964aa4 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -85,6 +85,23 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>  	return PSCI_RET_SUCCESS;
>  }
>  
> +static inline void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)

Loose the "inline". This is not performance critical, and the compiler
does a pretty good job doing that for you.

> +{
> +	memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
> +	vcpu->run->system_event.type = type;
> +	vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
> +}
> +
> +static void kvm_psci_system_off(struct kvm_vcpu *vcpu)
> +{
> +	kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_SHUTDOWN);
> +}
> +
> +static void kvm_psci_system_reset(struct kvm_vcpu *vcpu)
> +{
> +	kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_RESET);
> +}
> +
>  int kvm_psci_version(struct kvm_vcpu *vcpu)
>  {
>  	if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
> @@ -95,6 +112,7 @@ int kvm_psci_version(struct kvm_vcpu *vcpu)
>  
>  static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  {
> +	int ret = 1;
>  	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>  	unsigned long val;
>  
> @@ -114,13 +132,21 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  	case PSCI_0_2_FN64_CPU_ON:
>  		val = kvm_psci_vcpu_on(vcpu);
>  		break;
> +	case PSCI_0_2_FN_SYSTEM_OFF:
> +		kvm_psci_system_off(vcpu);
> +		val = PSCI_RET_SUCCESS;
> +		ret = 0;
> +		break;
> +	case PSCI_0_2_FN_SYSTEM_RESET:
> +		kvm_psci_system_reset(vcpu);
> +		val = PSCI_RET_SUCCESS;
> +		ret = 0;
> +		break;

What is the significance of setting val to PSCI_RET_SUCCESS here? We're
exiting to userspace, so surely only the platform emulation can set
that. Am I missing something?

>  	case PSCI_0_2_FN_CPU_SUSPEND:
>  	case PSCI_0_2_FN_AFFINITY_INFO:
>  	case PSCI_0_2_FN_MIGRATE:
>  	case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
>  	case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
> -	case PSCI_0_2_FN_SYSTEM_OFF:
> -	case PSCI_0_2_FN_SYSTEM_RESET:
>  	case PSCI_0_2_FN64_CPU_SUSPEND:
>  	case PSCI_0_2_FN64_AFFINITY_INFO:
>  	case PSCI_0_2_FN64_MIGRATE:
> @@ -132,7 +158,7 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  	}
>  
>  	*vcpu_reg(vcpu, 0) = val;
> -	return 1;
> +	return ret;
>  }
>  
>  static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
-- 
Jazz is not dead. It just smells funny.

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

* [PATCH v9 08/12] ARM/ARM64: KVM: Emulate PSCI v0.2 AFFINITY_INFO
  2014-04-15  6:14 ` [PATCH v9 08/12] ARM/ARM64: KVM: Emulate PSCI v0.2 AFFINITY_INFO Anup Patel
@ 2014-04-15 10:55   ` Marc Zyngier
  2014-04-15 11:15     ` Anup Patel
  0 siblings, 1 reply; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 10:55 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:11 am BST, Anup Patel <anup.patel@linaro.org> wrote:
> This patch adds emulation of PSCI v0.2 AFFINITY_INFO function call
> for KVM ARM/ARM64. This is a VCPU-level function call which will be
> used to determine current state of given affinity level.
>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
>  arch/arm/kvm/psci.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index b964aa4..f6f9290 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -27,6 +27,16 @@
>   * as described in ARM document number ARM DEN 0022A.
>   */
>  
> +#define AFFINITY_MASK(level)	~((0x1UL << ((level) * 8)) - 1)

Nit: Use MPIDR_LEVEL_MASK instead of 8.

> +
> +static unsigned long psci_affinity_mask(unsigned long affinity_level)
> +{
> +	if (affinity_level <= 3)
> +		return MPIDR_HWID_BITMASK & AFFINITY_MASK(affinity_level);
> +
> +	return 0;
> +}
> +
>  static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
>  {
>  	vcpu->arch.pause = true;
> @@ -85,6 +95,42 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>  	return PSCI_RET_SUCCESS;
>  }
>  
> +static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
> +{
> +	int i;
> +	unsigned long mpidr;
> +	unsigned long target_affinity;
> +	unsigned long target_affinity_mask;
> +	unsigned long lowest_affinity_level;
> +	struct kvm *kvm = vcpu->kvm;
> +	struct kvm_vcpu *tmp;
> +
> +	target_affinity = *vcpu_reg(vcpu, 1);
> +	lowest_affinity_level = *vcpu_reg(vcpu, 2);
> +
> +	/* Determine target affinity mask */
> +	target_affinity_mask = psci_affinity_mask(lowest_affinity_level);
> +	if (!target_affinity_mask)
> +		return PSCI_RET_INVALID_PARAMS;
> +
> +	/* Ignore other bits of target affinity */
> +	target_affinity &= target_affinity_mask;
> +
> +	/*
> +	 * If one or more VCPU matching target affinity are running
> +	 * then return 0 (ON) else return 1 (OFF)

Would be good to define symbolic values for these (and for ON_PENDING as
well).

> +	 */
> +	kvm_for_each_vcpu(i, tmp, kvm) {
> +		mpidr = kvm_vcpu_get_mpidr(tmp);
> +		if (((mpidr & target_affinity_mask) == target_affinity) &&
> +		    !tmp->arch.pause) {
> +			return 0;
> +		}
> +	}
> +
> +	return 1;
> +}
> +
>  static inline void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
>  {
>  	memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
> @@ -132,6 +178,10 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  	case PSCI_0_2_FN64_CPU_ON:
>  		val = kvm_psci_vcpu_on(vcpu);
>  		break;
> +	case PSCI_0_2_FN_AFFINITY_INFO:
> +	case PSCI_0_2_FN64_AFFINITY_INFO:
> +		val = kvm_psci_vcpu_affinity_info(vcpu);
> +		break;
>  	case PSCI_0_2_FN_SYSTEM_OFF:
>  		kvm_psci_system_off(vcpu);
>  		val = PSCI_RET_SUCCESS;
> @@ -143,12 +193,10 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  		ret = 0;
>  		break;
>  	case PSCI_0_2_FN_CPU_SUSPEND:
> -	case PSCI_0_2_FN_AFFINITY_INFO:
>  	case PSCI_0_2_FN_MIGRATE:
>  	case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
>  	case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
>  	case PSCI_0_2_FN64_CPU_SUSPEND:
> -	case PSCI_0_2_FN64_AFFINITY_INFO:
>  	case PSCI_0_2_FN64_MIGRATE:
>  	case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
>  		val = PSCI_RET_NOT_SUPPORTED;

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

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

* [PATCH v9 02/12] ARM/ARM64: KVM: Add common header for PSCI related defines
  2014-04-15 10:06   ` Marc Zyngier
@ 2014-04-15 11:10     ` Anup Patel
  2014-04-15 12:18       ` Marc Zyngier
  0 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15 11:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15, 2014 at 3:36 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On Tue, Apr 15 2014 at  7:14:05 am BST, Anup Patel <anup.patel@linaro.org> wrote:
>> We need a common place to share PSCI related defines among ARM kernel,
>> ARM64 kernel, KVM ARM/ARM64 PSCI emulation, and user space.
>>
>> We introduce uapi/linux/psci.h for this purpose. This newly added
>> header will be first used by KVM ARM/ARM64 in-kernel PSCI emulation
>> and user space (i.e. QEMU or KVMTOOL).
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
>> ---
>>  include/uapi/linux/Kbuild |    1 +
>>  include/uapi/linux/psci.h |   78 +++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 79 insertions(+)
>>  create mode 100644 include/uapi/linux/psci.h
>>
>> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
>> index 6929571..24e9033 100644
>> --- a/include/uapi/linux/Kbuild
>> +++ b/include/uapi/linux/Kbuild
>> @@ -317,6 +317,7 @@ header-y += ppp-ioctl.h
>>  header-y += ppp_defs.h
>>  header-y += pps.h
>>  header-y += prctl.h
>> +header-y += psci.h
>>  header-y += ptp_clock.h
>>  header-y += ptrace.h
>>  header-y += qnx4_fs.h
>> diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
>> new file mode 100644
>> index 0000000..077b623
>> --- /dev/null
>> +++ b/include/uapi/linux/psci.h
>> @@ -0,0 +1,78 @@
>> +/*
>> + * ARM Power State and Coordination Interface (PSCI) header
>> + *
>> + * This header holds common PSCI defines and macros shared
>> + * by: ARM kernel, ARM64 kernel, KVM ARM/ARM64 and user space.
>> + *
>> + * Copyright (C) 2014 Linaro Ltd.
>> + * Author: Anup Patel <anup.patel@linaro.org>
>> + */
>> +
>> +#ifndef _UAPI_LINUX_PSCI_H
>> +#define _UAPI_LINUX_PSCI_H
>> +
>> +/* PSCI v0.1 interface */
>> +#define PSCI_FN(base, n)                     ((base) + (n))
>> +
>> +#define PSCI_FN_CPU_SUSPEND(base)            PSCI_FN(base, 0)
>> +#define PSCI_FN_CPU_OFF(base)                        PSCI_FN(base, 1)
>> +#define PSCI_FN_CPU_ON(base)                 PSCI_FN(base, 2)
>> +#define PSCI_FN_MIGRATE(base)                        PSCI_FN(base, 3)
>
> For PSCI v0.1, all the function numbers are independent (the spec didn't
> specify any number...). You cannot assume contiguous numbering.

This is more like KVM recommended function numbering for PSCI v0.1.
We were already having it in uapi/asm/kvm.h to share between KVM and
user space so I felt this to be better place for it.

Do you want me to drop these or add some comment about PSCI v0.1
function numbering ?

>
>> +/* PSCI v0.2 interface */
>> +#define PSCI_0_2_FN_BASE                     0x84000000
>> +#define PSCI_0_2_FN(n)                               (PSCI_0_2_FN_BASE + (n))
>> +#define PSCI_0_2_64BIT                               0x40000000
>> +#define PSCI_0_2_FN64_BASE                   \
>> +                                     (PSCI_0_2_FN_BASE + PSCI_0_2_64BIT)
>> +#define PSCI_0_2_FN64(n)                     (PSCI_0_2_FN64_BASE + (n))
>> +
>> +#define PSCI_0_2_FN_PSCI_VERSION             PSCI_0_2_FN(0)
>> +#define PSCI_0_2_FN_CPU_SUSPEND                      PSCI_0_2_FN(1)
>> +#define PSCI_0_2_FN_CPU_OFF                  PSCI_0_2_FN(2)
>> +#define PSCI_0_2_FN_CPU_ON                   PSCI_0_2_FN(3)
>> +#define PSCI_0_2_FN_AFFINITY_INFO            PSCI_0_2_FN(4)
>> +#define PSCI_0_2_FN_MIGRATE                  PSCI_0_2_FN(5)
>> +#define PSCI_0_2_FN_MIGRATE_INFO_TYPE                PSCI_0_2_FN(6)
>> +#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU              PSCI_0_2_FN(7)
>> +#define PSCI_0_2_FN_SYSTEM_OFF                       PSCI_0_2_FN(8)
>> +#define PSCI_0_2_FN_SYSTEM_RESET             PSCI_0_2_FN(9)
>> +
>> +#define PSCI_0_2_FN64_CPU_SUSPEND            PSCI_0_2_FN64(1)
>> +#define PSCI_0_2_FN64_CPU_ON                 PSCI_0_2_FN64(3)
>> +#define PSCI_0_2_FN64_AFFINITY_INFO          PSCI_0_2_FN64(4)
>> +#define PSCI_0_2_FN64_MIGRATE                        PSCI_0_2_FN64(5)
>> +#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU    PSCI_0_2_FN64(7)
>> +
>> +#define PSCI_0_2_POWER_STATE_ID_MASK         0xffff
>> +#define PSCI_0_2_POWER_STATE_ID_SHIFT                0
>> +#define PSCI_0_2_POWER_STATE_TYPE_MASK               0x1
>> +#define PSCI_0_2_POWER_STATE_TYPE_SHIFT              16
>> +#define PSCI_0_2_POWER_STATE_AFFL_MASK               0x3
>> +#define PSCI_0_2_POWER_STATE_AFFL_SHIFT              24
>> +
>> +#define PSCI_0_2_TOS_UP_MIGRATE                      0
>> +#define PSCI_0_2_TOS_UP_NO_MIGRATE           1
>> +#define PSCI_0_2_TOS_MP                              2
>> +
>> +/* PSCI version decoding (independent of PSCI version) */
>> +#define PSCI_VERSION_MAJOR_MASK                      0xffff0000
>> +#define PSCI_VERSION_MINOR_MASK                      0x0000ffff
>> +#define PSCI_VERSION_MAJOR_SHIFT             16
>
> What about:
>
> #define PSCI_VERSION_MINOR_MASK ((1U << PSCI_VERSION_MAJOR_SHIFT) - 1)
> #define PSCI_VERSION_MAJOR_MASK ~PSCI_VERSION_MINOR_MASK

OK, I will update this.

>
>> +#define PSCI_VERSION_MAJOR(ver)                      \
>> +             (((ver) & PSCI_VERSION_MAJOR_MASK) >> PSCI_VERSION_MAJOR_SHIFT)
>> +#define PSCI_VERSION_MINOR(ver)                      \
>> +             ((ver) & PSCI_VERSION_MINOR_MASK)
>> +
>> +/* PSCI return values (inclusive of all PSCI versions) */
>> +#define PSCI_RET_SUCCESS                     0
>> +#define PSCI_RET_NOT_SUPPORTED                       -1
>> +#define PSCI_RET_INVALID_PARAMS                      -2
>> +#define PSCI_RET_DENIED                              -3
>> +#define PSCI_RET_ALREADY_ON                  -4
>> +#define PSCI_RET_ON_PENDING                  -5
>> +#define PSCI_RET_INTERNAL_FAILURE            -6
>> +#define PSCI_RET_NOT_PRESENT                 -7
>> +#define PSCI_RET_DISABLED                    -8
>> +
>> +#endif /* _UAPI_LINUX_PSCI_H */
>
> --
> Jazz is not dead. It just smells funny.
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

--
Anup

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

* [PATCH v9 05/12] ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible
  2014-04-15 10:21   ` Marc Zyngier
@ 2014-04-15 11:13     ` Anup Patel
  2014-04-15 12:23       ` Marc Zyngier
  0 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15 11:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15, 2014 at 3:51 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On Tue, Apr 15 2014 at  7:14:08 am BST, Anup Patel <anup.patel@linaro.org> wrote:
>> Currently, the kvm_psci_call() returns 'true' or 'false' based on whether
>> the PSCI function call was handled successfully or not. This does not help
>> us emulate system-level PSCI functions where the actual emulation work will
>> be done by user space (QEMU or KVMTOOL). Examples of such system-level PSCI
>> functions are: PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET.
>>
>> This patch updates kvm_psci_call() to return three types of values:
>> 1) > 0 (success)
>> 2) = 0 (success but exit to user space)
>> 3) < 0 (errors)
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
>> ---
>>  arch/arm/include/asm/kvm_psci.h   |    2 +-
>>  arch/arm/kvm/handle_exit.c        |   10 +++++++---
>>  arch/arm/kvm/psci.c               |   28 ++++++++++++++++------------
>>  arch/arm64/include/asm/kvm_psci.h |    2 +-
>>  arch/arm64/kvm/handle_exit.c      |   10 +++++++---
>>  5 files changed, 32 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_psci.h b/arch/arm/include/asm/kvm_psci.h
>> index 4c0e3e1..6bda945 100644
>> --- a/arch/arm/include/asm/kvm_psci.h
>> +++ b/arch/arm/include/asm/kvm_psci.h
>> @@ -22,6 +22,6 @@
>>  #define KVM_ARM_PSCI_0_2     2
>>
>>  int kvm_psci_version(struct kvm_vcpu *vcpu);
>> -bool kvm_psci_call(struct kvm_vcpu *vcpu);
>> +int kvm_psci_call(struct kvm_vcpu *vcpu);
>>
>>  #endif /* __ARM_KVM_PSCI_H__ */
>> diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
>> index 0de91fc..1270095 100644
>> --- a/arch/arm/kvm/handle_exit.c
>> +++ b/arch/arm/kvm/handle_exit.c
>> @@ -38,14 +38,18 @@ static int handle_svc_hyp(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>
>>  static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>  {
>> +     int ret;
>> +
>>       trace_kvm_hvc(*vcpu_pc(vcpu), *vcpu_reg(vcpu, 0),
>>                     kvm_vcpu_hvc_get_imm(vcpu));
>>
>> -     if (kvm_psci_call(vcpu))
>> +     ret = kvm_psci_call(vcpu);
>> +     if (ret == -EINVAL) {
>
> Please check for (ret < 0), so it actually matches with the comment (and
> will same some debugging when we introduce another error code).

Should we be injecting undefined exception for all types errors?

The intention here was to only inject undefined exception when
PSCI function number is invalid.

>
>> +             kvm_inject_undefined(vcpu);
>>               return 1;
>> +     }
>>
>> -     kvm_inject_undefined(vcpu);
>> -     return 1;
>> +     return ret;
>>  }
>>
>>  static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
>> index 8c42596c..14e6fa6 100644
>> --- a/arch/arm/kvm/psci.c
>> +++ b/arch/arm/kvm/psci.c
>> @@ -93,7 +93,7 @@ int kvm_psci_version(struct kvm_vcpu *vcpu)
>>       return KVM_ARM_PSCI_0_1;
>>  }
>>
>> -static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>> +static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>  {
>>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>       unsigned long val;
>> @@ -128,14 +128,14 @@ static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>               val = PSCI_RET_NOT_SUPPORTED;
>>               break;
>>       default:
>> -             return false;
>> +             return -EINVAL;
>>       }
>>
>>       *vcpu_reg(vcpu, 0) = val;
>> -     return true;
>> +     return 1;
>>  }
>>
>> -static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>> +static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>>  {
>>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>       unsigned long val;
>> @@ -153,11 +153,11 @@ static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>>               val = PSCI_RET_NOT_SUPPORTED;
>>               break;
>>       default:
>> -             return false;
>> +             return -EINVAL;
>>       }
>>
>>       *vcpu_reg(vcpu, 0) = val;
>> -     return true;
>> +     return 1;
>>  }
>>
>>  /**
>> @@ -165,12 +165,16 @@ static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>>   * @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.
>> + * The calling convention is similar to SMC calls to the secure world
>> + * where the function number is placed in r0.
>> + *
>> + * This function returns: > 0 (success), 0 (success but exit to user
>> + * space), and < 0 (errors)
>> + *
>> + * Errors:
>> + * -EINVAL: Unrecognized PSCI function
>>   */
>> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> +int kvm_psci_call(struct kvm_vcpu *vcpu)
>>  {
>>       switch (kvm_psci_version(vcpu)) {
>>       case KVM_ARM_PSCI_0_2:
>> @@ -178,6 +182,6 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>>       case KVM_ARM_PSCI_0_1:
>>               return kvm_psci_0_1_call(vcpu);
>>       default:
>> -             return false;
>> +             return -EINVAL;
>>       };
>>  }
>> diff --git a/arch/arm64/include/asm/kvm_psci.h b/arch/arm64/include/asm/kvm_psci.h
>> index e25c658..bc39e55 100644
>> --- a/arch/arm64/include/asm/kvm_psci.h
>> +++ b/arch/arm64/include/asm/kvm_psci.h
>> @@ -22,6 +22,6 @@
>>  #define KVM_ARM_PSCI_0_2     2
>>
>>  int kvm_psci_version(struct kvm_vcpu *vcpu);
>> -bool kvm_psci_call(struct kvm_vcpu *vcpu);
>> +int kvm_psci_call(struct kvm_vcpu *vcpu);
>>
>>  #endif /* __ARM64_KVM_PSCI_H__ */
>> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
>> index 7bc41ea..743a74d 100644
>> --- a/arch/arm64/kvm/handle_exit.c
>> +++ b/arch/arm64/kvm/handle_exit.c
>> @@ -30,11 +30,15 @@ typedef int (*exit_handle_fn)(struct kvm_vcpu *, struct kvm_run *);
>>
>>  static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>  {
>> -     if (kvm_psci_call(vcpu))
>> +     int ret;
>> +
>> +     ret = kvm_psci_call(vcpu);
>> +     if (ret == -EINVAL) {
>
> Same here.
>
>> +             kvm_inject_undefined(vcpu);
>>               return 1;
>> +     }
>>
>> -     kvm_inject_undefined(vcpu);
>> -     return 1;
>> +     return ret;
>>  }
>>
>>  static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>
> Thanks,
>
>         M.
> --
> Jazz is not dead. It just smells funny.
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

--
Anup

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

* [PATCH v9 08/12] ARM/ARM64: KVM: Emulate PSCI v0.2 AFFINITY_INFO
  2014-04-15 10:55   ` Marc Zyngier
@ 2014-04-15 11:15     ` Anup Patel
  0 siblings, 0 replies; 33+ messages in thread
From: Anup Patel @ 2014-04-15 11:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15, 2014 at 4:25 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On Tue, Apr 15 2014 at  7:14:11 am BST, Anup Patel <anup.patel@linaro.org> wrote:
>> This patch adds emulation of PSCI v0.2 AFFINITY_INFO function call
>> for KVM ARM/ARM64. This is a VCPU-level function call which will be
>> used to determine current state of given affinity level.
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
>> ---
>>  arch/arm/kvm/psci.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 50 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
>> index b964aa4..f6f9290 100644
>> --- a/arch/arm/kvm/psci.c
>> +++ b/arch/arm/kvm/psci.c
>> @@ -27,6 +27,16 @@
>>   * as described in ARM document number ARM DEN 0022A.
>>   */
>>
>> +#define AFFINITY_MASK(level) ~((0x1UL << ((level) * 8)) - 1)
>
> Nit: Use MPIDR_LEVEL_MASK instead of 8.

OK, will use MPIDR_LEVEL_MASK here.

>
>> +
>> +static unsigned long psci_affinity_mask(unsigned long affinity_level)
>> +{
>> +     if (affinity_level <= 3)
>> +             return MPIDR_HWID_BITMASK & AFFINITY_MASK(affinity_level);
>> +
>> +     return 0;
>> +}
>> +
>>  static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
>>  {
>>       vcpu->arch.pause = true;
>> @@ -85,6 +95,42 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>>       return PSCI_RET_SUCCESS;
>>  }
>>
>> +static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
>> +{
>> +     int i;
>> +     unsigned long mpidr;
>> +     unsigned long target_affinity;
>> +     unsigned long target_affinity_mask;
>> +     unsigned long lowest_affinity_level;
>> +     struct kvm *kvm = vcpu->kvm;
>> +     struct kvm_vcpu *tmp;
>> +
>> +     target_affinity = *vcpu_reg(vcpu, 1);
>> +     lowest_affinity_level = *vcpu_reg(vcpu, 2);
>> +
>> +     /* Determine target affinity mask */
>> +     target_affinity_mask = psci_affinity_mask(lowest_affinity_level);
>> +     if (!target_affinity_mask)
>> +             return PSCI_RET_INVALID_PARAMS;
>> +
>> +     /* Ignore other bits of target affinity */
>> +     target_affinity &= target_affinity_mask;
>> +
>> +     /*
>> +      * If one or more VCPU matching target affinity are running
>> +      * then return 0 (ON) else return 1 (OFF)
>
> Would be good to define symbolic values for these (and for ON_PENDING as
> well).

Sure, I will add them to uapi/linux/psci.h

>
>> +      */
>> +     kvm_for_each_vcpu(i, tmp, kvm) {
>> +             mpidr = kvm_vcpu_get_mpidr(tmp);
>> +             if (((mpidr & target_affinity_mask) == target_affinity) &&
>> +                 !tmp->arch.pause) {
>> +                     return 0;
>> +             }
>> +     }
>> +
>> +     return 1;
>> +}
>> +
>>  static inline void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
>>  {
>>       memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
>> @@ -132,6 +178,10 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>       case PSCI_0_2_FN64_CPU_ON:
>>               val = kvm_psci_vcpu_on(vcpu);
>>               break;
>> +     case PSCI_0_2_FN_AFFINITY_INFO:
>> +     case PSCI_0_2_FN64_AFFINITY_INFO:
>> +             val = kvm_psci_vcpu_affinity_info(vcpu);
>> +             break;
>>       case PSCI_0_2_FN_SYSTEM_OFF:
>>               kvm_psci_system_off(vcpu);
>>               val = PSCI_RET_SUCCESS;
>> @@ -143,12 +193,10 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>               ret = 0;
>>               break;
>>       case PSCI_0_2_FN_CPU_SUSPEND:
>> -     case PSCI_0_2_FN_AFFINITY_INFO:
>>       case PSCI_0_2_FN_MIGRATE:
>>       case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
>>       case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
>>       case PSCI_0_2_FN64_CPU_SUSPEND:
>> -     case PSCI_0_2_FN64_AFFINITY_INFO:
>>       case PSCI_0_2_FN64_MIGRATE:
>>       case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
>>               val = PSCI_RET_NOT_SUPPORTED;
>
> --
> Jazz is not dead. It just smells funny.
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

--
Anup

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

* [PATCH v9 07/12] ARM/ARM64: KVM: Emulate PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET
  2014-04-15 10:34   ` Marc Zyngier
@ 2014-04-15 11:26     ` Anup Patel
  2014-04-15 12:28       ` Marc Zyngier
  0 siblings, 1 reply; 33+ messages in thread
From: Anup Patel @ 2014-04-15 11:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15, 2014 at 4:04 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On Tue, Apr 15 2014 at  7:14:10 am BST, Anup Patel <anup.patel@linaro.org> wrote:
>> The PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET functions are system-level
>> functions hence cannot be fully emulated by in-kernel PSCI emulation code.
>>
>> To tackle this, we forward PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET function
>> calls from vcpu to user space (i.e. QEMU or KVMTOOL) via kvm_run structure
>> using KVM_EXIT_SYSTEM_EVENT exit reasons.
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
>> ---
>>  arch/arm/kvm/psci.c |   32 +++++++++++++++++++++++++++++---
>>  1 file changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
>> index 14e6fa6..b964aa4 100644
>> --- a/arch/arm/kvm/psci.c
>> +++ b/arch/arm/kvm/psci.c
>> @@ -85,6 +85,23 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>>       return PSCI_RET_SUCCESS;
>>  }
>>
>> +static inline void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
>
> Loose the "inline". This is not performance critical, and the compiler
> does a pretty good job doing that for you.

OK, I will drop the "inline" attribute.

>
>> +{
>> +     memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
>> +     vcpu->run->system_event.type = type;
>> +     vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
>> +}
>> +
>> +static void kvm_psci_system_off(struct kvm_vcpu *vcpu)
>> +{
>> +     kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_SHUTDOWN);
>> +}
>> +
>> +static void kvm_psci_system_reset(struct kvm_vcpu *vcpu)
>> +{
>> +     kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_RESET);
>> +}
>> +
>>  int kvm_psci_version(struct kvm_vcpu *vcpu)
>>  {
>>       if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>> @@ -95,6 +112,7 @@ int kvm_psci_version(struct kvm_vcpu *vcpu)
>>
>>  static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>  {
>> +     int ret = 1;
>>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>       unsigned long val;
>>
>> @@ -114,13 +132,21 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>       case PSCI_0_2_FN64_CPU_ON:
>>               val = kvm_psci_vcpu_on(vcpu);
>>               break;
>> +     case PSCI_0_2_FN_SYSTEM_OFF:
>> +             kvm_psci_system_off(vcpu);
>> +             val = PSCI_RET_SUCCESS;
>> +             ret = 0;
>> +             break;
>> +     case PSCI_0_2_FN_SYSTEM_RESET:
>> +             kvm_psci_system_reset(vcpu);
>> +             val = PSCI_RET_SUCCESS;
>> +             ret = 0;
>> +             break;
>
> What is the significance of setting val to PSCI_RET_SUCCESS here? We're
> exiting to userspace, so surely only the platform emulation can set
> that. Am I missing something?

Actually, return value is undefined for SYSTEM_OFF and SYSTEM_RESET
because these functions are not expected to return. Currently, we are updating
r0 (or x0) for all PSCI functions.

Are you suggesting that we update r0 (or x0) only when there are no error
(i.e. ret == 0) ?

>
>>       case PSCI_0_2_FN_CPU_SUSPEND:
>>       case PSCI_0_2_FN_AFFINITY_INFO:
>>       case PSCI_0_2_FN_MIGRATE:
>>       case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
>>       case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
>> -     case PSCI_0_2_FN_SYSTEM_OFF:
>> -     case PSCI_0_2_FN_SYSTEM_RESET:
>>       case PSCI_0_2_FN64_CPU_SUSPEND:
>>       case PSCI_0_2_FN64_AFFINITY_INFO:
>>       case PSCI_0_2_FN64_MIGRATE:
>> @@ -132,7 +158,7 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>       }
>>
>>       *vcpu_reg(vcpu, 0) = val;
>> -     return 1;
>> +     return ret;
>>  }
>>
>>  static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
> --
> Jazz is not dead. It just smells funny.
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

--
Anup

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

* [PATCH v9 09/12] ARM/ARM64: KVM: Emulate PSCI v0.2 MIGRATE_INFO_TYPE and related functions
  2014-04-15  6:14 ` [PATCH v9 09/12] ARM/ARM64: KVM: Emulate PSCI v0.2 MIGRATE_INFO_TYPE and related functions Anup Patel
@ 2014-04-15 12:05   ` Marc Zyngier
  0 siblings, 0 replies; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 12:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:12 am BST, Anup Patel <anup.patel@linaro.org> wrote:
> This patch adds emulation of PSCI v0.2 MIGRATE, MIGRATE_INFO_TYPE, and
> MIGRATE_INFO_UP_CPU function calls for KVM ARM/ARM64.
>
> KVM ARM/ARM64 being a hypervisor (and not a Trusted OS), we cannot provide
> this functions hence we emulate these functions in following way:
> 1. MIGRATE - Returns "Not Supported"
> 2. MIGRATE_INFO_TYPE - Return 2 i.e. Trusted OS is not present
> 3. MIGRATE_INFO_UP_CPU - Returns "Not Supported"
>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

> ---
>  arch/arm/kvm/psci.c |   21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index f6f9290..1af0016 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -182,6 +182,22 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  	case PSCI_0_2_FN64_AFFINITY_INFO:
>  		val = kvm_psci_vcpu_affinity_info(vcpu);
>  		break;
> +	case PSCI_0_2_FN_MIGRATE:
> +	case PSCI_0_2_FN64_MIGRATE:
> +		val = PSCI_RET_NOT_SUPPORTED;
> +		break;
> +	case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
> +		/*
> +		 * Trusted OS is MP hence does not require migration
> +	         * or
> +		 * Trusted OS is not present
> +		 */
> +		val = PSCI_0_2_TOS_MP;
> +		break;
> +	case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
> +	case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
> +		val = PSCI_RET_NOT_SUPPORTED;
> +		break;
>  	case PSCI_0_2_FN_SYSTEM_OFF:
>  		kvm_psci_system_off(vcpu);
>  		val = PSCI_RET_SUCCESS;
> @@ -193,12 +209,7 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  		ret = 0;
>  		break;
>  	case PSCI_0_2_FN_CPU_SUSPEND:
> -	case PSCI_0_2_FN_MIGRATE:
> -	case PSCI_0_2_FN_MIGRATE_INFO_TYPE:
> -	case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU:
>  	case PSCI_0_2_FN64_CPU_SUSPEND:
> -	case PSCI_0_2_FN64_MIGRATE:
> -	case PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU:
>  		val = PSCI_RET_NOT_SUPPORTED;
>  		break;
>  	default:

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

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

* [PATCH v9 10/12] ARM/ARM64: KVM: Fix CPU_ON emulation for PSCI v0.2
  2014-04-15  6:14 ` [PATCH v9 10/12] ARM/ARM64: KVM: Fix CPU_ON emulation for PSCI v0.2 Anup Patel
@ 2014-04-15 12:10   ` Marc Zyngier
  0 siblings, 0 replies; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 12:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:13 am BST, Anup Patel <anup.patel@linaro.org> wrote:
> As-per PSCI v0.2, the source CPU provides physical address of
> "entry point" and "context id" for starting a target CPU. Also,
> if target CPU is already running then we should return ALREADY_ON.
>
> Current emulation of CPU_ON function does not consider physical
> address of "context id" and returns INVALID_PARAMETERS if target
> CPU is already running.
>
> This patch updates kvm_psci_vcpu_on() such that it works for both
> PSCI v0.1 and PSCI v0.2.
>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

> ---
>  arch/arm/kvm/psci.c |   15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 1af0016..f59f09d 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -48,6 +48,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>  	struct kvm_vcpu *vcpu = NULL, *tmp;
>  	wait_queue_head_t *wq;
>  	unsigned long cpu_id;
> +	unsigned long context_id;
>  	unsigned long mpidr;
>  	phys_addr_t target_pc;
>  	int i;
> @@ -68,10 +69,17 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>  	 * Make sure the caller requested a valid CPU and that the CPU is
>  	 * turned off.
>  	 */
> -	if (!vcpu || !vcpu->arch.pause)
> +	if (!vcpu)
>  		return PSCI_RET_INVALID_PARAMS;
> +	if (!vcpu->arch.pause) {
> +		if (kvm_psci_version(source_vcpu) != KVM_ARM_PSCI_0_1)
> +			return PSCI_RET_ALREADY_ON;
> +		else
> +			return PSCI_RET_INVALID_PARAMS;
> +	}
>  
>  	target_pc = *vcpu_reg(source_vcpu, 2);
> +	context_id = *vcpu_reg(source_vcpu, 3);
>  
>  	kvm_reset_vcpu(vcpu);
>  
> @@ -86,6 +94,11 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>  		kvm_vcpu_set_be(vcpu);
>  
>  	*vcpu_pc(vcpu) = target_pc;
> +	/*
> +	 * NOTE: We always update r0 (or x0) because for PSCI v0.1
> +	 * the general puspose registers are undefined upon CPU_ON.
> +	 */
> +	*vcpu_reg(vcpu, 0) = context_id;
>  	vcpu->arch.pause = false;
>  	smp_mb();		/* Make sure the above is visible */

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

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

* [PATCH v9 11/12] ARM/ARM64: KVM: Emulate PSCI v0.2 CPU_SUSPEND
  2014-04-15  6:14 ` [PATCH v9 11/12] ARM/ARM64: KVM: Emulate PSCI v0.2 CPU_SUSPEND Anup Patel
@ 2014-04-15 12:14   ` Marc Zyngier
  0 siblings, 0 replies; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 12:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:14 am BST, Anup Patel <anup.patel@linaro.org> wrote:
> This patch adds emulation of PSCI v0.2 CPU_SUSPEND function call for
> KVM ARM/ARM64. This is a CPU-level function call which can suspend
> current CPU or current CPU cluster. We don't have VCPU clusters in
> KVM so we only suspend the current VCPU.
>
> The CPU_SUSPEND emulation is not tested much because currently there
> is no CPUIDLE driver in Linux kernel that uses PSCI CPU_SUSPEND. The
> PSCI CPU_SUSPEND implementation in ARM64 kernel was tested using a
> Simple CPUIDLE driver which is not published due to unstable DT-bindings
> for PSCI.
> (For more info, http://lwn.net/Articles/574950/)
>
> Even if we had stable DT-bindings for PSCI and CPUIDLE driver that
> uses PSCI CPU_SUSPEND then still we need to define SUSPEND states
> and WAKEUP events for KVM ARM/ARM64.
>
> Due to above, we implement CPU_SUSPEND emulation similar to WFI
> (Wait-for-interrupt) emulation and we also treat power-down request
> to be same as stand-by request. This is consistent with section
> 5.4.1 and section 5.4.2 of PSCI v0.2 specification.
>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> ---
>  arch/arm/kvm/psci.c |   29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index f59f09d..fdb347a 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -37,6 +37,27 @@ static unsigned long psci_affinity_mask(unsigned long affinity_level)
>  	return 0;
>  }
>  
> +static unsigned long kvm_psci_vcpu_suspend(struct kvm_vcpu *vcpu)
> +{
> +	/*
> +	 * NOTE: Currently, we don't have any wakeup events defined
> +	 * for KVM so for simplicity we make VCPU suspend emulation
> +	 * to be same-as WFI (Wait-for-interrupt) emulation.

Well, in this implementation, any interrupt is a wakeup event, so you
can remove the comment about "we don't have any wakeup events". We
arguably have too many of them.

> +	 *
> +	 * This means for KVM the wakeup events are interrupts and
> +	 * this is consistent with intended use of StateID as described
> +	 * in section 5.4.1 of PSCI v0.2 specification (ARM DEN 0022A).
> +	 *
> +	 * Further, we also treat power-down request to be same as
> +	 * stand-by request as-per section 5.4.2 clause 3 of PSCI v0.2
> +	 * specification (ARM DEN 0022A). This means all suspend states
> +	 * for KVM will preserve the register state.
> +	 */
> +	kvm_vcpu_block(vcpu);
> +
> +	return PSCI_RET_SUCCESS;
> +}
> +
>  static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
>  {
>  	vcpu->arch.pause = true;
> @@ -183,6 +204,10 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  		 */
>  		val = 2;
>  		break;
> +	case PSCI_0_2_FN_CPU_SUSPEND:
> +	case PSCI_0_2_FN64_CPU_SUSPEND:
> +		val = kvm_psci_vcpu_suspend(vcpu);
> +		break;
>  	case PSCI_0_2_FN_CPU_OFF:
>  		kvm_psci_vcpu_off(vcpu);
>  		val = PSCI_RET_SUCCESS;
> @@ -221,10 +246,6 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  		val = PSCI_RET_SUCCESS;
>  		ret = 0;
>  		break;
> -	case PSCI_0_2_FN_CPU_SUSPEND:
> -	case PSCI_0_2_FN64_CPU_SUSPEND:
> -		val = PSCI_RET_NOT_SUPPORTED;
> -		break;
>  	default:
>  		return -EINVAL;
>  	}

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

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

* [PATCH v9 12/12] ARM/ARM64: KVM: Advertise KVM_CAP_ARM_PSCI_0_2 to user space
  2014-04-15  6:14 ` [PATCH v9 12/12] ARM/ARM64: KVM: Advertise KVM_CAP_ARM_PSCI_0_2 to user space Anup Patel
@ 2014-04-15 12:16   ` Marc Zyngier
  0 siblings, 0 replies; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 12:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at  7:14:15 am BST, Anup Patel <anup.patel@linaro.org> wrote:
> We have PSCI v0.2 emulation available in KVM ARM/ARM64
> hence advertise this to user space (i.e. QEMU or KVMTOOL)
> via KVM_CHECK_EXTENSION ioctl.
>
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

> ---
>  arch/arm/kvm/arm.c |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index f0e50a0..3c82b37 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -197,6 +197,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:

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

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

* [PATCH v9 02/12] ARM/ARM64: KVM: Add common header for PSCI related defines
  2014-04-15 11:10     ` Anup Patel
@ 2014-04-15 12:18       ` Marc Zyngier
  0 siblings, 0 replies; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 12:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at 12:10:30 pm BST, Anup Patel <anup@brainfault.org> wrote:
> On Tue, Apr 15, 2014 at 3:36 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On Tue, Apr 15 2014 at  7:14:05 am BST, Anup Patel <anup.patel@linaro.org> wrote:
>>> We need a common place to share PSCI related defines among ARM kernel,
>>> ARM64 kernel, KVM ARM/ARM64 PSCI emulation, and user space.
>>>
>>> We introduce uapi/linux/psci.h for this purpose. This newly added
>>> header will be first used by KVM ARM/ARM64 in-kernel PSCI emulation
>>> and user space (i.e. QEMU or KVMTOOL).
>>>
>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
>>> ---
>>>  include/uapi/linux/Kbuild |    1 +
>>>  include/uapi/linux/psci.h |   78 +++++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 79 insertions(+)
>>>  create mode 100644 include/uapi/linux/psci.h
>>>
>>> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
>>> index 6929571..24e9033 100644
>>> --- a/include/uapi/linux/Kbuild
>>> +++ b/include/uapi/linux/Kbuild
>>> @@ -317,6 +317,7 @@ header-y += ppp-ioctl.h
>>>  header-y += ppp_defs.h
>>>  header-y += pps.h
>>>  header-y += prctl.h
>>> +header-y += psci.h
>>>  header-y += ptp_clock.h
>>>  header-y += ptrace.h
>>>  header-y += qnx4_fs.h
>>> diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
>>> new file mode 100644
>>> index 0000000..077b623
>>> --- /dev/null
>>> +++ b/include/uapi/linux/psci.h
>>> @@ -0,0 +1,78 @@
>>> +/*
>>> + * ARM Power State and Coordination Interface (PSCI) header
>>> + *
>>> + * This header holds common PSCI defines and macros shared
>>> + * by: ARM kernel, ARM64 kernel, KVM ARM/ARM64 and user space.
>>> + *
>>> + * Copyright (C) 2014 Linaro Ltd.
>>> + * Author: Anup Patel <anup.patel@linaro.org>
>>> + */
>>> +
>>> +#ifndef _UAPI_LINUX_PSCI_H
>>> +#define _UAPI_LINUX_PSCI_H
>>> +
>>> +/* PSCI v0.1 interface */
>>> +#define PSCI_FN(base, n)                     ((base) + (n))
>>> +
>>> +#define PSCI_FN_CPU_SUSPEND(base)            PSCI_FN(base, 0)
>>> +#define PSCI_FN_CPU_OFF(base)                        PSCI_FN(base, 1)
>>> +#define PSCI_FN_CPU_ON(base)                 PSCI_FN(base, 2)
>>> +#define PSCI_FN_MIGRATE(base)                        PSCI_FN(base, 3)
>>
>> For PSCI v0.1, all the function numbers are independent (the spec didn't
>> specify any number...). You cannot assume contiguous numbering.
>
> This is more like KVM recommended function numbering for PSCI v0.1.
> We were already having it in uapi/asm/kvm.h to share between KVM and
> user space so I felt this to be better place for it.
>
> Do you want me to drop these or add some comment about PSCI v0.1
> function numbering ?

Both. Add a comment saying that v0.1 function numbering is
implementation defined, and remove these defines.

>
>>
>>> +/* PSCI v0.2 interface */
>>> +#define PSCI_0_2_FN_BASE                     0x84000000
>>> +#define PSCI_0_2_FN(n)                               (PSCI_0_2_FN_BASE + (n))
>>> +#define PSCI_0_2_64BIT                               0x40000000
>>> +#define PSCI_0_2_FN64_BASE                   \
>>> +                                     (PSCI_0_2_FN_BASE + PSCI_0_2_64BIT)
>>> +#define PSCI_0_2_FN64(n)                     (PSCI_0_2_FN64_BASE + (n))
>>> +
>>> +#define PSCI_0_2_FN_PSCI_VERSION             PSCI_0_2_FN(0)
>>> +#define PSCI_0_2_FN_CPU_SUSPEND                      PSCI_0_2_FN(1)
>>> +#define PSCI_0_2_FN_CPU_OFF                  PSCI_0_2_FN(2)
>>> +#define PSCI_0_2_FN_CPU_ON                   PSCI_0_2_FN(3)
>>> +#define PSCI_0_2_FN_AFFINITY_INFO            PSCI_0_2_FN(4)
>>> +#define PSCI_0_2_FN_MIGRATE                  PSCI_0_2_FN(5)
>>> +#define PSCI_0_2_FN_MIGRATE_INFO_TYPE                PSCI_0_2_FN(6)
>>> +#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU              PSCI_0_2_FN(7)
>>> +#define PSCI_0_2_FN_SYSTEM_OFF                       PSCI_0_2_FN(8)
>>> +#define PSCI_0_2_FN_SYSTEM_RESET             PSCI_0_2_FN(9)
>>> +
>>> +#define PSCI_0_2_FN64_CPU_SUSPEND            PSCI_0_2_FN64(1)
>>> +#define PSCI_0_2_FN64_CPU_ON                 PSCI_0_2_FN64(3)
>>> +#define PSCI_0_2_FN64_AFFINITY_INFO          PSCI_0_2_FN64(4)
>>> +#define PSCI_0_2_FN64_MIGRATE                        PSCI_0_2_FN64(5)
>>> +#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU    PSCI_0_2_FN64(7)
>>> +
>>> +#define PSCI_0_2_POWER_STATE_ID_MASK         0xffff
>>> +#define PSCI_0_2_POWER_STATE_ID_SHIFT                0
>>> +#define PSCI_0_2_POWER_STATE_TYPE_MASK               0x1
>>> +#define PSCI_0_2_POWER_STATE_TYPE_SHIFT              16
>>> +#define PSCI_0_2_POWER_STATE_AFFL_MASK               0x3
>>> +#define PSCI_0_2_POWER_STATE_AFFL_SHIFT              24
>>> +
>>> +#define PSCI_0_2_TOS_UP_MIGRATE                      0
>>> +#define PSCI_0_2_TOS_UP_NO_MIGRATE           1
>>> +#define PSCI_0_2_TOS_MP                              2
>>> +
>>> +/* PSCI version decoding (independent of PSCI version) */
>>> +#define PSCI_VERSION_MAJOR_MASK                      0xffff0000
>>> +#define PSCI_VERSION_MINOR_MASK                      0x0000ffff
>>> +#define PSCI_VERSION_MAJOR_SHIFT             16
>>
>> What about:
>>
>> #define PSCI_VERSION_MINOR_MASK ((1U << PSCI_VERSION_MAJOR_SHIFT) - 1)
>> #define PSCI_VERSION_MAJOR_MASK ~PSCI_VERSION_MINOR_MASK
>
> OK, I will update this.

Thanks,

	M.

>>
>>> +#define PSCI_VERSION_MAJOR(ver)                      \
>>> +             (((ver) & PSCI_VERSION_MAJOR_MASK) >> PSCI_VERSION_MAJOR_SHIFT)
>>> +#define PSCI_VERSION_MINOR(ver)                      \
>>> +             ((ver) & PSCI_VERSION_MINOR_MASK)
>>> +
>>> +/* PSCI return values (inclusive of all PSCI versions) */
>>> +#define PSCI_RET_SUCCESS                     0
>>> +#define PSCI_RET_NOT_SUPPORTED                       -1
>>> +#define PSCI_RET_INVALID_PARAMS                      -2
>>> +#define PSCI_RET_DENIED                              -3
>>> +#define PSCI_RET_ALREADY_ON                  -4
>>> +#define PSCI_RET_ON_PENDING                  -5
>>> +#define PSCI_RET_INTERNAL_FAILURE            -6
>>> +#define PSCI_RET_NOT_PRESENT                 -7
>>> +#define PSCI_RET_DISABLED                    -8
>>> +
>>> +#endif /* _UAPI_LINUX_PSCI_H */
>>
>> --
>> Jazz is not dead. It just smells funny.
>> _______________________________________________
>> kvmarm mailing list
>> kvmarm at lists.cs.columbia.edu
>> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>
> --
> Anup
>

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

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

* [PATCH v9 05/12] ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible
  2014-04-15 11:13     ` Anup Patel
@ 2014-04-15 12:23       ` Marc Zyngier
  2014-04-15 13:07         ` Peter Maydell
  0 siblings, 1 reply; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 12:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at 12:13:30 pm BST, Anup Patel <anup@brainfault.org> wrote:
> On Tue, Apr 15, 2014 at 3:51 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On Tue, Apr 15 2014 at  7:14:08 am BST, Anup Patel <anup.patel@linaro.org> wrote:
>>> Currently, the kvm_psci_call() returns 'true' or 'false' based on whether
>>> the PSCI function call was handled successfully or not. This does not help
>>> us emulate system-level PSCI functions where the actual emulation work will
>>> be done by user space (QEMU or KVMTOOL). Examples of such system-level PSCI
>>> functions are: PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET.
>>>
>>> This patch updates kvm_psci_call() to return three types of values:
>>> 1) > 0 (success)
>>> 2) = 0 (success but exit to user space)
>>> 3) < 0 (errors)
>>>
>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
>>> ---
>>>  arch/arm/include/asm/kvm_psci.h   |    2 +-
>>>  arch/arm/kvm/handle_exit.c        |   10 +++++++---
>>>  arch/arm/kvm/psci.c               |   28 ++++++++++++++++------------
>>>  arch/arm64/include/asm/kvm_psci.h |    2 +-
>>>  arch/arm64/kvm/handle_exit.c      |   10 +++++++---
>>>  5 files changed, 32 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/kvm_psci.h b/arch/arm/include/asm/kvm_psci.h
>>> index 4c0e3e1..6bda945 100644
>>> --- a/arch/arm/include/asm/kvm_psci.h
>>> +++ b/arch/arm/include/asm/kvm_psci.h
>>> @@ -22,6 +22,6 @@
>>>  #define KVM_ARM_PSCI_0_2     2
>>>
>>>  int kvm_psci_version(struct kvm_vcpu *vcpu);
>>> -bool kvm_psci_call(struct kvm_vcpu *vcpu);
>>> +int kvm_psci_call(struct kvm_vcpu *vcpu);
>>>
>>>  #endif /* __ARM_KVM_PSCI_H__ */
>>> diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
>>> index 0de91fc..1270095 100644
>>> --- a/arch/arm/kvm/handle_exit.c
>>> +++ b/arch/arm/kvm/handle_exit.c
>>> @@ -38,14 +38,18 @@ static int handle_svc_hyp(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>>
>>>  static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>>  {
>>> +     int ret;
>>> +
>>>       trace_kvm_hvc(*vcpu_pc(vcpu), *vcpu_reg(vcpu, 0),
>>>                     kvm_vcpu_hvc_get_imm(vcpu));
>>>
>>> -     if (kvm_psci_call(vcpu))
>>> +     ret = kvm_psci_call(vcpu);
>>> +     if (ret == -EINVAL) {
>>
>> Please check for (ret < 0), so it actually matches with the comment (and
>> will same some debugging when we introduce another error code).
>
> Should we be injecting undefined exception for all types errors?

What would be the alternative? If we end-up being unable to provide the
expected service because of an internal error, I'd rather let the guest
know about it.

> The intention here was to only inject undefined exception when
> PSCI function number is invalid.

I understand that, and this is the only case at the moment. What I'm
foreseeing is a situation where we've been unable to perform the
expected service, and PSCI doesn't specify any "internal error". So an
error injection looks like a valid solution to me.

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

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

* [PATCH v9 07/12] ARM/ARM64: KVM: Emulate PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET
  2014-04-15 11:26     ` Anup Patel
@ 2014-04-15 12:28       ` Marc Zyngier
  0 siblings, 0 replies; 33+ messages in thread
From: Marc Zyngier @ 2014-04-15 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 15 2014 at 12:26:06 pm BST, Anup Patel <anup@brainfault.org> wrote:
> On Tue, Apr 15, 2014 at 4:04 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On Tue, Apr 15 2014 at  7:14:10 am BST, Anup Patel <anup.patel@linaro.org> wrote:
>>> The PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET functions are system-level
>>> functions hence cannot be fully emulated by in-kernel PSCI emulation code.
>>>
>>> To tackle this, we forward PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET function
>>> calls from vcpu to user space (i.e. QEMU or KVMTOOL) via kvm_run structure
>>> using KVM_EXIT_SYSTEM_EVENT exit reasons.
>>>
>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
>>> ---
>>>  arch/arm/kvm/psci.c |   32 +++++++++++++++++++++++++++++---
>>>  1 file changed, 29 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
>>> index 14e6fa6..b964aa4 100644
>>> --- a/arch/arm/kvm/psci.c
>>> +++ b/arch/arm/kvm/psci.c
>>> @@ -85,6 +85,23 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>>>       return PSCI_RET_SUCCESS;
>>>  }
>>>
>>> +static inline void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
>>
>> Loose the "inline". This is not performance critical, and the compiler
>> does a pretty good job doing that for you.
>
> OK, I will drop the "inline" attribute.
>
>>
>>> +{
>>> +     memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
>>> +     vcpu->run->system_event.type = type;
>>> +     vcpu->run->exit_reason = KVM_EXIT_SYSTEM_EVENT;
>>> +}
>>> +
>>> +static void kvm_psci_system_off(struct kvm_vcpu *vcpu)
>>> +{
>>> +     kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_SHUTDOWN);
>>> +}
>>> +
>>> +static void kvm_psci_system_reset(struct kvm_vcpu *vcpu)
>>> +{
>>> +     kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_RESET);
>>> +}
>>> +
>>>  int kvm_psci_version(struct kvm_vcpu *vcpu)
>>>  {
>>>       if (test_bit(KVM_ARM_VCPU_PSCI_0_2, vcpu->arch.features))
>>> @@ -95,6 +112,7 @@ int kvm_psci_version(struct kvm_vcpu *vcpu)
>>>
>>>  static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>>  {
>>> +     int ret = 1;
>>>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>>       unsigned long val;
>>>
>>> @@ -114,13 +132,21 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>>       case PSCI_0_2_FN64_CPU_ON:
>>>               val = kvm_psci_vcpu_on(vcpu);
>>>               break;
>>> +     case PSCI_0_2_FN_SYSTEM_OFF:
>>> +             kvm_psci_system_off(vcpu);
>>> +             val = PSCI_RET_SUCCESS;
>>> +             ret = 0;
>>> +             break;
>>> +     case PSCI_0_2_FN_SYSTEM_RESET:
>>> +             kvm_psci_system_reset(vcpu);
>>> +             val = PSCI_RET_SUCCESS;
>>> +             ret = 0;
>>> +             break;
>>
>> What is the significance of setting val to PSCI_RET_SUCCESS here? We're
>> exiting to userspace, so surely only the platform emulation can set
>> that. Am I missing something?
>
> Actually, return value is undefined for SYSTEM_OFF and SYSTEM_RESET
> because these functions are not expected to return. Currently, we are updating
> r0 (or x0) for all PSCI functions.
>
> Are you suggesting that we update r0 (or x0) only when there are no error
> (i.e. ret == 0) ?

What I'm suggesting is that the only valid return value should be
indicative of a failure. If you're coming back to the guest after a
SYSTEM_OFF or a SYSTEM_RESET, it means you've failed to perform the
operation.

So userspace either should report the failure (by putting it in x0), and
there is no need to pre-load x0 with SUCCESS, or you start by storing
FAILURE in x0, before exiting to userspace.

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

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

* [PATCH v9 05/12] ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible
  2014-04-15 12:23       ` Marc Zyngier
@ 2014-04-15 13:07         ` Peter Maydell
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2014-04-15 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

On 15 April 2014 13:23, Marc Zyngier <marc.zyngier@arm.com> wrote:
> I understand that, and this is the only case at the moment. What I'm
> foreseeing is a situation where we've been unable to perform the
> expected service, and PSCI doesn't specify any "internal error". So an
> error injection looks like a valid solution to me.

This sounds like a flaw in the PSCI spec which we should
try to get fixed...

thanks
-- PMM

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

end of thread, other threads:[~2014-04-15 13:07 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-15  6:14 [PATCH v9 00/12] In-kernel PSCI v0.2 emulation for KVM ARM/ARM64 Anup Patel
2014-04-15  6:14 ` [PATCH v9 01/12] KVM: Add capability to advertise PSCI v0.2 support Anup Patel
2014-04-15  9:49   ` Marc Zyngier
2014-04-15  6:14 ` [PATCH v9 02/12] ARM/ARM64: KVM: Add common header for PSCI related defines Anup Patel
2014-04-15 10:06   ` Marc Zyngier
2014-04-15 11:10     ` Anup Patel
2014-04-15 12:18       ` Marc Zyngier
2014-04-15  6:14 ` [PATCH v9 03/12] ARM/ARM64: KVM: Add base for PSCI v0.2 emulation Anup Patel
2014-04-15 10:19   ` Marc Zyngier
2014-04-15  6:14 ` [PATCH v9 04/12] KVM: Documentation: Add info regarding KVM_ARM_VCPU_PSCI_0_2 feature Anup Patel
2014-04-15 10:20   ` Marc Zyngier
2014-04-15  6:14 ` [PATCH v9 05/12] ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible Anup Patel
2014-04-15 10:21   ` Marc Zyngier
2014-04-15 11:13     ` Anup Patel
2014-04-15 12:23       ` Marc Zyngier
2014-04-15 13:07         ` Peter Maydell
2014-04-15  6:14 ` [PATCH v9 06/12] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header Anup Patel
2014-04-15 10:25   ` Marc Zyngier
2014-04-15  6:14 ` [PATCH v9 07/12] ARM/ARM64: KVM: Emulate PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET Anup Patel
2014-04-15 10:34   ` Marc Zyngier
2014-04-15 11:26     ` Anup Patel
2014-04-15 12:28       ` Marc Zyngier
2014-04-15  6:14 ` [PATCH v9 08/12] ARM/ARM64: KVM: Emulate PSCI v0.2 AFFINITY_INFO Anup Patel
2014-04-15 10:55   ` Marc Zyngier
2014-04-15 11:15     ` Anup Patel
2014-04-15  6:14 ` [PATCH v9 09/12] ARM/ARM64: KVM: Emulate PSCI v0.2 MIGRATE_INFO_TYPE and related functions Anup Patel
2014-04-15 12:05   ` Marc Zyngier
2014-04-15  6:14 ` [PATCH v9 10/12] ARM/ARM64: KVM: Fix CPU_ON emulation for PSCI v0.2 Anup Patel
2014-04-15 12:10   ` Marc Zyngier
2014-04-15  6:14 ` [PATCH v9 11/12] ARM/ARM64: KVM: Emulate PSCI v0.2 CPU_SUSPEND Anup Patel
2014-04-15 12:14   ` Marc Zyngier
2014-04-15  6:14 ` [PATCH v9 12/12] ARM/ARM64: KVM: Advertise KVM_CAP_ARM_PSCI_0_2 to user space Anup Patel
2014-04-15 12:16   ` Marc Zyngier

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.