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

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

[Kele Huang]

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;
+        }
+        break;
+    }
+
     return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
 }
 
-- 
2.17.1

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

[Philippe Mathieu-Daudé]

Cc'ing TCG MIPS maintainers *again*.

On 9/25/20 10: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;
> +        }
> +        break;
> +    }
> +
>      return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
>  }
>  
> 

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~

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

[Kele Huang]

[-- Attachment #1: Type: text/plain, Size: 2540 bytes --]

Fixed! I have resent a v4 patch which contains SUXC1.
Thank you!

On Fri, 25 Sep 2020 at 22:58, Richard Henderson <
richard.henderson@linaro.org> wrote:

> 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~
>

[-- Attachment #2: Type: text/html, Size: 3585 bytes --]

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

[Kele Huang]

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.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Kele Huang <kele.hwang@gmail.com>
Signed-off-by: Xu Zou <iwatchnima@gmail.com>
---
 accel/tcg/user-exec.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index bb039eb32d..9ecda6c0d0 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,42 @@ 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 MIPS32r2. */
+        switch (insn & 077) {
+        case 010: /* SWXC1 */
+        case 011: /* SDXC1 */
+        case 015: /* SDXC1 */
+            is_write = 1;
+        }
+        break;
+    }
+
     return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
 }
 
-- 
2.17.1

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

[Philippe Mathieu-Daudé]

On 9/27/20 10:20 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.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Kele Huang <kele.hwang@gmail.com>
> Signed-off-by: Xu Zou <iwatchnima@gmail.com>

I already Cc'ed the TCG MIPS maintainers twice for you,
but you don't mind, so this time I won't insist.

> ---
>  accel/tcg/user-exec.c | 39 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index bb039eb32d..9ecda6c0d0 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,42 @@ 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 MIPS32r2. */
> +        switch (insn & 077) {
> +        case 010: /* SWXC1 */
> +        case 011: /* SDXC1 */
> +        case 015: /* SDXC1 */
> +            is_write = 1;
> +        }
> +        break;
> +    }
> +
>      return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
>  }
>  
> 

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

[Kele Huang]

[-- Attachment #1: Type: text/plain, Size: 2989 bytes --]

Sorry about that, I only got maintainers by './scripts/get_maintainer.pl -f
accel/tcg/user-exec.c' and missed your advice about maintainers.
In another words, I thought I had Cc'ed the TCG MIPS maintainers. 😅
And sorry to maintainers. 😅

On Sun, 27 Sep 2020 at 16:41, Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> On 9/27/20 10:20 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.
> >
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > Signed-off-by: Kele Huang <kele.hwang@gmail.com>
> > Signed-off-by: Xu Zou <iwatchnima@gmail.com>
>
> I already Cc'ed the TCG MIPS maintainers twice for you,
> but you don't mind, so this time I won't insist.
>
> > ---
> >  accel/tcg/user-exec.c | 39 ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> > index bb039eb32d..9ecda6c0d0 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,42 @@ 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 MIPS32r2. */
> > +        switch (insn & 077) {
> > +        case 010: /* SWXC1 */
> > +        case 011: /* SDXC1 */
> > +        case 015: /* SDXC1 */
> > +            is_write = 1;
> > +        }
> > +        break;
> > +    }
> > +
> >      return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
> >  }
> >
> >
>
>

[-- Attachment #2: Type: text/html, Size: 4090 bytes --]

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

[Aleksandar Markovic]

[-- Attachment #1: Type: text/plain, Size: 3608 bytes --]

On Sunday, September 27, 2020, Kele Huang <kele.hwang@gmail.com> wrote:

> Sorry about that, I only got maintainers by './scripts/get_maintainer.pl
> -f accel/tcg/user-exec.c' and missed your advice about maintainers.
> In another words, I thought I had Cc'ed the TCG MIPS maintainers. 😅
> And sorry to maintainers. 😅
>
>>
>>
This is fine, Kele. :)

The granularity of get_maintainer.py is at file level, so this is one of
the cases where you can use your own judgement and include more email
addresses, even though get_maintainer.py doesn't list them.
get_maintainer.py is good most of the time, but not always. But not a big
deal.

Thanks for the patch! :)

I expect Richard is going to include it in his next tcg queue.

Yours,
Aleksandar


> On Sun, 27 Sep 2020 at 16:41, Philippe Mathieu-Daudé <f4bug@amsat.org>
> wrote:
>
>> On 9/27/20 10:20 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.
>> >
>> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> > Signed-off-by: Kele Huang <kele.hwang@gmail.com>
>> > Signed-off-by: Xu Zou <iwatchnima@gmail.com>
>>
>> I already Cc'ed the TCG MIPS maintainers twice for you,
>> but you don't mind, so this time I won't insist.
>>
>> > ---
>> >  accel/tcg/user-exec.c | 39 ++++++++++++++++++++++++++++++++++++++-
>> >  1 file changed, 38 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
>> > index bb039eb32d..9ecda6c0d0 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,42 @@ 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 MIPS32r2. */
>> > +        switch (insn & 077) {
>> > +        case 010: /* SWXC1 */
>> > +        case 011: /* SDXC1 */
>> > +        case 015: /* SDXC1 */
>> > +            is_write = 1;
>> > +        }
>> > +        break;
>> > +    }
>> > +
>> >      return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
>> >  }
>> >
>> >
>>
>>

[-- Attachment #2: Type: text/html, Size: 5169 bytes --]

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

[Kele Huang]

[-- Attachment #1: Type: text/plain, Size: 3860 bytes --]

Thank you so much!


On Mon, 28 Sep 2020 at 16:14, Aleksandar Markovic <
aleksandar.qemu.devel@gmail.com> wrote:

>
>
> On Sunday, September 27, 2020, Kele Huang <kele.hwang@gmail.com> wrote:
>
>> Sorry about that, I only got maintainers by './scripts/get_maintainer.pl
>> -f accel/tcg/user-exec.c' and missed your advice about maintainers.
>> In another words, I thought I had Cc'ed the TCG MIPS maintainers. 😅
>> And sorry to maintainers. 😅
>>
>>>
>>>
> This is fine, Kele. :)
>
> The granularity of get_maintainer.py is at file level, so this is one of
> the cases where you can use your own judgement and include more email
> addresses, even though get_maintainer.py doesn't list them.
> get_maintainer.py is good most of the time, but not always. But not a big
> deal.
>
> Thanks for the patch! :)
>
> I expect Richard is going to include it in his next tcg queue.
>
> Yours,
> Aleksandar
>
>
>> On Sun, 27 Sep 2020 at 16:41, Philippe Mathieu-Daudé <f4bug@amsat.org>
>> wrote:
>>
>>> On 9/27/20 10:20 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.
>>> >
>>> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> > Signed-off-by: Kele Huang <kele.hwang@gmail.com>
>>> > Signed-off-by: Xu Zou <iwatchnima@gmail.com>
>>>
>>> I already Cc'ed the TCG MIPS maintainers twice for you,
>>> but you don't mind, so this time I won't insist.
>>>
>>> > ---
>>> >  accel/tcg/user-exec.c | 39 ++++++++++++++++++++++++++++++++++++++-
>>> >  1 file changed, 38 insertions(+), 1 deletion(-)
>>> >
>>> > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
>>> > index bb039eb32d..9ecda6c0d0 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,42 @@ 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 MIPS32r2. */
>>> > +        switch (insn & 077) {
>>> > +        case 010: /* SWXC1 */
>>> > +        case 011: /* SDXC1 */
>>> > +        case 015: /* SDXC1 */
>>> > +            is_write = 1;
>>> > +        }
>>> > +        break;
>>> > +    }
>>> > +
>>> >      return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
>>> >  }
>>> >
>>> >
>>>
>>>

[-- Attachment #2: Type: text/html, Size: 5655 bytes --]

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

[Kele Huang]

[-- Attachment #1: Type: text/plain, Size: 4189 bytes --]

> +        case 015: /* SDXC1 */

I just found a comment mistake about SUXC1,  and I have rectified it and
resent a new patch.

On Tue, 29 Sep 2020 at 09:59, Kele Huang <kele.hwang@gmail.com> wrote:

> Thank you so much!
>
>
> On Mon, 28 Sep 2020 at 16:14, Aleksandar Markovic <
> aleksandar.qemu.devel@gmail.com> wrote:
>
>>
>>
>> On Sunday, September 27, 2020, Kele Huang <kele.hwang@gmail.com> wrote:
>>
>>> Sorry about that, I only got maintainers by './scripts/get_maintainer.pl
>>> -f accel/tcg/user-exec.c' and missed your advice about maintainers.
>>> In another words, I thought I had Cc'ed the TCG MIPS maintainers. 😅
>>> And sorry to maintainers. 😅
>>>
>>>>
>>>>
>> This is fine, Kele. :)
>>
>> The granularity of get_maintainer.py is at file level, so this is one of
>> the cases where you can use your own judgement and include more email
>> addresses, even though get_maintainer.py doesn't list them.
>> get_maintainer.py is good most of the time, but not always. But not a big
>> deal.
>>
>> Thanks for the patch! :)
>>
>> I expect Richard is going to include it in his next tcg queue.
>>
>> Yours,
>> Aleksandar
>>
>>
>>> On Sun, 27 Sep 2020 at 16:41, Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> wrote:
>>>
>>>> On 9/27/20 10:20 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.
>>>> >
>>>> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>> > Signed-off-by: Kele Huang <kele.hwang@gmail.com>
>>>> > Signed-off-by: Xu Zou <iwatchnima@gmail.com>
>>>>
>>>> I already Cc'ed the TCG MIPS maintainers twice for you,
>>>> but you don't mind, so this time I won't insist.
>>>>
>>>> > ---
>>>> >  accel/tcg/user-exec.c | 39 ++++++++++++++++++++++++++++++++++++++-
>>>> >  1 file changed, 38 insertions(+), 1 deletion(-)
>>>> >
>>>> > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
>>>> > index bb039eb32d..9ecda6c0d0 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,42 @@ 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 MIPS32r2. */
>>>> > +        switch (insn & 077) {
>>>> > +        case 010: /* SWXC1 */
>>>> > +        case 011: /* SDXC1 */
>>>> > +        case 015: /* SDXC1 */
>>>> > +            is_write = 1;
>>>> > +        }
>>>> > +        break;
>>>> > +    }
>>>> > +
>>>> >      return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
>>>> >  }
>>>> >
>>>> >
>>>>
>>>>

[-- Attachment #2: Type: text/html, Size: 6187 bytes --]