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(-)
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 anup.patel@linaro.org Signed-off-by: Pranavkumar Sawargaonkar pranavkumar@linaro.org --- Documentation/virtual/kvm/api.txt | 15 +++++++++++++++ include/uapi/linux/kvm.h | 8 ++++++++ 2 files changed, 23 insertions(+)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index aad3244..4ef180e 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -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]; }; diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 902f124..bb13faa 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -171,6 +171,7 @@ struct kvm_pit_config { #define KVM_EXIT_WATCHDOG 21 #define KVM_EXIT_S390_TSCH 22 #define KVM_EXIT_EPR 23 +#define KVM_EXIT_SYSTEM_EVENT 24
/* For KVM_EXIT_INTERNAL_ERROR */ /* Emulate instruction failed. */ @@ -301,6 +302,13 @@ struct kvm_run { struct { __u32 epr; } epr; + /* KVM_EXIT_SYSTEM_EVENT */ + struct { +#define KVM_SYSTEM_EVENT_SHUTDOWN 1 +#define KVM_SYSTEM_EVENT_RESET 2 + __u32 type; + __u64 flags; + } system_event; /* Fix the size of the union. */ char padding[256]; };
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 anup.patel@linaro.org Signed-off-by: Pranavkumar Sawargaonkar pranavkumar@linaro.org --- 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(-)
diff --git a/arch/arm/include/asm/kvm_psci.h b/arch/arm/include/asm/kvm_psci.h index 9a83d98..992d7f1 100644 --- a/arch/arm/include/asm/kvm_psci.h +++ b/arch/arm/include/asm/kvm_psci.h @@ -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__ */ diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h index c498b60..f4de20c 100644 --- a/arch/arm/include/uapi/asm/kvm.h +++ b/arch/arm/include/uapi/asm/kvm.h @@ -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) diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c index a920790..ea5f18e 100644 --- a/arch/arm/kvm/handle_exit.c +++ b/arch/arm/kvm/handle_exit.c @@ -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) diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c index 0881bf1..4885666 100644 --- a/arch/arm/kvm/psci.c +++ b/arch/arm/kvm/psci.c @@ -84,18 +84,40 @@ 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); +} + /** * 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. + * + * This function returns: > 0 (success), 0 (success but exit to user + * space), and < 0 (errors) + * + * Errors: + * -EINVAL: Unrecognized PSCI function */ -bool kvm_psci_call(struct kvm_vcpu *vcpu) +int kvm_psci_call(struct kvm_vcpu *vcpu) { + int ret = 1; unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0); unsigned long val;
@@ -111,11 +133,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; } diff --git a/arch/arm64/include/asm/kvm_psci.h b/arch/arm64/include/asm/kvm_psci.h index e301a48..9bd0ee8 100644 --- a/arch/arm64/include/asm/kvm_psci.h +++ b/arch/arm64/include/asm/kvm_psci.h @@ -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__ */ diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index d9f026b..f678902 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -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) diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index df84d7b..dbb6a11 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -30,11 +30,15 @@ typedef int (*exit_handle_fn)(struct kvm_vcpu *, struct kvm_run *);
static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) { - if (kvm_psci_call(vcpu)) + int ret; + + ret = kvm_psci_call(vcpu); + if (ret == -EINVAL) { + kvm_inject_undefined(vcpu); return 1; + }
- kvm_inject_undefined(vcpu); - return 1; + return ret; }
static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
Am 17.12.2013 um 12:35 schrieb Anup Patel anup.patel@linaro.org:
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)
Reviewed-by: Alexander Graf agraf@suse.de
Alex
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
I can merge this series if Marc acks it as well.
-Christoffer
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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.
On Wed, Dec 18, 2013 at 8:08 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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).
Should we emulate PSCI_VERSION call to help Guest determine the spec version emulated by KVM (i.e. v0.1 or v0.2) ??
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.-- Jazz is not dead. It just smells funny. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
Regards, Anup
Hi Anup,
On Wed, Dec 18 2013 at 03:03:43 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 8:08 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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).
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.
On Wed, Dec 18, 2013 at 9:11 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Hi Anup,
On Wed, Dec 18 2013 at 03:03:43 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 8:08 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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).
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)
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?
- 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.
On Wed, Dec 18 2013 at 03:52:29 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 9:11 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Hi Anup,
On Wed, Dec 18 2013 at 03:03:43 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 8:08 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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).
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)
Does this mean we should have first isolate v0.2 ID range from v0.1 ID range?
Yes.
And then...
Rebase this patchset based on new v0.2 ID range?
Indeed.
Thanks,
M.
On Wed, Dec 18, 2013 at 11:41 PM, Marc Zyngier marc.zyngier@arm.com wrote:
On Wed, Dec 18 2013 at 03:52:29 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 9:11 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Hi Anup,
On Wed, Dec 18 2013 at 03:03:43 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 8:08 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote: > 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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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).
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)
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?
And then...
Rebase this patchset based on new v0.2 ID range?
Indeed.
Thanks,
M.-- Jazz is not dead. It just smells funny.
Regards, Anup
On Wed, Dec 18 2013 at 06:18:54 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 11:41 PM, Marc Zyngier marc.zyngier@arm.com wrote:
On Wed, Dec 18 2013 at 03:52:29 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 9:11 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Hi Anup,
On Wed, Dec 18 2013 at 03:03:43 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 8:08 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
> On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote: >> 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) > > Reviewed-by: Christoffer Dall christoffer.dall@linaro.org > > 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).
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)
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.
On Wed, Dec 18, 2013 at 12:25 PM, Marc Zyngier marc.zyngier@arm.com wrote:
On Wed, Dec 18 2013 at 06:18:54 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 11:41 PM, Marc Zyngier marc.zyngier@arm.com wrote:
On Wed, Dec 18 2013 at 03:52:29 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 9:11 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Hi Anup,
On Wed, Dec 18 2013 at 03:03:43 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 8:08 PM, Marc Zyngier marc.zyngier@arm.com wrote: > Christoffer Dall christoffer.dall@linaro.org writes: > >> On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote: >>> 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) >> >> Reviewed-by: Christoffer Dall christoffer.dall@linaro.org >> >> 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).
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)
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.
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
On Wed, Dec 18, 2013 at 05:26:46PM -0600, Rob Herring wrote:
On Wed, Dec 18, 2013 at 12:25 PM, Marc Zyngier marc.zyngier@arm.com wrote:
On Wed, Dec 18 2013 at 06:18:54 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 11:41 PM, Marc Zyngier marc.zyngier@arm.com wrote:
On Wed, Dec 18 2013 at 03:52:29 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 9:11 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Hi Anup,
On Wed, Dec 18 2013 at 03:03:43 PM, Anup Patel anup@brainfault.org wrote: > On Wed, Dec 18, 2013 at 8:08 PM, Marc Zyngier marc.zyngier@arm.com wrote: >> Christoffer Dall christoffer.dall@linaro.org writes: >> >>> On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote: >>>> 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) >>> >>> Reviewed-by: Christoffer Dall christoffer.dall@linaro.org >>> >>> 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). > > 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)
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.
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
On Wed, Dec 18, 2013 at 03:41:27PM +0000, Marc Zyngier wrote:
Hi Anup,
On Wed, Dec 18 2013 at 03:03:43 PM, Anup Patel anup@brainfault.org wrote:
On Wed, Dec 18, 2013 at 8:08 PM, Marc Zyngier marc.zyngier@arm.com wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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).
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
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
Il 18/12/2013 21:38, Christoffer Dall ha scritto:
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.
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
Adding Mark Rutland.
On 12/18/2013 08:38 AM, Marc Zyngier wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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).
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.
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
Hi Rob,
On Wed, Dec 18 2013 at 03:42:09 PM, Rob Herring robherring2@gmail.com wrote:
Adding Mark Rutland.
On 12/18/2013 08:38 AM, Marc Zyngier wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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).
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?
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.
On Wed, Dec 18, 2013 at 06:10:27PM +0000, Marc Zyngier wrote:
Hi Rob,
On Wed, Dec 18 2013 at 03:42:09 PM, Rob Herring robherring2@gmail.com wrote:
Adding Mark Rutland.
On 12/18/2013 08:38 AM, Marc Zyngier wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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).
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.
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.
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.
On Tue, Jan 07, 2014 at 11:50:24AM +0000, Mark Rutland wrote:
On Wed, Dec 18, 2013 at 06:10:27PM +0000, Marc Zyngier wrote:
Hi Rob,
On Wed, Dec 18 2013 at 03:42:09 PM, Rob Herring robherring2@gmail.com wrote:
Adding Mark Rutland.
On 12/18/2013 08:38 AM, Marc Zyngier wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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).
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.
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
Does that sound reasonable? Sorry for leading you in circles.
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.
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.
On Tue, Jan 7, 2014 at 5:50 AM, Mark Rutland mark.rutland@arm.com wrote:
On Wed, Dec 18, 2013 at 06:10:27PM +0000, Marc Zyngier wrote:
Hi Rob,
On Wed, Dec 18 2013 at 03:42:09 PM, Rob Herring robherring2@gmail.com wrote:
Adding Mark Rutland.
On 12/18/2013 08:38 AM, Marc Zyngier wrote:
Christoffer Dall christoffer.dall@linaro.org writes:
On Tue, Dec 17, 2013 at 05:05:34PM +0530, Anup Patel wrote:
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)
Reviewed-by: Christoffer Dall christoffer.dall@linaro.org
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).
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.
Yes. Certainly the simplest solution is the best.
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.
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?
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
linaro-kernel@lists.linaro.org
-
Alexander Graf -
Anup Patel -
Anup Patel -
Christoffer Dall -
Marc Zyngier -
Mark Rutland -
Paolo Bonzini -
Rob Herring