Re: [PATCH 1/1] accel/tcg/user-exec: support computing is_write for mips32

[Peter Maydell]

On Fri, 11 Sep 2020 at 02:14, zou xu <iwatchnima@gmail.com> wrote:
>
> From 533ed02427bdaf0265f62fcb4f961854a41b7037 Mon Sep 17 00:00:00 2001
> From: ZouXu <iwatchnima@gmail.com>
> Date: Wed, 9 Sep 2020 21:59:13 +0800
> Subject: [PATCH 1/1] accel/tcg/user-exec: support computing is_write for
>  mips32
>
> Those MIPS32 instructions can cause store operation:
> SB/SH/SW/SD/SWL/SWR/SDL/SDR/CACHE
> SC/SCD/SWC1/SWC2/SDC1/SDC2
> ---
>  accel/tcg/user-exec.c | 34 +++++++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index bb039eb32d..b5ad721aa5 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -708,10 +708,38 @@ int cpu_signal_handler(int host_signum, void *pinfo,
>      siginfo_t *info = pinfo;
>      ucontext_t *uc = puc;
>      greg_t pc = uc->uc_mcontext.pc;
> -    int is_write;
> +    int 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) {

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 ?

> +        }
> +        break;
> +    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;
> +        }
> +        break;
> +    }

thanks
-- PMM