All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] target-arm: Implement vCPU reset via KVM_ARM_VCPU_INIT for 32-bit CPUs
@ 2014-06-26 17:16 Peter Maydell
  2014-06-27  7:38 ` Diana Craciun
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2014-06-26 17:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alex Bennée, kvmarm, Christoffer Dall, patches

Implement kvm_arm_vcpu_init() as a simple call to arm_arm_vcpu_init()
(which uses the KVM_ARM_VCPU_INIT vcpu ioctl to tell the kernel
to re-initialize the vCPU), rather than via the complicated code
which saves a copy of the register state on first init and then
writes it back to the kernel. This is much simpler and brings the
32-bit KVM code into line with the 64-bit code.


Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The kernel has always supported being able to call VCPU_INIT
multiple times for this reset effect; I just didn't realize it
was possible when I wrote the original reset code.

When kvm64.c grows support for system registers we can probably
coalesce the two kvm_arm_reset_cpu() functions into one.

I also have a vague recollection that somebody reported that
we had an actual bug in this area that this patch would fix;
however I can't now find that in the mailing list archives :-(

Testing appreciated: my ARMv7 box is being a bit flaky at the
moment; I don't *think* the occasional weird stuff I see is
the effect of this patch but it's hard to be certain.

---
 target-arm/cpu-qom.h |  4 ----
 target-arm/kvm32.c   | 19 +++++--------------
 2 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index eaee944..ee4fbb1 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -72,10 +72,6 @@ typedef struct ARMCPU {
     uint64_t *cpreg_indexes;
     /* Values of the registers (cpreg_indexes[i]'s value is cpreg_values[i]) */
     uint64_t *cpreg_values;
-    /* When using KVM, keeps a copy of the initial state of the VCPU,
-     * so that on reset we can feed the reset values back into the kernel.
-     */
-    uint64_t *cpreg_reset_values;
     /* Length of the indexes, values, reset_values arrays */
     int32_t cpreg_array_len;
     /* These are used only for migration: incoming data arrives in
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index 068af7d..5ec4eb1 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -270,13 +270,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
         goto out;
     }
 
-    /* Save a copy of the initial register values so that we can
-     * feed it back to the kernel on VCPU reset.
-     */
-    cpu->cpreg_reset_values = g_memdup(cpu->cpreg_values,
-                                       cpu->cpreg_array_len *
-                                       sizeof(cpu->cpreg_values[0]));
-
 out:
     g_free(rlp);
     return ret;
@@ -518,11 +511,9 @@ int kvm_arch_get_registers(CPUState *cs)
 
 void kvm_arm_reset_vcpu(ARMCPU *cpu)
 {
-    /* Feed the kernel back its initial register state */
-    memmove(cpu->cpreg_values, cpu->cpreg_reset_values,
-            cpu->cpreg_array_len * sizeof(cpu->cpreg_values[0]));
-
-    if (!write_list_to_kvmstate(cpu)) {
-        abort();
-    }
+    /* Re-init VCPU so that all registers are set to
+     * their respective reset values.
+     */
+    kvm_arm_vcpu_init(CPU(cpu));
+    write_kvmstate_to_list(cpu);
 }
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Implement vCPU reset via KVM_ARM_VCPU_INIT for 32-bit CPUs
  2014-06-26 17:16 [Qemu-devel] [PATCH] target-arm: Implement vCPU reset via KVM_ARM_VCPU_INIT for 32-bit CPUs Peter Maydell
@ 2014-06-27  7:38 ` Diana Craciun
  2014-06-27 12:26   ` Peter Maydell
  0 siblings, 1 reply; 5+ messages in thread
From: Diana Craciun @ 2014-06-27  7:38 UTC (permalink / raw)
  To: Peter Maydell; +Cc: patches, qemu-devel, kvmarm

On 06/26/2014 08:16 PM, Peter Maydell wrote:
> Implement kvm_arm_vcpu_init() as a simple call to arm_arm_vcpu_init()
> (which uses the KVM_ARM_VCPU_INIT vcpu ioctl to tell the kernel
> to re-initialize the vCPU), rather than via the complicated code
> which saves a copy of the register state on first init and then
> writes it back to the kernel. This is much simpler and brings the
> 32-bit KVM code into line with the 64-bit code.
>
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> The kernel has always supported being able to call VCPU_INIT
> multiple times for this reset effect; I just didn't realize it
> was possible when I wrote the original reset code.
>
> When kvm64.c grows support for system registers we can probably
> coalesce the two kvm_arm_reset_cpu() functions into one.
>
> I also have a vague recollection that somebody reported that
> we had an actual bug in this area that this patch would fix;
> however I can't now find that in the mailing list archives :-(

I did: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03131.html


>
> Testing appreciated: my ARMv7 box is being a bit flaky at the
> moment; I don't *think* the occasional weird stuff I see is
> the effect of this patch but it's hard to be certain.

I will test your patch in the following days.

Diana

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Implement vCPU reset via KVM_ARM_VCPU_INIT for 32-bit CPUs
  2014-06-27  7:38 ` Diana Craciun
@ 2014-06-27 12:26   ` Peter Maydell
  2014-06-30 14:22     ` Diana Craciun
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2014-06-27 12:26 UTC (permalink / raw)
  To: Diana Craciun; +Cc: Patch Tracking, QEMU Developers, kvmarm

On 27 June 2014 08:38, Diana Craciun <diana.craciun@freescale.com> wrote:
> On 06/26/2014 08:16 PM, Peter Maydell wrote:
>> Testing appreciated: my ARMv7 box is being a bit flaky at the
>> moment; I don't *think* the occasional weird stuff I see is
>> the effect of this patch but it's hard to be certain.
>
>
> I will test your patch in the following days.

Thanks. If you can manage to test by Monday I can fit
this in before we roll 2.1 rc0 on Tuesday; otherwise
it'll have to slip to rc1.

-- PMM

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Implement vCPU reset via KVM_ARM_VCPU_INIT for 32-bit CPUs
  2014-06-27 12:26   ` Peter Maydell
@ 2014-06-30 14:22     ` Diana Craciun
  2014-06-30 14:41       ` Peter Maydell
  0 siblings, 1 reply; 5+ messages in thread
From: Diana Craciun @ 2014-06-30 14:22 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Patch Tracking, QEMU Developers, kvmarm

Hi,

I tested the patch on a cubieboard2 and it is working fine.

However I did see some weird stuff, but I do not think that it is 
related with this patch as it reproduces without the patch also. What I 
am seeing is that after a number of resets (pretty random, sometimes I 
can reproduce it after 20,30 resets, sometimes after 5,6, but in general 
pretty hard to reproduce) is that the guest gets stuck somewhere (it 
does not boot to prompt) and further resets are not working. I remember 
I saw this also in the past but I did not have the chance to look into 
it in more depth.

Diana


On 06/27/2014 03:26 PM, Peter Maydell wrote:
> On 27 June 2014 08:38, Diana Craciun <diana.craciun@freescale.com> wrote:
>> On 06/26/2014 08:16 PM, Peter Maydell wrote:
>>> Testing appreciated: my ARMv7 box is being a bit flaky at the
>>> moment; I don't *think* the occasional weird stuff I see is
>>> the effect of this patch but it's hard to be certain.
>>
>> I will test your patch in the following days.
> Thanks. If you can manage to test by Monday I can fit
> this in before we roll 2.1 rc0 on Tuesday; otherwise
> it'll have to slip to rc1.
>
> -- PMM

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] target-arm: Implement vCPU reset via KVM_ARM_VCPU_INIT for 32-bit CPUs
  2014-06-30 14:22     ` Diana Craciun
@ 2014-06-30 14:41       ` Peter Maydell
  0 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2014-06-30 14:41 UTC (permalink / raw)
  To: Diana Craciun; +Cc: Patch Tracking, QEMU Developers, kvmarm

On 30 June 2014 15:22, Diana Craciun <diana.craciun@freescale.com> wrote:
> I tested the patch on a cubieboard2 and it is working fine.

OK, thanks. This just missed the bus for my pullrequest, but I can
add it in later since it's a bugfix.

> However I did see some weird stuff, but I do not think that it is related
> with this patch as it reproduces without the patch also. What I am seeing is
> that after a number of resets (pretty random, sometimes I can reproduce it
> after 20,30 resets, sometimes after 5,6, but in general pretty hard to
> reproduce) is that the guest gets stuck somewhere (it does not boot to
> prompt) and further resets are not working. I remember I saw this also in
> the past but I did not have the chance to look into it in more depth.

Yuck :-( That sounds pretty horrible to track down.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-06-30 14:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-26 17:16 [Qemu-devel] [PATCH] target-arm: Implement vCPU reset via KVM_ARM_VCPU_INIT for 32-bit CPUs Peter Maydell
2014-06-27  7:38 ` Diana Craciun
2014-06-27 12:26   ` Peter Maydell
2014-06-30 14:22     ` Diana Craciun
2014-06-30 14:41       ` 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.