Re: [PATCH v3 1/1] accel/tcg: Fix computing of is_write for MIPS

[Richard Henderson]

On 9/25/20 1:33 AM, Kele Huang wrote:
> Detect all MIPS store instructions in cpu_signal_handler for all available
> MIPS versions, and set is_write if encountering such store instructions.
> 
> This fixed the error while dealing with self-modified code for MIPS.
> 
> Signed-off-by: Kele Huang <kele.hwang@gmail.com>
> Signed-off-by: Xu Zou <iwatchnima@gmail.com>
> ---
>  accel/tcg/user-exec.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index bb039eb32d..c4494c93e7 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -702,6 +702,10 @@ int cpu_signal_handler(int host_signum, void *pinfo,
>  
>  #elif defined(__mips__)
>  
> +#if defined(__misp16) || defined(__mips_micromips)
> +#error "Unsupported encoding"
> +#endif
> +
>  int cpu_signal_handler(int host_signum, void *pinfo,
>                         void *puc)
>  {
> @@ -709,9 +713,41 @@ int cpu_signal_handler(int host_signum, void *pinfo,
>      ucontext_t *uc = puc;
>      greg_t pc = uc->uc_mcontext.pc;
>      int is_write;
> +    uint32_t insn;
>  
> -    /* XXX: compute is_write */
> +    /* Detect all store instructions at program counter. */
>      is_write = 0;
> +    insn = *(uint32_t *)pc;
> +    switch((insn >> 26) & 077) {
> +    case 050: /* SB */
> +    case 051: /* SH */
> +    case 052: /* SWL */
> +    case 053: /* SW */
> +    case 054: /* SDL */
> +    case 055: /* SDR */
> +    case 056: /* SWR */
> +    case 070: /* SC */
> +    case 071: /* SWC1 */
> +    case 074: /* SCD */
> +    case 075: /* SDC1 */
> +    case 077: /* SD */
> +#if !defined(__mips_isa_rev) || __mips_isa_rev < 6
> +    case 072: /* SWC2 */
> +    case 076: /* SDC2 */
> +#endif
> +        is_write = 1;
> +        break;
> +    case 023: /* COP1X */
> +        /* Required in all versions of MIPS64 since 
> +           MIPS64r1 and subsequent versions of MIPS32. */
> +        switch (insn & 077) {
> +        case 010: /* SWXC1 */
> +        case 011: /* SDXC1 */
> +            is_write = 1;

Much better.  I just noticed you're missing SUXC1 (COP1X minor 015).  With that
fixed,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~