Skip to contentSkip to conversation

arm64: KVM: Add Kconfig option for max VCPUs per-Guest

PATCH RESEND New Anup Patel 4 participants 16 messages Dec 12, 2013 – Dec 14, 2013

Subsystems (5)
Documentation Arm (32-bit) KVM arm64 +1 more subsystems
lore
Share & tools
lore

Anup Patel

author
#0 lore raw

PATCH RESEND 1/1 arm64: KVM: Add Kconfig option for max VCPUs per-Guest

Current max VCPUs per-Guest is set to 4 which is preventing us from creating a Guest (or VM) with 8 VCPUs on Host (e.g. X-Gene Storm SOC) with 8 Host CPUs.

The correct value of max VCPUs per-Guest should be same as the max CPUs supported by GICv2 which is 8 but, increasing value of max VCPUs per-Guest can make things slower hence we add Kconfig option to let KVM users select appropriate max VCPUs per-Guest.

Signed-off-by Anup Patel <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h |    7 ++++++-
 arch/arm64/kvm/Kconfig            |   11 +++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
arch/arm64/include/asm/kvm_host.h +6 -1
index 5d85a02..0a1d697 100644
@@ -26,7 +26,12 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_mmio.h>
 
-#define KVM_MAX_VCPUS 4
+#if defined(CONFIG_KVM_ARM_MAX_VCPUS)
+#define KVM_MAX_VCPUS CONFIG_KVM_ARM_MAX_VCPUS
+#else
+#define KVM_MAX_VCPUS 0
+#endif
+
 #define KVM_USER_MEM_SLOTS 32
 #define KVM_PRIVATE_MEM_SLOTS 4
 #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
arch/arm64/kvm/Kconfig +11 -0
index 4480ab3..8ba85e9 100644
@@ -36,6 +36,17 @@ config KVM_ARM_HOST
 	---help---
 	  Provides host support for ARM processors.
 
+config KVM_ARM_MAX_VCPUS
+	int "Number maximum supported virtual CPUs per VM"
+	depends on KVM_ARM_HOST
+	default 4
+	help
+	  Static number of max supported virtual CPUs per VM.
+
+	  If you choose a high number, the vcpu structures will be quite
+	  large, so only choose a reasonable number that you expect to
+	  actually use.
+
 config KVM_ARM_VGIC
 	bool
 	depends on KVM_ARM_HOST && OF
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH 1/1 arm64: KVM: Force undefined exception for Guest SMC intructions

The SMC-based PSCI emulation for Guest is going to be very different from the in-kernel HVC-based PSCI emulation hence for now just inject undefined exception when Guest executes SMC instruction.

Signed-off-by Anup Patel <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 arch/arm64/kvm/handle_exit.c |    3 ---
 1 file changed, 3 deletions(-)
arch/arm64/kvm/handle_exit.c +0 -3
index 8da5606..df84d7b 100644
@@ -39,9 +39,6 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
-	if (kvm_psci_call(vcpu))
-		return 1;
-
 	kvm_inject_undefined(vcpu);
 	return 1;
 }
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 1/1 arm64: KVM: Force undefined exception for Guest SMC intructions

Quoted 6 lines · Anup Patel

The SMC-based PSCI emulation for Guest is going to be very different from the in-kernel HVC-based PSCI emulation hence for now just inject undefined exception when Guest executes SMC instruction.

Signed-off-by Anup Patel <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
Acked-by Marc Zyngier <[email protected]>
	M.
Quoted 19 lines
---
 arch/arm64/kvm/handle_exit.c |    3 ---
 1 file changed, 3 deletions(-)
arch/arm64/kvm/handle_exit.c +0 -3
index 8da5606..df84d7b 100644
@@ -39,9 +39,6 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
-	if (kvm_psci_call(vcpu))
-		return 1;
-
 	kvm_inject_undefined(vcpu);
 	return 1;
 }
Signature · 1 line
Jazz is not dead. It just smells funny...
#3 lore raw

PATCH 1/1 arm64: KVM: Force undefined exception for Guest SMC intructions

In reply to Anup Patel
Quoted 3 lines · Anup Patel

The SMC-based PSCI emulation for Guest is going to be very different from the in-kernel HVC-based PSCI emulation hence for now just inject undefined exception when Guest executes SMC instruction.

Acked-by Christoffer Dall <[email protected]>
Quoted 24 lines
Signed-off-by Anup Patel <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 arch/arm64/kvm/handle_exit.c |    3 ---
 1 file changed, 3 deletions(-)
arch/arm64/kvm/handle_exit.c +0 -3
index 8da5606..df84d7b 100644
@@ -39,9 +39,6 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
-	if (kvm_psci_call(vcpu))
-		return 1;
-
 	kvm_inject_undefined(vcpu);
 	return 1;
 }
Signature · 1 line
1.7.9.5
Signature · 1 line
Christoffer
author
#4 lore raw

PATCH 1/1 KVM: Documentation: Fix typo for KVM_ARM_VCPU_INIT ioctl

In reply to Anup Patel

Fix minor typo in "Parameters:" of KVM_ARM_VCPU_INIT documentation.

Signed-off-by Anup Patel <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 Documentation/virtual/kvm/api.txt |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/virtual/kvm/api.txt +1 -1
index a30035d..aad3244 100644
@@ -2327,7 +2327,7 @@ current state.  "addr" is ignored.
 Capability: basic
 Architectures: arm, arm64
 Type: vcpu ioctl
-Parameters: struct struct kvm_vcpu_init (in)
+Parameters: struct kvm_vcpu_init (in)
 Returns: 0 on success; -1 on error
 Errors:
  ?EINVAL: ???the target is unknown, or the combination of features is invalid.
Signature · 1 line
1.7.9.5
#5 lore raw

PATCH 1/1 KVM: Documentation: Fix typo for KVM_ARM_VCPU_INIT ioctl

Quoted 1 line · Anup Patel

Fix minor typo in "Parameters:" of KVM_ARM_VCPU_INIT documentation.

Acked-by Christoffer Dall <[email protected]>
Quoted 23 lines
Signed-off-by Anup Patel <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 Documentation/virtual/kvm/api.txt |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/virtual/kvm/api.txt +1 -1
index a30035d..aad3244 100644
@@ -2327,7 +2327,7 @@ current state.  "addr" is ignored.
 Capability: basic
 Architectures: arm, arm64
 Type: vcpu ioctl
-Parameters: struct struct kvm_vcpu_init (in)
+Parameters: struct kvm_vcpu_init (in)
 Returns: 0 on success; -1 on error
 Errors:
  ?EINVAL: ???the target is unknown, or the combination of features is invalid.
Signature · 1 line
1.7.9.5
Signature · 1 line
Christoffer
author
#6 lore raw

v2 cover PSCI system off and reset for KVM ARM/ARM64

In reply to Anup Patel

The Power State and Coordination Interface (PSCI) specification defines SYSTEM_OFF and SYSTEM_RESET functions for system poweroff and reboot.

This patchset adds emulation of PSCI SYSTEM_OFF and SYSTEM_RESET functions in KVM ARM/ARM64 by forwarding them to user space (QEMU or KVMTOOL) using KVM_EXIT_SYSTEM_EVENT exit reason.

To try this patch from guest kernel, we will need PSCI-based restart and poweroff support in the guest kenel for both ARM and ARM64.

Rob Herring has already submitted patches for PSCI-based restart and poweroff in ARM kernel but these are not merged yet due unstable device tree bindings of kernel PSCI support. We will be having similar patches for PSCI-based restart and poweroff in ARM64 kernel.
(Refer http://www.spinics.net/lists/arm-kernel/msg262217.html)
(Refer http://www.spinics.net/lists/devicetree/msg05348.html)

Change Log:

V2:
- Replace KVM_EXIT_RESET with KVM_EXIT_SYSTEM_EVENT
- Make kvm_psci_call() return convention to match KVM ARM return convention

V1:
- Initial revised patch after RFC PATCH.

Anup Patel (2):
KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header ARM/ARM64: KVM: Forward PSCI SYSTEM_OFF and SYSTEM_RESET to user
space

 Documentation/virtual/kvm/api.txt |   15 ++++++++++++
 arch/arm/include/asm/kvm_psci.h   |    2 +-
 arch/arm/include/uapi/asm/kvm.h   |    2 ++
 arch/arm/kvm/handle_exit.c        |   13 +++++++---
 arch/arm/kvm/psci.c               |   48 ++++++++++++++++++++++++++++++-------
 arch/arm64/include/asm/kvm_psci.h |    2 +-
 arch/arm64/include/uapi/asm/kvm.h |    2 ++
 arch/arm64/kvm/handle_exit.c      |   12 +++++++---
 include/uapi/linux/kvm.h          |    8 +++++++
 9 files changed, 88 insertions(+), 16 deletions(-)
Signature · 1 line
1.7.9.5
#7 lore raw

v2 cover PSCI system off and reset for KVM ARM/ARM64

Quoted 10 lines · Anup Patel

The Power State and Coordination Interface (PSCI) specification defines SYSTEM_OFF and SYSTEM_RESET functions for system poweroff and reboot.

This patchset adds emulation of PSCI SYSTEM_OFF and SYSTEM_RESET functions in KVM ARM/ARM64 by forwarding them to user space (QEMU or KVMTOOL) using KVM_EXIT_SYSTEM_EVENT exit reason.

To try this patch from guest kernel, we will need PSCI-based restart and poweroff support in the guest kenel for both ARM and ARM64.

Quoted 6 lines

Rob Herring has already submitted patches for PSCI-based restart and poweroff in ARM kernel but these are not merged yet due unstable device tree bindings of kernel PSCI support. We will be having similar patches for PSCI-based restart and poweroff in ARM64 kernel.
(Refer http://www.spinics.net/lists/arm-kernel/msg262217.html)
(Refer http://www.spinics.net/lists/devicetree/msg05348.html)

There's something really funky about the threading and reply-to settings of all of these patches, please check your e-mail sending tool config.

Quoted 30 lines

Change Log:

V2:
- Replace KVM_EXIT_RESET with KVM_EXIT_SYSTEM_EVENT
- Make kvm_psci_call() return convention to match KVM ARM return convention

V1:
- Initial revised patch after RFC PATCH.

Anup Patel (2):
KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header ARM/ARM64: KVM: Forward PSCI SYSTEM_OFF and SYSTEM_RESET to user
space

 Documentation/virtual/kvm/api.txt |   15 ++++++++++++
 arch/arm/include/asm/kvm_psci.h   |    2 +-
 arch/arm/include/uapi/asm/kvm.h   |    2 ++
 arch/arm/kvm/handle_exit.c        |   13 +++++++---
 arch/arm/kvm/psci.c               |   48 ++++++++++++++++++++++++++++++-------
 arch/arm64/include/asm/kvm_psci.h |    2 +-
 arch/arm64/include/uapi/asm/kvm.h |    2 ++
 arch/arm64/kvm/handle_exit.c      |   12 +++++++---
 include/uapi/linux/kvm.h          |    8 +++++++
 9 files changed, 88 insertions(+), 16 deletions(-)
Signature · 1 line
1.7.9.5
Signature · 1 line
Christoffer
author
#8 lore raw

PATCH v2 1/2 KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header

In reply to Anup Patel

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 support to reset/shutdown VMs.

Signed-off-by Anup Patel <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 Documentation/virtual/kvm/api.txt |   15 +++++++++++++++
 include/uapi/linux/kvm.h          |    8 ++++++++
 2 files changed, 23 insertions(+)
Documentation/virtual/kvm/api.txt +15 -0
index aad3244..4ef180e 100644
@@ -2699,6 +2699,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];
 	};
include/uapi/linux/kvm.h +8 -0
index 902f124..bb13faa 100644
@@ -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];
 	};
Signature · 1 line
1.7.9.5
#9 lore raw

PATCH v2 1/2 KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header

Quoted 72 lines · Anup Patel

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 support to reset/shutdown VMs.

Signed-off-by Anup Patel <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 Documentation/virtual/kvm/api.txt |   15 +++++++++++++++
 include/uapi/linux/kvm.h          |    8 ++++++++
 2 files changed, 23 insertions(+)
Documentation/virtual/kvm/api.txt +15 -0
index aad3244..4ef180e 100644
@@ -2699,6 +2699,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];
 	};
include/uapi/linux/kvm.h +8 -0
index 902f124..bb13faa 100644
@@ -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];
 	};
Signature · 1 line
1.7.9.5

Looks good to me:

Reviewed-by Christoffer Dall <[email protected]>
author
#10 lore raw

PATCH v2 2/2 ARM/ARM64: KVM: Forward PSCI SYSTEM_OFF and SYSTEM_RESET to user space

In reply to Anup Patel

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

To tackle this, we forward PSCI 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 <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 arch/arm/include/asm/kvm_psci.h   |    2 +-
 arch/arm/include/uapi/asm/kvm.h   |    2 ++
 arch/arm/kvm/handle_exit.c        |   13 +++++++---
 arch/arm/kvm/psci.c               |   48 ++++++++++++++++++++++++++++++-------
 arch/arm64/include/asm/kvm_psci.h |    2 +-
 arch/arm64/include/uapi/asm/kvm.h |    2 ++
 arch/arm64/kvm/handle_exit.c      |   12 +++++++---
 7 files changed, 65 insertions(+), 16 deletions(-)
arch/arm/include/asm/kvm_psci.h +1 -1
index 9a83d98..992d7f1 100644
@@ -18,6 +18,6 @@
 #ifndef __ARM_KVM_PSCI_H__
 #define __ARM_KVM_PSCI_H__
 
-bool kvm_psci_call(struct kvm_vcpu *vcpu);
+int kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM_KVM_PSCI_H__ */
arch/arm/include/uapi/asm/kvm.h +2 -0
index c498b60..f4de20c 100644
@@ -172,6 +172,8 @@ struct kvm_arch_memory_slot {
 #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_SYSTEM_OFF		KVM_PSCI_FN(4)
+#define KVM_PSCI_FN_SYSTEM_RESET	KVM_PSCI_FN(5)
 
 #define KVM_PSCI_RET_SUCCESS		0
 #define KVM_PSCI_RET_NI			((unsigned long)-1)
arch/arm/kvm/handle_exit.c +10 -3
index a920790..c3f0e72 100644
@@ -40,14 +40,21 @@ 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 >= 0)
+		return ret;
+	else if (ret == -EINVAL) {
+		kvm_inject_undefined(vcpu);
 		return 1;
+	}
+
+	return ret;
 
-	kvm_inject_undefined(vcpu);
-	return 1;
 }
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
arch/arm/kvm/psci.c +40 -8

69 lines not on this page · raw message

arch/arm64/include/asm/kvm_psci.h +1 -1
index e301a48..9bd0ee8 100644
@@ -18,6 +18,6 @@
 #ifndef __ARM64_KVM_PSCI_H__
 #define __ARM64_KVM_PSCI_H__
 
-bool kvm_psci_call(struct kvm_vcpu *vcpu);
+int kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM64_KVM_PSCI_H__ */
arch/arm64/include/uapi/asm/kvm.h +2 -0
index d9f026b..f678902 100644
@@ -158,6 +158,8 @@ struct kvm_arch_memory_slot {
 #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_SYSTEM_OFF		KVM_PSCI_FN(4)
+#define KVM_PSCI_FN_SYSTEM_RESET	KVM_PSCI_FN(5)
 
 #define KVM_PSCI_RET_SUCCESS		0
 #define KVM_PSCI_RET_NI			((unsigned long)-1)
arch/arm64/kvm/handle_exit.c +9 -3
index df84d7b..e382eb8 100644
@@ -30,11 +30,17 @@ 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 >= 0)
+		return ret;
+	else 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)
Signature · 1 line
1.7.9.5
#11 lore raw

PATCH v2 2/2 ARM/ARM64: KVM: Forward PSCI SYSTEM_OFF and SYSTEM_RESET to user space

Quoted 58 lines · Anup Patel

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

To tackle this, we forward PSCI 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 <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 arch/arm/include/asm/kvm_psci.h   |    2 +-
 arch/arm/include/uapi/asm/kvm.h   |    2 ++
 arch/arm/kvm/handle_exit.c        |   13 +++++++---
 arch/arm/kvm/psci.c               |   48 ++++++++++++++++++++++++++++++-------
 arch/arm64/include/asm/kvm_psci.h |    2 +-
 arch/arm64/include/uapi/asm/kvm.h |    2 ++
 arch/arm64/kvm/handle_exit.c      |   12 +++++++---
 7 files changed, 65 insertions(+), 16 deletions(-)
arch/arm/include/asm/kvm_psci.h +1 -1
index 9a83d98..992d7f1 100644
@@ -18,6 +18,6 @@
 #ifndef __ARM_KVM_PSCI_H__
 #define __ARM_KVM_PSCI_H__
 
-bool kvm_psci_call(struct kvm_vcpu *vcpu);
+int kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM_KVM_PSCI_H__ */
arch/arm/include/uapi/asm/kvm.h +2 -0
index c498b60..f4de20c 100644
@@ -172,6 +172,8 @@ struct kvm_arch_memory_slot {
 #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_SYSTEM_OFF		KVM_PSCI_FN(4)
+#define KVM_PSCI_FN_SYSTEM_RESET	KVM_PSCI_FN(5)
 
 #define KVM_PSCI_RET_SUCCESS		0
 #define KVM_PSCI_RET_NI			((unsigned long)-1)
arch/arm/kvm/handle_exit.c +2 -0
index a920790..c3f0e72 100644
@@ -40,14 +40,21 @@ 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));

arch/arm/kvm/handle_exit.c static int handle_svc_hyp(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 56

Quoted 10 lines
arch/arm/kvm/handle_exit.c +8 -1
-	if (kvm_psci_call(vcpu))
+	ret = kvm_psci_call(vcpu);
+	if (ret >= 0)
+		return ret;
+	else if (ret == -EINVAL) {
+		kvm_inject_undefined(vcpu);
 		return 1;
+	}
+
+	return ret;

Wouldn't

ret = kvm_psci_call(vcpu);

if (ret == -EINVAL) {
	kvm_inject_undefined(vcpu);
	return 1;
}

return ret;

be simpler?

also, minor nit, but I think according to the CodingStyle if you have single statements in if cases, but they are part of a larger if statement with alternative clauses that use curly-brackets to encapsulate multiple statements, then you should use curly braces around the single-line statement as well...

Quoted 32 lines
arch/arm/kvm/handle_exit.c +0 -2
-	kvm_inject_undefined(vcpu);
-	return 1;
 }
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
arch/arm/kvm/psci.c +17 -0
index 0881bf1..8e246d6 100644
@@ -84,18 +84,41 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 	return KVM_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);
+}
+
 /**

arch/arm/kvm/psci.c static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) · line 110

Quoted 10 lines
arch/arm/kvm/psci.c +2 -4
  * 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.
+ * The calling convention is similar to SMC calls to the secure world
+ * where the function number is placed in r0 and function number

the function number

arch/arm/kvm/psci.c static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) · line 111

Quoted 1 line
arch/arm/kvm/psci.c +1 -0
+ * specified in r0 is withing the PSCI range.

this reads a little funny now. I think you can get rid of everything after the first mention of r0 (cut from "and function number...") since you explain that in the error codes below.

arch/arm/kvm/psci.c static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) · line 114

Quoted 3 lines
arch/arm/kvm/psci.c +3 -0
+ *
+ * This function returns: > 0 (success), 0 (success but exit to user
+ * space), and < 0 (errors)

you could use @return: instead of "This function returns", but I'm not sure if anyone actually cares.

Quoted 72 lines
arch/arm/kvm/psci.c +17 -4
+ *
+ * Errors:
+ * -EINVAL: Unrecognized PSCI function
  */
-bool kvm_psci_call(struct kvm_vcpu *vcpu)
+int kvm_psci_call(struct kvm_vcpu *vcpu)
 {
+	int ret = 1;
 	unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
 	unsigned long val;
 

@@ -111,11 +134,20 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
 	case KVM_PSCI_FN_MIGRATE:
 		val = KVM_PSCI_RET_NI;
 		break;
-
+	case KVM_PSCI_FN_SYSTEM_OFF:
+		kvm_psci_system_off(vcpu);
+		val = KVM_PSCI_RET_SUCCESS;
+		ret = 0;
+		break;
+	case KVM_PSCI_FN_SYSTEM_RESET:
+		kvm_psci_system_reset(vcpu);
+		val = KVM_PSCI_RET_SUCCESS;
+		ret = 0;
+		break;
 	default:
-		return false;
+		return -EINVAL;
 	}
 
 	*vcpu_reg(vcpu, 0) = val;
-	return true;
+	return ret;
 }
arch/arm64/include/asm/kvm_psci.h +1 -1
index e301a48..9bd0ee8 100644
@@ -18,6 +18,6 @@
 #ifndef __ARM64_KVM_PSCI_H__
 #define __ARM64_KVM_PSCI_H__
 
-bool kvm_psci_call(struct kvm_vcpu *vcpu);
+int kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM64_KVM_PSCI_H__ */
arch/arm64/include/uapi/asm/kvm.h +2 -0
index d9f026b..f678902 100644
@@ -158,6 +158,8 @@ struct kvm_arch_memory_slot {
 #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_SYSTEM_OFF		KVM_PSCI_FN(4)
+#define KVM_PSCI_FN_SYSTEM_RESET	KVM_PSCI_FN(5)
 
 #define KVM_PSCI_RET_SUCCESS		0
 #define KVM_PSCI_RET_NI			((unsigned long)-1)
arch/arm64/kvm/handle_exit.c +3 -1
index df84d7b..e382eb8 100644
@@ -30,11 +30,17 @@ 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);

arch/arm64/kvm/handle_exit.c typedef int (*exit_handle_fn)(struct kvm_vcpu *, struct kvm_run *); · line 43

Quoted 10 lines
arch/arm64/kvm/handle_exit.c +6 -2
+	if (ret >= 0)
+		return ret;
+	else if (ret == -EINVAL) {
+		kvm_inject_undefined(vcpu);
 		return 1;
+	}
 
-	kvm_inject_undefined(vcpu);
-	return 1;
+	return ret;

same as above.

Quoted 6 lines
arch/arm64/kvm/handle_exit.c +0 -0
 }
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
Signature · 1 line
1.7.9.5

Aside from the code formatting and commenting above, it looks pretty good to me.

-Christoffer

#12 lore raw

PATCH v2 2/2 ARM/ARM64: KVM: Forward PSCI SYSTEM_OFF and SYSTEM_RESET to user space

Quoted 73 lines · Christoffer Dall
Quoted 58 lines · Anup Patel

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

To tackle this, we forward PSCI 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 <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 arch/arm/include/asm/kvm_psci.h   |    2 +-
 arch/arm/include/uapi/asm/kvm.h   |    2 ++
 arch/arm/kvm/handle_exit.c        |   13 +++++++---
 arch/arm/kvm/psci.c               |   48 ++++++++++++++++++++++++++++++-------
 arch/arm64/include/asm/kvm_psci.h |    2 +-
 arch/arm64/include/uapi/asm/kvm.h |    2 ++
 arch/arm64/kvm/handle_exit.c      |   12 +++++++---
 7 files changed, 65 insertions(+), 16 deletions(-)
arch/arm/include/asm/kvm_psci.h +1 -1
index 9a83d98..992d7f1 100644
@@ -18,6 +18,6 @@
 #ifndef __ARM_KVM_PSCI_H__
 #define __ARM_KVM_PSCI_H__
 
-bool kvm_psci_call(struct kvm_vcpu *vcpu);
+int kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM_KVM_PSCI_H__ */
arch/arm/include/uapi/asm/kvm.h +2 -0
index c498b60..f4de20c 100644
@@ -172,6 +172,8 @@ struct kvm_arch_memory_slot {
 #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_SYSTEM_OFF               KVM_PSCI_FN(4)
+#define KVM_PSCI_FN_SYSTEM_RESET     KVM_PSCI_FN(5)
 
 #define KVM_PSCI_RET_SUCCESS         0
 #define KVM_PSCI_RET_NI                      ((unsigned long)-1)
arch/arm/kvm/handle_exit.c +2 -0
index a920790..c3f0e72 100644
@@ -40,14 +40,21 @@ 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));

arch/arm/kvm/handle_exit.c static int handle_svc_hyp(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 56

Quoted 10 lines
arch/arm/kvm/handle_exit.c +8 -1
-     if (kvm_psci_call(vcpu))
+     ret = kvm_psci_call(vcpu);
+     if (ret >= 0)
+             return ret;
+     else if (ret == -EINVAL) {
+             kvm_inject_undefined(vcpu);
              return 1;
+     }
+
+     return ret;

Wouldn't

ret = kvm_psci_call(vcpu);

Quoted 8 lines
if (ret == -EINVAL) {
        kvm_inject_undefined(vcpu);
        return 1;
}

return ret;

be simpler?

Yes, I will make this simpler.

Quoted 52 lines

also, minor nit, but I think according to the CodingStyle if you have single statements in if cases, but they are part of a larger if statement with alternative clauses that use curly-brackets to encapsulate multiple statements, then you should use curly braces around the single-line statement as well...

Quoted 32 lines
arch/arm/kvm/handle_exit.c +0 -2
-     kvm_inject_undefined(vcpu);
-     return 1;
 }
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
arch/arm/kvm/psci.c +17 -0
index 0881bf1..8e246d6 100644
@@ -84,18 +84,41 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
      return KVM_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);
+}
+
 /**

arch/arm/kvm/psci.c static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) · line 110

Quoted 10 lines
arch/arm/kvm/psci.c +2 -4
  * 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.
+ * The calling convention is similar to SMC calls to the secure world
+ * where the function number is placed in r0 and function number

the function number

Quoted 5 lines

arch/arm/kvm/psci.c static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) · line 111

Quoted 1 line
arch/arm/kvm/psci.c +1 -0
+ * specified in r0 is withing the PSCI range.

this reads a little funny now. I think you can get rid of everything after the first mention of r0 (cut from "and function number...") since you explain that in the error codes below.

Sure, I will update this comment.

Quoted 7 lines

arch/arm/kvm/psci.c static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) · line 114

Quoted 4 lines
arch/arm/kvm/psci.c +3 -0
+ *
+ * This function returns: > 0 (success), 0 (success but exit to user
+ * space), and < 0 (errors)

you could use @return: instead of "This function returns", but I'm not sure if anyone actually cares.

Actually, "@returns" is correct way to specify as-per doxygen comment style for C but I have seen "Returns:" or "Returns" being used commonly in other parts of kernel. I will replace "The function returns:" with "Returns:".

Quoted 80 lines
Quoted 80 lines
arch/arm/kvm/psci.c +17 -4
+ *
+ * Errors:
+ * -EINVAL: Unrecognized PSCI function
  */
-bool kvm_psci_call(struct kvm_vcpu *vcpu)
+int kvm_psci_call(struct kvm_vcpu *vcpu)
 {
+     int ret = 1;
      unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
      unsigned long val;
 

@@ -111,11 +134,20 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
      case KVM_PSCI_FN_MIGRATE:
              val = KVM_PSCI_RET_NI;
              break;
-
+     case KVM_PSCI_FN_SYSTEM_OFF:
+             kvm_psci_system_off(vcpu);
+             val = KVM_PSCI_RET_SUCCESS;
+             ret = 0;
+             break;
+     case KVM_PSCI_FN_SYSTEM_RESET:
+             kvm_psci_system_reset(vcpu);
+             val = KVM_PSCI_RET_SUCCESS;
+             ret = 0;
+             break;
      default:
-             return false;
+             return -EINVAL;
      }
 
      *vcpu_reg(vcpu, 0) = val;
-     return true;
+     return ret;
 }
arch/arm64/include/asm/kvm_psci.h +1 -1
index e301a48..9bd0ee8 100644
@@ -18,6 +18,6 @@
 #ifndef __ARM64_KVM_PSCI_H__
 #define __ARM64_KVM_PSCI_H__
 
-bool kvm_psci_call(struct kvm_vcpu *vcpu);
+int kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM64_KVM_PSCI_H__ */
arch/arm64/include/uapi/asm/kvm.h +2 -0
index d9f026b..f678902 100644
@@ -158,6 +158,8 @@ struct kvm_arch_memory_slot {
 #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_SYSTEM_OFF               KVM_PSCI_FN(4)
+#define KVM_PSCI_FN_SYSTEM_RESET     KVM_PSCI_FN(5)
 
 #define KVM_PSCI_RET_SUCCESS         0
 #define KVM_PSCI_RET_NI                      ((unsigned long)-1)
arch/arm64/kvm/handle_exit.c +8 -1
index df84d7b..e382eb8 100644
@@ -30,11 +30,17 @@ 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 >= 0)
+             return ret;
+     else if (ret == -EINVAL) {
+             kvm_inject_undefined(vcpu);
              return 1;
+     }
Quoted 5 lines

arch/arm64/kvm/handle_exit.c typedef int (*exit_handle_fn)(struct kvm_vcpu *, struct kvm_run *); · line 43

Quoted 3 lines
arch/arm64/kvm/handle_exit.c +1 -2
-     kvm_inject_undefined(vcpu);
-     return 1;
+     return ret;

same as above.

OK.

Quoted 16 lines
Quoted 7 lines
arch/arm64/kvm/handle_exit.c +0 -1
 }
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
--

1.7.9.5

Aside from the code formatting and commenting above, it looks pretty good to me.

-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm at lists.cs.columbia.edu
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

#13 lore raw

PATCH v2 2/2 ARM/ARM64: KVM: Forward PSCI SYSTEM_OFF and SYSTEM_RESET to user space

Quoted 151 lines · Anup Patel
Quoted 73 lines · Christoffer Dall
Quoted 58 lines · Anup Patel

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

To tackle this, we forward PSCI 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 <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 arch/arm/include/asm/kvm_psci.h   |    2 +-
 arch/arm/include/uapi/asm/kvm.h   |    2 ++
 arch/arm/kvm/handle_exit.c        |   13 +++++++---
 arch/arm/kvm/psci.c               |   48 ++++++++++++++++++++++++++++++-------
 arch/arm64/include/asm/kvm_psci.h |    2 +-
 arch/arm64/include/uapi/asm/kvm.h |    2 ++
 arch/arm64/kvm/handle_exit.c      |   12 +++++++---
 7 files changed, 65 insertions(+), 16 deletions(-)
arch/arm/include/asm/kvm_psci.h +1 -1
index 9a83d98..992d7f1 100644
@@ -18,6 +18,6 @@
 #ifndef __ARM_KVM_PSCI_H__
 #define __ARM_KVM_PSCI_H__
 
-bool kvm_psci_call(struct kvm_vcpu *vcpu);
+int kvm_psci_call(struct kvm_vcpu *vcpu);
 
 #endif /* __ARM_KVM_PSCI_H__ */
arch/arm/include/uapi/asm/kvm.h +2 -0
index c498b60..f4de20c 100644
@@ -172,6 +172,8 @@ struct kvm_arch_memory_slot {
 #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_SYSTEM_OFF               KVM_PSCI_FN(4)
+#define KVM_PSCI_FN_SYSTEM_RESET     KVM_PSCI_FN(5)
 
 #define KVM_PSCI_RET_SUCCESS         0
 #define KVM_PSCI_RET_NI                      ((unsigned long)-1)
arch/arm/kvm/handle_exit.c +2 -0
index a920790..c3f0e72 100644
@@ -40,14 +40,21 @@ 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));

arch/arm/kvm/handle_exit.c static int handle_svc_hyp(struct kvm_vcpu *vcpu, struct kvm_run *run) · line 56

Quoted 10 lines
arch/arm/kvm/handle_exit.c +8 -1
-     if (kvm_psci_call(vcpu))
+     ret = kvm_psci_call(vcpu);
+     if (ret >= 0)
+             return ret;
+     else if (ret == -EINVAL) {
+             kvm_inject_undefined(vcpu);
              return 1;
+     }
+
+     return ret;

Wouldn't

ret = kvm_psci_call(vcpu);

Quoted 8 lines
if (ret == -EINVAL) {
        kvm_inject_undefined(vcpu);
        return 1;
}

return ret;

be simpler?

Yes, I will make this simpler.

Quoted 52 lines

also, minor nit, but I think according to the CodingStyle if you have single statements in if cases, but they are part of a larger if statement with alternative clauses that use curly-brackets to encapsulate multiple statements, then you should use curly braces around the single-line statement as well...

Quoted 32 lines
arch/arm/kvm/handle_exit.c +0 -2
-     kvm_inject_undefined(vcpu);
-     return 1;
 }
 
 static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
arch/arm/kvm/psci.c +17 -0
index 0881bf1..8e246d6 100644
@@ -84,18 +84,41 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
      return KVM_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);
+}
+
 /**

arch/arm/kvm/psci.c static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) · line 110

Quoted 10 lines
arch/arm/kvm/psci.c +2 -4
  * 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.
+ * The calling convention is similar to SMC calls to the secure world
+ * where the function number is placed in r0 and function number

the function number

Quoted 5 lines

arch/arm/kvm/psci.c static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu) · line 111

Quoted 1 line
arch/arm/kvm/psci.c +1 -0
+ * specified in r0 is withing the PSCI range.

this reads a little funny now. I think you can get rid of everything after the first mention of r0 (cut from "and function number...") since you explain that in the error codes below.

Sure, I will update this comment.

Quoted 5 lines
Quoted 4 lines
arch/arm/kvm/psci.c +3 -0
+ *
+ * This function returns: > 0 (success), 0 (success but exit to user
+ * space), and < 0 (errors)
Quoted 8 lines
Quoted 2 lines

you could use @return: instead of "This function returns", but I'm not sure if anyone actually cares.

Actually, "@returns" is correct way to specify as-per doxygen comment style for C but I have seen "Returns:" or "Returns" being used commonly in other parts of kernel. I will replace "The function returns:" with "Returns:".

Yes, but it doesn't seem that's what the kernel preferred style is following:

$ grep -sInR '@return' * | wc -l
583
$ grep -sInR '@returns' * | wc -l
82
$ grep -sInR '@Returns' * | wc -l
1
$ grep -sInR '@Return' * | wc -l
1

Anyway, it's not a big deal in any case.

[...]

-Christoffer

#14 lore raw

PATCH RESEND 1/1 arm64: KVM: Add Kconfig option for max VCPUs per-Guest

In reply to Anup Patel
Quoted 4 lines · Anup Patel

Current max VCPUs per-Guest is set to 4 which is preventing us from creating a Guest (or VM) with 8 VCPUs on Host (e.g. X-Gene Storm SOC) with 8 Host CPUs.

Quoted 8 lines

The correct value of max VCPUs per-Guest should be same as the max CPUs supported by GICv2 which is 8 but, increasing value of max VCPUs per-Guest can make things slower hence we add Kconfig option to let KVM users select appropriate max VCPUs per-Guest.

Signed-off-by Anup Patel <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
Acked-by Marc Zyngier <[email protected]>
	M.
Quoted 46 lines
---
 arch/arm64/include/asm/kvm_host.h |    7 ++++++-
 arch/arm64/kvm/Kconfig            |   11 +++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
arch/arm64/include/asm/kvm_host.h +6 -1
index 5d85a02..0a1d697 100644
@@ -26,7 +26,12 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_mmio.h>
 
-#define KVM_MAX_VCPUS 4
+#if defined(CONFIG_KVM_ARM_MAX_VCPUS)
+#define KVM_MAX_VCPUS CONFIG_KVM_ARM_MAX_VCPUS
+#else
+#define KVM_MAX_VCPUS 0
+#endif
+
 #define KVM_USER_MEM_SLOTS 32
 #define KVM_PRIVATE_MEM_SLOTS 4
 #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
arch/arm64/kvm/Kconfig +11 -0
index 4480ab3..8ba85e9 100644
@@ -36,6 +36,17 @@ config KVM_ARM_HOST
 	---help---
 	  Provides host support for ARM processors.
 
+config KVM_ARM_MAX_VCPUS
+	int "Number maximum supported virtual CPUs per VM"
+	depends on KVM_ARM_HOST
+	default 4
+	help
+	  Static number of max supported virtual CPUs per VM.
+
+	  If you choose a high number, the vcpu structures will be quite
+	  large, so only choose a reasonable number that you expect to
+	  actually use.
+
 config KVM_ARM_VGIC
 	bool
 	depends on KVM_ARM_HOST && OF
Signature · 1 line
Jazz is not dead. It just smells funny...
#15 lore raw

PATCH RESEND 1/1 arm64: KVM: Add Kconfig option for max VCPUs per-Guest

In reply to Anup Patel
Quoted 10 lines · Anup Patel

Current max VCPUs per-Guest is set to 4 which is preventing us from creating a Guest (or VM) with 8 VCPUs on Host (e.g. X-Gene Storm SOC) with 8 Host CPUs.

The correct value of max VCPUs per-Guest should be same as the max CPUs supported by GICv2 which is 8 but, increasing value of max VCPUs per-Guest can make things slower hence we add Kconfig option to let KVM users select appropriate max VCPUs per-Guest.

Acked-by Christoffer Dall <[email protected]>
Quoted 50 lines
Signed-off-by Anup Patel <[email protected]>
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h |    7 ++++++-
 arch/arm64/kvm/Kconfig            |   11 +++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
arch/arm64/include/asm/kvm_host.h +6 -1
index 5d85a02..0a1d697 100644
@@ -26,7 +26,12 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_mmio.h>
 
-#define KVM_MAX_VCPUS 4
+#if defined(CONFIG_KVM_ARM_MAX_VCPUS)
+#define KVM_MAX_VCPUS CONFIG_KVM_ARM_MAX_VCPUS
+#else
+#define KVM_MAX_VCPUS 0
+#endif
+
 #define KVM_USER_MEM_SLOTS 32
 #define KVM_PRIVATE_MEM_SLOTS 4
 #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
arch/arm64/kvm/Kconfig +11 -0
index 4480ab3..8ba85e9 100644
@@ -36,6 +36,17 @@ config KVM_ARM_HOST
 	---help---
 	  Provides host support for ARM processors.
 
+config KVM_ARM_MAX_VCPUS
+	int "Number maximum supported virtual CPUs per VM"
+	depends on KVM_ARM_HOST
+	default 4
+	help
+	  Static number of max supported virtual CPUs per VM.
+
+	  If you choose a high number, the vcpu structures will be quite
+	  large, so only choose a reasonable number that you expect to
+	  actually use.
+
 config KVM_ARM_VGIC
 	bool
 	depends on KVM_ARM_HOST && OF
Signature · 1 line
1.7.9.5
Signature · 1 line
Christoffer