* [PATCH 0/6] QEMU ARM64 Migration Fixes
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, marc.zyngier, linux-arm-kernel, kvmarm
In conjunction with the kernel patch series I've just sent:
http://thread.gmane.org/gmane.comp.emulators.kvm.arm.devel/101/focus=100
These patches have been fairly heavily tested on the xgene systems
I've got access to and I'm pretty confident we've caught all the
corner cases.
From QEMU's point of view the fixes are fairly simple. We need to take
a little care when restoring the GIC that config that affects the
later restoration is restored first. The rest was simply missing
serialisation code for SPSR and FP registers.
The pl011 patch was mainly to reduce noise on re-asserting level
triggered interrupt lines. And the cpu.h documentation was for my own
sanity.
Cheers,
Alex.
Alex Bennée (5):
target-arm: kvm: save/restore mp state
arm_gic_kvm.c: restore config before pending IRQs
hw/char/pl011: don't keep setting the IRQ if nothing changed
target-arm/kvm64.c: sync FP register state
target-arm/cpu.h: document why env->spsr exists
Christoffer Dall (1):
target-arm/kvm64: fix save/restore of SPSR regs
hw/char/pl011.c | 12 ++++--
hw/intc/arm_gic_kvm.c | 7 +++-
target-arm/cpu.h | 5 +++
target-arm/kvm.c | 1 +
target-arm/kvm64.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++---
target-arm/machine.c | 38 ++++++++++++++++++
6 files changed, 156 insertions(+), 11 deletions(-)
--
2.3.0
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 73+ messages in thread
* [Qemu-devel] [PATCH 0/6] QEMU ARM64 Migration Fixes
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, marc.zyngier, linux-arm-kernel, Alex Bennée, kvmarm,
christoffer.dall
In conjunction with the kernel patch series I've just sent:
http://thread.gmane.org/gmane.comp.emulators.kvm.arm.devel/101/focus=100
These patches have been fairly heavily tested on the xgene systems
I've got access to and I'm pretty confident we've caught all the
corner cases.
>From QEMU's point of view the fixes are fairly simple. We need to take
a little care when restoring the GIC that config that affects the
later restoration is restored first. The rest was simply missing
serialisation code for SPSR and FP registers.
The pl011 patch was mainly to reduce noise on re-asserting level
triggered interrupt lines. And the cpu.h documentation was for my own
sanity.
Cheers,
Alex.
Alex Bennée (5):
target-arm: kvm: save/restore mp state
arm_gic_kvm.c: restore config before pending IRQs
hw/char/pl011: don't keep setting the IRQ if nothing changed
target-arm/kvm64.c: sync FP register state
target-arm/cpu.h: document why env->spsr exists
Christoffer Dall (1):
target-arm/kvm64: fix save/restore of SPSR regs
hw/char/pl011.c | 12 ++++--
hw/intc/arm_gic_kvm.c | 7 +++-
target-arm/cpu.h | 5 +++
target-arm/kvm.c | 1 +
target-arm/kvm64.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++---
target-arm/machine.c | 38 ++++++++++++++++++
6 files changed, 156 insertions(+), 11 deletions(-)
--
2.3.0
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 0/6] QEMU ARM64 Migration Fixes
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: linux-arm-kernel
In conjunction with the kernel patch series I've just sent:
http://thread.gmane.org/gmane.comp.emulators.kvm.arm.devel/101/focus=100
These patches have been fairly heavily tested on the xgene systems
I've got access to and I'm pretty confident we've caught all the
corner cases.
>From QEMU's point of view the fixes are fairly simple. We need to take
a little care when restoring the GIC that config that affects the
later restoration is restored first. The rest was simply missing
serialisation code for SPSR and FP registers.
The pl011 patch was mainly to reduce noise on re-asserting level
triggered interrupt lines. And the cpu.h documentation was for my own
sanity.
Cheers,
Alex.
Alex Benn?e (5):
target-arm: kvm: save/restore mp state
arm_gic_kvm.c: restore config before pending IRQs
hw/char/pl011: don't keep setting the IRQ if nothing changed
target-arm/kvm64.c: sync FP register state
target-arm/cpu.h: document why env->spsr exists
Christoffer Dall (1):
target-arm/kvm64: fix save/restore of SPSR regs
hw/char/pl011.c | 12 ++++--
hw/intc/arm_gic_kvm.c | 7 +++-
target-arm/cpu.h | 5 +++
target-arm/kvm.c | 1 +
target-arm/kvm64.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++---
target-arm/machine.c | 38 ++++++++++++++++++
6 files changed, 156 insertions(+), 11 deletions(-)
--
2.3.0
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 1/6] target-arm: kvm: save/restore mp state
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-02-25 16:02 ` Alex Bennée
-1 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, marc.zyngier, linux-arm-kernel, Paolo Bonzini, kvmarm
This adds the saving and restore of the current Multi-Processing state
of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
potential states for x86 we only use two for ARM. Either the process is
running or not.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 23cefe9..8732854 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -25,6 +25,7 @@
#include "hw/arm/arm.h"
const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
+ KVM_CAP_INFO(MP_STATE),
KVM_CAP_LAST_INFO
};
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 9446e5a..70b1bc4 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -161,6 +161,34 @@ static const VMStateInfo vmstate_cpsr = {
.put = put_cpsr,
};
+#if defined CONFIG_KVM
+static int get_mpstate(QEMUFile *f, void *opaque, size_t size)
+{
+ ARMCPU *cpu = opaque;
+ struct kvm_mp_state mp_state = { .mp_state = qemu_get_be32(f)};
+ return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
+}
+
+static void put_mpstate(QEMUFile *f, void *opaque, size_t size)
+{
+ ARMCPU *cpu = opaque;
+ struct kvm_mp_state mp_state;
+ int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, &mp_state);
+ if (ret) {
+ fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
+ __func__, ret, strerror(ret));
+ abort();
+ }
+ qemu_put_be32(f, mp_state.mp_state);
+}
+
+static const VMStateInfo vmstate_mpstate = {
+ .name = "mpstate",
+ .get = get_mpstate,
+ .put = put_mpstate,
+};
+#endif
+
static void cpu_pre_save(void *opaque)
{
ARMCPU *cpu = opaque;
@@ -244,6 +272,16 @@ const VMStateDescription vmstate_arm_cpu = {
VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
VMSTATE_UINT64(env.pc, ARMCPU),
+#if defined CONFIG_KVM
+ {
+ .name = "mp_state",
+ .version_id = 0,
+ .size = sizeof(uint32_t),
+ .info = &vmstate_mpstate,
+ .flags = VMS_SINGLE,
+ .offset = 0,
+ },
+#endif
{
.name = "cpsr",
.version_id = 0,
--
2.3.0
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [Qemu-devel] [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, kvm, marc.zyngier, linux-arm-kernel,
Paolo Bonzini, Alex Bennée, kvmarm, christoffer.dall
This adds the saving and restore of the current Multi-Processing state
of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
potential states for x86 we only use two for ARM. Either the process is
running or not.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 23cefe9..8732854 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -25,6 +25,7 @@
#include "hw/arm/arm.h"
const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
+ KVM_CAP_INFO(MP_STATE),
KVM_CAP_LAST_INFO
};
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 9446e5a..70b1bc4 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -161,6 +161,34 @@ static const VMStateInfo vmstate_cpsr = {
.put = put_cpsr,
};
+#if defined CONFIG_KVM
+static int get_mpstate(QEMUFile *f, void *opaque, size_t size)
+{
+ ARMCPU *cpu = opaque;
+ struct kvm_mp_state mp_state = { .mp_state = qemu_get_be32(f)};
+ return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
+}
+
+static void put_mpstate(QEMUFile *f, void *opaque, size_t size)
+{
+ ARMCPU *cpu = opaque;
+ struct kvm_mp_state mp_state;
+ int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, &mp_state);
+ if (ret) {
+ fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
+ __func__, ret, strerror(ret));
+ abort();
+ }
+ qemu_put_be32(f, mp_state.mp_state);
+}
+
+static const VMStateInfo vmstate_mpstate = {
+ .name = "mpstate",
+ .get = get_mpstate,
+ .put = put_mpstate,
+};
+#endif
+
static void cpu_pre_save(void *opaque)
{
ARMCPU *cpu = opaque;
@@ -244,6 +272,16 @@ const VMStateDescription vmstate_arm_cpu = {
VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
VMSTATE_UINT64(env.pc, ARMCPU),
+#if defined CONFIG_KVM
+ {
+ .name = "mp_state",
+ .version_id = 0,
+ .size = sizeof(uint32_t),
+ .info = &vmstate_mpstate,
+ .flags = VMS_SINGLE,
+ .offset = 0,
+ },
+#endif
{
.name = "cpsr",
.version_id = 0,
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: linux-arm-kernel
This adds the saving and restore of the current Multi-Processing state
of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
potential states for x86 we only use two for ARM. Either the process is
running or not.
Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 23cefe9..8732854 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -25,6 +25,7 @@
#include "hw/arm/arm.h"
const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
+ KVM_CAP_INFO(MP_STATE),
KVM_CAP_LAST_INFO
};
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 9446e5a..70b1bc4 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -161,6 +161,34 @@ static const VMStateInfo vmstate_cpsr = {
.put = put_cpsr,
};
+#if defined CONFIG_KVM
+static int get_mpstate(QEMUFile *f, void *opaque, size_t size)
+{
+ ARMCPU *cpu = opaque;
+ struct kvm_mp_state mp_state = { .mp_state = qemu_get_be32(f)};
+ return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
+}
+
+static void put_mpstate(QEMUFile *f, void *opaque, size_t size)
+{
+ ARMCPU *cpu = opaque;
+ struct kvm_mp_state mp_state;
+ int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, &mp_state);
+ if (ret) {
+ fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
+ __func__, ret, strerror(ret));
+ abort();
+ }
+ qemu_put_be32(f, mp_state.mp_state);
+}
+
+static const VMStateInfo vmstate_mpstate = {
+ .name = "mpstate",
+ .get = get_mpstate,
+ .put = put_mpstate,
+};
+#endif
+
static void cpu_pre_save(void *opaque)
{
ARMCPU *cpu = opaque;
@@ -244,6 +272,16 @@ const VMStateDescription vmstate_arm_cpu = {
VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
VMSTATE_UINT64(env.pc, ARMCPU),
+#if defined CONFIG_KVM
+ {
+ .name = "mp_state",
+ .version_id = 0,
+ .size = sizeof(uint32_t),
+ .info = &vmstate_mpstate,
+ .flags = VMS_SINGLE,
+ .offset = 0,
+ },
+#endif
{
.name = "cpsr",
.version_id = 0,
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 2/6] arm_gic_kvm.c: restore config before pending IRQs
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-02-25 16:02 ` Alex Bennée
-1 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, marc.zyngier, linux-arm-kernel, kvmarm
As there is logic to deal with the difference between edge and level
triggered interrupts in the kernel we must ensure it knows the
configuration of the IRQs before we restore the pending state.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 1ad3eb0..2f21ae7 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -370,6 +370,11 @@ static void kvm_arm_gic_put(GICState *s)
* the appropriate CPU interfaces in the kernel) */
kvm_dist_put(s, 0x800, 8, s->num_irq, translate_targets);
+ /* irq_state[n].trigger -> GICD_ICFGRn
+ * (restore targets before pending IRQs so we treat level/edge
+ * correctly */
+ kvm_dist_put(s, 0xc00, 2, s->num_irq, translate_trigger);
+
/* irq_state[n].pending + irq_state[n].level -> GICD_ISPENDRn */
kvm_dist_put(s, 0x280, 1, s->num_irq, translate_clear);
kvm_dist_put(s, 0x200, 1, s->num_irq, translate_pending);
@@ -378,8 +383,6 @@ static void kvm_arm_gic_put(GICState *s)
kvm_dist_put(s, 0x380, 1, s->num_irq, translate_clear);
kvm_dist_put(s, 0x300, 1, s->num_irq, translate_active);
- /* irq_state[n].trigger -> GICD_ICFRn */
- kvm_dist_put(s, 0xc00, 2, s->num_irq, translate_trigger);
/* s->priorityX[irq] -> ICD_IPRIORITYRn */
kvm_dist_put(s, 0x400, 8, s->num_irq, translate_priority);
--
2.3.0
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [Qemu-devel] [PATCH 2/6] arm_gic_kvm.c: restore config before pending IRQs
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, marc.zyngier, linux-arm-kernel, Alex Bennée, kvmarm,
christoffer.dall
As there is logic to deal with the difference between edge and level
triggered interrupts in the kernel we must ensure it knows the
configuration of the IRQs before we restore the pending state.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 1ad3eb0..2f21ae7 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -370,6 +370,11 @@ static void kvm_arm_gic_put(GICState *s)
* the appropriate CPU interfaces in the kernel) */
kvm_dist_put(s, 0x800, 8, s->num_irq, translate_targets);
+ /* irq_state[n].trigger -> GICD_ICFGRn
+ * (restore targets before pending IRQs so we treat level/edge
+ * correctly */
+ kvm_dist_put(s, 0xc00, 2, s->num_irq, translate_trigger);
+
/* irq_state[n].pending + irq_state[n].level -> GICD_ISPENDRn */
kvm_dist_put(s, 0x280, 1, s->num_irq, translate_clear);
kvm_dist_put(s, 0x200, 1, s->num_irq, translate_pending);
@@ -378,8 +383,6 @@ static void kvm_arm_gic_put(GICState *s)
kvm_dist_put(s, 0x380, 1, s->num_irq, translate_clear);
kvm_dist_put(s, 0x300, 1, s->num_irq, translate_active);
- /* irq_state[n].trigger -> GICD_ICFRn */
- kvm_dist_put(s, 0xc00, 2, s->num_irq, translate_trigger);
/* s->priorityX[irq] -> ICD_IPRIORITYRn */
kvm_dist_put(s, 0x400, 8, s->num_irq, translate_priority);
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 2/6] arm_gic_kvm.c: restore config before pending IRQs
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: linux-arm-kernel
As there is logic to deal with the difference between edge and level
triggered interrupts in the kernel we must ensure it knows the
configuration of the IRQs before we restore the pending state.
Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 1ad3eb0..2f21ae7 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -370,6 +370,11 @@ static void kvm_arm_gic_put(GICState *s)
* the appropriate CPU interfaces in the kernel) */
kvm_dist_put(s, 0x800, 8, s->num_irq, translate_targets);
+ /* irq_state[n].trigger -> GICD_ICFGRn
+ * (restore targets before pending IRQs so we treat level/edge
+ * correctly */
+ kvm_dist_put(s, 0xc00, 2, s->num_irq, translate_trigger);
+
/* irq_state[n].pending + irq_state[n].level -> GICD_ISPENDRn */
kvm_dist_put(s, 0x280, 1, s->num_irq, translate_clear);
kvm_dist_put(s, 0x200, 1, s->num_irq, translate_pending);
@@ -378,8 +383,6 @@ static void kvm_arm_gic_put(GICState *s)
kvm_dist_put(s, 0x380, 1, s->num_irq, translate_clear);
kvm_dist_put(s, 0x300, 1, s->num_irq, translate_active);
- /* irq_state[n].trigger -> GICD_ICFRn */
- kvm_dist_put(s, 0xc00, 2, s->num_irq, translate_trigger);
/* s->priorityX[irq] -> ICD_IPRIORITYRn */
kvm_dist_put(s, 0x400, 8, s->num_irq, translate_priority);
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 3/6] hw/char/pl011: don't keep setting the IRQ if nothing changed
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-02-25 16:02 ` Alex Bennée
-1 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, marc.zyngier, linux-arm-kernel, kvmarm
While observing KVM traces I can see additional IRQ calls on pretty much
every MMIO access which is just plain inefficient. Only update the QEMU
IRQ level if something has actually changed from last time. Otherwise we
may be papering over other failure modes.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 0a45115..bb554bc 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -36,6 +36,9 @@ typedef struct PL011State {
CharDriverState *chr;
qemu_irq irq;
const unsigned char *id;
+
+ /* not serialised, prevents pl011_update doing extra set_irqs */
+ uint32_t current_irq;
} PL011State;
#define PL011_INT_TX 0x20
@@ -53,10 +56,11 @@ static const unsigned char pl011_id_luminary[8] =
static void pl011_update(PL011State *s)
{
- uint32_t flags;
-
- flags = s->int_level & s->int_enabled;
- qemu_set_irq(s->irq, flags != 0);
+ uint32_t flags = s->int_level & s->int_enabled;
+ if (flags != s->current_irq) {
+ s->current_irq = flags;
+ qemu_set_irq(s->irq, s->current_irq != 0);
+ }
}
static uint64_t pl011_read(void *opaque, hwaddr offset,
--
2.3.0
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [Qemu-devel] [PATCH 3/6] hw/char/pl011: don't keep setting the IRQ if nothing changed
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, marc.zyngier, linux-arm-kernel, Alex Bennée, kvmarm,
christoffer.dall
While observing KVM traces I can see additional IRQ calls on pretty much
every MMIO access which is just plain inefficient. Only update the QEMU
IRQ level if something has actually changed from last time. Otherwise we
may be papering over other failure modes.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 0a45115..bb554bc 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -36,6 +36,9 @@ typedef struct PL011State {
CharDriverState *chr;
qemu_irq irq;
const unsigned char *id;
+
+ /* not serialised, prevents pl011_update doing extra set_irqs */
+ uint32_t current_irq;
} PL011State;
#define PL011_INT_TX 0x20
@@ -53,10 +56,11 @@ static const unsigned char pl011_id_luminary[8] =
static void pl011_update(PL011State *s)
{
- uint32_t flags;
-
- flags = s->int_level & s->int_enabled;
- qemu_set_irq(s->irq, flags != 0);
+ uint32_t flags = s->int_level & s->int_enabled;
+ if (flags != s->current_irq) {
+ s->current_irq = flags;
+ qemu_set_irq(s->irq, s->current_irq != 0);
+ }
}
static uint64_t pl011_read(void *opaque, hwaddr offset,
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 3/6] hw/char/pl011: don't keep setting the IRQ if nothing changed
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: linux-arm-kernel
While observing KVM traces I can see additional IRQ calls on pretty much
every MMIO access which is just plain inefficient. Only update the QEMU
IRQ level if something has actually changed from last time. Otherwise we
may be papering over other failure modes.
Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 0a45115..bb554bc 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -36,6 +36,9 @@ typedef struct PL011State {
CharDriverState *chr;
qemu_irq irq;
const unsigned char *id;
+
+ /* not serialised, prevents pl011_update doing extra set_irqs */
+ uint32_t current_irq;
} PL011State;
#define PL011_INT_TX 0x20
@@ -53,10 +56,11 @@ static const unsigned char pl011_id_luminary[8] =
static void pl011_update(PL011State *s)
{
- uint32_t flags;
-
- flags = s->int_level & s->int_enabled;
- qemu_set_irq(s->irq, flags != 0);
+ uint32_t flags = s->int_level & s->int_enabled;
+ if (flags != s->current_irq) {
+ s->current_irq = flags;
+ qemu_set_irq(s->irq, s->current_irq != 0);
+ }
}
static uint64_t pl011_read(void *opaque, hwaddr offset,
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 4/6] target-arm/kvm64.c: sync FP register state
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-02-25 16:02 ` Alex Bennée
-1 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée, kvm, marc.zyngier, linux-arm-kernel, kvmarm
From: Alex Bennée <alex@bennee.com>
For migration to work we need to sync all of the register state. This is
especially noticeable when GCC starts using FP registers as spill
registers even with integer programs.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 8cf3a62..c60e989 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -126,9 +126,17 @@ bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
#define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
+/* The linux headers don't define a 128 bit wide SIMD macro for us */
+#define AARCH64_SIMD_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U128 | \
+ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
+
+#define AARCH64_SIMD_CTRL_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U32 | \
+ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
+
int kvm_arch_put_registers(CPUState *cs, int level)
{
struct kvm_one_reg reg;
+ uint32_t fpr;
uint64_t val;
int i;
int ret;
@@ -207,13 +215,36 @@ int kvm_arch_put_registers(CPUState *cs, int level)
}
}
+ /* Advanced SIMD and FP registers */
+ for (i = 0; i < 32; i++) {
+ reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
+ reg.addr = (uintptr_t)(&env->vfp.regs[i]);
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ reg.id++;
+ }
+
+ reg.addr = (uintptr_t)(&fpr);
+ fpr = vfp_get_fpsr(env);
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr);
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+
+ fpr = vfp_get_fpcr(env);
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr);
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+
if (!write_list_to_kvmstate(cpu)) {
return EINVAL;
}
- /* TODO:
- * FP state
- */
return ret;
}
@@ -221,6 +252,7 @@ int kvm_arch_get_registers(CPUState *cs)
{
struct kvm_one_reg reg;
uint64_t val;
+ uint32_t fpr;
int i;
int ret;
@@ -302,9 +334,36 @@ int kvm_arch_get_registers(CPUState *cs)
}
}
+ /* Advanced SIMD and FP registers */
+ for (i = 0; i < 32; i++) {
+ reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
+ reg.addr = (uintptr_t)(&env->vfp.regs[i]);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ reg.id++;
+ }
+
+ reg.addr = (uintptr_t)(&fpr);
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ vfp_set_fpsr(env, fpr);
+
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ vfp_set_fpcr(env, fpr);
+
if (!write_kvmstate_to_list(cpu)) {
return EINVAL;
}
+
/* Note that it's OK to have registers which aren't in CPUState,
* so we can ignore a failure return here.
*/
--
2.3.0
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [Qemu-devel] [PATCH 4/6] target-arm/kvm64.c: sync FP register state
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Alex Bennée, kvm, marc.zyngier,
linux-arm-kernel, Alex Bennée, kvmarm, christoffer.dall
From: Alex Bennée <alex@bennee.com>
For migration to work we need to sync all of the register state. This is
especially noticeable when GCC starts using FP registers as spill
registers even with integer programs.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 8cf3a62..c60e989 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -126,9 +126,17 @@ bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
#define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
+/* The linux headers don't define a 128 bit wide SIMD macro for us */
+#define AARCH64_SIMD_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U128 | \
+ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
+
+#define AARCH64_SIMD_CTRL_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U32 | \
+ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
+
int kvm_arch_put_registers(CPUState *cs, int level)
{
struct kvm_one_reg reg;
+ uint32_t fpr;
uint64_t val;
int i;
int ret;
@@ -207,13 +215,36 @@ int kvm_arch_put_registers(CPUState *cs, int level)
}
}
+ /* Advanced SIMD and FP registers */
+ for (i = 0; i < 32; i++) {
+ reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
+ reg.addr = (uintptr_t)(&env->vfp.regs[i]);
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ reg.id++;
+ }
+
+ reg.addr = (uintptr_t)(&fpr);
+ fpr = vfp_get_fpsr(env);
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr);
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+
+ fpr = vfp_get_fpcr(env);
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr);
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+
if (!write_list_to_kvmstate(cpu)) {
return EINVAL;
}
- /* TODO:
- * FP state
- */
return ret;
}
@@ -221,6 +252,7 @@ int kvm_arch_get_registers(CPUState *cs)
{
struct kvm_one_reg reg;
uint64_t val;
+ uint32_t fpr;
int i;
int ret;
@@ -302,9 +334,36 @@ int kvm_arch_get_registers(CPUState *cs)
}
}
+ /* Advanced SIMD and FP registers */
+ for (i = 0; i < 32; i++) {
+ reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
+ reg.addr = (uintptr_t)(&env->vfp.regs[i]);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ reg.id++;
+ }
+
+ reg.addr = (uintptr_t)(&fpr);
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ vfp_set_fpsr(env, fpr);
+
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ vfp_set_fpcr(env, fpr);
+
if (!write_kvmstate_to_list(cpu)) {
return EINVAL;
}
+
/* Note that it's OK to have registers which aren't in CPUState,
* so we can ignore a failure return here.
*/
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 4/6] target-arm/kvm64.c: sync FP register state
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: linux-arm-kernel
From: Alex Benn?e <alex@bennee.com>
For migration to work we need to sync all of the register state. This is
especially noticeable when GCC starts using FP registers as spill
registers even with integer programs.
Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 8cf3a62..c60e989 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -126,9 +126,17 @@ bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
#define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
+/* The linux headers don't define a 128 bit wide SIMD macro for us */
+#define AARCH64_SIMD_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U128 | \
+ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
+
+#define AARCH64_SIMD_CTRL_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U32 | \
+ KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
+
int kvm_arch_put_registers(CPUState *cs, int level)
{
struct kvm_one_reg reg;
+ uint32_t fpr;
uint64_t val;
int i;
int ret;
@@ -207,13 +215,36 @@ int kvm_arch_put_registers(CPUState *cs, int level)
}
}
+ /* Advanced SIMD and FP registers */
+ for (i = 0; i < 32; i++) {
+ reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
+ reg.addr = (uintptr_t)(&env->vfp.regs[i]);
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ reg.id++;
+ }
+
+ reg.addr = (uintptr_t)(&fpr);
+ fpr = vfp_get_fpsr(env);
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr);
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+
+ fpr = vfp_get_fpcr(env);
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr);
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+
if (!write_list_to_kvmstate(cpu)) {
return EINVAL;
}
- /* TODO:
- * FP state
- */
return ret;
}
@@ -221,6 +252,7 @@ int kvm_arch_get_registers(CPUState *cs)
{
struct kvm_one_reg reg;
uint64_t val;
+ uint32_t fpr;
int i;
int ret;
@@ -302,9 +334,36 @@ int kvm_arch_get_registers(CPUState *cs)
}
}
+ /* Advanced SIMD and FP registers */
+ for (i = 0; i < 32; i++) {
+ reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
+ reg.addr = (uintptr_t)(&env->vfp.regs[i]);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ reg.id++;
+ }
+
+ reg.addr = (uintptr_t)(&fpr);
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ vfp_set_fpsr(env, fpr);
+
+ reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr);
+ ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+ if (ret) {
+ return ret;
+ }
+ vfp_set_fpcr(env, fpr);
+
if (!write_kvmstate_to_list(cpu)) {
return EINVAL;
}
+
/* Note that it's OK to have registers which aren't in CPUState,
* so we can ignore a failure return here.
*/
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR registers
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-02-25 16:02 ` Alex Bennée
-1 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, marc.zyngier, linux-arm-kernel, kvmarm
From: Christoffer Dall <christoffer.dall@linaro.org>
The current code was negatively indexing the cpu state array and not
synchronizing banked spsr register state with the current mode's spsr
state, causing occasional failures with migration.
Some munging is done to take care of the aarch64 mapping and also to
ensure the most current value of the spsr is updated to the banked
registers (relevant for KVM<->TCG migration).
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2 (ajb)
- minor tweaks and clarifications
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index c60e989..6d30e10 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
uint64_t val;
int i;
int ret;
+ unsigned int el;
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
@@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
return ret;
}
- for (i = 0; i < KVM_NR_SPSR; i++) {
+ /* Saved Program State Registers
+ *
+ * Before we restore from the banked_spsr[] array we need to
+ * ensure that any modifications to env->spsr are correctly
+ * reflected and map aarch64 exception levels if required.
+ */
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[1] = env->banked_spsr[0];
+ env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
+ } else {
+ env->banked_spsr[el] = env->spsr;
+ }
+
+ for (i=0; i<KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
if (ret) {
return ret;
@@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
struct kvm_one_reg reg;
uint64_t val;
uint32_t fpr;
+ unsigned int el;
int i;
int ret;
@@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
return ret;
}
- for (i = 0; i < KVM_NR_SPSR; i++) {
+ /* Fetch the SPSR registers
+ *
+ * KVM has an array of state indexed for all the possible aarch32
+ * privilage levels. Although not all are valid at all points
+ * there are some transitions possible which can access old state
+ * so it is worth keeping them all.
+ */
+ for (i=0; i<KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
if (ret) {
return ret;
}
}
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[0] = env->banked_spsr[1];
+ env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
+ } else {
+ env->spsr = env->banked_spsr[el];
+ }
+
/* Advanced SIMD and FP registers */
for (i = 0; i < 32; i++) {
reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
--
2.3.0
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [Qemu-devel] [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR registers
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, kvm, marc.zyngier, linux-arm-kernel,
Alex Bennée, kvmarm, christoffer.dall
From: Christoffer Dall <christoffer.dall@linaro.org>
The current code was negatively indexing the cpu state array and not
synchronizing banked spsr register state with the current mode's spsr
state, causing occasional failures with migration.
Some munging is done to take care of the aarch64 mapping and also to
ensure the most current value of the spsr is updated to the banked
registers (relevant for KVM<->TCG migration).
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2 (ajb)
- minor tweaks and clarifications
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index c60e989..6d30e10 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
uint64_t val;
int i;
int ret;
+ unsigned int el;
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
@@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
return ret;
}
- for (i = 0; i < KVM_NR_SPSR; i++) {
+ /* Saved Program State Registers
+ *
+ * Before we restore from the banked_spsr[] array we need to
+ * ensure that any modifications to env->spsr are correctly
+ * reflected and map aarch64 exception levels if required.
+ */
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[1] = env->banked_spsr[0];
+ env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
+ } else {
+ env->banked_spsr[el] = env->spsr;
+ }
+
+ for (i=0; i<KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
if (ret) {
return ret;
@@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
struct kvm_one_reg reg;
uint64_t val;
uint32_t fpr;
+ unsigned int el;
int i;
int ret;
@@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
return ret;
}
- for (i = 0; i < KVM_NR_SPSR; i++) {
+ /* Fetch the SPSR registers
+ *
+ * KVM has an array of state indexed for all the possible aarch32
+ * privilage levels. Although not all are valid at all points
+ * there are some transitions possible which can access old state
+ * so it is worth keeping them all.
+ */
+ for (i=0; i<KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
if (ret) {
return ret;
}
}
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[0] = env->banked_spsr[1];
+ env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
+ } else {
+ env->spsr = env->banked_spsr[el];
+ }
+
/* Advanced SIMD and FP registers */
for (i = 0; i < 32; i++) {
reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR registers
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: linux-arm-kernel
From: Christoffer Dall <christoffer.dall@linaro.org>
The current code was negatively indexing the cpu state array and not
synchronizing banked spsr register state with the current mode's spsr
state, causing occasional failures with migration.
Some munging is done to take care of the aarch64 mapping and also to
ensure the most current value of the spsr is updated to the banked
registers (relevant for KVM<->TCG migration).
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
---
v2 (ajb)
- minor tweaks and clarifications
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index c60e989..6d30e10 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
uint64_t val;
int i;
int ret;
+ unsigned int el;
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
@@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
return ret;
}
- for (i = 0; i < KVM_NR_SPSR; i++) {
+ /* Saved Program State Registers
+ *
+ * Before we restore from the banked_spsr[] array we need to
+ * ensure that any modifications to env->spsr are correctly
+ * reflected and map aarch64 exception levels if required.
+ */
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[1] = env->banked_spsr[0];
+ env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
+ } else {
+ env->banked_spsr[el] = env->spsr;
+ }
+
+ for (i=0; i<KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
if (ret) {
return ret;
@@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
struct kvm_one_reg reg;
uint64_t val;
uint32_t fpr;
+ unsigned int el;
int i;
int ret;
@@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
return ret;
}
- for (i = 0; i < KVM_NR_SPSR; i++) {
+ /* Fetch the SPSR registers
+ *
+ * KVM has an array of state indexed for all the possible aarch32
+ * privilage levels. Although not all are valid@all points
+ * there are some transitions possible which can access old state
+ * so it is worth keeping them all.
+ */
+ for (i=0; i<KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
if (ret) {
return ret;
}
}
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[0] = env->banked_spsr[1];
+ env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
+ } else {
+ env->spsr = env->banked_spsr[el];
+ }
+
/* Advanced SIMD and FP registers */
for (i = 0; i < 32; i++) {
reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-02-25 16:02 ` Alex Bennée
-1 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, marc.zyngier, linux-arm-kernel, kvmarm
From: Christoffer Dall <christoffer.dall@linaro.org>
The current code was negatively indexing the cpu state array and not
synchronizing banked spsr register state with the current mode's spsr
state, causing occasional failures with migration.
Some munging is done to take care of the aarch64 mapping and also to
ensure the most current value of the spsr is updated to the banked
registers (relevant for KVM<->TCG migration).
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2 (ajb)
- minor tweaks and clarifications
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index c60e989..1e36b0a 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
uint64_t val;
int i;
int ret;
+ unsigned int el;
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
@@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
return ret;
}
+ /* Saved Program State Registers
+ *
+ * Before we restore from the banked_spsr[] array we need to
+ * ensure that any modifications to env->spsr are correctly
+ * reflected and map aarch64 exception levels if required.
+ */
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[1] = env->banked_spsr[0];
+ env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
+ } else {
+ env->banked_spsr[el] = env->spsr;
+ }
+
for (i = 0; i < KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
if (ret) {
return ret;
@@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
struct kvm_one_reg reg;
uint64_t val;
uint32_t fpr;
+ unsigned int el;
int i;
int ret;
@@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
return ret;
}
+ /* Fetch the SPSR registers
+ *
+ * KVM has an array of state indexed for all the possible aarch32
+ * privilage levels. Although not all are valid at all points
+ * there are some transitions possible which can access old state
+ * so it is worth keeping them all.
+ */
for (i = 0; i < KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
if (ret) {
return ret;
}
}
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[0] = env->banked_spsr[1];
+ env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
+ } else {
+ env->spsr = env->banked_spsr[el];
+ }
+
/* Advanced SIMD and FP registers */
for (i = 0; i < 32; i++) {
reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
--
2.3.0
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [Qemu-devel] [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, kvm, marc.zyngier, linux-arm-kernel,
Alex Bennée, kvmarm, christoffer.dall
From: Christoffer Dall <christoffer.dall@linaro.org>
The current code was negatively indexing the cpu state array and not
synchronizing banked spsr register state with the current mode's spsr
state, causing occasional failures with migration.
Some munging is done to take care of the aarch64 mapping and also to
ensure the most current value of the spsr is updated to the banked
registers (relevant for KVM<->TCG migration).
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2 (ajb)
- minor tweaks and clarifications
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index c60e989..1e36b0a 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
uint64_t val;
int i;
int ret;
+ unsigned int el;
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
@@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
return ret;
}
+ /* Saved Program State Registers
+ *
+ * Before we restore from the banked_spsr[] array we need to
+ * ensure that any modifications to env->spsr are correctly
+ * reflected and map aarch64 exception levels if required.
+ */
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[1] = env->banked_spsr[0];
+ env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
+ } else {
+ env->banked_spsr[el] = env->spsr;
+ }
+
for (i = 0; i < KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
if (ret) {
return ret;
@@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
struct kvm_one_reg reg;
uint64_t val;
uint32_t fpr;
+ unsigned int el;
int i;
int ret;
@@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
return ret;
}
+ /* Fetch the SPSR registers
+ *
+ * KVM has an array of state indexed for all the possible aarch32
+ * privilage levels. Although not all are valid at all points
+ * there are some transitions possible which can access old state
+ * so it is worth keeping them all.
+ */
for (i = 0; i < KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
if (ret) {
return ret;
}
}
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[0] = env->banked_spsr[1];
+ env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
+ } else {
+ env->spsr = env->banked_spsr[el];
+ }
+
/* Advanced SIMD and FP registers */
for (i = 0; i < 32; i++) {
reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: linux-arm-kernel
From: Christoffer Dall <christoffer.dall@linaro.org>
The current code was negatively indexing the cpu state array and not
synchronizing banked spsr register state with the current mode's spsr
state, causing occasional failures with migration.
Some munging is done to take care of the aarch64 mapping and also to
ensure the most current value of the spsr is updated to the banked
registers (relevant for KVM<->TCG migration).
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
---
v2 (ajb)
- minor tweaks and clarifications
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index c60e989..1e36b0a 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
uint64_t val;
int i;
int ret;
+ unsigned int el;
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
@@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
return ret;
}
+ /* Saved Program State Registers
+ *
+ * Before we restore from the banked_spsr[] array we need to
+ * ensure that any modifications to env->spsr are correctly
+ * reflected and map aarch64 exception levels if required.
+ */
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[1] = env->banked_spsr[0];
+ env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
+ } else {
+ env->banked_spsr[el] = env->spsr;
+ }
+
for (i = 0; i < KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
if (ret) {
return ret;
@@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
struct kvm_one_reg reg;
uint64_t val;
uint32_t fpr;
+ unsigned int el;
int i;
int ret;
@@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
return ret;
}
+ /* Fetch the SPSR registers
+ *
+ * KVM has an array of state indexed for all the possible aarch32
+ * privilage levels. Although not all are valid at all points
+ * there are some transitions possible which can access old state
+ * so it is worth keeping them all.
+ */
for (i = 0; i < KVM_NR_SPSR; i++) {
reg.id = AARCH64_CORE_REG(spsr[i]);
- reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
+ reg.addr = (uintptr_t) &env->banked_spsr[i+1];
ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
if (ret) {
return ret;
}
}
+ el = arm_current_el(env);
+ if (is_a64(env) && el > 0) {
+ g_assert(el == 1);
+ /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
+ env->banked_spsr[0] = env->banked_spsr[1];
+ env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
+ } else {
+ env->spsr = env->banked_spsr[el];
+ }
+
/* Advanced SIMD and FP registers */
for (i = 0; i < 32; i++) {
reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 6/6] target-arm/cpu.h: document why env->spsr exists
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-02-25 16:02 ` Alex Bennée
-1 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm, marc.zyngier, linux-arm-kernel, kvmarm
I was getting very confused about the duplication of state. Perhaps we
should just get rid of env->spsr and use helpers that understand the
banking?
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 11845a6..d7fd13f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -155,6 +155,11 @@ typedef struct CPUARMState {
This contains all the other bits. Use cpsr_{read,write} to access
the whole CPSR. */
uint32_t uncached_cpsr;
+ /* The spsr is a alias for spsr_elN where N is the current
+ * exception level. It is provided for here so the TCG msr/mrs
+ * implementation can access one register. Care needs to be taken
+ * to ensure the banked_spsr[] is also updated.
+ */
uint32_t spsr;
/* Banked registers. */
--
2.3.0
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [Qemu-devel] [PATCH 6/6] target-arm/cpu.h: document why env->spsr exists
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, kvm, marc.zyngier, linux-arm-kernel,
Alex Bennée, kvmarm, christoffer.dall
I was getting very confused about the duplication of state. Perhaps we
should just get rid of env->spsr and use helpers that understand the
banking?
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 11845a6..d7fd13f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -155,6 +155,11 @@ typedef struct CPUARMState {
This contains all the other bits. Use cpsr_{read,write} to access
the whole CPSR. */
uint32_t uncached_cpsr;
+ /* The spsr is a alias for spsr_elN where N is the current
+ * exception level. It is provided for here so the TCG msr/mrs
+ * implementation can access one register. Care needs to be taken
+ * to ensure the banked_spsr[] is also updated.
+ */
uint32_t spsr;
/* Banked registers. */
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* [PATCH 6/6] target-arm/cpu.h: document why env->spsr exists
@ 2015-02-25 16:02 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-02-25 16:02 UTC (permalink / raw)
To: linux-arm-kernel
I was getting very confused about the duplication of state. Perhaps we
should just get rid of env->spsr and use helpers that understand the
banking?
Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 11845a6..d7fd13f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -155,6 +155,11 @@ typedef struct CPUARMState {
This contains all the other bits. Use cpsr_{read,write} to access
the whole CPSR. */
uint32_t uncached_cpsr;
+ /* The spsr is a alias for spsr_elN where N is the current
+ * exception level. It is provided for here so the TCG msr/mrs
+ * implementation can access one register. Care needs to be taken
+ * to ensure the banked_spsr[] is also updated.
+ */
uint32_t spsr;
/* Banked registers. */
--
2.3.0
^ permalink raw reply related [flat|nested] 73+ messages in thread
* Re: [PATCH 1/6] target-arm: kvm: save/restore mp state
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-02-25 23:36 ` Peter Maydell
-1 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-02-25 23:36 UTC (permalink / raw)
To: Alex Bennée
Cc: QEMU Developers, kvm-devel, arm-mail-list, kvmarm,
Christoffer Dall, Marc Zyngier, Paolo Bonzini
On 26 February 2015 at 01:02, Alex Bennée <alex.bennee@linaro.org> wrote:
> This adds the saving and restore of the current Multi-Processing state
> of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
> potential states for x86 we only use two for ARM. Either the process is
> running or not.
By this you mean "is the CPU in the PSCI powered down state or not",
right?
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index 23cefe9..8732854 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -25,6 +25,7 @@
> #include "hw/arm/arm.h"
>
> const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
> + KVM_CAP_INFO(MP_STATE),
Does this really want to be a required cap? I haven't checked,
but assuming 'required' means what it says this presumably
means we'll stop working on host kernels we previously ran
fine on (even if migration didn't work there).
> KVM_CAP_LAST_INFO
> };
>
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 9446e5a..70b1bc4 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -161,6 +161,34 @@ static const VMStateInfo vmstate_cpsr = {
> .put = put_cpsr,
> };
>
> +#if defined CONFIG_KVM
> +static int get_mpstate(QEMUFile *f, void *opaque, size_t size)
> +{
> + ARMCPU *cpu = opaque;
> + struct kvm_mp_state mp_state = { .mp_state = qemu_get_be32(f)};
> + return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
> +}
Won't this break if you're running a QEMU built with KVM
support in TCG mode on an aarch64 host?
In any case, for that configuration we should be migrating the
TCG cpu->powered_off flag, which is where we keep the PSCI
power state for TCG.
> +
> +static void put_mpstate(QEMUFile *f, void *opaque, size_t size)
> +{
> + ARMCPU *cpu = opaque;
> + struct kvm_mp_state mp_state;
> + int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, &mp_state);
> + if (ret) {
> + fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
> + __func__, ret, strerror(ret));
> + abort();
> + }
> + qemu_put_be32(f, mp_state.mp_state);
> +}
> +
> +static const VMStateInfo vmstate_mpstate = {
> + .name = "mpstate",
> + .get = get_mpstate,
> + .put = put_mpstate,
> +};
> +#endif
> +
> static void cpu_pre_save(void *opaque)
> {
> ARMCPU *cpu = opaque;
> @@ -244,6 +272,16 @@ const VMStateDescription vmstate_arm_cpu = {
> VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
> VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
> VMSTATE_UINT64(env.pc, ARMCPU),
> +#if defined CONFIG_KVM
> + {
> + .name = "mp_state",
> + .version_id = 0,
> + .size = sizeof(uint32_t),
> + .info = &vmstate_mpstate,
> + .flags = VMS_SINGLE,
> + .offset = 0,
> + },
> +#endif
This means the migration format will be different on different
build configurations, which seems like a really bad idea.
Have you considered having the KVM state sync functions just
sync the kernel's MP state into cpu->powered_off, and then
migrating that flag here unconditionally?
> {
> .name = "cpsr",
> .version_id = 0,
> --
> 2.3.0
>
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-02-25 23:36 ` Peter Maydell
0 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-02-25 23:36 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm-devel, Marc Zyngier, QEMU Developers, Christoffer Dall,
Paolo Bonzini, kvmarm, arm-mail-list
On 26 February 2015 at 01:02, Alex Bennée <alex.bennee@linaro.org> wrote:
> This adds the saving and restore of the current Multi-Processing state
> of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
> potential states for x86 we only use two for ARM. Either the process is
> running or not.
By this you mean "is the CPU in the PSCI powered down state or not",
right?
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index 23cefe9..8732854 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -25,6 +25,7 @@
> #include "hw/arm/arm.h"
>
> const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
> + KVM_CAP_INFO(MP_STATE),
Does this really want to be a required cap? I haven't checked,
but assuming 'required' means what it says this presumably
means we'll stop working on host kernels we previously ran
fine on (even if migration didn't work there).
> KVM_CAP_LAST_INFO
> };
>
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 9446e5a..70b1bc4 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -161,6 +161,34 @@ static const VMStateInfo vmstate_cpsr = {
> .put = put_cpsr,
> };
>
> +#if defined CONFIG_KVM
> +static int get_mpstate(QEMUFile *f, void *opaque, size_t size)
> +{
> + ARMCPU *cpu = opaque;
> + struct kvm_mp_state mp_state = { .mp_state = qemu_get_be32(f)};
> + return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
> +}
Won't this break if you're running a QEMU built with KVM
support in TCG mode on an aarch64 host?
In any case, for that configuration we should be migrating the
TCG cpu->powered_off flag, which is where we keep the PSCI
power state for TCG.
> +
> +static void put_mpstate(QEMUFile *f, void *opaque, size_t size)
> +{
> + ARMCPU *cpu = opaque;
> + struct kvm_mp_state mp_state;
> + int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, &mp_state);
> + if (ret) {
> + fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
> + __func__, ret, strerror(ret));
> + abort();
> + }
> + qemu_put_be32(f, mp_state.mp_state);
> +}
> +
> +static const VMStateInfo vmstate_mpstate = {
> + .name = "mpstate",
> + .get = get_mpstate,
> + .put = put_mpstate,
> +};
> +#endif
> +
> static void cpu_pre_save(void *opaque)
> {
> ARMCPU *cpu = opaque;
> @@ -244,6 +272,16 @@ const VMStateDescription vmstate_arm_cpu = {
> VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
> VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
> VMSTATE_UINT64(env.pc, ARMCPU),
> +#if defined CONFIG_KVM
> + {
> + .name = "mp_state",
> + .version_id = 0,
> + .size = sizeof(uint32_t),
> + .info = &vmstate_mpstate,
> + .flags = VMS_SINGLE,
> + .offset = 0,
> + },
> +#endif
This means the migration format will be different on different
build configurations, which seems like a really bad idea.
Have you considered having the KVM state sync functions just
sync the kernel's MP state into cpu->powered_off, and then
migrating that flag here unconditionally?
> {
> .name = "cpsr",
> .version_id = 0,
> --
> 2.3.0
>
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-02-25 23:36 ` Peter Maydell
0 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-02-25 23:36 UTC (permalink / raw)
To: linux-arm-kernel
On 26 February 2015 at 01:02, Alex Benn?e <alex.bennee@linaro.org> wrote:
> This adds the saving and restore of the current Multi-Processing state
> of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
> potential states for x86 we only use two for ARM. Either the process is
> running or not.
By this you mean "is the CPU in the PSCI powered down state or not",
right?
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
>
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index 23cefe9..8732854 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -25,6 +25,7 @@
> #include "hw/arm/arm.h"
>
> const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
> + KVM_CAP_INFO(MP_STATE),
Does this really want to be a required cap? I haven't checked,
but assuming 'required' means what it says this presumably
means we'll stop working on host kernels we previously ran
fine on (even if migration didn't work there).
> KVM_CAP_LAST_INFO
> };
>
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 9446e5a..70b1bc4 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -161,6 +161,34 @@ static const VMStateInfo vmstate_cpsr = {
> .put = put_cpsr,
> };
>
> +#if defined CONFIG_KVM
> +static int get_mpstate(QEMUFile *f, void *opaque, size_t size)
> +{
> + ARMCPU *cpu = opaque;
> + struct kvm_mp_state mp_state = { .mp_state = qemu_get_be32(f)};
> + return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
> +}
Won't this break if you're running a QEMU built with KVM
support in TCG mode on an aarch64 host?
In any case, for that configuration we should be migrating the
TCG cpu->powered_off flag, which is where we keep the PSCI
power state for TCG.
> +
> +static void put_mpstate(QEMUFile *f, void *opaque, size_t size)
> +{
> + ARMCPU *cpu = opaque;
> + struct kvm_mp_state mp_state;
> + int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, &mp_state);
> + if (ret) {
> + fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
> + __func__, ret, strerror(ret));
> + abort();
> + }
> + qemu_put_be32(f, mp_state.mp_state);
> +}
> +
> +static const VMStateInfo vmstate_mpstate = {
> + .name = "mpstate",
> + .get = get_mpstate,
> + .put = put_mpstate,
> +};
> +#endif
> +
> static void cpu_pre_save(void *opaque)
> {
> ARMCPU *cpu = opaque;
> @@ -244,6 +272,16 @@ const VMStateDescription vmstate_arm_cpu = {
> VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
> VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
> VMSTATE_UINT64(env.pc, ARMCPU),
> +#if defined CONFIG_KVM
> + {
> + .name = "mp_state",
> + .version_id = 0,
> + .size = sizeof(uint32_t),
> + .info = &vmstate_mpstate,
> + .flags = VMS_SINGLE,
> + .offset = 0,
> + },
> +#endif
This means the migration format will be different on different
build configurations, which seems like a really bad idea.
Have you considered having the KVM state sync functions just
sync the kernel's MP state into cpu->powered_off, and then
migrating that flag here unconditionally?
> {
> .name = "cpsr",
> .version_id = 0,
> --
> 2.3.0
>
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 1/6] target-arm: kvm: save/restore mp state
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-02-26 12:57 ` Paolo Bonzini
-1 siblings, 0 replies; 73+ messages in thread
From: Paolo Bonzini @ 2015-02-26 12:57 UTC (permalink / raw)
To: Alex Bennée, qemu-devel; +Cc: kvm, marc.zyngier, linux-arm-kernel, kvmarm
On 25/02/2015 17:02, Alex Bennée wrote:
> +#if defined CONFIG_KVM
> + {
> + .name = "mp_state",
> + .version_id = 0,
> + .size = sizeof(uint32_t),
> + .info = &vmstate_mpstate,
> + .flags = VMS_SINGLE,
> + .offset = 0,
> + },
> +#endif
This makes TCG migration state incompatible depending on whether QEMU
was built on ARM or x86.
You can instead add a subsection, and mark it as needed only iff
kvm_enabled().
Paolo
> {
> .name = "cpsr",
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-02-26 12:57 ` Paolo Bonzini
0 siblings, 0 replies; 73+ messages in thread
From: Paolo Bonzini @ 2015-02-26 12:57 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Peter Maydell, kvm, marc.zyngier, linux-arm-kernel, kvmarm,
christoffer.dall
On 25/02/2015 17:02, Alex Bennée wrote:
> +#if defined CONFIG_KVM
> + {
> + .name = "mp_state",
> + .version_id = 0,
> + .size = sizeof(uint32_t),
> + .info = &vmstate_mpstate,
> + .flags = VMS_SINGLE,
> + .offset = 0,
> + },
> +#endif
This makes TCG migration state incompatible depending on whether QEMU
was built on ARM or x86.
You can instead add a subsection, and mark it as needed only iff
kvm_enabled().
Paolo
> {
> .name = "cpsr",
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-02-26 12:57 ` Paolo Bonzini
0 siblings, 0 replies; 73+ messages in thread
From: Paolo Bonzini @ 2015-02-26 12:57 UTC (permalink / raw)
To: linux-arm-kernel
On 25/02/2015 17:02, Alex Benn?e wrote:
> +#if defined CONFIG_KVM
> + {
> + .name = "mp_state",
> + .version_id = 0,
> + .size = sizeof(uint32_t),
> + .info = &vmstate_mpstate,
> + .flags = VMS_SINGLE,
> + .offset = 0,
> + },
> +#endif
This makes TCG migration state incompatible depending on whether QEMU
was built on ARM or x86.
You can instead add a subsection, and mark it as needed only iff
kvm_enabled().
Paolo
> {
> .name = "cpsr",
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-03-02 17:22 ` Christoffer Dall
-1 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-02 17:22 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, kvm, linux-arm-kernel, kvmarm, marc.zyngier, Peter Maydell
Hi Alex,
Seems like you accidentally sent out two copies of this patch, hopefully
I'm reviewing the right one...
On Wed, Feb 25, 2015 at 04:02:38PM +0000, Alex Bennée wrote:
> From: Christoffer Dall <christoffer.dall@linaro.org>
>
> The current code was negatively indexing the cpu state array and not
> synchronizing banked spsr register state with the current mode's spsr
> state, causing occasional failures with migration.
>
> Some munging is done to take care of the aarch64 mapping and also to
> ensure the most current value of the spsr is updated to the banked
> registers (relevant for KVM<->TCG migration).
>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2 (ajb)
> - minor tweaks and clarifications
>
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index c60e989..1e36b0a 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> uint64_t val;
> int i;
> int ret;
> + unsigned int el;
>
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> @@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> return ret;
> }
>
> + /* Saved Program State Registers
> + *
> + * Before we restore from the banked_spsr[] array we need to
> + * ensure that any modifications to env->spsr are correctly
> + * reflected and map aarch64 exception levels if required.
> + */
> + el = arm_current_el(env);
> + if (is_a64(env) && el > 0) {
> + g_assert(el == 1);
> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
> + env->banked_spsr[1] = env->banked_spsr[0];
> + env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
> + } else {
> + env->banked_spsr[el] = env->spsr;
is this valid if (is_a64(env) && el == 0)) ? I thought that if you're
in el == 0, then env->banked_spsr[x] is the most up-to-date one, not
env->spsr ?
for !is_a64(env) this looks wrong, because of the same as above if el ==
0, but also because I think you need
bank_number(env->uncached_cpsr & CPSR_M) to index into the array.
> + }
> +
> for (i = 0; i < KVM_NR_SPSR; i++) {
> reg.id = AARCH64_CORE_REG(spsr[i]);
> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
> ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> if (ret) {
> return ret;
> @@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
> struct kvm_one_reg reg;
> uint64_t val;
> uint32_t fpr;
> + unsigned int el;
> int i;
> int ret;
>
> @@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
> return ret;
> }
>
> + /* Fetch the SPSR registers
> + *
> + * KVM has an array of state indexed for all the possible aarch32
> + * privilage levels. Although not all are valid at all points
> + * there are some transitions possible which can access old state
> + * so it is worth keeping them all.
> + */
> for (i = 0; i < KVM_NR_SPSR; i++) {
> reg.id = AARCH64_CORE_REG(spsr[i]);
> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
> ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> if (ret) {
> return ret;
> }
> }
>
> + el = arm_current_el(env);
> + if (is_a64(env) && el > 0) {
> + g_assert(el == 1);
> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
> + env->banked_spsr[0] = env->banked_spsr[1];
> + env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
> + } else {
> + env->spsr = env->banked_spsr[el];
same concern with bank_number as above.
> + }
> +
> /* Advanced SIMD and FP registers */
> for (i = 0; i < 32; i++) {
> reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
> --
> 2.3.0
>
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-02 17:22 ` Christoffer Dall
0 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-02 17:22 UTC (permalink / raw)
To: Alex Bennée
Cc: Peter Maydell, kvm, marc.zyngier, qemu-devel, kvmarm, linux-arm-kernel
Hi Alex,
Seems like you accidentally sent out two copies of this patch, hopefully
I'm reviewing the right one...
On Wed, Feb 25, 2015 at 04:02:38PM +0000, Alex Bennée wrote:
> From: Christoffer Dall <christoffer.dall@linaro.org>
>
> The current code was negatively indexing the cpu state array and not
> synchronizing banked spsr register state with the current mode's spsr
> state, causing occasional failures with migration.
>
> Some munging is done to take care of the aarch64 mapping and also to
> ensure the most current value of the spsr is updated to the banked
> registers (relevant for KVM<->TCG migration).
>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2 (ajb)
> - minor tweaks and clarifications
>
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index c60e989..1e36b0a 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> uint64_t val;
> int i;
> int ret;
> + unsigned int el;
>
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> @@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> return ret;
> }
>
> + /* Saved Program State Registers
> + *
> + * Before we restore from the banked_spsr[] array we need to
> + * ensure that any modifications to env->spsr are correctly
> + * reflected and map aarch64 exception levels if required.
> + */
> + el = arm_current_el(env);
> + if (is_a64(env) && el > 0) {
> + g_assert(el == 1);
> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
> + env->banked_spsr[1] = env->banked_spsr[0];
> + env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
> + } else {
> + env->banked_spsr[el] = env->spsr;
is this valid if (is_a64(env) && el == 0)) ? I thought that if you're
in el == 0, then env->banked_spsr[x] is the most up-to-date one, not
env->spsr ?
for !is_a64(env) this looks wrong, because of the same as above if el ==
0, but also because I think you need
bank_number(env->uncached_cpsr & CPSR_M) to index into the array.
> + }
> +
> for (i = 0; i < KVM_NR_SPSR; i++) {
> reg.id = AARCH64_CORE_REG(spsr[i]);
> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
> ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> if (ret) {
> return ret;
> @@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
> struct kvm_one_reg reg;
> uint64_t val;
> uint32_t fpr;
> + unsigned int el;
> int i;
> int ret;
>
> @@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
> return ret;
> }
>
> + /* Fetch the SPSR registers
> + *
> + * KVM has an array of state indexed for all the possible aarch32
> + * privilage levels. Although not all are valid at all points
> + * there are some transitions possible which can access old state
> + * so it is worth keeping them all.
> + */
> for (i = 0; i < KVM_NR_SPSR; i++) {
> reg.id = AARCH64_CORE_REG(spsr[i]);
> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
> ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> if (ret) {
> return ret;
> }
> }
>
> + el = arm_current_el(env);
> + if (is_a64(env) && el > 0) {
> + g_assert(el == 1);
> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
> + env->banked_spsr[0] = env->banked_spsr[1];
> + env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
> + } else {
> + env->spsr = env->banked_spsr[el];
same concern with bank_number as above.
> + }
> +
> /* Advanced SIMD and FP registers */
> for (i = 0; i < 32; i++) {
> reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
> --
> 2.3.0
>
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-02 17:22 ` Christoffer Dall
0 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-02 17:22 UTC (permalink / raw)
To: linux-arm-kernel
Hi Alex,
Seems like you accidentally sent out two copies of this patch, hopefully
I'm reviewing the right one...
On Wed, Feb 25, 2015 at 04:02:38PM +0000, Alex Benn?e wrote:
> From: Christoffer Dall <christoffer.dall@linaro.org>
>
> The current code was negatively indexing the cpu state array and not
> synchronizing banked spsr register state with the current mode's spsr
> state, causing occasional failures with migration.
>
> Some munging is done to take care of the aarch64 mapping and also to
> ensure the most current value of the spsr is updated to the banked
> registers (relevant for KVM<->TCG migration).
>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
>
> ---
> v2 (ajb)
> - minor tweaks and clarifications
>
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index c60e989..1e36b0a 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> uint64_t val;
> int i;
> int ret;
> + unsigned int el;
>
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> @@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> return ret;
> }
>
> + /* Saved Program State Registers
> + *
> + * Before we restore from the banked_spsr[] array we need to
> + * ensure that any modifications to env->spsr are correctly
> + * reflected and map aarch64 exception levels if required.
> + */
> + el = arm_current_el(env);
> + if (is_a64(env) && el > 0) {
> + g_assert(el == 1);
> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
> + env->banked_spsr[1] = env->banked_spsr[0];
> + env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
> + } else {
> + env->banked_spsr[el] = env->spsr;
is this valid if (is_a64(env) && el == 0)) ? I thought that if you're
in el == 0, then env->banked_spsr[x] is the most up-to-date one, not
env->spsr ?
for !is_a64(env) this looks wrong, because of the same as above if el ==
0, but also because I think you need
bank_number(env->uncached_cpsr & CPSR_M) to index into the array.
> + }
> +
> for (i = 0; i < KVM_NR_SPSR; i++) {
> reg.id = AARCH64_CORE_REG(spsr[i]);
> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
> ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> if (ret) {
> return ret;
> @@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
> struct kvm_one_reg reg;
> uint64_t val;
> uint32_t fpr;
> + unsigned int el;
> int i;
> int ret;
>
> @@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
> return ret;
> }
>
> + /* Fetch the SPSR registers
> + *
> + * KVM has an array of state indexed for all the possible aarch32
> + * privilage levels. Although not all are valid at all points
> + * there are some transitions possible which can access old state
> + * so it is worth keeping them all.
> + */
> for (i = 0; i < KVM_NR_SPSR; i++) {
> reg.id = AARCH64_CORE_REG(spsr[i]);
> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
> ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> if (ret) {
> return ret;
> }
> }
>
> + el = arm_current_el(env);
> + if (is_a64(env) && el > 0) {
> + g_assert(el == 1);
> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
> + env->banked_spsr[0] = env->banked_spsr[1];
> + env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
> + } else {
> + env->spsr = env->banked_spsr[el];
same concern with bank_number as above.
> + }
> +
> /* Advanced SIMD and FP registers */
> for (i = 0; i < 32; i++) {
> reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
> --
> 2.3.0
>
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 2/6] arm_gic_kvm.c: restore config before pending IRQs
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-03-02 22:14 ` Christoffer Dall
-1 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-02 22:14 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel, kvm, linux-arm-kernel, kvmarm, marc.zyngier
On Wed, Feb 25, 2015 at 04:02:34PM +0000, Alex Bennée wrote:
> As there is logic to deal with the difference between edge and level
> triggered interrupts in the kernel we must ensure it knows the
> configuration of the IRQs before we restore the pending state.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 2/6] arm_gic_kvm.c: restore config before pending IRQs
@ 2015-03-02 22:14 ` Christoffer Dall
0 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-02 22:14 UTC (permalink / raw)
To: Alex Bennée; +Cc: linux-arm-kernel, marc.zyngier, qemu-devel, kvm, kvmarm
On Wed, Feb 25, 2015 at 04:02:34PM +0000, Alex Bennée wrote:
> As there is logic to deal with the difference between edge and level
> triggered interrupts in the kernel we must ensure it knows the
> configuration of the IRQs before we restore the pending state.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 2/6] arm_gic_kvm.c: restore config before pending IRQs
@ 2015-03-02 22:14 ` Christoffer Dall
0 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-02 22:14 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 25, 2015 at 04:02:34PM +0000, Alex Benn?e wrote:
> As there is logic to deal with the difference between edge and level
> triggered interrupts in the kernel we must ensure it knows the
> configuration of the IRQs before we restore the pending state.
>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 1/6] target-arm: kvm: save/restore mp state
2015-02-25 23:36 ` [Qemu-devel] " Peter Maydell
(?)
@ 2015-03-03 10:56 ` Alex Bennée
-1 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-03 10:56 UTC (permalink / raw)
To: Peter Maydell
Cc: kvm-devel, Marc Zyngier, QEMU Developers, Paolo Bonzini, kvmarm,
arm-mail-list
Peter Maydell <peter.maydell@linaro.org> writes:
> On 26 February 2015 at 01:02, Alex Bennée <alex.bennee@linaro.org> wrote:
>> This adds the saving and restore of the current Multi-Processing state
>> of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
>> potential states for x86 we only use two for ARM. Either the process is
>> running or not.
>
> By this you mean "is the CPU in the PSCI powered down state or not",
> right?
From the vcpu's perspective it is either running or not. However it is
the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
VM, internally setting vcpu->arch.paused.
>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
>> index 23cefe9..8732854 100644
>> --- a/target-arm/kvm.c
>> +++ b/target-arm/kvm.c
>> @@ -25,6 +25,7 @@
>> #include "hw/arm/arm.h"
>>
>> const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>> + KVM_CAP_INFO(MP_STATE),
>
> Does this really want to be a required cap? I haven't checked,
> but assuming 'required' means what it says this presumably
> means we'll stop working on host kernels we previously ran
> fine on (even if migration didn't work there).
You are right, I'll move the CAP check to the kvm_enabled() leg of
get/set functions.
>
>> KVM_CAP_LAST_INFO
>> };
>>
>> diff --git a/target-arm/machine.c b/target-arm/machine.c
>> index 9446e5a..70b1bc4 100644
>> --- a/target-arm/machine.c
>> +++ b/target-arm/machine.c
>> @@ -161,6 +161,34 @@ static const VMStateInfo vmstate_cpsr = {
>> .put = put_cpsr,
>> };
>>
>> +#if defined CONFIG_KVM
>> +static int get_mpstate(QEMUFile *f, void *opaque, size_t size)
>> +{
>> + ARMCPU *cpu = opaque;
>> + struct kvm_mp_state mp_state = { .mp_state = qemu_get_be32(f)};
>> + return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
>> +}
>
> Won't this break if you're running a QEMU built with KVM
> support in TCG mode on an aarch64 host?
>
> In any case, for that configuration we should be migrating the
> TCG cpu->powered_off flag, which is where we keep the PSCI
> power state for TCG.
Yeah, I'll make the access functions aware of the mode. In itself it
won't enable KVM<->TCG migration though!
>
>> +
>> +static void put_mpstate(QEMUFile *f, void *opaque, size_t size)
>> +{
>> + ARMCPU *cpu = opaque;
>> + struct kvm_mp_state mp_state;
>> + int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, &mp_state);
>> + if (ret) {
>> + fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
>> + __func__, ret, strerror(ret));
>> + abort();
>> + }
>> + qemu_put_be32(f, mp_state.mp_state);
>> +}
>> +
>> +static const VMStateInfo vmstate_mpstate = {
>> + .name = "mpstate",
>> + .get = get_mpstate,
>> + .put = put_mpstate,
>> +};
>> +#endif
>> +
>> static void cpu_pre_save(void *opaque)
>> {
>> ARMCPU *cpu = opaque;
>> @@ -244,6 +272,16 @@ const VMStateDescription vmstate_arm_cpu = {
>> VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
>> VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
>> VMSTATE_UINT64(env.pc, ARMCPU),
>> +#if defined CONFIG_KVM
>> + {
>> + .name = "mp_state",
>> + .version_id = 0,
>> + .size = sizeof(uint32_t),
>> + .info = &vmstate_mpstate,
>> + .flags = VMS_SINGLE,
>> + .offset = 0,
>> + },
>> +#endif
>
> This means the migration format will be different on different
> build configurations, which seems like a really bad idea.
>
> Have you considered having the KVM state sync functions just
> sync the kernel's MP state into cpu->powered_off, and then
> migrating that flag here unconditionally?
I was looking at using:
.field_exists = mpstate_valid
for the field. If we have the field in the migration stream and the
kernel doesn't have the capability to set the state we need to fail hard
right?
>
>> {
>> .name = "cpsr",
>> .version_id = 0,
>> --
>> 2.3.0
>>
>
> -- PMM
--
Alex Bennée
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-03-03 10:56 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-03 10:56 UTC (permalink / raw)
To: Peter Maydell
Cc: kvm-devel, Marc Zyngier, QEMU Developers, Christoffer Dall,
Paolo Bonzini, kvmarm, arm-mail-list
Peter Maydell <peter.maydell@linaro.org> writes:
> On 26 February 2015 at 01:02, Alex Bennée <alex.bennee@linaro.org> wrote:
>> This adds the saving and restore of the current Multi-Processing state
>> of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
>> potential states for x86 we only use two for ARM. Either the process is
>> running or not.
>
> By this you mean "is the CPU in the PSCI powered down state or not",
> right?
>From the vcpu's perspective it is either running or not. However it is
the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
VM, internally setting vcpu->arch.paused.
>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
>> index 23cefe9..8732854 100644
>> --- a/target-arm/kvm.c
>> +++ b/target-arm/kvm.c
>> @@ -25,6 +25,7 @@
>> #include "hw/arm/arm.h"
>>
>> const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>> + KVM_CAP_INFO(MP_STATE),
>
> Does this really want to be a required cap? I haven't checked,
> but assuming 'required' means what it says this presumably
> means we'll stop working on host kernels we previously ran
> fine on (even if migration didn't work there).
You are right, I'll move the CAP check to the kvm_enabled() leg of
get/set functions.
>
>> KVM_CAP_LAST_INFO
>> };
>>
>> diff --git a/target-arm/machine.c b/target-arm/machine.c
>> index 9446e5a..70b1bc4 100644
>> --- a/target-arm/machine.c
>> +++ b/target-arm/machine.c
>> @@ -161,6 +161,34 @@ static const VMStateInfo vmstate_cpsr = {
>> .put = put_cpsr,
>> };
>>
>> +#if defined CONFIG_KVM
>> +static int get_mpstate(QEMUFile *f, void *opaque, size_t size)
>> +{
>> + ARMCPU *cpu = opaque;
>> + struct kvm_mp_state mp_state = { .mp_state = qemu_get_be32(f)};
>> + return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
>> +}
>
> Won't this break if you're running a QEMU built with KVM
> support in TCG mode on an aarch64 host?
>
> In any case, for that configuration we should be migrating the
> TCG cpu->powered_off flag, which is where we keep the PSCI
> power state for TCG.
Yeah, I'll make the access functions aware of the mode. In itself it
won't enable KVM<->TCG migration though!
>
>> +
>> +static void put_mpstate(QEMUFile *f, void *opaque, size_t size)
>> +{
>> + ARMCPU *cpu = opaque;
>> + struct kvm_mp_state mp_state;
>> + int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, &mp_state);
>> + if (ret) {
>> + fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
>> + __func__, ret, strerror(ret));
>> + abort();
>> + }
>> + qemu_put_be32(f, mp_state.mp_state);
>> +}
>> +
>> +static const VMStateInfo vmstate_mpstate = {
>> + .name = "mpstate",
>> + .get = get_mpstate,
>> + .put = put_mpstate,
>> +};
>> +#endif
>> +
>> static void cpu_pre_save(void *opaque)
>> {
>> ARMCPU *cpu = opaque;
>> @@ -244,6 +272,16 @@ const VMStateDescription vmstate_arm_cpu = {
>> VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
>> VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
>> VMSTATE_UINT64(env.pc, ARMCPU),
>> +#if defined CONFIG_KVM
>> + {
>> + .name = "mp_state",
>> + .version_id = 0,
>> + .size = sizeof(uint32_t),
>> + .info = &vmstate_mpstate,
>> + .flags = VMS_SINGLE,
>> + .offset = 0,
>> + },
>> +#endif
>
> This means the migration format will be different on different
> build configurations, which seems like a really bad idea.
>
> Have you considered having the KVM state sync functions just
> sync the kernel's MP state into cpu->powered_off, and then
> migrating that flag here unconditionally?
I was looking at using:
.field_exists = mpstate_valid
for the field. If we have the field in the migration stream and the
kernel doesn't have the capability to set the state we need to fail hard
right?
>
>> {
>> .name = "cpsr",
>> .version_id = 0,
>> --
>> 2.3.0
>>
>
> -- PMM
--
Alex Bennée
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-03-03 10:56 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-03 10:56 UTC (permalink / raw)
To: linux-arm-kernel
Peter Maydell <peter.maydell@linaro.org> writes:
> On 26 February 2015 at 01:02, Alex Benn?e <alex.bennee@linaro.org> wrote:
>> This adds the saving and restore of the current Multi-Processing state
>> of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
>> potential states for x86 we only use two for ARM. Either the process is
>> running or not.
>
> By this you mean "is the CPU in the PSCI powered down state or not",
> right?
>From the vcpu's perspective it is either running or not. However it is
the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
VM, internally setting vcpu->arch.paused.
>
>> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
>>
>> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
>> index 23cefe9..8732854 100644
>> --- a/target-arm/kvm.c
>> +++ b/target-arm/kvm.c
>> @@ -25,6 +25,7 @@
>> #include "hw/arm/arm.h"
>>
>> const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>> + KVM_CAP_INFO(MP_STATE),
>
> Does this really want to be a required cap? I haven't checked,
> but assuming 'required' means what it says this presumably
> means we'll stop working on host kernels we previously ran
> fine on (even if migration didn't work there).
You are right, I'll move the CAP check to the kvm_enabled() leg of
get/set functions.
>
>> KVM_CAP_LAST_INFO
>> };
>>
>> diff --git a/target-arm/machine.c b/target-arm/machine.c
>> index 9446e5a..70b1bc4 100644
>> --- a/target-arm/machine.c
>> +++ b/target-arm/machine.c
>> @@ -161,6 +161,34 @@ static const VMStateInfo vmstate_cpsr = {
>> .put = put_cpsr,
>> };
>>
>> +#if defined CONFIG_KVM
>> +static int get_mpstate(QEMUFile *f, void *opaque, size_t size)
>> +{
>> + ARMCPU *cpu = opaque;
>> + struct kvm_mp_state mp_state = { .mp_state = qemu_get_be32(f)};
>> + return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
>> +}
>
> Won't this break if you're running a QEMU built with KVM
> support in TCG mode on an aarch64 host?
>
> In any case, for that configuration we should be migrating the
> TCG cpu->powered_off flag, which is where we keep the PSCI
> power state for TCG.
Yeah, I'll make the access functions aware of the mode. In itself it
won't enable KVM<->TCG migration though!
>
>> +
>> +static void put_mpstate(QEMUFile *f, void *opaque, size_t size)
>> +{
>> + ARMCPU *cpu = opaque;
>> + struct kvm_mp_state mp_state;
>> + int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, &mp_state);
>> + if (ret) {
>> + fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
>> + __func__, ret, strerror(ret));
>> + abort();
>> + }
>> + qemu_put_be32(f, mp_state.mp_state);
>> +}
>> +
>> +static const VMStateInfo vmstate_mpstate = {
>> + .name = "mpstate",
>> + .get = get_mpstate,
>> + .put = put_mpstate,
>> +};
>> +#endif
>> +
>> static void cpu_pre_save(void *opaque)
>> {
>> ARMCPU *cpu = opaque;
>> @@ -244,6 +272,16 @@ const VMStateDescription vmstate_arm_cpu = {
>> VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
>> VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
>> VMSTATE_UINT64(env.pc, ARMCPU),
>> +#if defined CONFIG_KVM
>> + {
>> + .name = "mp_state",
>> + .version_id = 0,
>> + .size = sizeof(uint32_t),
>> + .info = &vmstate_mpstate,
>> + .flags = VMS_SINGLE,
>> + .offset = 0,
>> + },
>> +#endif
>
> This means the migration format will be different on different
> build configurations, which seems like a really bad idea.
>
> Have you considered having the KVM state sync functions just
> sync the kernel's MP state into cpu->powered_off, and then
> migrating that flag here unconditionally?
I was looking at using:
.field_exists = mpstate_valid
for the field. If we have the field in the migration stream and the
kernel doesn't have the capability to set the state we need to fail hard
right?
>
>> {
>> .name = "cpsr",
>> .version_id = 0,
>> --
>> 2.3.0
>>
>
> -- PMM
--
Alex Benn?e
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 1/6] target-arm: kvm: save/restore mp state
2015-03-03 10:56 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-03-03 11:06 ` Paolo Bonzini
-1 siblings, 0 replies; 73+ messages in thread
From: Paolo Bonzini @ 2015-03-03 11:06 UTC (permalink / raw)
To: Alex Bennée, Peter Maydell
Cc: kvm-devel, Marc Zyngier, QEMU Developers, kvmarm, arm-mail-list
On 03/03/2015 11:56, Alex Bennée wrote:
> > > This adds the saving and restore of the current Multi-Processing state
> > > of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
> > > potential states for x86 we only use two for ARM. Either the process is
> > > running or not.
> >
> > By this you mean "is the CPU in the PSCI powered down state or not",
> > right?
>
> From the vcpu's perspective it is either running or not. However it is
> the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
> VM, internally setting vcpu->arch.paused.
I suggest that you define a new MP_STATE constant for this. HALTED in
x86 and s390 is the state an ARM processor enters when you execute wfi.
Right now this is not migrated on ARM if I remember correctly, but
perhaps you'll want to add it in the future.
Paolo
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-03-03 11:06 ` Paolo Bonzini
0 siblings, 0 replies; 73+ messages in thread
From: Paolo Bonzini @ 2015-03-03 11:06 UTC (permalink / raw)
To: Alex Bennée, Peter Maydell
Cc: kvm-devel, Marc Zyngier, QEMU Developers, Christoffer Dall,
kvmarm, arm-mail-list
On 03/03/2015 11:56, Alex Bennée wrote:
> > > This adds the saving and restore of the current Multi-Processing state
> > > of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
> > > potential states for x86 we only use two for ARM. Either the process is
> > > running or not.
> >
> > By this you mean "is the CPU in the PSCI powered down state or not",
> > right?
>
> From the vcpu's perspective it is either running or not. However it is
> the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
> VM, internally setting vcpu->arch.paused.
I suggest that you define a new MP_STATE constant for this. HALTED in
x86 and s390 is the state an ARM processor enters when you execute wfi.
Right now this is not migrated on ARM if I remember correctly, but
perhaps you'll want to add it in the future.
Paolo
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-03-03 11:06 ` Paolo Bonzini
0 siblings, 0 replies; 73+ messages in thread
From: Paolo Bonzini @ 2015-03-03 11:06 UTC (permalink / raw)
To: linux-arm-kernel
On 03/03/2015 11:56, Alex Benn?e wrote:
> > > This adds the saving and restore of the current Multi-Processing state
> > > of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
> > > potential states for x86 we only use two for ARM. Either the process is
> > > running or not.
> >
> > By this you mean "is the CPU in the PSCI powered down state or not",
> > right?
>
> From the vcpu's perspective it is either running or not. However it is
> the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
> VM, internally setting vcpu->arch.paused.
I suggest that you define a new MP_STATE constant for this. HALTED in
x86 and s390 is the state an ARM processor enters when you execute wfi.
Right now this is not migrated on ARM if I remember correctly, but
perhaps you'll want to add it in the future.
Paolo
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
2015-03-02 17:22 ` [Qemu-devel] " Christoffer Dall
(?)
@ 2015-03-03 11:28 ` Alex Bennée
-1 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-03 11:28 UTC (permalink / raw)
To: Christoffer Dall; +Cc: kvm, marc.zyngier, qemu-devel, kvmarm, linux-arm-kernel
Christoffer Dall <christoffer.dall@linaro.org> writes:
> Hi Alex,
>
> Seems like you accidentally sent out two copies of this patch, hopefully
> I'm reviewing the right one...
Oops, perils of not wiping your output directory. I think it was just a
title tweak!
> On Wed, Feb 25, 2015 at 04:02:38PM +0000, Alex Bennée wrote:
>> From: Christoffer Dall <christoffer.dall@linaro.org>
>>
>> The current code was negatively indexing the cpu state array and not
>> synchronizing banked spsr register state with the current mode's spsr
>> state, causing occasional failures with migration.
>>
>> Some munging is done to take care of the aarch64 mapping and also to
>> ensure the most current value of the spsr is updated to the banked
>> registers (relevant for KVM<->TCG migration).
>>
>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> ---
>> v2 (ajb)
>> - minor tweaks and clarifications
>>
>> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
>> index c60e989..1e36b0a 100644
>> --- a/target-arm/kvm64.c
>> +++ b/target-arm/kvm64.c
>> @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>> uint64_t val;
>> int i;
>> int ret;
>> + unsigned int el;
>>
>> ARMCPU *cpu = ARM_CPU(cs);
>> CPUARMState *env = &cpu->env;
>> @@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>> return ret;
>> }
>>
>> + /* Saved Program State Registers
>> + *
>> + * Before we restore from the banked_spsr[] array we need to
>> + * ensure that any modifications to env->spsr are correctly
>> + * reflected and map aarch64 exception levels if required.
>> + */
>> + el = arm_current_el(env);
>> + if (is_a64(env) && el > 0) {
>> + g_assert(el == 1);
>> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
>> + env->banked_spsr[1] = env->banked_spsr[0];
>> + env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
>> + } else {
>> + env->banked_spsr[el] = env->spsr;
>
> is this valid if (is_a64(env) && el == 0)) ? I thought that if you're
> in el == 0, then env->banked_spsr[x] is the most up-to-date one, not
> env->spsr ?
Actually they will both be the same (at least for KVM). In the end both:
VMSTATE_UINT32(env.spsr, ARMCPU),
VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 8),
get serialised in the stream and when we save the stream out we make
sure everything is in sync (i.e. env->spsr is correct). In reality this
makes more sense for TCG than KVM which is the only reason env->spsr
exists.
>
> for !is_a64(env) this looks wrong, because of the same as above if el ==
> 0, but also because I think you need
> bank_number(env->uncached_cpsr & CPSR_M) to index into the array.
>
Good catch. For some reason I was treating the number from
arm_current_el() as equivalent. How about:
el = arm_current_el(env);
if (is_a64(env) && el > 0) {
g_assert(el == 1);
/* KVM only maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 ATM */
env->banked_spsr[1] = env->banked_spsr[0];
}
i = is_a64(env) ?
aarch64_banked_spsr_index(el) : bank_number(env->unached_cpsr & CPSR_M);
env->banked_spsr[i] = env->spsr;
>> + }
>> +
>> for (i = 0; i < KVM_NR_SPSR; i++) {
>> reg.id = AARCH64_CORE_REG(spsr[i]);
>> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
>> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
>> ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
>> if (ret) {
>> return ret;
>> @@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
>> struct kvm_one_reg reg;
>> uint64_t val;
>> uint32_t fpr;
>> + unsigned int el;
>> int i;
>> int ret;
>>
>> @@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
>> return ret;
>> }
>>
>> + /* Fetch the SPSR registers
>> + *
>> + * KVM has an array of state indexed for all the possible aarch32
>> + * privilage levels. Although not all are valid at all points
>> + * there are some transitions possible which can access old state
>> + * so it is worth keeping them all.
>> + */
>> for (i = 0; i < KVM_NR_SPSR; i++) {
>> reg.id = AARCH64_CORE_REG(spsr[i]);
>> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
>> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
>> ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
>> if (ret) {
>> return ret;
>> }
>> }
>>
>> + el = arm_current_el(env);
>> + if (is_a64(env) && el > 0) {
>> + g_assert(el == 1);
>> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
>> + env->banked_spsr[0] = env->banked_spsr[1];
>> + env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
>> + } else {
>> + env->spsr = env->banked_spsr[el];
>
> same concern with bank_number as above.
>
>> + }
>> +
>> /* Advanced SIMD and FP registers */
>> for (i = 0; i < 32; i++) {
>> reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
>> --
>> 2.3.0
>>
>
> Thanks,
> -Christoffer
--
Alex Bennée
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-03 11:28 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-03 11:28 UTC (permalink / raw)
To: Christoffer Dall
Cc: Peter Maydell, kvm, marc.zyngier, qemu-devel, kvmarm, linux-arm-kernel
Christoffer Dall <christoffer.dall@linaro.org> writes:
> Hi Alex,
>
> Seems like you accidentally sent out two copies of this patch, hopefully
> I'm reviewing the right one...
Oops, perils of not wiping your output directory. I think it was just a
title tweak!
> On Wed, Feb 25, 2015 at 04:02:38PM +0000, Alex Bennée wrote:
>> From: Christoffer Dall <christoffer.dall@linaro.org>
>>
>> The current code was negatively indexing the cpu state array and not
>> synchronizing banked spsr register state with the current mode's spsr
>> state, causing occasional failures with migration.
>>
>> Some munging is done to take care of the aarch64 mapping and also to
>> ensure the most current value of the spsr is updated to the banked
>> registers (relevant for KVM<->TCG migration).
>>
>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> ---
>> v2 (ajb)
>> - minor tweaks and clarifications
>>
>> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
>> index c60e989..1e36b0a 100644
>> --- a/target-arm/kvm64.c
>> +++ b/target-arm/kvm64.c
>> @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>> uint64_t val;
>> int i;
>> int ret;
>> + unsigned int el;
>>
>> ARMCPU *cpu = ARM_CPU(cs);
>> CPUARMState *env = &cpu->env;
>> @@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>> return ret;
>> }
>>
>> + /* Saved Program State Registers
>> + *
>> + * Before we restore from the banked_spsr[] array we need to
>> + * ensure that any modifications to env->spsr are correctly
>> + * reflected and map aarch64 exception levels if required.
>> + */
>> + el = arm_current_el(env);
>> + if (is_a64(env) && el > 0) {
>> + g_assert(el == 1);
>> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
>> + env->banked_spsr[1] = env->banked_spsr[0];
>> + env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
>> + } else {
>> + env->banked_spsr[el] = env->spsr;
>
> is this valid if (is_a64(env) && el == 0)) ? I thought that if you're
> in el == 0, then env->banked_spsr[x] is the most up-to-date one, not
> env->spsr ?
Actually they will both be the same (at least for KVM). In the end both:
VMSTATE_UINT32(env.spsr, ARMCPU),
VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 8),
get serialised in the stream and when we save the stream out we make
sure everything is in sync (i.e. env->spsr is correct). In reality this
makes more sense for TCG than KVM which is the only reason env->spsr
exists.
>
> for !is_a64(env) this looks wrong, because of the same as above if el ==
> 0, but also because I think you need
> bank_number(env->uncached_cpsr & CPSR_M) to index into the array.
>
Good catch. For some reason I was treating the number from
arm_current_el() as equivalent. How about:
el = arm_current_el(env);
if (is_a64(env) && el > 0) {
g_assert(el == 1);
/* KVM only maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 ATM */
env->banked_spsr[1] = env->banked_spsr[0];
}
i = is_a64(env) ?
aarch64_banked_spsr_index(el) : bank_number(env->unached_cpsr & CPSR_M);
env->banked_spsr[i] = env->spsr;
>> + }
>> +
>> for (i = 0; i < KVM_NR_SPSR; i++) {
>> reg.id = AARCH64_CORE_REG(spsr[i]);
>> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
>> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
>> ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
>> if (ret) {
>> return ret;
>> @@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
>> struct kvm_one_reg reg;
>> uint64_t val;
>> uint32_t fpr;
>> + unsigned int el;
>> int i;
>> int ret;
>>
>> @@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
>> return ret;
>> }
>>
>> + /* Fetch the SPSR registers
>> + *
>> + * KVM has an array of state indexed for all the possible aarch32
>> + * privilage levels. Although not all are valid at all points
>> + * there are some transitions possible which can access old state
>> + * so it is worth keeping them all.
>> + */
>> for (i = 0; i < KVM_NR_SPSR; i++) {
>> reg.id = AARCH64_CORE_REG(spsr[i]);
>> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
>> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
>> ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
>> if (ret) {
>> return ret;
>> }
>> }
>>
>> + el = arm_current_el(env);
>> + if (is_a64(env) && el > 0) {
>> + g_assert(el == 1);
>> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
>> + env->banked_spsr[0] = env->banked_spsr[1];
>> + env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
>> + } else {
>> + env->spsr = env->banked_spsr[el];
>
> same concern with bank_number as above.
>
>> + }
>> +
>> /* Advanced SIMD and FP registers */
>> for (i = 0; i < 32; i++) {
>> reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
>> --
>> 2.3.0
>>
>
> Thanks,
> -Christoffer
--
Alex Bennée
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-03 11:28 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-03 11:28 UTC (permalink / raw)
To: linux-arm-kernel
Christoffer Dall <christoffer.dall@linaro.org> writes:
> Hi Alex,
>
> Seems like you accidentally sent out two copies of this patch, hopefully
> I'm reviewing the right one...
Oops, perils of not wiping your output directory. I think it was just a
title tweak!
> On Wed, Feb 25, 2015 at 04:02:38PM +0000, Alex Benn?e wrote:
>> From: Christoffer Dall <christoffer.dall@linaro.org>
>>
>> The current code was negatively indexing the cpu state array and not
>> synchronizing banked spsr register state with the current mode's spsr
>> state, causing occasional failures with migration.
>>
>> Some munging is done to take care of the aarch64 mapping and also to
>> ensure the most current value of the spsr is updated to the banked
>> registers (relevant for KVM<->TCG migration).
>>
>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
>> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
>>
>> ---
>> v2 (ajb)
>> - minor tweaks and clarifications
>>
>> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
>> index c60e989..1e36b0a 100644
>> --- a/target-arm/kvm64.c
>> +++ b/target-arm/kvm64.c
>> @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>> uint64_t val;
>> int i;
>> int ret;
>> + unsigned int el;
>>
>> ARMCPU *cpu = ARM_CPU(cs);
>> CPUARMState *env = &cpu->env;
>> @@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>> return ret;
>> }
>>
>> + /* Saved Program State Registers
>> + *
>> + * Before we restore from the banked_spsr[] array we need to
>> + * ensure that any modifications to env->spsr are correctly
>> + * reflected and map aarch64 exception levels if required.
>> + */
>> + el = arm_current_el(env);
>> + if (is_a64(env) && el > 0) {
>> + g_assert(el == 1);
>> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
>> + env->banked_spsr[1] = env->banked_spsr[0];
>> + env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
>> + } else {
>> + env->banked_spsr[el] = env->spsr;
>
> is this valid if (is_a64(env) && el == 0)) ? I thought that if you're
> in el == 0, then env->banked_spsr[x] is the most up-to-date one, not
> env->spsr ?
Actually they will both be the same (at least for KVM). In the end both:
VMSTATE_UINT32(env.spsr, ARMCPU),
VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 8),
get serialised in the stream and when we save the stream out we make
sure everything is in sync (i.e. env->spsr is correct). In reality this
makes more sense for TCG than KVM which is the only reason env->spsr
exists.
>
> for !is_a64(env) this looks wrong, because of the same as above if el ==
> 0, but also because I think you need
> bank_number(env->uncached_cpsr & CPSR_M) to index into the array.
>
Good catch. For some reason I was treating the number from
arm_current_el() as equivalent. How about:
el = arm_current_el(env);
if (is_a64(env) && el > 0) {
g_assert(el == 1);
/* KVM only maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 ATM */
env->banked_spsr[1] = env->banked_spsr[0];
}
i = is_a64(env) ?
aarch64_banked_spsr_index(el) : bank_number(env->unached_cpsr & CPSR_M);
env->banked_spsr[i] = env->spsr;
>> + }
>> +
>> for (i = 0; i < KVM_NR_SPSR; i++) {
>> reg.id = AARCH64_CORE_REG(spsr[i]);
>> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
>> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
>> ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
>> if (ret) {
>> return ret;
>> @@ -253,6 +270,7 @@ int kvm_arch_get_registers(CPUState *cs)
>> struct kvm_one_reg reg;
>> uint64_t val;
>> uint32_t fpr;
>> + unsigned int el;
>> int i;
>> int ret;
>>
>> @@ -325,15 +343,32 @@ int kvm_arch_get_registers(CPUState *cs)
>> return ret;
>> }
>>
>> + /* Fetch the SPSR registers
>> + *
>> + * KVM has an array of state indexed for all the possible aarch32
>> + * privilage levels. Although not all are valid at all points
>> + * there are some transitions possible which can access old state
>> + * so it is worth keeping them all.
>> + */
>> for (i = 0; i < KVM_NR_SPSR; i++) {
>> reg.id = AARCH64_CORE_REG(spsr[i]);
>> - reg.addr = (uintptr_t) &env->banked_spsr[i - 1];
>> + reg.addr = (uintptr_t) &env->banked_spsr[i+1];
>> ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
>> if (ret) {
>> return ret;
>> }
>> }
>>
>> + el = arm_current_el(env);
>> + if (is_a64(env) && el > 0) {
>> + g_assert(el == 1);
>> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
>> + env->banked_spsr[0] = env->banked_spsr[1];
>> + env->spsr = env->banked_spsr[aarch64_banked_spsr_index(el)];
>> + } else {
>> + env->spsr = env->banked_spsr[el];
>
> same concern with bank_number as above.
>
>> + }
>> +
>> /* Advanced SIMD and FP registers */
>> for (i = 0; i < 32; i++) {
>> reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
>> --
>> 2.3.0
>>
>
> Thanks,
> -Christoffer
--
Alex Benn?e
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 1/6] target-arm: kvm: save/restore mp state
2015-03-03 11:06 ` [Qemu-devel] " Paolo Bonzini
(?)
@ 2015-03-03 11:51 ` Peter Maydell
-1 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-03-03 11:51 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Alex Bennée, QEMU Developers, kvm-devel, arm-mail-list,
kvmarm, Christoffer Dall, Marc Zyngier
On 3 March 2015 at 20:06, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 03/03/2015 11:56, Alex Bennée wrote:
>> > > This adds the saving and restore of the current Multi-Processing state
>> > > of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
>> > > potential states for x86 we only use two for ARM. Either the process is
>> > > running or not.
>> >
>> > By this you mean "is the CPU in the PSCI powered down state or not",
>> > right?
>>
>> From the vcpu's perspective it is either running or not. However it is
>> the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
>> VM, internally setting vcpu->arch.paused.
Well, it has to be (ABI defined to be) identical with being PSCI
powered down/up, because that's how userspace is going to be
treating it. If we might tell userspace we're in the "not running"
state for other cases than PSCI-powered-down then we probably need
to consider separating those out into separate states.
> I suggest that you define a new MP_STATE constant for this. HALTED in
> x86 and s390 is the state an ARM processor enters when you execute wfi.
Architecturally the CPU doesn't have to enter any state at all
if you execute a WFI -- it might be implemented as "go to low
power state and wait for an interrupt" or "go low power but
maybe be unnecessarily woken up" or "nop, do nothing"...
> Right now this is not migrated on ARM if I remember correctly, but
> perhaps you'll want to add it in the future.
...which is why we don't need to migrate this: it just means
that migration during WFI causes an unnecessary-wakeup, which
is architecturally fine.
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-03-03 11:51 ` Peter Maydell
0 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-03-03 11:51 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kvm-devel, Marc Zyngier, QEMU Developers, Christoffer Dall,
Alex Bennée, kvmarm, arm-mail-list
On 3 March 2015 at 20:06, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 03/03/2015 11:56, Alex Bennée wrote:
>> > > This adds the saving and restore of the current Multi-Processing state
>> > > of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
>> > > potential states for x86 we only use two for ARM. Either the process is
>> > > running or not.
>> >
>> > By this you mean "is the CPU in the PSCI powered down state or not",
>> > right?
>>
>> From the vcpu's perspective it is either running or not. However it is
>> the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
>> VM, internally setting vcpu->arch.paused.
Well, it has to be (ABI defined to be) identical with being PSCI
powered down/up, because that's how userspace is going to be
treating it. If we might tell userspace we're in the "not running"
state for other cases than PSCI-powered-down then we probably need
to consider separating those out into separate states.
> I suggest that you define a new MP_STATE constant for this. HALTED in
> x86 and s390 is the state an ARM processor enters when you execute wfi.
Architecturally the CPU doesn't have to enter any state at all
if you execute a WFI -- it might be implemented as "go to low
power state and wait for an interrupt" or "go low power but
maybe be unnecessarily woken up" or "nop, do nothing"...
> Right now this is not migrated on ARM if I remember correctly, but
> perhaps you'll want to add it in the future.
...which is why we don't need to migrate this: it just means
that migration during WFI causes an unnecessary-wakeup, which
is architecturally fine.
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-03-03 11:51 ` Peter Maydell
0 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-03-03 11:51 UTC (permalink / raw)
To: linux-arm-kernel
On 3 March 2015 at 20:06, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 03/03/2015 11:56, Alex Benn?e wrote:
>> > > This adds the saving and restore of the current Multi-Processing state
>> > > of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
>> > > potential states for x86 we only use two for ARM. Either the process is
>> > > running or not.
>> >
>> > By this you mean "is the CPU in the PSCI powered down state or not",
>> > right?
>>
>> From the vcpu's perspective it is either running or not. However it is
>> the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
>> VM, internally setting vcpu->arch.paused.
Well, it has to be (ABI defined to be) identical with being PSCI
powered down/up, because that's how userspace is going to be
treating it. If we might tell userspace we're in the "not running"
state for other cases than PSCI-powered-down then we probably need
to consider separating those out into separate states.
> I suggest that you define a new MP_STATE constant for this. HALTED in
> x86 and s390 is the state an ARM processor enters when you execute wfi.
Architecturally the CPU doesn't have to enter any state at all
if you execute a WFI -- it might be implemented as "go to low
power state and wait for an interrupt" or "go low power but
maybe be unnecessarily woken up" or "nop, do nothing"...
> Right now this is not migrated on ARM if I remember correctly, but
> perhaps you'll want to add it in the future.
...which is why we don't need to migrate this: it just means
that migration during WFI causes an unnecessary-wakeup, which
is architecturally fine.
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 1/6] target-arm: kvm: save/restore mp state
2015-03-03 11:51 ` [Qemu-devel] " Peter Maydell
(?)
@ 2015-03-03 16:30 ` Alex Bennée
-1 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-03 16:30 UTC (permalink / raw)
To: Peter Maydell
Cc: kvm-devel, Marc Zyngier, QEMU Developers, Paolo Bonzini, kvmarm,
arm-mail-list
Peter Maydell <peter.maydell@linaro.org> writes:
> On 3 March 2015 at 20:06, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 03/03/2015 11:56, Alex Bennée wrote:
>>> > > This adds the saving and restore of the current Multi-Processing state
>>> > > of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
>>> > > potential states for x86 we only use two for ARM. Either the process is
>>> > > running or not.
>>> >
>>> > By this you mean "is the CPU in the PSCI powered down state or not",
>>> > right?
>>>
>>> From the vcpu's perspective it is either running or not. However it is
>>> the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
>>> VM, internally setting vcpu->arch.paused.
>
> Well, it has to be (ABI defined to be) identical with being PSCI
> powered down/up, because that's how userspace is going to be
> treating it. If we might tell userspace we're in the "not running"
> state for other cases than PSCI-powered-down then we probably need
> to consider separating those out into separate states.
>
>> I suggest that you define a new MP_STATE constant for this. HALTED in
>> x86 and s390 is the state an ARM processor enters when you execute wfi.
>
> Architecturally the CPU doesn't have to enter any state at all
> if you execute a WFI -- it might be implemented as "go to low
> power state and wait for an interrupt" or "go low power but
> maybe be unnecessarily woken up" or "nop, do nothing"...
>
>> Right now this is not migrated on ARM if I remember correctly, but
>> perhaps you'll want to add it in the future.
>
> ...which is why we don't need to migrate this: it just means
> that migration during WFI causes an unnecessary-wakeup, which
> is architecturally fine.
What happens when you boot a SMP system but only ever power up one of the
CPUs? You can't just randomly start the second CPU if it's in the
powered off state, who knows what it would do?
--
Alex Bennée
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-03-03 16:30 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-03 16:30 UTC (permalink / raw)
To: Peter Maydell
Cc: kvm-devel, Marc Zyngier, QEMU Developers, Christoffer Dall,
Paolo Bonzini, kvmarm, arm-mail-list
Peter Maydell <peter.maydell@linaro.org> writes:
> On 3 March 2015 at 20:06, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 03/03/2015 11:56, Alex Bennée wrote:
>>> > > This adds the saving and restore of the current Multi-Processing state
>>> > > of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
>>> > > potential states for x86 we only use two for ARM. Either the process is
>>> > > running or not.
>>> >
>>> > By this you mean "is the CPU in the PSCI powered down state or not",
>>> > right?
>>>
>>> From the vcpu's perspective it is either running or not. However it is
>>> the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
>>> VM, internally setting vcpu->arch.paused.
>
> Well, it has to be (ABI defined to be) identical with being PSCI
> powered down/up, because that's how userspace is going to be
> treating it. If we might tell userspace we're in the "not running"
> state for other cases than PSCI-powered-down then we probably need
> to consider separating those out into separate states.
>
>> I suggest that you define a new MP_STATE constant for this. HALTED in
>> x86 and s390 is the state an ARM processor enters when you execute wfi.
>
> Architecturally the CPU doesn't have to enter any state at all
> if you execute a WFI -- it might be implemented as "go to low
> power state and wait for an interrupt" or "go low power but
> maybe be unnecessarily woken up" or "nop, do nothing"...
>
>> Right now this is not migrated on ARM if I remember correctly, but
>> perhaps you'll want to add it in the future.
>
> ...which is why we don't need to migrate this: it just means
> that migration during WFI causes an unnecessary-wakeup, which
> is architecturally fine.
What happens when you boot a SMP system but only ever power up one of the
CPUs? You can't just randomly start the second CPU if it's in the
powered off state, who knows what it would do?
--
Alex Bennée
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-03-03 16:30 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-03 16:30 UTC (permalink / raw)
To: linux-arm-kernel
Peter Maydell <peter.maydell@linaro.org> writes:
> On 3 March 2015 at 20:06, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 03/03/2015 11:56, Alex Benn?e wrote:
>>> > > This adds the saving and restore of the current Multi-Processing state
>>> > > of the machine. While the KVM_GET/SET_MP_STATE API exposes a number of
>>> > > potential states for x86 we only use two for ARM. Either the process is
>>> > > running or not.
>>> >
>>> > By this you mean "is the CPU in the PSCI powered down state or not",
>>> > right?
>>>
>>> From the vcpu's perspective it is either running or not. However it is
>>> the same mechanism that is used when PSCI_0_2_FN_CPU_OFF is passed the
>>> VM, internally setting vcpu->arch.paused.
>
> Well, it has to be (ABI defined to be) identical with being PSCI
> powered down/up, because that's how userspace is going to be
> treating it. If we might tell userspace we're in the "not running"
> state for other cases than PSCI-powered-down then we probably need
> to consider separating those out into separate states.
>
>> I suggest that you define a new MP_STATE constant for this. HALTED in
>> x86 and s390 is the state an ARM processor enters when you execute wfi.
>
> Architecturally the CPU doesn't have to enter any state at all
> if you execute a WFI -- it might be implemented as "go to low
> power state and wait for an interrupt" or "go low power but
> maybe be unnecessarily woken up" or "nop, do nothing"...
>
>> Right now this is not migrated on ARM if I remember correctly, but
>> perhaps you'll want to add it in the future.
>
> ...which is why we don't need to migrate this: it just means
> that migration during WFI causes an unnecessary-wakeup, which
> is architecturally fine.
What happens when you boot a SMP system but only ever power up one of the
CPUs? You can't just randomly start the second CPU if it's in the
powered off state, who knows what it would do?
--
Alex Benn?e
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 1/6] target-arm: kvm: save/restore mp state
2015-03-03 16:30 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-03-03 17:10 ` Paolo Bonzini
-1 siblings, 0 replies; 73+ messages in thread
From: Paolo Bonzini @ 2015-03-03 17:10 UTC (permalink / raw)
To: Alex Bennée, Peter Maydell
Cc: kvm-devel, Marc Zyngier, QEMU Developers, kvmarm, arm-mail-list
On 03/03/2015 17:30, Alex Bennée wrote:
>> >
>>> >> Right now this is not migrated on ARM if I remember correctly, but
>>> >> perhaps you'll want to add it in the future.
>> >
>> > ...which is why we don't need to migrate this: it just means
>> > that migration during WFI causes an unnecessary-wakeup, which
>> > is architecturally fine.
> What happens when you boot a SMP system but only ever power up one of the
> CPUs? You can't just randomly start the second CPU if it's in the
> powered off state, who knows what it would do?
The second CPU would not be in the WFI state, which is what Peter is
talking about.
I agree that this state should be saved/restored. I'm just saying that
HALTED is not the right constant to use.
Paolo
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-03-03 17:10 ` Paolo Bonzini
0 siblings, 0 replies; 73+ messages in thread
From: Paolo Bonzini @ 2015-03-03 17:10 UTC (permalink / raw)
To: Alex Bennée, Peter Maydell
Cc: Marc Zyngier, arm-mail-list, QEMU Developers, kvm-devel, kvmarm
On 03/03/2015 17:30, Alex Bennée wrote:
>> >
>>> >> Right now this is not migrated on ARM if I remember correctly, but
>>> >> perhaps you'll want to add it in the future.
>> >
>> > ...which is why we don't need to migrate this: it just means
>> > that migration during WFI causes an unnecessary-wakeup, which
>> > is architecturally fine.
> What happens when you boot a SMP system but only ever power up one of the
> CPUs? You can't just randomly start the second CPU if it's in the
> powered off state, who knows what it would do?
The second CPU would not be in the WFI state, which is what Peter is
talking about.
I agree that this state should be saved/restored. I'm just saying that
HALTED is not the right constant to use.
Paolo
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 1/6] target-arm: kvm: save/restore mp state
@ 2015-03-03 17:10 ` Paolo Bonzini
0 siblings, 0 replies; 73+ messages in thread
From: Paolo Bonzini @ 2015-03-03 17:10 UTC (permalink / raw)
To: linux-arm-kernel
On 03/03/2015 17:30, Alex Benn?e wrote:
>> >
>>> >> Right now this is not migrated on ARM if I remember correctly, but
>>> >> perhaps you'll want to add it in the future.
>> >
>> > ...which is why we don't need to migrate this: it just means
>> > that migration during WFI causes an unnecessary-wakeup, which
>> > is architecturally fine.
> What happens when you boot a SMP system but only ever power up one of the
> CPUs? You can't just randomly start the second CPU if it's in the
> powered off state, who knows what it would do?
The second CPU would not be in the WFI state, which is what Peter is
talking about.
I agree that this state should be saved/restored. I'm just saying that
HALTED is not the right constant to use.
Paolo
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
2015-03-03 11:28 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-03-09 12:56 ` Christoffer Dall
-1 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-09 12:56 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, kvm, linux-arm-kernel, kvmarm, marc.zyngier, Peter Maydell
On Tue, Mar 03, 2015 at 11:28:21AM +0000, Alex Bennée wrote:
>
> Christoffer Dall <christoffer.dall@linaro.org> writes:
>
> > Hi Alex,
> >
> > Seems like you accidentally sent out two copies of this patch, hopefully
> > I'm reviewing the right one...
>
> Oops, perils of not wiping your output directory. I think it was just a
> title tweak!
>
> > On Wed, Feb 25, 2015 at 04:02:38PM +0000, Alex Bennée wrote:
> >> From: Christoffer Dall <christoffer.dall@linaro.org>
> >>
> >> The current code was negatively indexing the cpu state array and not
> >> synchronizing banked spsr register state with the current mode's spsr
> >> state, causing occasional failures with migration.
> >>
> >> Some munging is done to take care of the aarch64 mapping and also to
> >> ensure the most current value of the spsr is updated to the banked
> >> registers (relevant for KVM<->TCG migration).
> >>
> >> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >>
> >> ---
> >> v2 (ajb)
> >> - minor tweaks and clarifications
> >>
> >> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> >> index c60e989..1e36b0a 100644
> >> --- a/target-arm/kvm64.c
> >> +++ b/target-arm/kvm64.c
> >> @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> >> uint64_t val;
> >> int i;
> >> int ret;
> >> + unsigned int el;
> >>
> >> ARMCPU *cpu = ARM_CPU(cs);
> >> CPUARMState *env = &cpu->env;
> >> @@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> >> return ret;
> >> }
> >>
> >> + /* Saved Program State Registers
> >> + *
> >> + * Before we restore from the banked_spsr[] array we need to
> >> + * ensure that any modifications to env->spsr are correctly
> >> + * reflected and map aarch64 exception levels if required.
> >> + */
> >> + el = arm_current_el(env);
> >> + if (is_a64(env) && el > 0) {
> >> + g_assert(el == 1);
> >> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
> >> + env->banked_spsr[1] = env->banked_spsr[0];
> >> + env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
> >> + } else {
> >> + env->banked_spsr[el] = env->spsr;
> >
> > is this valid if (is_a64(env) && el == 0)) ? I thought that if you're
> > in el == 0, then env->banked_spsr[x] is the most up-to-date one, not
> > env->spsr ?
>
> Actually they will both be the same (at least for KVM). In the end both:
>
> VMSTATE_UINT32(env.spsr, ARMCPU),
> VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 8),
>
> get serialised in the stream and when we save the stream out we make
> sure everything is in sync (i.e. env->spsr is correct). In reality this
> makes more sense for TCG than KVM which is the only reason env->spsr
> exists.
>
this function, however, is not used only when migration, but should
generally cover the case where you want to synchronize QEMU's state into
KVM's state, right? So while it may not be harmful in currently
supported use cases, is there ever a situation where (is_a64(env) && el
== 0) and env->spsr != banked_spsr[el], and where env->spsr is
out-of-date?
If that's the case, I think it would be better to have an assert that
says "don't call the code in this situation" than relying on limited use
cases for callers of this function.
> >
> > for !is_a64(env) this looks wrong, because of the same as above if el ==
> > 0, but also because I think you need
> > bank_number(env->uncached_cpsr & CPSR_M) to index into the array.
> >
>
> Good catch. For some reason I was treating the number from
> arm_current_el() as equivalent. How about:
>
> el = arm_current_el(env);
> if (is_a64(env) && el > 0) {
> g_assert(el == 1);
> /* KVM only maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 ATM */
> env->banked_spsr[1] = env->banked_spsr[0];
> }
> i = is_a64(env) ?
> aarch64_banked_spsr_index(el) : bank_number(env->unached_cpsr & CPSR_M);
I think that will fail due to the assert in aarch64_banked_spsr_index()
for el == 0.
-Christoffer
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-09 12:56 ` Christoffer Dall
0 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-09 12:56 UTC (permalink / raw)
To: Alex Bennée
Cc: Peter Maydell, kvm, marc.zyngier, qemu-devel, kvmarm, linux-arm-kernel
On Tue, Mar 03, 2015 at 11:28:21AM +0000, Alex Bennée wrote:
>
> Christoffer Dall <christoffer.dall@linaro.org> writes:
>
> > Hi Alex,
> >
> > Seems like you accidentally sent out two copies of this patch, hopefully
> > I'm reviewing the right one...
>
> Oops, perils of not wiping your output directory. I think it was just a
> title tweak!
>
> > On Wed, Feb 25, 2015 at 04:02:38PM +0000, Alex Bennée wrote:
> >> From: Christoffer Dall <christoffer.dall@linaro.org>
> >>
> >> The current code was negatively indexing the cpu state array and not
> >> synchronizing banked spsr register state with the current mode's spsr
> >> state, causing occasional failures with migration.
> >>
> >> Some munging is done to take care of the aarch64 mapping and also to
> >> ensure the most current value of the spsr is updated to the banked
> >> registers (relevant for KVM<->TCG migration).
> >>
> >> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >>
> >> ---
> >> v2 (ajb)
> >> - minor tweaks and clarifications
> >>
> >> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> >> index c60e989..1e36b0a 100644
> >> --- a/target-arm/kvm64.c
> >> +++ b/target-arm/kvm64.c
> >> @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> >> uint64_t val;
> >> int i;
> >> int ret;
> >> + unsigned int el;
> >>
> >> ARMCPU *cpu = ARM_CPU(cs);
> >> CPUARMState *env = &cpu->env;
> >> @@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> >> return ret;
> >> }
> >>
> >> + /* Saved Program State Registers
> >> + *
> >> + * Before we restore from the banked_spsr[] array we need to
> >> + * ensure that any modifications to env->spsr are correctly
> >> + * reflected and map aarch64 exception levels if required.
> >> + */
> >> + el = arm_current_el(env);
> >> + if (is_a64(env) && el > 0) {
> >> + g_assert(el == 1);
> >> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
> >> + env->banked_spsr[1] = env->banked_spsr[0];
> >> + env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
> >> + } else {
> >> + env->banked_spsr[el] = env->spsr;
> >
> > is this valid if (is_a64(env) && el == 0)) ? I thought that if you're
> > in el == 0, then env->banked_spsr[x] is the most up-to-date one, not
> > env->spsr ?
>
> Actually they will both be the same (at least for KVM). In the end both:
>
> VMSTATE_UINT32(env.spsr, ARMCPU),
> VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 8),
>
> get serialised in the stream and when we save the stream out we make
> sure everything is in sync (i.e. env->spsr is correct). In reality this
> makes more sense for TCG than KVM which is the only reason env->spsr
> exists.
>
this function, however, is not used only when migration, but should
generally cover the case where you want to synchronize QEMU's state into
KVM's state, right? So while it may not be harmful in currently
supported use cases, is there ever a situation where (is_a64(env) && el
== 0) and env->spsr != banked_spsr[el], and where env->spsr is
out-of-date?
If that's the case, I think it would be better to have an assert that
says "don't call the code in this situation" than relying on limited use
cases for callers of this function.
> >
> > for !is_a64(env) this looks wrong, because of the same as above if el ==
> > 0, but also because I think you need
> > bank_number(env->uncached_cpsr & CPSR_M) to index into the array.
> >
>
> Good catch. For some reason I was treating the number from
> arm_current_el() as equivalent. How about:
>
> el = arm_current_el(env);
> if (is_a64(env) && el > 0) {
> g_assert(el == 1);
> /* KVM only maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 ATM */
> env->banked_spsr[1] = env->banked_spsr[0];
> }
> i = is_a64(env) ?
> aarch64_banked_spsr_index(el) : bank_number(env->unached_cpsr & CPSR_M);
I think that will fail due to the assert in aarch64_banked_spsr_index()
for el == 0.
-Christoffer
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-09 12:56 ` Christoffer Dall
0 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-09 12:56 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 03, 2015 at 11:28:21AM +0000, Alex Benn?e wrote:
>
> Christoffer Dall <christoffer.dall@linaro.org> writes:
>
> > Hi Alex,
> >
> > Seems like you accidentally sent out two copies of this patch, hopefully
> > I'm reviewing the right one...
>
> Oops, perils of not wiping your output directory. I think it was just a
> title tweak!
>
> > On Wed, Feb 25, 2015 at 04:02:38PM +0000, Alex Benn?e wrote:
> >> From: Christoffer Dall <christoffer.dall@linaro.org>
> >>
> >> The current code was negatively indexing the cpu state array and not
> >> synchronizing banked spsr register state with the current mode's spsr
> >> state, causing occasional failures with migration.
> >>
> >> Some munging is done to take care of the aarch64 mapping and also to
> >> ensure the most current value of the spsr is updated to the banked
> >> registers (relevant for KVM<->TCG migration).
> >>
> >> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> >> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
> >>
> >> ---
> >> v2 (ajb)
> >> - minor tweaks and clarifications
> >>
> >> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> >> index c60e989..1e36b0a 100644
> >> --- a/target-arm/kvm64.c
> >> +++ b/target-arm/kvm64.c
> >> @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> >> uint64_t val;
> >> int i;
> >> int ret;
> >> + unsigned int el;
> >>
> >> ARMCPU *cpu = ARM_CPU(cs);
> >> CPUARMState *env = &cpu->env;
> >> @@ -206,9 +207,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> >> return ret;
> >> }
> >>
> >> + /* Saved Program State Registers
> >> + *
> >> + * Before we restore from the banked_spsr[] array we need to
> >> + * ensure that any modifications to env->spsr are correctly
> >> + * reflected and map aarch64 exception levels if required.
> >> + */
> >> + el = arm_current_el(env);
> >> + if (is_a64(env) && el > 0) {
> >> + g_assert(el == 1);
> >> + /* KVM maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 */
> >> + env->banked_spsr[1] = env->banked_spsr[0];
> >> + env->banked_spsr[aarch64_banked_spsr_index(el)] = env->spsr;
> >> + } else {
> >> + env->banked_spsr[el] = env->spsr;
> >
> > is this valid if (is_a64(env) && el == 0)) ? I thought that if you're
> > in el == 0, then env->banked_spsr[x] is the most up-to-date one, not
> > env->spsr ?
>
> Actually they will both be the same (at least for KVM). In the end both:
>
> VMSTATE_UINT32(env.spsr, ARMCPU),
> VMSTATE_UINT64_ARRAY(env.banked_spsr, ARMCPU, 8),
>
> get serialised in the stream and when we save the stream out we make
> sure everything is in sync (i.e. env->spsr is correct). In reality this
> makes more sense for TCG than KVM which is the only reason env->spsr
> exists.
>
this function, however, is not used only when migration, but should
generally cover the case where you want to synchronize QEMU's state into
KVM's state, right? So while it may not be harmful in currently
supported use cases, is there ever a situation where (is_a64(env) && el
== 0) and env->spsr != banked_spsr[el], and where env->spsr is
out-of-date?
If that's the case, I think it would be better to have an assert that
says "don't call the code in this situation" than relying on limited use
cases for callers of this function.
> >
> > for !is_a64(env) this looks wrong, because of the same as above if el ==
> > 0, but also because I think you need
> > bank_number(env->uncached_cpsr & CPSR_M) to index into the array.
> >
>
> Good catch. For some reason I was treating the number from
> arm_current_el() as equivalent. How about:
>
> el = arm_current_el(env);
> if (is_a64(env) && el > 0) {
> g_assert(el == 1);
> /* KVM only maps KVM_SPSR_SVC to KVM_SPSR_EL1 for aarch64 ATM */
> env->banked_spsr[1] = env->banked_spsr[0];
> }
> i = is_a64(env) ?
> aarch64_banked_spsr_index(el) : bank_number(env->unached_cpsr & CPSR_M);
I think that will fail due to the assert in aarch64_banked_spsr_index()
for el == 0.
-Christoffer
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
2015-03-09 12:56 ` [Qemu-devel] " Christoffer Dall
(?)
@ 2015-03-09 13:31 ` Peter Maydell
-1 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-03-09 13:31 UTC (permalink / raw)
To: Christoffer Dall
Cc: Alex Bennée, QEMU Developers, kvm-devel, arm-mail-list,
kvmarm, Marc Zyngier
On 9 March 2015 at 21:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> this function, however, is not used only when migration, but should
> generally cover the case where you want to synchronize QEMU's state into
> KVM's state, right? So while it may not be harmful in currently
> supported use cases, is there ever a situation where (is_a64(env) && el
> == 0) and env->spsr != banked_spsr[el], and where env->spsr is
> out-of-date?
If EL == 0 then you can't access any SPSR, so env->spsr is by
definition out of date.
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-09 13:31 ` Peter Maydell
0 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-03-09 13:31 UTC (permalink / raw)
To: Christoffer Dall
Cc: kvm-devel, Marc Zyngier, QEMU Developers, Alex Bennée,
kvmarm, arm-mail-list
On 9 March 2015 at 21:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> this function, however, is not used only when migration, but should
> generally cover the case where you want to synchronize QEMU's state into
> KVM's state, right? So while it may not be harmful in currently
> supported use cases, is there ever a situation where (is_a64(env) && el
> == 0) and env->spsr != banked_spsr[el], and where env->spsr is
> out-of-date?
If EL == 0 then you can't access any SPSR, so env->spsr is by
definition out of date.
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-09 13:31 ` Peter Maydell
0 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-03-09 13:31 UTC (permalink / raw)
To: linux-arm-kernel
On 9 March 2015 at 21:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> this function, however, is not used only when migration, but should
> generally cover the case where you want to synchronize QEMU's state into
> KVM's state, right? So while it may not be harmful in currently
> supported use cases, is there ever a situation where (is_a64(env) && el
> == 0) and env->spsr != banked_spsr[el], and where env->spsr is
> out-of-date?
If EL == 0 then you can't access any SPSR, so env->spsr is by
definition out of date.
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
2015-03-09 13:31 ` [Qemu-devel] " Peter Maydell
(?)
(?)
@ 2015-03-09 16:25 ` Alex Bennée
-1 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-09 16:25 UTC (permalink / raw)
To: Peter Maydell
Cc: Christoffer Dall, QEMU Developers, kvm-devel, arm-mail-list,
kvmarm, Marc Zyngier
Peter Maydell <peter.maydell@linaro.org> writes:
> On 9 March 2015 at 21:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>> this function, however, is not used only when migration, but should
>> generally cover the case where you want to synchronize QEMU's state into
>> KVM's state, right? So while it may not be harmful in currently
>> supported use cases, is there ever a situation where (is_a64(env) && el
>> == 0) and env->spsr != banked_spsr[el], and where env->spsr is
>> out-of-date?
>
> If EL == 0 then you can't access any SPSR, so env->spsr is by
> definition out of date.
Indeed and in v2 the whole thing is wrapped in if (el > 0)
>
> -- PMM
--
Alex Bennée
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-09 16:25 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-09 16:25 UTC (permalink / raw)
To: Peter Maydell
Cc: kvm-devel, Marc Zyngier, QEMU Developers, Christoffer Dall,
kvmarm, arm-mail-list
Peter Maydell <peter.maydell@linaro.org> writes:
> On 9 March 2015 at 21:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>> this function, however, is not used only when migration, but should
>> generally cover the case where you want to synchronize QEMU's state into
>> KVM's state, right? So while it may not be harmful in currently
>> supported use cases, is there ever a situation where (is_a64(env) && el
>> == 0) and env->spsr != banked_spsr[el], and where env->spsr is
>> out-of-date?
>
> If EL == 0 then you can't access any SPSR, so env->spsr is by
> definition out of date.
Indeed and in v2 the whole thing is wrapped in if (el > 0)
>
> -- PMM
--
Alex Bennée
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-09 16:25 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-09 16:25 UTC (permalink / raw)
To: Peter Maydell
Cc: Christoffer Dall, QEMU Developers, kvm-devel, arm-mail-list,
kvmarm, Marc Zyngier
Peter Maydell <peter.maydell@linaro.org> writes:
> On 9 March 2015 at 21:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>> this function, however, is not used only when migration, but should
>> generally cover the case where you want to synchronize QEMU's state into
>> KVM's state, right? So while it may not be harmful in currently
>> supported use cases, is there ever a situation where (is_a64(env) && el
>> == 0) and env->spsr != banked_spsr[el], and where env->spsr is
>> out-of-date?
>
> If EL == 0 then you can't access any SPSR, so env->spsr is by
> definition out of date.
Indeed and in v2 the whole thing is wrapped in if (el > 0)
>
> -- PMM
--
Alex Bennée
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-09 16:25 ` Alex Bennée
0 siblings, 0 replies; 73+ messages in thread
From: Alex Bennée @ 2015-03-09 16:25 UTC (permalink / raw)
To: linux-arm-kernel
Peter Maydell <peter.maydell@linaro.org> writes:
> On 9 March 2015 at 21:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>> this function, however, is not used only when migration, but should
>> generally cover the case where you want to synchronize QEMU's state into
>> KVM's state, right? So while it may not be harmful in currently
>> supported use cases, is there ever a situation where (is_a64(env) && el
>> == 0) and env->spsr != banked_spsr[el], and where env->spsr is
>> out-of-date?
>
> If EL == 0 then you can't access any SPSR, so env->spsr is by
> definition out of date.
Indeed and in v2 the whole thing is wrapped in if (el > 0)
>
> -- PMM
--
Alex Benn?e
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
2015-03-09 13:31 ` [Qemu-devel] " Peter Maydell
(?)
@ 2015-03-09 19:31 ` Christoffer Dall
-1 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-09 19:31 UTC (permalink / raw)
To: Peter Maydell
Cc: Alex Bennée, QEMU Developers, kvm-devel, arm-mail-list,
kvmarm, Marc Zyngier
On Mon, Mar 09, 2015 at 10:31:19PM +0900, Peter Maydell wrote:
> On 9 March 2015 at 21:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > this function, however, is not used only when migration, but should
> > generally cover the case where you want to synchronize QEMU's state into
> > KVM's state, right? So while it may not be harmful in currently
> > supported use cases, is there ever a situation where (is_a64(env) && el
> > == 0) and env->spsr != banked_spsr[el], and where env->spsr is
> > out-of-date?
>
> If EL == 0 then you can't access any SPSR, so env->spsr is by
> definition out of date.
>
ok, never mind then.
-Christoffer
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-09 19:31 ` Christoffer Dall
0 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-09 19:31 UTC (permalink / raw)
To: Peter Maydell
Cc: kvm-devel, Marc Zyngier, QEMU Developers, Alex Bennée,
kvmarm, arm-mail-list
On Mon, Mar 09, 2015 at 10:31:19PM +0900, Peter Maydell wrote:
> On 9 March 2015 at 21:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > this function, however, is not used only when migration, but should
> > generally cover the case where you want to synchronize QEMU's state into
> > KVM's state, right? So while it may not be harmful in currently
> > supported use cases, is there ever a situation where (is_a64(env) && el
> > == 0) and env->spsr != banked_spsr[el], and where env->spsr is
> > out-of-date?
>
> If EL == 0 then you can't access any SPSR, so env->spsr is by
> definition out of date.
>
ok, never mind then.
-Christoffer
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs
@ 2015-03-09 19:31 ` Christoffer Dall
0 siblings, 0 replies; 73+ messages in thread
From: Christoffer Dall @ 2015-03-09 19:31 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 09, 2015 at 10:31:19PM +0900, Peter Maydell wrote:
> On 9 March 2015 at 21:56, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > this function, however, is not used only when migration, but should
> > generally cover the case where you want to synchronize QEMU's state into
> > KVM's state, right? So while it may not be harmful in currently
> > supported use cases, is there ever a situation where (is_a64(env) && el
> > == 0) and env->spsr != banked_spsr[el], and where env->spsr is
> > out-of-date?
>
> If EL == 0 then you can't access any SPSR, so env->spsr is by
> definition out of date.
>
ok, never mind then.
-Christoffer
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] target-arm/cpu.h: document why env->spsr exists
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-03-11 15:39 ` Greg Bellows
-1 siblings, 0 replies; 73+ messages in thread
From: Greg Bellows @ 2015-03-11 15:39 UTC (permalink / raw)
To: Alex Bennée
Cc: QEMU Developers, Peter Maydell, kvm, Marc Zyngier,
linux-arm-kernel, kvmarm, Christoffer Dall
On Wed, Feb 25, 2015 at 10:02 AM, Alex Bennée <alex.bennee@linaro.org> wrote:
> I was getting very confused about the duplication of state. Perhaps we
> should just get rid of env->spsr and use helpers that understand the
> banking?
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 11845a6..d7fd13f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -155,6 +155,11 @@ typedef struct CPUARMState {
> This contains all the other bits. Use cpsr_{read,write} to access
> the whole CPSR. */
> uint32_t uncached_cpsr;
> + /* The spsr is a alias for spsr_elN where N is the current
"an alias"
> + * exception level. It is provided for here so the TCG msr/mrs
remove "for here"
> + * implementation can access one register. Care needs to be taken
> + * to ensure the banked_spsr[] is also updated.
> + */
> uint32_t spsr;
>
> /* Banked registers. */
> --
> 2.3.0
>
>
Otherwise...
Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] target-arm/cpu.h: document why env->spsr exists
@ 2015-03-11 15:39 ` Greg Bellows
0 siblings, 0 replies; 73+ messages in thread
From: Greg Bellows @ 2015-03-11 15:39 UTC (permalink / raw)
To: Alex Bennée
Cc: Peter Maydell, kvm, Marc Zyngier, QEMU Developers,
Christoffer Dall, kvmarm, linux-arm-kernel
On Wed, Feb 25, 2015 at 10:02 AM, Alex Bennée <alex.bennee@linaro.org> wrote:
> I was getting very confused about the duplication of state. Perhaps we
> should just get rid of env->spsr and use helpers that understand the
> banking?
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 11845a6..d7fd13f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -155,6 +155,11 @@ typedef struct CPUARMState {
> This contains all the other bits. Use cpsr_{read,write} to access
> the whole CPSR. */
> uint32_t uncached_cpsr;
> + /* The spsr is a alias for spsr_elN where N is the current
"an alias"
> + * exception level. It is provided for here so the TCG msr/mrs
remove "for here"
> + * implementation can access one register. Care needs to be taken
> + * to ensure the banked_spsr[] is also updated.
> + */
> uint32_t spsr;
>
> /* Banked registers. */
> --
> 2.3.0
>
>
Otherwise...
Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
^ permalink raw reply [flat|nested] 73+ messages in thread
* [Qemu-devel] [PATCH 6/6] target-arm/cpu.h: document why env->spsr exists
@ 2015-03-11 15:39 ` Greg Bellows
0 siblings, 0 replies; 73+ messages in thread
From: Greg Bellows @ 2015-03-11 15:39 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 25, 2015 at 10:02 AM, Alex Benn?e <alex.bennee@linaro.org> wrote:
> I was getting very confused about the duplication of state. Perhaps we
> should just get rid of env->spsr and use helpers that understand the
> banking?
>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 11845a6..d7fd13f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -155,6 +155,11 @@ typedef struct CPUARMState {
> This contains all the other bits. Use cpsr_{read,write} to access
> the whole CPSR. */
> uint32_t uncached_cpsr;
> + /* The spsr is a alias for spsr_elN where N is the current
"an alias"
> + * exception level. It is provided for here so the TCG msr/mrs
remove "for here"
> + * implementation can access one register. Care needs to be taken
> + * to ensure the banked_spsr[] is also updated.
> + */
> uint32_t spsr;
>
> /* Banked registers. */
> --
> 2.3.0
>
>
Otherwise...
Reviewed-by: Greg Bellows <greg.bellows@linaro.org>
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [PATCH 6/6] target-arm/cpu.h: document why env->spsr exists
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
(?)
@ 2015-03-11 15:47 ` Peter Maydell
-1 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-03-11 15:47 UTC (permalink / raw)
To: Alex Bennée
Cc: QEMU Developers, kvm-devel, arm-mail-list, kvmarm,
Christoffer Dall, Marc Zyngier
On 25 February 2015 at 16:02, Alex Bennée <alex.bennee@linaro.org> wrote:
> I was getting very confused about the duplication of state. Perhaps we
> should just get rid of env->spsr and use helpers that understand the
> banking?
I've already disagreed with this. I would suggest putting
tentative questions about future direction of the codebase
below the '---' rather than in the commit log :-)
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 11845a6..d7fd13f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -155,6 +155,11 @@ typedef struct CPUARMState {
> This contains all the other bits. Use cpsr_{read,write} to access
> the whole CPSR. */
> uint32_t uncached_cpsr;
> + /* The spsr is a alias for spsr_elN where N is the current
> + * exception level.
This isn't true for AArch32 (which has multiple different SPSRs
any of which might be the one in env->spsr when we're at EL1).
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] target-arm/cpu.h: document why env->spsr exists
@ 2015-03-11 15:47 ` Peter Maydell
0 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-03-11 15:47 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm-devel, Marc Zyngier, QEMU Developers, Christoffer Dall,
kvmarm, arm-mail-list
On 25 February 2015 at 16:02, Alex Bennée <alex.bennee@linaro.org> wrote:
> I was getting very confused about the duplication of state. Perhaps we
> should just get rid of env->spsr and use helpers that understand the
> banking?
I've already disagreed with this. I would suggest putting
tentative questions about future direction of the codebase
below the '---' rather than in the commit log :-)
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 11845a6..d7fd13f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -155,6 +155,11 @@ typedef struct CPUARMState {
> This contains all the other bits. Use cpsr_{read,write} to access
> the whole CPSR. */
> uint32_t uncached_cpsr;
> + /* The spsr is a alias for spsr_elN where N is the current
> + * exception level.
This isn't true for AArch32 (which has multiple different SPSRs
any of which might be the one in env->spsr when we're at EL1).
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
* [PATCH 6/6] target-arm/cpu.h: document why env->spsr exists
@ 2015-03-11 15:47 ` Peter Maydell
0 siblings, 0 replies; 73+ messages in thread
From: Peter Maydell @ 2015-03-11 15:47 UTC (permalink / raw)
To: linux-arm-kernel
On 25 February 2015 at 16:02, Alex Benn?e <alex.bennee@linaro.org> wrote:
> I was getting very confused about the duplication of state. Perhaps we
> should just get rid of env->spsr and use helpers that understand the
> banking?
I've already disagreed with this. I would suggest putting
tentative questions about future direction of the codebase
below the '---' rather than in the commit log :-)
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 11845a6..d7fd13f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -155,6 +155,11 @@ typedef struct CPUARMState {
> This contains all the other bits. Use cpsr_{read,write} to access
> the whole CPSR. */
> uint32_t uncached_cpsr;
> + /* The spsr is a alias for spsr_elN where N is the current
> + * exception level.
This isn't true for AArch32 (which has multiple different SPSRs
any of which might be the one in env->spsr when we're at EL1).
-- PMM
^ permalink raw reply [flat|nested] 73+ messages in thread
end of thread, other threads:[~2015-03-11 15:48 UTC | newest]
Thread overview: 73+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-25 16:02 [PATCH 0/6] QEMU ARM64 Migration Fixes Alex Bennée
2015-02-25 16:02 ` Alex Bennée
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
2015-02-25 16:02 ` [PATCH 1/6] target-arm: kvm: save/restore mp state Alex Bennée
2015-02-25 16:02 ` Alex Bennée
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
2015-02-25 23:36 ` Peter Maydell
2015-02-25 23:36 ` Peter Maydell
2015-02-25 23:36 ` [Qemu-devel] " Peter Maydell
2015-03-03 10:56 ` Alex Bennée
2015-03-03 10:56 ` Alex Bennée
2015-03-03 10:56 ` [Qemu-devel] " Alex Bennée
2015-03-03 11:06 ` Paolo Bonzini
2015-03-03 11:06 ` Paolo Bonzini
2015-03-03 11:06 ` [Qemu-devel] " Paolo Bonzini
2015-03-03 11:51 ` Peter Maydell
2015-03-03 11:51 ` Peter Maydell
2015-03-03 11:51 ` [Qemu-devel] " Peter Maydell
2015-03-03 16:30 ` Alex Bennée
2015-03-03 16:30 ` Alex Bennée
2015-03-03 16:30 ` [Qemu-devel] " Alex Bennée
2015-03-03 17:10 ` Paolo Bonzini
2015-03-03 17:10 ` Paolo Bonzini
2015-03-03 17:10 ` [Qemu-devel] " Paolo Bonzini
2015-02-26 12:57 ` Paolo Bonzini
2015-02-26 12:57 ` Paolo Bonzini
2015-02-26 12:57 ` [Qemu-devel] " Paolo Bonzini
2015-02-25 16:02 ` [PATCH 2/6] arm_gic_kvm.c: restore config before pending IRQs Alex Bennée
2015-02-25 16:02 ` Alex Bennée
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
2015-03-02 22:14 ` Christoffer Dall
2015-03-02 22:14 ` Christoffer Dall
2015-03-02 22:14 ` [Qemu-devel] " Christoffer Dall
2015-02-25 16:02 ` [PATCH 3/6] hw/char/pl011: don't keep setting the IRQ if nothing changed Alex Bennée
2015-02-25 16:02 ` Alex Bennée
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
2015-02-25 16:02 ` [PATCH 4/6] target-arm/kvm64.c: sync FP register state Alex Bennée
2015-02-25 16:02 ` Alex Bennée
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
2015-02-25 16:02 ` [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR registers Alex Bennée
2015-02-25 16:02 ` Alex Bennée
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
2015-02-25 16:02 ` [PATCH 5/6] target-arm/kvm64: fix save/restore of SPSR regs Alex Bennée
2015-02-25 16:02 ` Alex Bennée
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
2015-03-02 17:22 ` Christoffer Dall
2015-03-02 17:22 ` Christoffer Dall
2015-03-02 17:22 ` [Qemu-devel] " Christoffer Dall
2015-03-03 11:28 ` Alex Bennée
2015-03-03 11:28 ` Alex Bennée
2015-03-03 11:28 ` [Qemu-devel] " Alex Bennée
2015-03-09 12:56 ` Christoffer Dall
2015-03-09 12:56 ` Christoffer Dall
2015-03-09 12:56 ` [Qemu-devel] " Christoffer Dall
2015-03-09 13:31 ` Peter Maydell
2015-03-09 13:31 ` Peter Maydell
2015-03-09 13:31 ` [Qemu-devel] " Peter Maydell
2015-03-09 16:25 ` Alex Bennée
2015-03-09 16:25 ` Alex Bennée
2015-03-09 16:25 ` Alex Bennée
2015-03-09 16:25 ` [Qemu-devel] " Alex Bennée
2015-03-09 19:31 ` Christoffer Dall
2015-03-09 19:31 ` Christoffer Dall
2015-03-09 19:31 ` [Qemu-devel] " Christoffer Dall
2015-02-25 16:02 ` [PATCH 6/6] target-arm/cpu.h: document why env->spsr exists Alex Bennée
2015-02-25 16:02 ` Alex Bennée
2015-02-25 16:02 ` [Qemu-devel] " Alex Bennée
2015-03-11 15:39 ` Greg Bellows
2015-03-11 15:39 ` Greg Bellows
2015-03-11 15:39 ` Greg Bellows
2015-03-11 15:47 ` Peter Maydell
2015-03-11 15:47 ` Peter Maydell
2015-03-11 15:47 ` [Qemu-devel] " Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.