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

[Khansa Butt]

On Wed, Nov 30, 2011 at 4:54 PM, Andreas Färber <address@hidden> wrote:
> 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) { ... }".
>
Thanks for your response!
Here is a confusion. I found that cpu_save() and cpu_load() are not called in
user mode emulation, here is the code from exec.c
#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
    vmstate_register(NULL, cpu_index, &vmstate_cpu_common, env);
    register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
                    cpu_save, cpu_load, env);
#endif
As these patches are related to UME so we decided to postpone Octeon
specific changes ( registers and instructions) and will include them in our
SME work( we are currently working on system mode emulation of Octeon board)
that's why yesterday I sent only MIPS64 user mode emulation patches
here is the link
http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg03527.html
Please review them
I'm sorry, the words "with      Cavium specific instruction support" are
mistakenly added
 into subject line.

Please guide me towards the right track. should we stick to are old patches or
submit MIPS64 UME patches only(which are  without Cavium's Instruction support)


> 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
>