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

[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>:
>
> HTML again :(
>
> >     > From: Khansa Butt <address@hidden <mailto:address@hidden>>
> >
> >     Commit message should mention here at least that new registers are
> >     introduced and that load/save format is being changed.
> >
> >     > Signed-off-by: Khansa Butt <address@hidden
> >     <mailto:address@hidden>>
> >     > Signed-off-by: Ehsan Ul Haq <address@hidden
> >     <mailto:address@hidden>>
> >     > Signed-off-by: Abdul Qadeer <address@hidden
> >     <mailto:address@hidden>>
> >     > Signed-off-by: Abdul Waheed <address@hidden
> >     <mailto:address@hidden>>
> >     > ---
> >
> >     > diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> >     > index 79e2558..9180ee9 100644
> >     > --- a/target-mips/cpu.h
> >     > +++ b/target-mips/cpu.h
> >     > @@ -173,6 +173,13 @@ struct TCState {
> >     >      target_ulong CP0_TCSchedule;
> >     >      target_ulong CP0_TCScheFBack;
> >     >      int32_t CP0_Debug_tcstatus;
> >     > +    /* Multiplier registers for Octeon */
> >     > +    target_ulong MPL0;
> >     > +    target_ulong MPL1;
> >     > +    target_ulong MPL2;
> >     > +    target_ulong P0;
> >     > +    target_ulong P1;
> >     > +    target_ulong P2;
> >     >  };
> >     >
> >     >  typedef struct CPUMIPSState CPUMIPSState;
> >
> >     > 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
> >     > @@ -25,6 +25,12 @@ static void save_tc(QEMUFile *f, TCState *tc)
> >     >      qemu_put_betls(f, &tc->CP0_TCSchedule);
> >     >      qemu_put_betls(f, &tc->CP0_TCScheFBack);
> >     >      qemu_put_sbe32s(f, &tc->CP0_Debug_tcstatus);
> >     > +    qemu_put_betls(f, &tc->MPL0);
> >     > +    qemu_put_betls(f, &tc->MPL1);
> >
> >     MPL2 is not being saved but loaded below.
> >
> >     > +    qemu_put_betls(f, &tc->P0);
> >     > +    qemu_put_betls(f, &tc->P1);
> >     > +    qemu_put_betls(f, &tc->P2);
> >     > +
> >     >  }
> >     >
> >     >  static void save_fpu(QEMUFile *f, CPUMIPSFPUContext *fpu)
> >     > @@ -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? Also please tell me about mips maintainer,
Aurelien Jarno. I think he is not active in developer list for a
while.

>
> >     I'm wondering whether those register and serialization additions could
> >     and should be limited to TARGET_MIPS64.
> >
> > you want me to limit these registers to TARGET_OCTEON
>
> No, there shouldn't be a TARGET_OCTEON, there's no Octeon-specific
> executable.
>
> My point is, IIUC, qemu-system-mips will never have Octeon registers
> because they're in qemu-system-mips64 only. So without #ifdef it would
> save and load unused registers.
>
> Andreas