| Message ID | 20230215084754.3816747-4-marcin.nowakowski@fungible.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/mips: misc microMIPS fixes | expand |
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, I'm not sure about: + .CP0_Config5 = MIPS_CONFIG5, + .CP0_Config5_rw_bitmask = 0, > + .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? > diff --git a/target/mips/cpu.c b/target/mips/cpu.c > index 7a565466cb..7ba359696f 100644 > --- a/target/mips/cpu.c > +++ b/target/mips/cpu.c > @@ -144,12 +144,14 @@ static bool mips_cpu_has_work(CPUState *cs) > /* > * Prior to MIPS Release 6 it is implementation dependent if non-enabled > * interrupts wake-up the CPU, however most of the implementations only > - * check for interrupts that can be taken. > + * check for interrupts that can be taken. For pre-release 6 CPUs, > + * check for CP0 Config7 'Wait IE ignore' bit. > */ > if ((cs->interrupt_request & CPU_INTERRUPT_HARD) && > cpu_mips_hw_interrupts_pending(env)) { > if (cpu_mips_hw_interrupts_enabled(env) || > - (env->insn_flags & ISA_MIPS_R6)) { > + (env->insn_flags & ISA_MIPS_R6) || > + (env->CP0_Config7 & (1 << CP0C7_WII))) { > has_work = true; > } > } > diff --git a/target/mips/cpu.h b/target/mips/cpu.h > index 0a085643a3..abee7a99d7 100644 > --- a/target/mips/cpu.h > +++ b/target/mips/cpu.h > @@ -980,6 +980,7 @@ typedef struct CPUArchState { > #define CP0C6_DATAPREF 0 > int32_t CP0_Config7; > int64_t CP0_Config7_rw_bitmask; > +#define CP0C7_WII 31 > #define CP0C7_NAPCGEN 2 > #define CP0C7_UNIMUEN 1 > #define CP0C7_VFPUCGEN 0
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) 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. > > + .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. Marcin > > diff --git a/target/mips/cpu.c b/target/mips/cpu.c > > index 7a565466cb..7ba359696f 100644 > > --- a/target/mips/cpu.c > > +++ b/target/mips/cpu.c > > @@ -144,12 +144,14 @@ static bool mips_cpu_has_work(CPUState *cs) > > /* > > * Prior to MIPS Release 6 it is implementation dependent if non-enabled > > * interrupts wake-up the CPU, however most of the implementations only > > - * check for interrupts that can be taken. > > + * check for interrupts that can be taken. For pre-release 6 CPUs, > > + * check for CP0 Config7 'Wait IE ignore' bit. > > */ > > if ((cs->interrupt_request & CPU_INTERRUPT_HARD) && > > cpu_mips_hw_interrupts_pending(env)) { > > if (cpu_mips_hw_interrupts_enabled(env) || > > - (env->insn_flags & ISA_MIPS_R6)) { > > + (env->insn_flags & ISA_MIPS_R6) || > > + (env->CP0_Config7 & (1 << CP0C7_WII))) { > > has_work = true; > > } > > } > > diff --git a/target/mips/cpu.h b/target/mips/cpu.h > > index 0a085643a3..abee7a99d7 100644 > > --- a/target/mips/cpu.h > > +++ b/target/mips/cpu.h > > @@ -980,6 +980,7 @@ typedef struct CPUArchState { > > #define CP0C6_DATAPREF 0 > > int32_t CP0_Config7; > > int64_t CP0_Config7_rw_bitmask; > > +#define CP0C7_WII 31 > > #define CP0C7_NAPCGEN 2 > > #define CP0C7_UNIMUEN 1 > > #define CP0C7_VFPUCGEN 0 >
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 :)
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), + .CP0_Config7 = 0x1 << CP0C7_WII, .CP0_LLAddr_rw_bitmask = 0, .CP0_LLAddr_shift = 4, .SYNCI_Step = 32, diff --git a/target/mips/cpu.c b/target/mips/cpu.c index 7a565466cb..7ba359696f 100644 --- a/target/mips/cpu.c +++ b/target/mips/cpu.c @@ -144,12 +144,14 @@ static bool mips_cpu_has_work(CPUState *cs) /* * Prior to MIPS Release 6 it is implementation dependent if non-enabled * interrupts wake-up the CPU, however most of the implementations only - * check for interrupts that can be taken. + * check for interrupts that can be taken. For pre-release 6 CPUs, + * check for CP0 Config7 'Wait IE ignore' bit. */ if ((cs->interrupt_request & CPU_INTERRUPT_HARD) && cpu_mips_hw_interrupts_pending(env)) { if (cpu_mips_hw_interrupts_enabled(env) || - (env->insn_flags & ISA_MIPS_R6)) { + (env->insn_flags & ISA_MIPS_R6) || + (env->CP0_Config7 & (1 << CP0C7_WII))) { has_work = true; } } diff --git a/target/mips/cpu.h b/target/mips/cpu.h index 0a085643a3..abee7a99d7 100644 --- a/target/mips/cpu.h +++ b/target/mips/cpu.h @@ -980,6 +980,7 @@ typedef struct CPUArchState { #define CP0C6_DATAPREF 0 int32_t CP0_Config7; int64_t CP0_Config7_rw_bitmask; +#define CP0C7_WII 31 #define CP0C7_NAPCGEN 2 #define CP0C7_UNIMUEN 1 #define CP0C7_VFPUCGEN 0
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. 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(-)