Re: [Qemu-devel] [PATCH v3 5/6] target-mips: Adding support for Cavium specific instructions

[Andreas Färber]

Am 22.11.2011 09:31, schrieb Khansa Butt:
> On Tue, Nov 1, 2011 at 1:24 AM, Andreas Färber <address@hidden> wrote:
>>
>> Am 28.10.2011 06:42, schrieb Khansa Butt:
>>>
>>>
>>> On Sat, Oct 22, 2011 at 4:36 PM, Andreas Färber <address@hidden
>>> <mailto:address@hidden>> wrote:
>>>
>>>     Am 22.10.2011 12:11, schrieb address@hidden
>>>     <mailto:address@hidden>:
>>>
>>>     > diff --git a/target-mips/machine.c b/target-mips/machine.c
>>>     > index be72b36..a274ce2 100644
>>>     > --- a/target-mips/machine.c
>>>     > +++ b/target-mips/machine.c
>>>     > @@ -173,6 +179,12 @@ static void load_tc(QEMUFile *f, TCState *tc)
>>>     >      qemu_get_betls(f, &tc->CP0_TCSchedule);
>>>     >      qemu_get_betls(f, &tc->CP0_TCScheFBack);
>>>     >      qemu_get_sbe32s(f, &tc->CP0_Debug_tcstatus);
>>>     > +    qemu_get_betls(f, &tc->MPL0);
>>>     > +    qemu_get_betls(f, &tc->MPL1);
>>>     > +    qemu_get_betls(f, &tc->MPL2);
>>>     > +    qemu_get_betls(f, &tc->P0);
>>>     > +    qemu_get_betls(f, &tc->P1);
>>>     > +    qemu_get_betls(f, &tc->P2);
>>>     >  }
>>>     >
>>>     >  static void load_fpu(QEMUFile *f, CPUMIPSFPUContext *fpu)
>>>
>>>     You're saving new fields, so you'll need to bump the version somewhere.
>>>     For loading, since you're adding at the end, you might be able to make
>>>     your additions conditional on the to-be-bumped version.
>>>
>>>
>>> I 'm not able to understand " bump the version somewhere"  kindly
>>> explain this.
>>
>> "Somewhere" indicates I don't know the exact line for mips. Compare the
>> recent patch to arm_gic.
>> The general idea is that QEMU needs to be able to load files saved with
>> an older version, the file format is therefore versioned. If you
>> unconditionally try to load your new registers, you break loading older
>> files that don't include them.
> 
> Thanks for your response.
> As I can't see any example of bumping the version of registers  in
> mips ( 32 or 64) so i'm in a bit difficult situation
> From arm_gic what i understand is that version_id is related to
> devices which are specific to some board
> as gic is related to RealView board. considering that i'm in user
> mode, can i do the same thing with Cavium's  registers as these are
> related to multiplier unit?

No, this is not board- or device-specific, it's CPU-specific. Cf.
target-mips/cpu.h:CPU_SAVE_VERSION
target-mips/savevm.c:cpu_load()

My suggestion was to bump CPU_SAVE_VERSION to 4, change the error check
to "if (version_id < 3)" and to enclose your cpuo_load() additions in
"if (version_id >= 4) { ... }".

Depending how long you need to resend, note that Juan is working on a
VMState refactoring of machine.c, which will make it more like devices.

Andreas