Re: [PATCH-for-5.2] target/mips: Report unimplemented cache() operations

[Philippe Mathieu-Daudé]

On 8/6/20 11:37 PM, Philippe Mathieu-Daudé wrote:
> On 8/6/20 10:51 PM, Peter Maydell wrote:
>> On Thu, 6 Aug 2020 at 21:31, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>>
>>> On 8/6/20 8:01 PM, Jiaxun Yang wrote:
>>>> 在 2020/8/6 下午8:26, Philippe Mathieu-Daudé 写道:
>>>>> We only implement the Index[Store/Load]Tag from the 'cache' opcode.
>>>>> Instead of ignoring the other cache operations, report them as
>>>>> unimplemented.
>>>>
>>>> Hmm, I don't think we have anything to do with Invalidate/Writeback etc.
>>>> opcodes
>>>> in QEMU. Why do we log this?
>>>
>>> I'm noticed this code is run on Linux 3.3.8 (4KEc):
>>>
>>>     8880:       3082000f        andi    v0,a0,0xf
>>>     8884:       10800008        beqz    a0,88a8
>>>     8888:       00a21021        addu    v0,a1,v0
>>>     888c:       08002227        j       889c
>>>     8890:       00001821        move    v1,zero
>>>     8894:       bcf90000        cache   0x19,0(a3)
>>>     8898:       24630010        addiu   v1,v1,16
>>>     889c:       0062302b        sltu    a2,v1,v0
>>>     88a0:       14c0fffc        bnez    a2,8894
>>>     88a4:       00833821        addu    a3,a0,v1
>>>     88a8:       03e00008        jr      ra
>>>     88ac:       00000000        nop
>>>
>>> Why silently ignore the opcode is not implemented instead of logging it?
>>
>> I think the question is whether the opcode is supposed to have
>> some behaviour which we're not implementing, or whether "no-op"
>> is the correct behaviour for it (which it usually is for
>> cache invalidate type operations; compare the way the Arm
>> cache ops like IC_IALLU are just ARM_CP_NOP ops).
> 
> OK now I understand better, thanks.
> 
> I haven't found useful information about this 0x19=25 opcode value.

Just to close this thread, some findings from last WE:

- I couldn't find where Linux 3.3.8 use that op
- I eventually figured out it comes from a kernel module called 'tiatm'.
- This kmod is released by OpenWRT in packages named kmod-sangam-atm-annex
- Googling for strings from the object, this file has been added in [1]
based on the file included in [2]
- Someone imported these files in a git repo and published
- There is a commented reference [4] as:

#define        DataCacheHitInvalidate(a)     {__asm__(" cache  17, (%0)"
:   : "r" (a));}
#define        DataCacheHitWriteback(a)      {__asm__(" cache  25, (%0)"
:   : "r" (a));}

- Also referenced (not commented) in [5] "Linux atm module implementation".

For my use I'm happy using a trace event:

-- >8 --
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 7f87e57c8e..71b28ede2d 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -30,2 +30,3 @@
 #include "sysemu/kvm.h"
+#include "trace.h"

@@ -1577,2 +1578,4 @@ void helper_cache(CPUMIPSState *env, target_ulong
addr, uint32_t op)
     target_ulong index = addr & 0x1fffffff;
+
+    trace_cache_op(op, addr);
     if (op == 9) {
diff --git a/target/mips/trace-events b/target/mips/trace-events
index ba87fe6062..8a60f23bbd 100644
--- a/target/mips/trace-events
+++ b/target/mips/trace-events
@@ -2,2 +2,5 @@

+# op_helper.c
+cache_op(uint32_t op, uint64_t addr) "cache op:%u paddr:0x%" PRIx64
+
 # translate.c
---

[1]
https://git.openwrt.org/?p=openwrt/svn-archive/archive.git;a=commit;h=5a8a8f35c5a356f7167c3b3a3ca00f0780d86473
[2] https://dev.archive.openwrt.org/ticket/1411.html
[3] https://github.com/wolfhechel/ar7-atm
[4] https://github.com/wolfhechel/ar7-atm/blob/master/cpswhal_cpsar.h#L84
[5] https://github.com/wolfhechel/ar7-atm/blob/master/tn7atm.c#L479

> 
> On a r10k core it is listed as 'Hit Writeback Invalidate (D)' but here
> this is a 4kEc. The address used is a SRAM shared with a embedded DSP
> on the same SoC. From a RevEng PoV it is helpful to see there is a such
> cache access, as I can separate better the peripheral involved.
> I'm happy using a trace event instead.
> 
> Jiaxun, can you list me the list of opcodes QEMU can safely ignore from
> the TCG emulation PoV? That way we can comment them in the code such:
> 
>     switch (op) {
>     case 9:
>         /* Index Store Tag */
>         ...
>         break;
>     case 5:
>         /* Index Load Tag */
>         ...
>         break;
>     case X:
>     case Y:
>     case Z:
>         /* No-Op for QEMU */
>         ...
>         break;
>     default:
>         qemu_log_mask(LOG_UNIMP, "cache %u\n", op);
>     }
> 
> Thanks,
> 
> Phil.
> 
>>
>> thanks
>> -- PMM
>>
>