Hi Richard, Cc'ing developers who introduced nanoMIPS.
On 9/11/20 7:23 PM, Philippe Mathieu-Daudé wrote: > On 9/11/20 6:55 PM, Richard Henderson wrote: >> On 9/11/20 3:41 AM, Peter Maydell wrote: >>>> + /* Detect store by reading the instruction at the program counter. */ >>>> + uint32_t insn = *(uint32_t *)pc; >>>> + switch(insn>>29) { >>>> + case 0x5: >>>> + switch((insn>>26) & 0x7) { >>> >>> Here we mask to get a 3-bit field... >>> >>>> + case 0x0: /* SB */ >>>> + case 0x1: /* SH */ >>>> + case 0x2: /* SWL */ >>>> + case 0x3: /* SW */ >>>> + case 0x4: /* SDL */ >>>> + case 0x5: /* SDR */ >>>> + case 0x6: /* SWR */ >>>> + case 0x7: /* CACHE */ >>>> + is_write = 1; >>> >>> ...but here all 8 cases are handled identically. >>> Is there a typo/logic error here, or should this >>> really just be >>> >>> case 0x5: >>> /* SB, SH, SWL, SW, SDL, SDR, SWR, CACHE */ >>> is_write = 1; >>> >>> ? >>> >>> Is CACHE really a write insn ? >> >> Indeed not. However, it's also illegal for user-mode, so we cannot arrive >> here >> with SIGSEGV by executing it. So we could ignore that case and not decode >> this >> field. >> >>>> + case 0x7: >>>> + switch((insn>>26) & 0x7) { >>>> + case 0x0: /* SC */ >>>> + case 0x1: /* SWC1 */ >>>> + case 0x2: /* SWC2 */ >>>> + case 0x4: /* SCD */ >>>> + case 0x5: /* SDC1 */ >>>> + case 0x6: /* SDC2 */ >>>> + case 0x7: /* SD */ >>>> + is_write = 1; >> >> Well, the unconditional check of SWC2/SDC2 is not quite right. MIPS64R6 >> removes >> them and replaces them with some compact branches. That's easy enough to >> include here, using >> >> #if !defined(__mips_isa_rev) || __mips_isa_rev < 6 >> case 2: /* SWC2 */ >> case 6: /* SDC2 */ >> #endif >> >> We should also add >> >> #if defined(__mips16) || defined(__mips_micromips) >> # error "Unsupported encoding" >> #endif >> >> I see no upstream compiler support for nanomips at all, so there's no point >> in >> checking for that encoding. (Indeed, I wonder at the code in target/mips... >> how could it be tested?) > > I took the information from commit f7d257cb4a1 > ("qemu-doc: Add nanoMIPS ISA information") to add > the tests in f375ad6a0d6 ("BootLinuxConsoleTest: > Test nanoMIPS kernels on the I7200 CPU"), but I > haven't tried to recompile these files myself. I checked the various nanoMIPS announcements: GCC: https://gcc.gnu.org/legacy-ml/gcc/2018-05/msg00012.html Linux: https://lwn.net/Articles/753605/ QEMU: https://www.mail-archive.com/qemu-devel@nongnu.org/msg530721.html Unfortunately www.mips.com doesn't work anymore. >From this Wayback machine link: https://web.archive.org/web/20180904044530/https://www.mips.com/develop/tools/compilers/ I found this working web (the toolchain is a later release than the one referenced in the announcement mails): http://codescape.mips.com/components/toolchain/nanomips/2018.04-02/downloads.html The toolchain page mention LLVM but simply links http://llvm.org/ where I couldn't find any reference on nanoMIPS. The only reference in the GCC mailing list, is the nanoMIPS announcement: https://gcc.gnu.org/pipermail/gcc/2018-May.txt It looks safe for QEMU to declare nanoMIPS deprecated (it has no maintainer), to give time to interested parties to finish upstreaming process and step in to maintain it. Thoughts? Regards, Phil.