Skip to contentSkip to conversation

PSCI system off and reset for KVM ARM/ARM64

PATCH New Anup Patel 8 participants 21 messages Dec 17, 2013 – Jan 10, 2014

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

Anup Patel

author
#0 lore raw

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

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:

V3:
- Simplify handle_hvc() for both ARM and ARM64
- Minor fix in comments for kvm_psci_call()

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        |   11 ++++++---
 arch/arm/kvm/psci.c               |   47 ++++++++++++++++++++++++++++++-------
 arch/arm64/include/asm/kvm_psci.h |    2 +-
 arch/arm64/include/uapi/asm/kvm.h |    2 ++
 arch/arm64/kvm/handle_exit.c      |   10 +++++---
 include/uapi/linux/kvm.h          |    8 +++++++
 9 files changed, 83 insertions(+), 16 deletions(-)
Signature · 1 line
1.7.9.5
author
#1 lore raw

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

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.

---
 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
author
#2 lore raw

PATCH v3 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.

---
 arch/arm/include/asm/kvm_psci.h   |    2 +-
 arch/arm/include/uapi/asm/kvm.h   |    2 ++
 arch/arm/kvm/handle_exit.c        |   11 ++++++---
 arch/arm/kvm/psci.c               |   47 ++++++++++++++++++++++++++++++-------
 arch/arm64/include/asm/kvm_psci.h |    2 +-
 arch/arm64/include/uapi/asm/kvm.h |    2 ++
 arch/arm64/kvm/handle_exit.c      |   10 +++++---
 7 files changed, 60 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 +8 -3
index a920790..ea5f18e 100644
@@ -40,14 +40,19 @@ 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;
+	}
+
+	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 +39 -8

68 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 +7 -3
index df84d7b..dbb6a11 100644
@@ -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)
Signature · 1 line
1.7.9.5
#3 lore raw

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

In reply to Anup Patel
Quoted 14 lines

Am 17.12.2013 um 12:35 schrieb Anup Patel <[email protected]>:

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)

Alex

#4 lore raw

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

In reply to Anup Patel
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)

I can merge this series if Marc acks it as well.

-Christoffer

#5 lore raw

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

Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

So I rekon we need to create a separate range for those. Also, I'd like to progress the DT and kernel side of things as well (otherwise this is all a bit pointless).

Rob: what are your plans regarding your PSCI v0.2 patches?

Thanks,

        M.
Signature · 1 line
Jazz is not dead. It just smells funny.
#6 lore raw

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

Quoted 24 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

Quoted 8 lines

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

Should we emulate PSCI_VERSION call to help Guest determine the spec version emulated by KVM (i.e. v0.1 or v0.2) ??

Quoted 9 lines

So I rekon we need to create a separate range for those. Also, I'd like to progress the DT and kernel side of things as well (otherwise this is all a bit pointless).

Rob: what are your plans regarding your PSCI v0.2 patches?

Thanks,

Quoted 7 lines
        M.

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

Regards,
Anup

#7 lore raw

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

Hi Anup,

Quoted 29 lines · Anup Patel
Quoted 28 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

Quoted 7 lines
Quoted 4 lines

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

Should we emulate PSCI_VERSION call to help Guest determine the spec version emulated by KVM (i.e. v0.1 or v0.2) ??

I think that'd be a nice to have, but the guest is likely to get its information from the DT anyway. Plus I don't think the original PSCI spec specified PSCI_VERSION, which only make it useful for whatever comes after v0.2.

So I think we need to:
- Use the new range for PSCI v0.2 (while still supporting v0.1 and the old range)
- Get the kernel and DT bindings into shape
- Merge all of that at the same time

Cheers,

        M.
Signature · 1 line
Jazz is not dead. It just smells funny.
#8 lore raw

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

Quoted 40 lines · Marc Zyngier

Hi Anup,

Quoted 29 lines · Anup Patel
Quoted 28 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

Quoted 7 lines
Quoted 4 lines

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

Should we emulate PSCI_VERSION call to help Guest determine the spec version emulated by KVM (i.e. v0.1 or v0.2) ??

Quoted 8 lines

I think that'd be a nice to have, but the guest is likely to get its information from the DT anyway. Plus I don't think the original PSCI spec specified PSCI_VERSION, which only make it useful for whatever comes after v0.2.

So I think we need to:
- Use the new range for PSCI v0.2 (while still supporting v0.1 and the old range)

Does this mean we should have first isolate v0.2 ID range from v0.1 ID range?

And then...

Rebase this patchset based on new v0.2 ID range?

Quoted 8 lines

- Get the kernel and DT bindings into shape
- Merge all of that at the same time

Cheers,

        M.

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

#9 lore raw

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

Quoted 46 lines · Anup Patel
Quoted 45 lines · Marc Zyngier

Hi Anup,

Quoted 29 lines · Anup Patel
Quoted 28 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

Quoted 7 lines
Quoted 4 lines

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

Should we emulate PSCI_VERSION call to help Guest determine the spec version emulated by KVM (i.e. v0.1 or v0.2) ??

I think that'd be a nice to have, but the guest is likely to get its information from the DT anyway. Plus I don't think the original PSCI spec specified PSCI_VERSION, which only make it useful for whatever comes after v0.2.

Quoted 6 lines
Quoted 3 lines

So I think we need to:
- Use the new range for PSCI v0.2 (while still supporting v0.1 and the old range)

Does this mean we should have first isolate v0.2 ID range from v0.1 ID range?

Yes.

Quoted 3 lines

And then...

Rebase this patchset based on new v0.2 ID range?

Indeed.

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny.
#10 lore raw

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

Quoted 47 lines · Marc Zyngier
Quoted 46 lines · Anup Patel
Quoted 45 lines · Marc Zyngier

Hi Anup,

Quoted 29 lines · Anup Patel
Quoted 28 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

Quoted 7 lines
Quoted 4 lines

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

Should we emulate PSCI_VERSION call to help Guest determine the spec version emulated by KVM (i.e. v0.1 or v0.2) ??

I think that'd be a nice to have, but the guest is likely to get its information from the DT anyway. Plus I don't think the original PSCI spec specified PSCI_VERSION, which only make it useful for whatever comes after v0.2.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

So I think we need to:
- Use the new range for PSCI v0.2 (while still supporting v0.1 and the old range)

Does this mean we should have first isolate v0.2 ID range from v0.1 ID range?

Yes.

Are you planning to do it ?
OR
Do you expect me to do it because this patchset would depend on that?

Quoted 5 lines
Quoted 4 lines

And then...

Rebase this patchset based on new v0.2 ID range?

Quoted 7 lines

Indeed.

Thanks,

        M.

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

Regards,
Anup

#11 lore raw

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

Quoted 52 lines · Anup Patel
Quoted 51 lines · Marc Zyngier
Quoted 50 lines · Anup Patel
Quoted 48 lines · Marc Zyngier

Hi Anup,

Quoted 29 lines · Anup Patel
Quoted 28 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

Quoted 7 lines
Quoted 4 lines

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

Should we emulate PSCI_VERSION call to help Guest determine the spec version emulated by KVM (i.e. v0.1 or v0.2) ??

I think that'd be a nice to have, but the guest is likely to get its information from the DT anyway. Plus I don't think the original PSCI spec specified PSCI_VERSION, which only make it useful for whatever comes after v0.2.

So I think we need to:
- Use the new range for PSCI v0.2 (while still supporting v0.1 and the old range)

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

Does this mean we should have first isolate v0.2 ID range from v0.1 ID range?

Yes.

Are you planning to do it ?
OR
Do you expect me to do it because this patchset would depend on that?

First, I want to understand the objections that were raised against using the Function IDs as defined in the spec.

Then, assuming we move in that direction, I'd expect you to create a separate range of IDs and update the PSCI code to handle both PSCI versions.

But as this has direct implication with userspace (DT generation), I'd rather take it slow and first try to understand the issues Mark raised.

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny.
#12 lore raw

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

Quoted 65 lines · Marc Zyngier
Quoted 52 lines · Anup Patel
Quoted 51 lines · Marc Zyngier
Quoted 50 lines · Anup Patel
Quoted 48 lines · Marc Zyngier

Hi Anup,

Quoted 29 lines · Anup Patel
Quoted 28 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

Quoted 7 lines
Quoted 4 lines

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

Should we emulate PSCI_VERSION call to help Guest determine the spec version emulated by KVM (i.e. v0.1 or v0.2) ??

I think that'd be a nice to have, but the guest is likely to get its information from the DT anyway. Plus I don't think the original PSCI spec specified PSCI_VERSION, which only make it useful for whatever comes after v0.2.

So I think we need to:
- Use the new range for PSCI v0.2 (while still supporting v0.1 and the old range)

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

Does this mean we should have first isolate v0.2 ID range from v0.1 ID range?

Yes.

Are you planning to do it ?
OR
Do you expect me to do it because this patchset would depend on that?

First, I want to understand the objections that were raised against using the Function IDs as defined in the spec.

Quoted 6 lines

Then, assuming we move in that direction, I'd expect you to create a separate range of IDs and update the PSCI code to handle both PSCI versions.

But as this has direct implication with userspace (DT generation), I'd rather take it slow and first try to understand the issues Mark raised.

Here is the thread where relying on SMC calling convention was discussed:

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/188057.html

Rob

#13 lore raw

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

Quoted 70 lines · Rob Herring
Quoted 69 lines · Marc Zyngier
Quoted 52 lines · Anup Patel
Quoted 51 lines · Marc Zyngier
Quoted 50 lines · Anup Patel
Quoted 48 lines · Marc Zyngier

Hi Anup,

Quoted 29 lines · Anup Patel
Quoted 28 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

Quoted 7 lines
Quoted 4 lines

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

Should we emulate PSCI_VERSION call to help Guest determine the spec version emulated by KVM (i.e. v0.1 or v0.2) ??

I think that'd be a nice to have, but the guest is likely to get its information from the DT anyway. Plus I don't think the original PSCI spec specified PSCI_VERSION, which only make it useful for whatever comes after v0.2.

So I think we need to:
- Use the new range for PSCI v0.2 (while still supporting v0.1 and the old range)

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

Does this mean we should have first isolate v0.2 ID range from v0.1 ID range?

Yes.

Are you planning to do it ?
OR
Do you expect me to do it because this patchset would depend on that?

First, I want to understand the objections that were raised against using the Function IDs as defined in the spec.

Then, assuming we move in that direction, I'd expect you to create a separate range of IDs and update the PSCI code to handle both PSCI versions.

Quoted 8 lines
Quoted 2 lines

But as this has direct implication with userspace (DT generation), I'd rather take it slow and first try to understand the issues Mark raised.

Here is the thread where relying on SMC calling convention was discussed:

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/188057.html

From reading that thread and the PSCI and SMC calling convention docs I

don't see anything that suggests that PSCI v0.2 should not follow the function IDs in the PSCI document.

Am I missing something?

-Christoffer

#14 lore raw

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

In reply to Marc Zyngier
Quoted 45 lines · Marc Zyngier

Hi Anup,

Quoted 29 lines · Anup Patel
Quoted 28 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

Quoted 7 lines
Quoted 4 lines

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

Should we emulate PSCI_VERSION call to help Guest determine the spec version emulated by KVM (i.e. v0.1 or v0.2) ??

I think that'd be a nice to have, but the guest is likely to get its information from the DT anyway. Plus I don't think the original PSCI spec specified PSCI_VERSION, which only make it useful for whatever comes after v0.2.

Quoted 6 lines

So I think we need to:
- Use the new range for PSCI v0.2 (while still supporting v0.1 and the old range)
- Get the kernel and DT bindings into shape
- Merge all of that at the same time

Don't we also need a way for user space to tell KVM if it should emulate v0.1 or v0.2 of PSCI so we don't break backwards compatibility with tools that spit out a device tree and use guest kernels based on v0.1?

This could be a new feature for KVM_ARM_VCPU_INIT, but perhaps it should be something on the VM level, hmmm.

-Christoffer

maintainer
#15 lore raw

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

Il 18/12/2013 21:38, Christoffer Dall ha scritto:

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

So I think we need to:
- Use the new range for PSCI v0.2 (while still supporting v0.1 and the old range)
- Get the kernel and DT bindings into shape
- Merge all of that at the same time

Quoted 6 lines

Don't we also need a way for user space to tell KVM if it should emulate v0.1 or v0.2 of PSCI so we don't break backwards compatibility with tools that spit out a device tree and use guest kernels based on v0.1?

This could be a new feature for KVM_ARM_VCPU_INIT, but perhaps it should be something on the VM level, hmmm.

You can use KVM_ENABLE_CAP. It is currently documented as a VCPU ioctl, but you can reuse it for VMs. However, it is best if you also add a new capability KVM_CAP_VM_ENABLE_CAP. Then rename the old KVM_CAP_ENABLE_CAP to KVM_CAP_VCPU_ENABLE_CAP, while leaving the old name for backwards compatibility.

Paolo

#16 lore raw

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

In reply to Marc Zyngier

Adding Mark Rutland.

Quoted 24 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

Quoted 8 lines

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

News to me. That is exactly the opposite of what Mark Rutland told me. This would certainly simplify things since the SMC calling convention IDs encode the size and there would be no reason to put the IDs into DT.

Quoted 5 lines

So I rekon we need to create a separate range for those. Also, I'd like to progress the DT and kernel side of things as well (otherwise this is all a bit pointless).

Rob: what are your plans regarding your PSCI v0.2 patches?

My plan was to simply add 2 optional properties for reset/off and be done with it like is done here. I'll leave it to ARM to sort out all of v0.2 ID and 32-bit vs. 64-bit issues.

Rob

#17 lore raw

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

Hi Rob,

Quoted 36 lines · Rob Herring

Adding Mark Rutland.

Quoted 24 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

Quoted 8 lines

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

Quoted 4 lines

News to me. That is exactly the opposite of what Mark Rutland told me. This would certainly simplify things since the SMC calling convention IDs encode the size and there would be no reason to put the IDs into DT.

Hmmm. I'd hate to contredict Mark. But as he's on holiday (and hopefully unlikely to reply immediately), I win the argument! ;-)

More seriously, given that we have a document specifying the IDs, I'd be inclined to follow it. It is not that often that ARM actually *mandates* something... ;-)

Mark (assuming you're reading this): what were your objections about following the ID mentioned in the spec?

Quoted 9 lines
Quoted 5 lines

So I rekon we need to create a separate range for those. Also, I'd like to progress the DT and kernel side of things as well (otherwise this is all a bit pointless).

Rob: what are your plans regarding your PSCI v0.2 patches?

My plan was to simply add 2 optional properties for reset/off and be done with it like is done here. I'll leave it to ARM to sort out all of v0.2 ID and 32-bit vs. 64-bit issues.

Thankfully, RESET and OFF are not of those property with two IDs.

Thanks,

        M.
Signature · 1 line
Jazz is not dead. It just smells funny.
reviewer
#18 lore raw

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

Quoted 43 lines · Marc Zyngier

Hi Rob,

Quoted 40 lines · Rob Herring

Adding Mark Rutland.

Quoted 24 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

Quoted 8 lines

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

News to me. That is exactly the opposite of what Mark Rutland told me. This would certainly simplify things since the SMC calling convention IDs encode the size and there would be no reason to put the IDs into DT.

Hi Rob,

Sorry for the confusion here. My position changed over the course of the discussion, so I'm probably arguing against my original side now.

I think the key point was that I wanted a PSCI 0.2 system to function with an existing kernel if possible, as all the existing calls still exist. That implied placing the numbers (redundantly) into the DT. This is important for extending KVM with PSCI 0.2 support without breaking support for existing guests and having to have a load of messy flags depending on the guest you want to boot.

I agree that we should use the specified function IDs, and therefore we do not need to specify the IDs. A new system that old kernels can't run on anyway can just have:

psci {
	compatible = "arm,psci-0.2";
	methoc = "smc";
};

However, it would be nice to have a set of function IDs in the DT for existing clients as a fallback. Preferably in the same node (we have the compatible list, we may as well use it and make it clear we expect clients to use one or the other):

psci {
	compatible = "arm,psci-0.2", "arm,psci";
	method = "smc";

	/* 
	 * The mandated PSCI 0.2 IDs are used by the client if it
	 * understands PSCI 0.2. These IDs are for older clients that
	 * only support the kernel's old PSCI version.
	 *
	 * These IDs might not match any of the PSCI 0.2 IDs, and are
	 * purely here for compatibility with existing software.
	 */
	cpu_off = <...>;
	cpu_on = <...>;
	cpu_suspend = <...>;
};

I believe KVM currently uses the same ID numbers regardless of guest register-width, which is unfortunately different from what PSCI 0.2 says it should. As KVM can figure out the register width of the caller it might be able to do the right thing (unless that's in conflict with PSCI 0.2). At worst we should be able to allocate a third set of width-agnostic IDs for older clients that imply the current behaviour.

Does that sound reasonable? Sorry for leading you in circles.

Quoted 10 lines

Hmmm. I'd hate to contredict Mark. But as he's on holiday (and hopefully unlikely to reply immediately), I win the argument! ;-)

More seriously, given that we have a document specifying the IDs, I'd be inclined to follow it. It is not that often that ARM actually *mandates* something... ;-)

Mark (assuming you're reading this): what were your objections about following the ID mentioned in the spec?

Originally I was worried about more spectacularly bad implementations with wrong IDs or a subset of IDs supported. Now I agree that we can handle those later if and when they arise, and using the correct IDs is a far better option.

The other issue was KVM forwards-compatibility, but I think the above covers that.

Quoted 10 lines
Quoted 10 lines
Quoted 6 lines

So I rekon we need to create a separate range for those. Also, I'd like to progress the DT and kernel side of things as well (otherwise this is all a bit pointless).

Rob: what are your plans regarding your PSCI v0.2 patches?

My plan was to simply add 2 optional properties for reset/off and be done with it like is done here. I'll leave it to ARM to sort out all of v0.2 ID and 32-bit vs. 64-bit issues.

Does the above sound OK for adding that support?

Thanks,
Mark.

#19 lore raw

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

Quoted 88 lines · Mark Rutland
Quoted 43 lines · Marc Zyngier

Hi Rob,

Quoted 40 lines · Rob Herring

Adding Mark Rutland.

Quoted 24 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

Quoted 8 lines

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

News to me. That is exactly the opposite of what Mark Rutland told me. This would certainly simplify things since the SMC calling convention IDs encode the size and there would be no reason to put the IDs into DT.

Hi Rob,

Sorry for the confusion here. My position changed over the course of the discussion, so I'm probably arguing against my original side now.

I think the key point was that I wanted a PSCI 0.2 system to function with an existing kernel if possible, as all the existing calls still exist. That implied placing the numbers (redundantly) into the DT. This is important for extending KVM with PSCI 0.2 support without breaking support for existing guests and having to have a load of messy flags depending on the guest you want to boot.

I agree that we should use the specified function IDs, and therefore we do not need to specify the IDs. A new system that old kernels can't run on anyway can just have:

psci {
	compatible = "arm,psci-0.2";
	methoc = "smc";
};

However, it would be nice to have a set of function IDs in the DT for existing clients as a fallback. Preferably in the same node (we have the compatible list, we may as well use it and make it clear we expect clients to use one or the other):

psci {
	compatible = "arm,psci-0.2", "arm,psci";
	method = "smc";

	/* 
	 * The mandated PSCI 0.2 IDs are used by the client if it
	 * understands PSCI 0.2. These IDs are for older clients that
	 * only support the kernel's old PSCI version.
	 *
	 * These IDs might not match any of the PSCI 0.2 IDs, and are
	 * purely here for compatibility with existing software.
	 */
	cpu_off = <...>;
	cpu_on = <...>;
	cpu_suspend = <...>;
};
Quoted 6 lines

I believe KVM currently uses the same ID numbers regardless of guest register-width, which is unfortunately different from what PSCI 0.2 says it should. As KVM can figure out the register width of the caller it might be able to do the right thing (unless that's in conflict with PSCI 0.2). At worst we should be able to allocate a third set of width-agnostic IDs for older clients that imply the current behaviour.

I would assume we keep the current PSCI 0.1 implementation in KVM the way it is, and add PSCI 0.2 as an additional 'feature' and leave it up to user space to decide how to put it into the device tree. But without doing much more work on the current PSCI 0.1 implementation. User space can then choose either of your proposed DT nodes depending on the level of backwards compatibility with VM kernels it wants.

Is this in line with your thinking?

-Christoffer

Quoted 37 lines

Does that sound reasonable? Sorry for leading you in circles.

Quoted 10 lines

Hmmm. I'd hate to contredict Mark. But as he's on holiday (and hopefully unlikely to reply immediately), I win the argument! ;-)

More seriously, given that we have a document specifying the IDs, I'd be inclined to follow it. It is not that often that ARM actually *mandates* something... ;-)

Mark (assuming you're reading this): what were your objections about following the ID mentioned in the spec?

Originally I was worried about more spectacularly bad implementations with wrong IDs or a subset of IDs supported. Now I agree that we can handle those later if and when they arise, and using the correct IDs is a far better option.

The other issue was KVM forwards-compatibility, but I think the above covers that.

Quoted 10 lines
Quoted 10 lines
Quoted 6 lines

So I rekon we need to create a separate range for those. Also, I'd like to progress the DT and kernel side of things as well (otherwise this is all a bit pointless).

Rob: what are your plans regarding your PSCI v0.2 patches?

My plan was to simply add 2 optional properties for reset/off and be done with it like is done here. I'll leave it to ARM to sort out all of v0.2 ID and 32-bit vs. 64-bit issues.

Does the above sound OK for adding that support?

Thanks,
Mark.

#20 lore raw

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

In reply to Mark Rutland
Quoted 88 lines · Mark Rutland
Quoted 43 lines · Marc Zyngier

Hi Rob,

Quoted 40 lines · Rob Herring

Adding Mark Rutland.

Quoted 24 lines · Marc Zyngier
Quoted 18 lines · Christoffer Dall
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)

Quoted 3 lines

I can merge this series if Marc acks it as well.

Quoted 8 lines

The patches themselves are mostly fine. One issue though: They implement part of the v0.2 spec, but keep on using the range of function IDs that we made up for v0.1.

I just had a chat with the person responsible for the spec, and realized that the Function IDs mentionned in the v0.2 spec are not optional, and not using them would be in direct violation of the spec (the new numbers now come directly from the SMC calling convention).

News to me. That is exactly the opposite of what Mark Rutland told me. This would certainly simplify things since the SMC calling convention IDs encode the size and there would be no reason to put the IDs into DT.

Hi Rob,

Sorry for the confusion here. My position changed over the course of the discussion, so I'm probably arguing against my original side now.

I think the key point was that I wanted a PSCI 0.2 system to function with an existing kernel if possible, as all the existing calls still exist. That implied placing the numbers (redundantly) into the DT. This is important for extending KVM with PSCI 0.2 support without breaking support for existing guests and having to have a load of messy flags depending on the guest you want to boot.

I agree that we should use the specified function IDs, and therefore we do not need to specify the IDs. A new system that old kernels can't run on anyway can just have:

psci {
        compatible = "arm,psci-0.2";
        methoc = "smc";
};

However, it would be nice to have a set of function IDs in the DT for existing clients as a fallback. Preferably in the same node (we have the compatible list, we may as well use it and make it clear we expect clients to use one or the other):

psci {
        compatible = "arm,psci-0.2", "arm,psci";
        method = "smc";

        /*
         * The mandated PSCI 0.2 IDs are used by the client if it
         * understands PSCI 0.2. These IDs are for older clients that
         * only support the kernel's old PSCI version.
         *
         * These IDs might not match any of the PSCI 0.2 IDs, and are
         * purely here for compatibility with existing software.
         */
        cpu_off = <...>;
        cpu_on = <...>;
        cpu_suspend = <...>;
};
Quoted 8 lines

I believe KVM currently uses the same ID numbers regardless of guest register-width, which is unfortunately different from what PSCI 0.2 says it should. As KVM can figure out the register width of the caller it might be able to do the right thing (unless that's in conflict with PSCI 0.2). At worst we should be able to allocate a third set of width-agnostic IDs for older clients that imply the current behaviour.

Does that sound reasonable? Sorry for leading you in circles.

Yes. Certainly the simplest solution is the best.

Quoted 17 lines
Quoted 3 lines

Mark (assuming you're reading this): what were your objections about following the ID mentioned in the spec?

Originally I was worried about more spectacularly bad implementations with wrong IDs or a subset of IDs supported. Now I agree that we can handle those later if and when they arise, and using the correct IDs is a far better option.

The other issue was KVM forwards-compatibility, but I think the above covers that.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

So I rekon we need to create a separate range for those. Also, I'd like to progress the DT and kernel side of things as well (otherwise this is all a bit pointless).

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 1 line

Rob: what are your plans regarding your PSCI v0.2 patches?

My plan was to simply add 2 optional properties for reset/off and be done with it like is done here. I'll leave it to ARM to sort out all of v0.2 ID and 32-bit vs. 64-bit issues.

Does the above sound OK for adding that support?

Yes, but this doesn't actually work for highbank/midway without a firmware update because they are using IDs from a pre-release of the v0.2 spec. Fortunately, they do not claim v0.2 compatibility, so we should be okay. It is only a question of removing the highbank reset/poweroff code from the kernel and I guess that will just not happen now.

Rob