Re: [PATCH 3/3] target/mips: implement CP0.Config7.WII bit support

[Philippe Mathieu-Daudé]

On 15/2/23 20:19, Marcin Nowakowski wrote:
> On Wed, Feb 15, 2023 at 7:33 PM Philippe Mathieu-Daudé
> <philmd@linaro.org> wrote:
> > Hi Marcin,
> >
> > On 15/2/23 09:47, Marcin Nowakowski wrote:
> > > Some older cores use CP0.Config7.WII bit to indicate that a disabled
> > > interrupt should wake up a sleeping CPU.
> > > Enable this bit by default for M14Kc, which supports that. There are
> > > potentially other cores that support this feature, but I do not have a
> > > complete list.
> >
> > Also the P5600 (MIPS-MD01025-2B-P5600-Software-TRM-01.60.pdf,
> > "MIPS32® P5600 Multiprocessing System Software UM, Revision 01.60).
> >
> > > Signed-off-by: Marcin Nowakowski <marcin.nowakowski@fungible.com>
> > > ---
> > >    target/mips/cpu-defs.c.inc | 1 +
> > >    target/mips/cpu.c          | 6 ++++--
> > >    target/mips/cpu.h          | 1 +
> > >    3 files changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/target/mips/cpu-defs.c.inc b/target/mips/cpu-defs.c.inc
> > > index 480e60aeec..57856e2e72 100644
> > > --- a/target/mips/cpu-defs.c.inc
> > > +++ b/target/mips/cpu-defs.c.inc
> > > @@ -354,6 +354,7 @@ const mips_def_t mips_defs[] =
> > >                           (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA),
> > >            .CP0_Config2 = MIPS_CONFIG2,
> > >            .CP0_Config3 = MIPS_CONFIG3 | (0x2 << CP0C3_ISA) | (0 << CP0C3_VInt),
> >
> > Per the P5600 doc on Config5.M:
> >
> >     Configuration continuation bit. Even though the Config6 and Config7
> >     registers are used in the P5600 Multiprocessing System, they are both
> >     defined as implementation-specific registers. As such, this bit is
> >     zero and is not used to indicate the presence of Config6.
> >
> > Still I suppose we need to set at least Config4.M:
> >
> >     +        .CP0_Config4 = MIPS_CONFIG4,
> >     +        .CP0_Config4_rw_bitmask = 0,
>
> The definition of MIPS_CONFIG4 doesn't set M-bit, so I assume what you
> really meant here is
> .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M)

Yes :)

> Config3 also doesn't have M-bit set right now, I'll fix that in the
> next patch revision.
>
> > I'm not sure about:
> >
> >     +        .CP0_Config5 = MIPS_CONFIG5,
> >     +        .CP0_Config5_rw_bitmask = 0,
>
> M14Kc specification (MD00674-2B-M14Kc-SUM-02.04.pdf) notes the following:
> "This bit is reserved. With the current architectural definition, this
> bit should always read as a 0."
> But I'll add
> .CP0_Config5 = MIPS_CONFIG5 | (1U << CP0C5_NFExists)
> for completeness of the definition.

Thanks.

> > > +        .CP0_Config7 = 0x1 << CP0C7_WII,
> > >            .CP0_LLAddr_rw_bitmask = 0,
> > >            .CP0_LLAddr_shift = 4,
> > >            .SYNCI_Step = 32,
> >
> > Could you also set CP0C7_WII to the P5600 definition?
>
> OK, will add that.

Great, thank you :)