[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 23/35] vmstate: port arm cpu

From: Juan Quintela
Subject: Re: [Qemu-devel] [PATCH 23/35] vmstate: port arm cpu
Date: Fri, 04 May 2012 17:28:27 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.93 (gnu/linux)

Peter Maydell <address@hidden> wrote:
> On 4 May 2012 11:54, Juan Quintela <address@hidden> wrote:
>> 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 <address@hidden>
>> ---
>>  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 9434902..37744c6 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -236,6 +236,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;
> I still think this is the wrong approach. We need to support
> "this is how you read/write this field" functions. See also
> target-alpha handling of the fpcr.

It is easy to change to support that (see alpha example), but then, each
time that we change protocol format (and we have a pending change for
1.2), every user that is "like" uint64_t, but with accesor and setter,
needs to get a new change.  This other way (with pre_save/post_load)
instead makes trivial to change protocols.

So, we can have "cleaniness" or "flexibility", take one.
I still think that alpha should be changed to this other approach, for
the same reason.

Notice that we _know_ that what we sent is an uint32_t, it is how we get
that uint32_t what changes, that is why I preffer a new field and
pre_load/post_load functions.

The only other sane approach that I can think is changing:

#define VMSTATE_INT32_V(_f, _s, _v)                                   \

into something like:

VMSTATE_INT32_GENERAL(_f, _s, _v, _getter, _setter)

and then _getter/_setter functions for normal ints the obvious copy this

Would that work for you?

Later, Juan.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]