* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
@ 2013-12-17 11:35 Anup Patel
2013-12-17 11:35 ` [PATCH v3 1/2] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header Anup Patel
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Anup Patel @ 2013-12-17 11:35 UTC (permalink / raw)
To: linux-arm-kernel
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(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 1/2] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header
2013-12-17 11:35 [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64 Anup Patel
@ 2013-12-17 11:35 ` Anup Patel
2013-12-17 11:35 ` [PATCH v3 2/2] ARM/ARM64: KVM: Forward PSCI SYSTEM_OFF and SYSTEM_RESET to user space Anup Patel
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Anup Patel @ 2013-12-17 11:35 UTC (permalink / raw)
To: linux-arm-kernel
Currently, we don't have an exit reason to notify user space about
a system-level event (for e.g. system reset or shutdown) triggered
by the VCPU. This patch adds exit reason KVM_EXIT_SYSTEM_EVENT for
this purpose. We can also inform user space about the 'type' and
architecture specific 'flags' of a system-level event using the
kvm_run structure.
This newly added KVM_EXIT_SYSTEM_EVENT will be used by KVM arm/arm64
in-kernel PSCI 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];
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 2/2] ARM/ARM64: KVM: Forward PSCI SYSTEM_OFF and SYSTEM_RESET to user space
2013-12-17 11:35 [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64 Anup Patel
2013-12-17 11:35 ` [PATCH v3 1/2] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header Anup Patel
@ 2013-12-17 11:35 ` Anup Patel
2013-12-17 11:53 ` [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64 Alexander Graf
2013-12-17 18:51 ` Christoffer Dall
3 siblings, 0 replies; 21+ messages in thread
From: Anup Patel @ 2013-12-17 11:35 UTC (permalink / raw)
To: linux-arm-kernel
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)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-17 11:35 [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64 Anup Patel
2013-12-17 11:35 ` [PATCH v3 1/2] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header Anup Patel
2013-12-17 11:35 ` [PATCH v3 2/2] ARM/ARM64: KVM: Forward PSCI SYSTEM_OFF and SYSTEM_RESET to user space Anup Patel
@ 2013-12-17 11:53 ` Alexander Graf
2013-12-17 18:51 ` Christoffer Dall
3 siblings, 0 replies; 21+ messages in thread
From: Alexander Graf @ 2013-12-17 11:53 UTC (permalink / raw)
To: linux-arm-kernel
> 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-17 11:35 [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64 Anup Patel
` (2 preceding siblings ...)
2013-12-17 11:53 ` [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64 Alexander Graf
@ 2013-12-17 18:51 ` Christoffer Dall
2013-12-18 14:38 ` Marc Zyngier
3 siblings, 1 reply; 21+ messages in thread
From: Christoffer Dall @ 2013-12-17 18:51 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-17 18:51 ` Christoffer Dall
@ 2013-12-18 14:38 ` Marc Zyngier
2013-12-18 15:03 ` Anup Patel
2013-12-18 15:42 ` Rob Herring
0 siblings, 2 replies; 21+ messages in thread
From: Marc Zyngier @ 2013-12-18 14:38 UTC (permalink / raw)
To: linux-arm-kernel
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.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 14:38 ` Marc Zyngier
@ 2013-12-18 15:03 ` Anup Patel
2013-12-18 15:41 ` Marc Zyngier
2013-12-18 15:42 ` Rob Herring
1 sibling, 1 reply; 21+ messages in thread
From: Anup Patel @ 2013-12-18 15:03 UTC (permalink / raw)
To: linux-arm-kernel
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 at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
Regards,
Anup
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 15:03 ` Anup Patel
@ 2013-12-18 15:41 ` Marc Zyngier
2013-12-18 15:52 ` Anup Patel
2013-12-18 20:38 ` Christoffer Dall
0 siblings, 2 replies; 21+ messages in thread
From: Marc Zyngier @ 2013-12-18 15:41 UTC (permalink / raw)
To: linux-arm-kernel
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.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 14:38 ` Marc Zyngier
2013-12-18 15:03 ` Anup Patel
@ 2013-12-18 15:42 ` Rob Herring
2013-12-18 18:10 ` Marc Zyngier
1 sibling, 1 reply; 21+ messages in thread
From: Rob Herring @ 2013-12-18 15:42 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 15:41 ` Marc Zyngier
@ 2013-12-18 15:52 ` Anup Patel
2013-12-18 18:11 ` Marc Zyngier
2013-12-18 20:38 ` Christoffer Dall
1 sibling, 1 reply; 21+ messages in thread
From: Anup Patel @ 2013-12-18 15:52 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 15:42 ` Rob Herring
@ 2013-12-18 18:10 ` Marc Zyngier
2014-01-07 11:50 ` Mark Rutland
0 siblings, 1 reply; 21+ messages in thread
From: Marc Zyngier @ 2013-12-18 18:10 UTC (permalink / raw)
To: linux-arm-kernel
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.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 15:52 ` Anup Patel
@ 2013-12-18 18:11 ` Marc Zyngier
2013-12-18 18:18 ` Anup Patel
0 siblings, 1 reply; 21+ messages in thread
From: Marc Zyngier @ 2013-12-18 18:11 UTC (permalink / raw)
To: linux-arm-kernel
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.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 18:11 ` Marc Zyngier
@ 2013-12-18 18:18 ` Anup Patel
2013-12-18 18:25 ` Marc Zyngier
0 siblings, 1 reply; 21+ messages in thread
From: Anup Patel @ 2013-12-18 18:18 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 18:18 ` Anup Patel
@ 2013-12-18 18:25 ` Marc Zyngier
2013-12-18 23:26 ` Rob Herring
0 siblings, 1 reply; 21+ messages in thread
From: Marc Zyngier @ 2013-12-18 18:25 UTC (permalink / raw)
To: linux-arm-kernel
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.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 15:41 ` Marc Zyngier
2013-12-18 15:52 ` Anup Patel
@ 2013-12-18 20:38 ` Christoffer Dall
2013-12-19 14:17 ` Paolo Bonzini
1 sibling, 1 reply; 21+ messages in thread
From: Christoffer Dall @ 2013-12-18 20:38 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 18:25 ` Marc Zyngier
@ 2013-12-18 23:26 ` Rob Herring
2013-12-19 4:30 ` Christoffer Dall
0 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2013-12-18 23:26 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 23:26 ` Rob Herring
@ 2013-12-19 4:30 ` Christoffer Dall
0 siblings, 0 replies; 21+ messages in thread
From: Christoffer Dall @ 2013-12-19 4:30 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 20:38 ` Christoffer Dall
@ 2013-12-19 14:17 ` Paolo Bonzini
0 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2013-12-19 14:17 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2013-12-18 18:10 ` Marc Zyngier
@ 2014-01-07 11:50 ` Mark Rutland
2014-01-08 22:02 ` Christoffer Dall
2014-01-10 14:47 ` Rob Herring
0 siblings, 2 replies; 21+ messages in thread
From: Mark Rutland @ 2014-01-07 11:50 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2014-01-07 11:50 ` Mark Rutland
@ 2014-01-08 22:02 ` Christoffer Dall
2014-01-10 14:47 ` Rob Herring
1 sibling, 0 replies; 21+ messages in thread
From: Christoffer Dall @ 2014-01-08 22:02 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64
2014-01-07 11:50 ` Mark Rutland
2014-01-08 22:02 ` Christoffer Dall
@ 2014-01-10 14:47 ` Rob Herring
1 sibling, 0 replies; 21+ messages in thread
From: Rob Herring @ 2014-01-10 14:47 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2014-01-10 14:47 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-17 11:35 [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64 Anup Patel
2013-12-17 11:35 ` [PATCH v3 1/2] KVM: Add KVM_EXIT_SYSTEM_EVENT to user space API header Anup Patel
2013-12-17 11:35 ` [PATCH v3 2/2] ARM/ARM64: KVM: Forward PSCI SYSTEM_OFF and SYSTEM_RESET to user space Anup Patel
2013-12-17 11:53 ` [PATCH v3 0/2] PSCI system off and reset for KVM ARM/ARM64 Alexander Graf
2013-12-17 18:51 ` Christoffer Dall
2013-12-18 14:38 ` Marc Zyngier
2013-12-18 15:03 ` Anup Patel
2013-12-18 15:41 ` Marc Zyngier
2013-12-18 15:52 ` Anup Patel
2013-12-18 18:11 ` Marc Zyngier
2013-12-18 18:18 ` Anup Patel
2013-12-18 18:25 ` Marc Zyngier
2013-12-18 23:26 ` Rob Herring
2013-12-19 4:30 ` Christoffer Dall
2013-12-18 20:38 ` Christoffer Dall
2013-12-19 14:17 ` Paolo Bonzini
2013-12-18 15:42 ` Rob Herring
2013-12-18 18:10 ` Marc Zyngier
2014-01-07 11:50 ` Mark Rutland
2014-01-08 22:02 ` Christoffer Dall
2014-01-10 14:47 ` Rob Herring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).