| Message ID | 20201215225757.764263-4-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/mips: Convert MSA ASE to decodetree | expand |
On 12/15/20 4:57 PM, Philippe Mathieu-Daudé wrote: > +bool isa_rel6_available(const CPUMIPSState *env) > +{ > + if (TARGET_LONG_BITS == 64) { > + return cpu_supports_isa(env, ISA_MIPS64R6); > + } > + return cpu_supports_isa(env, ISA_MIPS32R6); > +} So... does qemu-system-mips64 support 32-bit cpus? If so, this needs to be written if (TARGET_LONG_BITS == 64 && cpu_supports_isa(...)) { return true; } Otherwise, this will return false for a mips32r6 cpu. r~
On 12/16/20 12:27 AM, Richard Henderson wrote: > On 12/15/20 4:57 PM, Philippe Mathieu-Daudé wrote: >> +bool isa_rel6_available(const CPUMIPSState *env) >> +{ >> + if (TARGET_LONG_BITS == 64) { >> + return cpu_supports_isa(env, ISA_MIPS64R6); >> + } >> + return cpu_supports_isa(env, ISA_MIPS32R6); >> +} > > So... does qemu-system-mips64 support 32-bit cpus? Well... TBH I never tested it :S It looks the TCG code is compiled with 64-bit TL registers, the machine address space is 64-bit regardless the CPU, and I see various #ifdef MIPS64 code that look dubious with 32-bit CPU. > > If so, this needs to be written > > if (TARGET_LONG_BITS == 64 && cpu_supports_isa(...)) { > return true; > } > > Otherwise, this will return false for a mips32r6 cpu. I see. Rel6 is new to me, so I'll have to look at the ISA manuals before returning to this thread with an answer. Thanks for reviewing the series! Phil.
在 2020/12/16 上午7:48, Philippe Mathieu-Daudé 写道: > On 12/16/20 12:27 AM, Richard Henderson wrote: >> On 12/15/20 4:57 PM, Philippe Mathieu-Daudé wrote: >>> +bool isa_rel6_available(const CPUMIPSState *env) >>> +{ >>> + if (TARGET_LONG_BITS == 64) { >>> + return cpu_supports_isa(env, ISA_MIPS64R6); >>> + } >>> + return cpu_supports_isa(env, ISA_MIPS32R6); >>> +} >> So... does qemu-system-mips64 support 32-bit cpus? > Well... TBH I never tested it :S It looks the TCG code > is compiled with 64-bit TL registers, the machine address > space is 64-bit regardless the CPU, and I see various > #ifdef MIPS64 code that look dubious with 32-bit CPU. qemu-system-mips64 and qemu-system-mips64el do support 32bit CPUs like M14Kc and P5600 :-) Sometimes I'm just curious about the necessity of having mips/mipsel targets >> If so, this needs to be written >> >> if (TARGET_LONG_BITS == 64 && cpu_supports_isa(...)) { >> return true; >> } >> >> Otherwise, this will return false for a mips32r6 cpu. > I see. Rel6 is new to me, so I'll have to look at the ISA > manuals before returning to this thread with an answer. TBH I do think it doesn't sounds like a good idea to make 32-bit and 64-bit different. In fact ISA_MIPS32R6 is always set for targets with ISA_MIPS64R6. We're treating MIPS64R6 as a superset of MIPS32R6, and ISA_MIPS3 is used to tell if a CPU supports 64-bit. FYI: https://commons.wikimedia.org/wiki/File:MIPS_instruction_set_family.svg Thanks. - Jiaxun > > Thanks for reviewing the series! > > Phil.
在 2020/12/16 上午10:50, Jiaxun Yang 写道: > > > TBH I do think it doesn't sounds like a good idea to make 32-bit > and 64-bit different. In fact ISA_MIPS32R6 is always set for targets > with ISA_MIPS64R6. > > We're treating MIPS64R6 as a superset of MIPS32R6, and ISA_MIPS3 > is used to tell if a CPU supports 64-bit. > > FYI: > https://commons.wikimedia.org/wiki/File:MIPS_instruction_set_family.svg Just add more cents here... The current method we handle R6 makes me a little bit annoying. Given that MIPS is backward compatible until R5, and R6 reorganized a lot of opcodes, I do think decoding procdure of R6 should be dedicated from the rest, otherwise we may fall into the hell of finding difference between R6 and previous ISAs, also I've heard some R6 only ASEs is occupying opcodes marked as "removed in R6", so it doesn't looks like a wise idea to check removed in R6 in helpers. So we may end up having four series of decodetrees for ISA Series1: MIPS-II, MIPS32, MIPS32R2, MIPS32R5 (32bit "old" ISAs) Series2: MIPS-III, MIPS64, MIPS64R2, MIPS64R5 (64bit "old" ISAs) Series3: MIPS32R6 (32bit "new" ISAs) Series4: MIPS64R6 (64bit "new" ISAs) Thanks - Jiaxun > > Thanks. > > - Jiaxun > > >> >> Thanks for reviewing the series! >> >> Phil. >
On 12/16/20 4:14 AM, Jiaxun Yang wrote: > 在 2020/12/16 上午10:50, Jiaxun Yang 写道: >> >> >> TBH I do think it doesn't sounds like a good idea to make 32-bit >> and 64-bit different. In fact ISA_MIPS32R6 is always set for targets >> with ISA_MIPS64R6. >> >> We're treating MIPS64R6 as a superset of MIPS32R6, and ISA_MIPS3 >> is used to tell if a CPU supports 64-bit. Which is why I don't understand why they are 2 ISA_MIPS32R6/ISA_MIPS64R6 definitions. >> >> FYI: >> https://commons.wikimedia.org/wiki/File:MIPS_instruction_set_family.svg > > Just add more cents here... > The current method we handle R6 makes me a little bit annoying. > > Given that MIPS is backward compatible until R5, and R6 reorganized a lot > of opcodes, I do think decoding procdure of R6 should be dedicated from > the rest, > otherwise we may fall into the hell of finding difference between R6 and > previous > ISAs, also I've heard some R6 only ASEs is occupying opcodes marked as > "removed in R6", so it doesn't looks like a wise idea to check removed > in R6 > in helpers. I'm not sure I understood well your comment, but I also find how R6 is handled messy... I'm doing this removal (from helper to decoder) with the decodetree conversion. > So we may end up having four series of decodetrees for ISA > Series1: MIPS-II, MIPS32, MIPS32R2, MIPS32R5 (32bit "old" ISAs) > Series2: MIPS-III, MIPS64, MIPS64R2, MIPS64R5 (64bit "old" ISAs) > > Series3: MIPS32R6 (32bit "new" ISAs) > Series4: MIPS64R6 (64bit "new" ISAs) Something like that, I'm starting by converting the messier leaves first, so the R6 and ASEs. My approach is from your "series4" to "series1" last. Regards, Phil.
On 12/16/20 12:48 AM, Philippe Mathieu-Daudé wrote: > On 12/16/20 12:27 AM, Richard Henderson wrote: >> On 12/15/20 4:57 PM, Philippe Mathieu-Daudé wrote: >>> +bool isa_rel6_available(const CPUMIPSState *env) >>> +{ >>> + if (TARGET_LONG_BITS == 64) { >>> + return cpu_supports_isa(env, ISA_MIPS64R6); >>> + } >>> + return cpu_supports_isa(env, ISA_MIPS32R6); >>> +} >> >> So... does qemu-system-mips64 support 32-bit cpus? > > Well... TBH I never tested it :S It looks the TCG code > is compiled with 64-bit TL registers, the machine address > space is 64-bit regardless the CPU, and I see various > #ifdef MIPS64 code that look dubious with 32-bit CPU. I don't think 32-bit CPUs on 64-bit build currently work well, see: 382 env->CP0_Status = (MIPS_HFLAG_UM << CP0St_KSU); 383 # ifdef TARGET_MIPS64 384 /* Enable 64-bit register mode. */ 385 env->CP0_Status |= (1 << CP0St_PX); 386 # endif 387 # ifdef TARGET_ABI_MIPSN64 388 /* Enable 64-bit address mode. */ 389 env->CP0_Status |= (1 << CP0St_UX); 390 # endif 391 /* 392 * Enable access to the CPUNum, SYNCI_Step, CC, and CCRes RDHWR 393 * hardware registers. 394 */ 395 env->CP0_HWREna |= 0x0000000F; 396 if (env->CP0_Config1 & (1 << CP0C1_FP)) { 397 env->CP0_Status |= (1 << CP0St_CU1); 398 } 399 if (env->CP0_Config3 & (1 << CP0C3_DSPP)) { 400 env->CP0_Status |= (1 << CP0St_MX); 401 } 402 # if defined(TARGET_MIPS64) 403 /* For MIPS64, init FR bit to 1 if FPU unit is there and bit is writable. */ 404 if ((env->CP0_Config1 & (1 << CP0C1_FP)) && 405 (env->CP0_Status_rw_bitmask & (1 << CP0St_FR))) { 406 env->CP0_Status |= (1 << CP0St_FR); 407 } 408 # endif Or: 1018 void helper_mtc0_pwsize(CPUMIPSState *env, target_ulong arg1) 1019 { 1020 #if defined(TARGET_MIPS64) 1021 env->CP0_PWSize = arg1 & 0x3F7FFFFFFFULL; 1022 #else 1023 env->CP0_PWSize = arg1 & 0x3FFFFFFF; 1024 #endif 1025 } 1038 void helper_mtc0_pwctl(CPUMIPSState *env, target_ulong arg1) 1039 { 1040 #if defined(TARGET_MIPS64) 1041 /* PWEn = 0. Hardware page table walking is not implemented. */ 1042 env->CP0_PWCtl = (env->CP0_PWCtl & 0x000000C0) | (arg1 & 0x5C00003F); 1043 #else 1044 env->CP0_PWCtl = (arg1 & 0x800000FF); 1045 #endif 1046 }
On 12/16/20 11:50 AM, Philippe Mathieu-Daudé wrote: > On 12/16/20 4:14 AM, Jiaxun Yang wrote: >> 在 2020/12/16 上午10:50, Jiaxun Yang 写道: >>> >>> >>> TBH I do think it doesn't sounds like a good idea to make 32-bit >>> and 64-bit different. In fact ISA_MIPS32R6 is always set for targets >>> with ISA_MIPS64R6. >>> >>> We're treating MIPS64R6 as a superset of MIPS32R6, and ISA_MIPS3 >>> is used to tell if a CPU supports 64-bit. I suppose you are talking about the CPU definitions (CPU_MIPS32R6/CPU_MIPS64R6). > > Which is why I don't understand why they are 2 ISA_MIPS32R6/ISA_MIPS64R6 > definitions. My comment is about the ISA definitions: #define ISA_MIPS32 0x0000000000000020ULL #define ISA_MIPS32R2 0x0000000000000040ULL #define ISA_MIPS64 0x0000000000000080ULL #define ISA_MIPS64R2 0x0000000000000100ULL #define ISA_MIPS32R3 0x0000000000000200ULL #define ISA_MIPS64R3 0x0000000000000400ULL #define ISA_MIPS32R5 0x0000000000000800ULL #define ISA_MIPS64R5 0x0000000000001000ULL #define ISA_MIPS32R6 0x0000000000002000ULL #define ISA_MIPS64R6 0x0000000000004000ULL > >>> >>> FYI: >>> https://commons.wikimedia.org/wiki/File:MIPS_instruction_set_family.svg >> >> Just add more cents here... >> The current method we handle R6 makes me a little bit annoying. >> >> Given that MIPS is backward compatible until R5, and R6 reorganized a lot >> of opcodes, I do think decoding procdure of R6 should be dedicated from >> the rest, >> otherwise we may fall into the hell of finding difference between R6 and >> previous >> ISAs, also I've heard some R6 only ASEs is occupying opcodes marked as >> "removed in R6", so it doesn't looks like a wise idea to check removed >> in R6 >> in helpers. > > I'm not sure I understood well your comment, but I also find how > R6 is handled messy... > > I'm doing this removal (from helper to decoder) with the decodetree > conversion. > >> So we may end up having four series of decodetrees for ISA >> Series1: MIPS-II, MIPS32, MIPS32R2, MIPS32R5 (32bit "old" ISAs) >> Series2: MIPS-III, MIPS64, MIPS64R2, MIPS64R5 (64bit "old" ISAs) >> >> Series3: MIPS32R6 (32bit "new" ISAs) >> Series4: MIPS64R6 (64bit "new" ISAs) > > Something like that, I'm starting by converting the messier leaves > first, so the R6 and ASEs. My approach is from your "series4" to > "series1" last. > > Regards, > > Phil. >
在 2020/12/16 下午6:59, Philippe Mathieu-Daudé 写道: > On 12/16/20 11:50 AM, Philippe Mathieu-Daudé wrote: >> On 12/16/20 4:14 AM, Jiaxun Yang wrote: >>> 在 2020/12/16 上午10:50, Jiaxun Yang 写道: >>>> >>>> TBH I do think it doesn't sounds like a good idea to make 32-bit >>>> and 64-bit different. In fact ISA_MIPS32R6 is always set for targets >>>> with ISA_MIPS64R6. >>>> >>>> We're treating MIPS64R6 as a superset of MIPS32R6, and ISA_MIPS3 >>>> is used to tell if a CPU supports 64-bit. > I suppose you are talking about the CPU definitions > (CPU_MIPS32R6/CPU_MIPS64R6). > >> Which is why I don't understand why they are 2 ISA_MIPS32R6/ISA_MIPS64R6 >> definitions. > My comment is about the ISA definitions: > > #define ISA_MIPS32 0x0000000000000020ULL > #define ISA_MIPS32R2 0x0000000000000040ULL > #define ISA_MIPS64 0x0000000000000080ULL > #define ISA_MIPS64R2 0x0000000000000100ULL > #define ISA_MIPS32R3 0x0000000000000200ULL > #define ISA_MIPS64R3 0x0000000000000400ULL > #define ISA_MIPS32R5 0x0000000000000800ULL > #define ISA_MIPS64R5 0x0000000000001000ULL > #define ISA_MIPS32R6 0x0000000000002000ULL > #define ISA_MIPS64R6 0x0000000000004000ULL Yes, as insn_flags is set by CPU definitions. ...... /* MIPS Technologies "Release 1" */ #define CPU_MIPS32 (CPU_MIPS2 | ISA_MIPS32) #define CPU_MIPS64 (CPU_MIPS5 | CPU_MIPS32 | ISA_MIPS64) /* MIPS Technologies "Release 2" */ #define CPU_MIPS32R2 (CPU_MIPS32 | ISA_MIPS32R2) #define CPU_MIPS64R2 (CPU_MIPS64 | CPU_MIPS32R2 | ISA_MIPS64R2) ...... As you can see when you're set insn_flags to CPU_MIPS64R2 the ISA flags for ISA_MIPS64R2 ISA_MIPS32R2 ISA_MIPS32 ISA_MIPS64 ISA_MIPS5 ISA_MIPS3 ISA_MIPS2 ISA_MIPS1 all get set as well. >>> So we may end up having four series of decodetrees for ISA >>> Series1: MIPS-II, MIPS32, MIPS32R2, MIPS32R5 (32bit "old" ISAs) >>> Series2: MIPS-III, MIPS64, MIPS64R2, MIPS64R5 (64bit "old" ISAs) >>> >>> Series3: MIPS32R6 (32bit "new" ISAs) >>> Series4: MIPS64R6 (64bit "new" ISAs) >> Something like that, I'm starting by converting the messier leaves >> first, so the R6 and ASEs. My approach is from your "series4" to >> "series1" last. Sounds neat! Thanks - Jiaxun
在 2020/12/16 下午6:59, Philippe Mathieu-Daudé 写道: > On 12/16/20 11:50 AM, Philippe Mathieu-Daudé wrote: >> On 12/16/20 4:14 AM, Jiaxun Yang wrote: >>> 在 2020/12/16 上午10:50, Jiaxun Yang 写道: >>>> TBH I do think it doesn't sounds like a good idea to make 32-bit >>>> and 64-bit different. In fact ISA_MIPS32R6 is always set for targets >>>> with ISA_MIPS64R6. >>>> >>>> We're treating MIPS64R6 as a superset of MIPS32R6, and ISA_MIPS3 >>>> is used to tell if a CPU supports 64-bit. > I suppose you are talking about the CPU definitions > (CPU_MIPS32R6/CPU_MIPS64R6). > >> Which is why I don't understand why they are 2 ISA_MIPS32R6/ISA_MIPS64R6 >> definitions. > My comment is about the ISA definitions: > > #define ISA_MIPS32 0x0000000000000020ULL > #define ISA_MIPS32R2 0x0000000000000040ULL > #define ISA_MIPS64 0x0000000000000080ULL > #define ISA_MIPS64R2 0x0000000000000100ULL > #define ISA_MIPS32R3 0x0000000000000200ULL > #define ISA_MIPS64R3 0x0000000000000400ULL > #define ISA_MIPS32R5 0x0000000000000800ULL > #define ISA_MIPS64R5 0x0000000000001000ULL > #define ISA_MIPS32R6 0x0000000000002000ULL > #define ISA_MIPS64R6 0x0000000000004000ULL Yes, as insn_flags is /* MIPS Technologies "Release 1" */ #define CPU_MIPS32 (CPU_MIPS2 | ISA_MIPS32) #define CPU_MIPS64 (CPU_MIPS5 | CPU_MIPS32 | ISA_MIPS64) /* MIPS Technologies "Release 2" */ #define CPU_MIPS32R2 (CPU_MIPS32 | ISA_MIPS32R2) #define CPU_MIPS64R2 (CPU_MIPS64 | CPU_MIPS32R2 | ISA_MIPS64R2) ...... As you can see when you're set insn_flags to CPU_MIPS64R2 the ISA flags for ISA_MIPS64R2 ISA_MIPS32R2 ISA_MIPS32 ISA_MIPS64 ISA_MIPS5 ISA_MIPS3 ISA_MIPS2 ISA_MIPS1 all get set as well. >> >>> So we may end up having four series of decodetrees for ISA >>> Series1: MIPS-II, MIPS32, MIPS32R2, MIPS32R5 (32bit "old" ISAs) >>> Series2: MIPS-III, MIPS64, MIPS64R2, MIPS64R5 (64bit "old" ISAs) >>> >>> Series3: MIPS32R6 (32bit "new" ISAs) >>> Series4: MIPS64R6 (64bit "new" ISAs) >> Something like that, I'm starting by converting the messier leaves >> first, so the R6 and ASEs. My approach is from your "series4" to >> "series1" last. Sounds neat! Thanks. - Jiaxun >> >> Regards, >> >> Phil. >>
On 12/16/20 4:14 AM, Jiaxun Yang wrote: > 在 2020/12/16 上午10:50, Jiaxun Yang 写道: >> TBH I do think it doesn't sounds like a good idea to make 32-bit >> and 64-bit different. In fact ISA_MIPS32R6 is always set for targets >> with ISA_MIPS64R6. >> >> We're treating MIPS64R6 as a superset of MIPS32R6, and ISA_MIPS3 >> is used to tell if a CPU supports 64-bit. >> >> FYI: >> https://commons.wikimedia.org/wiki/File:MIPS_instruction_set_family.svg > > Just add more cents here... > The current method we handle R6 makes me a little bit annoying. > > Given that MIPS is backward compatible until R5, and R6 reorganized a lot > of opcodes, I do think decoding procdure of R6 should be dedicated from > the rest, > otherwise we may fall into the hell of finding difference between R6 and > previous > ISAs, also I've heard some R6 only ASEs is occupying opcodes marked as > "removed in R6", so it doesn't looks like a wise idea to check removed > in R6 > in helpers. I think we are in agreement :) Your comment seems what I addressed last month as this series: https://gitlab.com/philmd/qemu/-/commits/mips_decodetree_lsa_r6/ (I'll try to rebase it and post during the week-end.) > So we may end up having four series of decodetrees for ISA > Series1: MIPS-II, MIPS32, MIPS32R2, MIPS32R5 (32bit "old" ISAs) > Series2: MIPS-III, MIPS64, MIPS64R2, MIPS64R5 (64bit "old" ISAs) > > Series3: MIPS32R6 (32bit "new" ISAs) > Series4: MIPS64R6 (64bit "new" ISAs) > > Thanks > > - Jiaxun
On 1/7/21 10:04 AM, Philippe Mathieu-Daudé wrote: > On 12/16/20 4:14 AM, Jiaxun Yang wrote: >> 在 2020/12/16 上午10:50, Jiaxun Yang 写道: >>> TBH I do think it doesn't sounds like a good idea to make 32-bit >>> and 64-bit different. In fact ISA_MIPS32R6 is always set for targets >>> with ISA_MIPS64R6. >>> >>> We're treating MIPS64R6 as a superset of MIPS32R6, and ISA_MIPS3 >>> is used to tell if a CPU supports 64-bit. >>> >>> FYI: >>> https://commons.wikimedia.org/wiki/File:MIPS_instruction_set_family.svg >> >> Just add more cents here... >> The current method we handle R6 makes me a little bit annoying. >> >> Given that MIPS is backward compatible until R5, and R6 reorganized a lot >> of opcodes, I do think decoding procdure of R6 should be dedicated from >> the rest, >> otherwise we may fall into the hell of finding difference between R6 and >> previous >> ISAs, also I've heard some R6 only ASEs is occupying opcodes marked as >> "removed in R6", so it doesn't looks like a wise idea to check removed >> in R6 >> in helpers. > > I think we are in agreement :) Your comment seems what I addressed > last month as this series: > https://gitlab.com/philmd/qemu/-/commits/mips_decodetree_lsa_r6/ > (I'll try to rebase it and post during the week-end.) Bah actually it is already on the list and reviewed =) https://www.mail-archive.com/qemu-devel@nongnu.org/msg765234.html >> So we may end up having four series of decodetrees for ISA >> Series1: MIPS-II, MIPS32, MIPS32R2, MIPS32R5 (32bit "old" ISAs) >> Series2: MIPS-III, MIPS64, MIPS64R2, MIPS64R5 (64bit "old" ISAs) >> >> Series3: MIPS32R6 (32bit "new" ISAs) >> Series4: MIPS64R6 (64bit "new" ISAs) >> >> Thanks >> >> - Jiaxun >
diff --git a/target/mips/cpu.h b/target/mips/cpu.h index 3ac21d0e9c0..c6a556efad5 100644 --- a/target/mips/cpu.h +++ b/target/mips/cpu.h @@ -1289,6 +1289,7 @@ int cpu_mips_signal_handler(int host_signum, void *pinfo, void *puc); bool cpu_type_supports_cps_smp(const char *cpu_type); bool cpu_supports_isa(const CPUMIPSState *env, uint64_t isa_mask); bool cpu_type_supports_isa(const char *cpu_type, uint64_t isa); +bool isa_rel6_available(const CPUMIPSState *env); /* Check presence of multi-threading ASE implementation */ static inline bool ase_mt_available(CPUMIPSState *env) diff --git a/target/mips/cpu.c b/target/mips/cpu.c index 4191c0741f4..9f082518076 100644 --- a/target/mips/cpu.c +++ b/target/mips/cpu.c @@ -789,6 +789,14 @@ bool cpu_supports_isa(const CPUMIPSState *env, uint64_t isa_mask) return (env->cpu_model->insn_flags & isa_mask) != 0; } +bool isa_rel6_available(const CPUMIPSState *env) +{ + if (TARGET_LONG_BITS == 64) { + return cpu_supports_isa(env, ISA_MIPS64R6); + } + return cpu_supports_isa(env, ISA_MIPS32R6); +} + bool cpu_type_supports_isa(const char *cpu_type, uint64_t isa) { const MIPSCPUClass *mcc = MIPS_CPU_CLASS(object_class_by_name(cpu_type));
Introduce the isa_rel6_available() helper to check if the CPU supports the Release 6 ISA. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- target/mips/cpu.h | 1 + target/mips/cpu.c | 8 ++++++++ 2 files changed, 9 insertions(+)