Re: [Qemu-devel] [PATCH 24/36] vmstate: port arm cpu

2012-03-21 Thread Juan Quintela 
Andreas Färber  wrote:
> Am 19.03.2012 23:57, schrieb Juan Quintela:
>> Use one subsection for each feature.  This means that we don't need to
>> bump the version field each time that a new feature gets introduced.
>> 
>> Introduce cpsr_vmstate field, as I am not sure if I can "use"
>> uncached_cpsr for saving state.
>> 
>> Signed-off-by: Juan Quintela 
>
> As stated previously, I still think we should hold off converting the
> ARM CPU to VMState until the cp15 rework by Peter takes on shape.

This converts all cpus to vmstate.  Conversion of cp15 is indiferent of
vmstate.  Basically we will need to send something like one array of 
pairs: [(register_id, value)], and have some validation before we
receive it.

Notice that something like that is also needed for x86 MSR's, just now
we are sending them ad-hoc, but it would be great to do something like:
- notice what msr's have been read/write by guest
- just send that msr's.
- maintain backwards compatibility is going to be a mess, but that don't
  affect arm at this moment.

I guess that cp15 registers have some similar meaning (they are
optional, and not all of them have been touched by the guest).  But I
still haven't a good idea on how to handle it.

> On another matter, had you prefixed this patch with "target-arm: "
> rather than hiding that essential keyword where my mailer cuts it off,
> it would be much easier to find in this series than amidst lots of
> "vmstate: " patches.

clearly, we need: keywords in each patch, in the gmail sense.  one patch
can be related to several subsystems.  general topic here was vmstate,
but I fully agree that if I had used target-foo: as starting it would
have been as good (just for a different definition of good).

Thanks, Juan.

Re: [Qemu-devel] [PATCH 24/36] vmstate: port arm cpu

2012-03-21 Thread Peter Maydell 
On 21 March 2012 16:29, Andreas Färber  wrote:
> Am 19.03.2012 23:57, schrieb Juan Quintela:
>> Use one subsection for each feature.  This means that we don't need to
>> bump the version field each time that a new feature gets introduced.
>>
>> Introduce cpsr_vmstate field, as I am not sure if I can "use"
>> uncached_cpsr for saving state.

You could, I guess, but you'd need a 'post-save' hook to reset the
bits not stored in uncached_cpsr normally, and it's a bit ugly.
On the other hand having a field in CPUARMState just for save
load is also pretty ugly.

VMState should support a way to mark a migration state field as
"not actually stored in a field in the struct, use these functions
to save and load it", and we should use that.

> As stated previously, I still think we should hold off converting the
> ARM CPU to VMState until the cp15 rework by Peter takes on shape.

I don't think this code actually clashes with what I've done so
far, and I think switching to a lot of separate subsections is
probably a step in the right direction even if the details might
not be quite what they'll end up eventually. So I don't think
we need to block it until the cp15 stuff lands. (Plus I know I've
been taking forever on cp15 so I feel bad about blocking other
peoples' patches on it.)

-- PMM

Re: [Qemu-devel] [PATCH 24/36] vmstate: port arm cpu

2012-03-21 Thread Andreas Färber 
Am 19.03.2012 23:57, schrieb Juan Quintela:
> Use one subsection for each feature.  This means that we don't need to
> bump the version field each time that a new feature gets introduced.
> 
> Introduce cpsr_vmstate field, as I am not sure if I can "use"
> uncached_cpsr for saving state.
> 
> Signed-off-by: Juan Quintela 

As stated previously, I still think we should hold off converting the
ARM CPU to VMState until the cp15 rework by Peter takes on shape.

On another matter, had you prefixed this patch with "target-arm: "
rather than hiding that essential keyword where my mailer cuts it off,
it would be much easier to find in this series than amidst lots of
"vmstate: " patches.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 24/36] vmstate: port arm cpu

2012-03-19 Thread Juan Quintela 
Use one subsection for each feature.  This means that we don't need to
bump the version field each time that a new feature gets introduced.

Introduce cpsr_vmstate field, as I am not sure if I can "use"
uncached_cpsr for saving state.

Signed-off-by: Juan Quintela 
---
 target-arm/cpu.h |5 +-
 target-arm/machine.c |  344 ++
 2 files changed, 156 insertions(+), 193 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index a6e8c7e..aafb4a7 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -233,6 +233,9 @@ typedef struct CPUARMState {
 } cp[15];
 void *nvic;
 const struct arm_boot_info *boot_info;
+
+/* Fields needed as intermediate for vmstate */
+uint32_t cpsr_vmstate;
 } CPUARMState;

 CPUARMState *cpu_arm_init(const char *cpu_model);
@@ -455,8 +458,6 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define cpu_signal_handler cpu_arm_signal_handler
 #define cpu_list arm_cpu_list

-#define CPU_SAVE_VERSION 7
-
 /* MMU modes definitions */
 #define MMU_MODE0_SUFFIX _kernel
 #define MMU_MODE1_SUFFIX _user
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 9c0f773..31e49ac 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -1,215 +1,177 @@
 #include "hw/hw.h"
 #include "hw/boards.h"

-void cpu_save(QEMUFile *f, void *opaque)
+static bool feature_vfp_needed(void *opaque)
 {
-int i;
-CPUARMState *env = (CPUARMState *)opaque;
+CPUARMState *env = opaque;

-for (i = 0; i < 16; i++) {
-qemu_put_be32(f, env->regs[i]);
-}
-qemu_put_be32(f, cpsr_read(env));
-qemu_put_be32(f, env->spsr);
-for (i = 0; i < 6; i++) {
-qemu_put_be32(f, env->banked_spsr[i]);
-qemu_put_be32(f, env->banked_r13[i]);
-qemu_put_be32(f, env->banked_r14[i]);
-}
-for (i = 0; i < 5; i++) {
-qemu_put_be32(f, env->usr_regs[i]);
-qemu_put_be32(f, env->fiq_regs[i]);
-}
-qemu_put_be32(f, env->cp15.c0_cpuid);
-qemu_put_be32(f, env->cp15.c0_cachetype);
-qemu_put_be32(f, env->cp15.c0_cssel);
-qemu_put_be32(f, env->cp15.c1_sys);
-qemu_put_be32(f, env->cp15.c1_coproc);
-qemu_put_be32(f, env->cp15.c1_xscaleauxcr);
-qemu_put_be32(f, env->cp15.c1_scr);
-qemu_put_be32(f, env->cp15.c2_base0);
-qemu_put_be32(f, env->cp15.c2_base1);
-qemu_put_be32(f, env->cp15.c2_control);
-qemu_put_be32(f, env->cp15.c2_mask);
-qemu_put_be32(f, env->cp15.c2_base_mask);
-qemu_put_be32(f, env->cp15.c2_data);
-qemu_put_be32(f, env->cp15.c2_insn);
-qemu_put_be32(f, env->cp15.c3);
-qemu_put_be32(f, env->cp15.c5_insn);
-qemu_put_be32(f, env->cp15.c5_data);
-for (i = 0; i < 8; i++) {
-qemu_put_be32(f, env->cp15.c6_region[i]);
-}
-qemu_put_be32(f, env->cp15.c6_insn);
-qemu_put_be32(f, env->cp15.c6_data);
-qemu_put_be32(f, env->cp15.c7_par);
-qemu_put_be32(f, env->cp15.c9_insn);
-qemu_put_be32(f, env->cp15.c9_data);
-qemu_put_be32(f, env->cp15.c9_pmcr);
-qemu_put_be32(f, env->cp15.c9_pmcnten);
-qemu_put_be32(f, env->cp15.c9_pmovsr);
-qemu_put_be32(f, env->cp15.c9_pmxevtyper);
-qemu_put_be32(f, env->cp15.c9_pmuserenr);
-qemu_put_be32(f, env->cp15.c9_pminten);
-qemu_put_be32(f, env->cp15.c13_fcse);
-qemu_put_be32(f, env->cp15.c13_context);
-qemu_put_be32(f, env->cp15.c13_tls1);
-qemu_put_be32(f, env->cp15.c13_tls2);
-qemu_put_be32(f, env->cp15.c13_tls3);
-qemu_put_be32(f, env->cp15.c15_cpar);
-qemu_put_be32(f, env->cp15.c15_power_control);
-qemu_put_be32(f, env->cp15.c15_diagnostic);
-qemu_put_be32(f, env->cp15.c15_power_diagnostic);
-
-qemu_put_be32(f, env->features);
-
-if (arm_feature(env, ARM_FEATURE_VFP)) {
-for (i = 0;  i < 32; i++) {
-CPU_DoubleU u;
-u.d = env->vfp.regs[i];
-qemu_put_be32(f, u.l.upper);
-qemu_put_be32(f, u.l.lower);
-}
-for (i = 0; i < 16; i++) {
-qemu_put_be32(f, env->vfp.xregs[i]);
-}
+return arm_feature(env, ARM_FEATURE_VFP);
+}

+static const VMStateDescription vmstate_feature_vfp = {
+.name = "feature_vfp",
+.version_id = 1,
+.minimum_version_id = 1,
+.minimum_version_id_old = 1,
+.fields = (VMStateField[]) {
+VMSTATE_FLOAT64_ARRAY(vfp.regs, CPUARMState, 32),
+VMSTATE_UINT32_ARRAY(vfp.xregs, CPUARMState, 16),
 /* TODO: Should use proper FPSCR access functions.  */
-qemu_put_be32(f, env->vfp.vec_len);
-qemu_put_be32(f, env->vfp.vec_stride);
+VMSTATE_INT32(vfp.vec_len, CPUARMState),
+VMSTATE_INT32(vfp.vec_stride, CPUARMState),
+VMSTATE_END_OF_LIST()
 }
+};

-if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
-for (i = 0; i < 16; i++) {
-qemu_put_be64(f, env->iwmmxt.regs[i]);
-}
-for (i = 0; i < 16; i++) {
-qemu_put_be32(f, env->iwmmxt.cregs[i]);
-}
-