Re: [Qemu-devel] [PATCH v9 40/84] target/mips: Fix pre-nanoMIPS MT ASE instructions availability control
> > I think some of the previously-implemented similar cases involving > > read-only bits were handled the same way, and we just built on that. What > > would you suggest as a more appropriate solution in such cases (of > > accessing "preset by hardware" bits)? > > Well, ctx->insn_flags and ctx->CP0_Config1 are good examples. > These are 100% read-only and fixed at cpu instantiation. > > I see that CP0_Config3 has one writable bit for micromips, but > is fully readonly for nanomips. Therefore XNP and MT need not > be copied to hflags because they will never vary. > > I'd suggest copying CP0_Config3 to ctx as with Config1. > > > r~ Hi, Richard, We ended up implementing this feature the way you suggested in the v11. Sorry about snafu. Regards, Aleksandar
Re: [Qemu-devel] [PATCH v9 40/84] target/mips: Fix pre-nanoMIPS MT ASE instructions availability control
> > I think some of the previously-implemented similar cases involving > > read-only bits were handled the same way, and we just built on that. What > > would you suggest as a more appropriate solution in such cases (of > > accessing "preset by hardware" bits)? > > Well, ctx->insn_flags and ctx->CP0_Config1 are good examples. > These are 100% read-only and fixed at cpu instantiation. > > I see that CP0_Config3 has one writable bit for micromips, but > is fully readonly for nanomips. Therefore XNP and MT need not > be copied to hflags because they will never vary. > > I'd suggest copying CP0_Config3 to ctx as with Config1. > > > r~ Hi, Richard, The opinion within the team is that we should leave such changes for follow-up clean-up - clean-up of CP0-related functionalities is scheduled anyway soon. The reason is that the current implementation (in v9) works fine, and this is very late in our dev cycle to change features with no observed bugs. All other your concerns will be addressed in v10, which is planned to be sent soon. Yours, Aleksandar
Re: [Qemu-devel] [PATCH v9 40/84] target/mips: Fix pre-nanoMIPS MT ASE instructions availability control
On 08/16/2018 10:06 AM, Aleksandar Markovic wrote: > I think some of the previously-implemented similar cases involving read-only > bits were handled the same way, and we just built on that. What would you > suggest as a more appropriate solution in such cases (of accessing "preset by > hardware" bits)? Well, ctx->insn_flags and ctx->CP0_Config1 are good examples. These are 100% read-only and fixed at cpu instantiation. I see that CP0_Config3 has one writable bit for micromips, but is fully readonly for nanomips. Therefore XNP and MT need not be copied to hflags because they will never vary. I'd suggest copying CP0_Config3 to ctx as with Config1. r~
Re: [Qemu-devel] [PATCH v9 40/84] target/mips: Fix pre-nanoMIPS MT ASE instructions availability control
On 08/16/2018 07:57 AM, Aleksandar Markovic wrote: > From: Aleksandar Rikalo > > Use bits from configuration registers for availability control > of MT ASE instructions, rather than only ISA_MT bit in insn_flags. > This is done by adding a field in hflags for MT bit, and adding > functions check_mt() and check_cp0_mt(). > > Reviewed-by: Aleksandar Markovic > Signed-off-by: Aleksandar Markovic > Signed-off-by: Stefan Markovic > --- > target/mips/cpu.h | 3 ++- > target/mips/internal.h | 6 +- > target/mips/translate.c | 45 + > 3 files changed, 44 insertions(+), 10 deletions(-) What was wrong with using insn_flags? I'll note that hflags should be reserved for things that can change at runtime. I thought all of these configuration registers were read-only. Anyway, with this plus the XNP patch from earlier, you now only have one remaining bit within hflags and then that resource is exhausted. r~
Re: [Qemu-devel] [PATCH v9 40/84] target/mips: Fix pre-nanoMIPS MT ASE instructions availability control
> From: Richard Henderson > Sent: Thursday, August 16, 2018 6:37 PM > > Subject: Re: [PATCH v9 40/84] target/mips: Fix pre-nanoMIPS MT ASE > instructions availability control > > On 08/16/2018 07:57 AM, Aleksandar Markovic wrote: > > From: Aleksandar Rikalo > > > > Use bits from configuration registers for availability control > > of MT ASE instructions, rather than only ISA_MT bit in insn_flags. > > This is done by adding a field in hflags for MT bit, and adding > > functions check_mt() and check_cp0_mt(). > > > > Reviewed-by: Aleksandar Markovic > > Signed-off-by: Aleksandar Markovic > > Signed-off-by: Stefan Markovic > > --- > > target/mips/cpu.h | 3 ++- > > target/mips/internal.h | 6 +- > > target/mips/translate.c | 45 + > > 3 files changed, 44 insertions(+), 10 deletions(-) > > What was wrong with using insn_flags? > The problem with using ISA_MT from insn_flags is that it doesn't provide the correct availability control. Even though all instructions from MT ASE are related, they are available under different conditions: Case 1 - DMT, DVPE, EMT, EVPE: if IsCoprocessorEnabled(0) then if Config3 MT then else SignalException(ReservedInstruction) endif else SignalException(CoprocessorUnusable, 0) endif Case 2 - FORK, YIELD: if Config3 MT then else SignalException(ReservedInstruction) endif Case 3 - MFTR, MTTR: if IsCoprocessorEnabled(0) then else SignalException(CoprocessorUnusable, 0) endif > I'll note that hflags should be reserved for things that can change at > runtime. > I thought all of these configuration registers were read-only. > > Anyway, with this plus the XNP patch from earlier, you now only have one > remaining bit within hflags and then that resource is exhausted. > I think some of the previously-implemented similar cases involving read-only bits were handled the same way, and we just built on that. What would you suggest as a more appropriate solution in such cases (of accessing "preset by hardware" bits)? Regards, Aleksandar
