Re: [PATCH] target/mips: Fix PageMask with variable page size

[Peter Maydell]

On Mon, 15 Jun 2020 at 10:17, Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
>
>
> 在 2020/6/15 17:13, Peter Maydell 写道:
> > On Sun, 14 Jun 2020 at 22:52, Aleksandar Markovic
> > <aleksandar.qemu.devel@gmail.com> wrote:
> >> When you change machine.c the way you did it, you need to bump the 
> >> version. Please see git log on machine.c for details.
> >
> >>> --- a/target/mips/cpu.h
> >>> +++ b/target/mips/cpu.h
> >>> @@ -617,7 +617,8 @@ struct CPUMIPSState {
> >>>   /*
> >>>    * CP0 Register 5
> >>>    */
> >>> -    int32_t CP0_PageMask;
> >>> +    target_ulong CP0_PageMask;
> >>> +#define CP0PM_MASK 13
> >
> > Does CP0_PageMask ever actually hold a value that won't fit
> > in an int32_t? If not, it might be preferable to avoid changing
> > its type to avoid the migration compat break, even if MIPS
> > doesn't have any versioned boards where we have a strict
> > don't-break-compat promise to users.
>
> In Release2, PageMask was extended to 64bit on MIPS64 processors.
>
> Is it necessary to follow that?

Ah, I see. I'd assumed that you were only fixing the
variable-page-size change (which shouldn't require a
change in the type), but as Aleksandar says you've
mixed in a new feature implementation in the same commit
(which from what you're saying does need the type to change).
If the new feature means the register is now 64 bits then
it is possible to implement this in a migration-compatible
way by using a vmstate subsection; I'll leave it up to
Aleksandar whether that complexity is something that makes
sense for MIPS targets or if it's better to just break migration
compat by bumping the version.

thanks
-- PMM