Re: [PATCH v2 1/1] accel/tcg: Fix computing of is_write for mips

[Kele Huang]

Got it. Thank you very much! 

I will resend the same v2 patch to v1 thread.

On Wed, 23 Sep 2020 at 19:08, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

Cc'ing the TCG MIPS maintainers, and also
Cc'ing Richard who made a comment in v1.

On 9/23/20 11:38 AM, Kele Huang wrote:
> Detect mips store instructions in cpu_signal_handler for all MIPS
> versions, and set is_write if encountering such store instructions.
>
> This fixed the error while dealing with self-modifed code for MIPS.

Quoting Eric Blake:

"It's better to post a v2 as a new top-level thread rather
than buried in-reply-to the v1 thread; among other things,
burying a reply can cause automated patch tooling to miss
the updated series."

>
> Signed-off-by: Kele Huang <kele.hwang@gmail.com>
> Signed-off-by: Xu Zou <iwatchnima@gmail.com>
> ---
>  accel/tcg/user-exec.c | 51 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index bb039eb32d..18784516e5 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -710,11 +710,60 @@ int cpu_signal_handler(int host_signum, void *pinfo,
>      greg_t pc = uc->uc_mcontext.pc;
>      int is_write;
> 
> -    /* XXX: compute is_write */
>      is_write = 0;
> +
> +    /* 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) {
> +        case 0x0: /* SB */
> +        case 0x1: /* SH */
> +        case 0x2: /* SWL */
> +        case 0x3: /* SW */
> +        case 0x4: /* SDL */
> +        case 0x5: /* SDR */
> +        case 0x6: /* SWR */
> +            is_write = 1;
> +        }
> +        break;
> +    case 0x7:
> +        switch((insn>>26) & 0x7) {
> +        case 0x0: /* SC */
> +        case 0x1: /* SWC1 */
> +        case 0x4: /* SCD */
> +        case 0x5: /* SDC1 */
> +        case 0x7: /* SD */
> +#if !defined(__mips_isa_rev) || __mips_isa_rev < 6
> +        case 0x2: /* SWC2 */
> +        case 0x6: /* SDC2 */
> +#endif
> +            is_write = 1;
> +        }
> +        break;
> +    }
> +
> +    /*
> +     * Required in all versions of MIPS64 since MIPS64r1. Not available
> +     * in MIPS32r1. Required by MIPS32r2 and subsequent versions of MIPS32.
> +     */
> +    switch ((insn >> 3) & 0x7) {
> +    case 0x1:
> +        switch (insn & 0x7) {
> +        case 0x0: /* SWXC1 */
> +        case 0x1: /* SDXC1 */
> +            is_write = 1;
> +        }
> +        break;
> +    }
> +
>      return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
>  }
> 
> +#elif defined(__misp16) || defined(__mips_micromips)
> +
> +#error "Unsupported encoding"
> +
>  #elif defined(__riscv)
> 
>  int cpu_signal_handler(int host_signum, void *pinfo,
>