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.
Regards,
Phil.
