| Message ID | 20200927082033.8716-1-kele.hwang@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/1] accel/tcg: Fix computing of is_write for MIPS | expand |
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); > } > >
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); > > } > > > > > > <div dir="ltr">Sorry about that, I only got maintainers by './scripts/<a href="http://get_maintainer.pl">get_maintainer.pl</a> -f accel/tcg/user-exec.c' and missed your advice about maintainers. <div>In another words, I thought I had Cc'ed the TCG MIPS maintainers. 😅</div><div>And sorry to maintainers. 😅</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, 27 Sep 2020 at 16:41, Philippe Mathieu-Daudé <<a href="mailto:f4bug@amsat.org">f4bug@amsat.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 9/27/20 10:20 AM, Kele Huang wrote:<br> > Detect all MIPS store instructions in cpu_signal_handler for all available<br> > MIPS versions, and set is_write if encountering such store instructions.<br> > <br> > This fixed the error while dealing with self-modified code for MIPS.<br> > <br> > Reviewed-by: Richard Henderson <<a href="mailto:richard.henderson@linaro.org" target="_blank">richard.henderson@linaro.org</a>><br> > Signed-off-by: Kele Huang <<a href="mailto:kele.hwang@gmail.com" target="_blank">kele.hwang@gmail.com</a>><br> > Signed-off-by: Xu Zou <<a href="mailto:iwatchnima@gmail.com" target="_blank">iwatchnima@gmail.com</a>><br> <br> I already Cc'ed the TCG MIPS maintainers twice for you,<br> but you don't mind, so this time I won't insist.<br> <br> > ---<br> > accel/tcg/user-exec.c | 39 ++++++++++++++++++++++++++++++++++++++-<br> > 1 file changed, 38 insertions(+), 1 deletion(-)<br> > <br> > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c<br> > index bb039eb32d..9ecda6c0d0 100644<br> > --- a/accel/tcg/user-exec.c<br> > +++ b/accel/tcg/user-exec.c<br> > @@ -702,6 +702,10 @@ int cpu_signal_handler(int host_signum, void *pinfo,<br> > <br> > #elif defined(__mips__)<br> > <br> > +#if defined(__misp16) || defined(__mips_micromips)<br> > +#error "Unsupported encoding"<br> > +#endif<br> > +<br> > int cpu_signal_handler(int host_signum, void *pinfo,<br> > void *puc)<br> > {<br> > @@ -709,9 +713,42 @@ int cpu_signal_handler(int host_signum, void *pinfo,<br> > ucontext_t *uc = puc;<br> > greg_t pc = uc->uc_mcontext.pc;<br> > int is_write;<br> > + uint32_t insn;<br> > <br> > - /* XXX: compute is_write */<br> > + /* Detect all store instructions at program counter. */<br> > is_write = 0;<br> > + insn = *(uint32_t *)pc;<br> > + switch((insn >> 26) & 077) {<br> > + case 050: /* SB */<br> > + case 051: /* SH */<br> > + case 052: /* SWL */<br> > + case 053: /* SW */<br> > + case 054: /* SDL */<br> > + case 055: /* SDR */<br> > + case 056: /* SWR */<br> > + case 070: /* SC */<br> > + case 071: /* SWC1 */<br> > + case 074: /* SCD */<br> > + case 075: /* SDC1 */<br> > + case 077: /* SD */<br> > +#if !defined(__mips_isa_rev) || __mips_isa_rev < 6<br> > + case 072: /* SWC2 */<br> > + case 076: /* SDC2 */<br> > +#endif<br> > + is_write = 1;<br> > + break;<br> > + case 023: /* COP1X */<br> > + /* Required in all versions of MIPS64 since <br> > + MIPS64r1 and subsequent versions of MIPS32r2. */<br> > + switch (insn & 077) {<br> > + case 010: /* SWXC1 */<br> > + case 011: /* SDXC1 */<br> > + case 015: /* SDXC1 */<br> > + is_write = 1;<br> > + }<br> > + break;<br> > + }<br> > +<br> > return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);<br> > }<br> > <br> > <br> <br> </blockquote></div>
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); >> > } >> > >> > >> >> <br><br>On Sunday, September 27, 2020, Kele Huang <<a href="mailto:kele.hwang@gmail.com">kele.hwang@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Sorry about that, I only got maintainers by './scripts/<a href="http://get_maintainer.pl" target="_blank">get_maintainer.pl</a> -f accel/tcg/user-exec.c' and missed your advice about maintainers. <div>In another words, I thought I had Cc'ed the TCG MIPS maintainers. 😅</div><div>And sorry to maintainers. 😅</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br></blockquote></blockquote><div><br></div><div>This is fine, Kele. :)</div><div><br></div><div>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.</div><div><br></div><div>Thanks for the patch! :)</div><div><br></div><div>I expect Richard is going to include it in his next tcg queue.</div><div><br></div><div>Yours,</div><div>Aleksandar</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, 27 Sep 2020 at 16:41, Philippe Mathieu-Daudé <<a href="mailto:f4bug@amsat.org" target="_blank">f4bug@amsat.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 9/27/20 10:20 AM, Kele Huang wrote:<br> > Detect all MIPS store instructions in cpu_signal_handler for all available<br> > MIPS versions, and set is_write if encountering such store instructions.<br> > <br> > This fixed the error while dealing with self-modified code for MIPS.<br> > <br> > Reviewed-by: Richard Henderson <<a href="mailto:richard.henderson@linaro.org" target="_blank">richard.henderson@linaro.org</a>><br> > Signed-off-by: Kele Huang <<a href="mailto:kele.hwang@gmail.com" target="_blank">kele.hwang@gmail.com</a>><br> > Signed-off-by: Xu Zou <<a href="mailto:iwatchnima@gmail.com" target="_blank">iwatchnima@gmail.com</a>><br> <br> I already Cc'ed the TCG MIPS maintainers twice for you,<br> but you don't mind, so this time I won't insist.<br> <br> > ---<br> > accel/tcg/user-exec.c | 39 ++++++++++++++++++++++++++++++<wbr>++++++++-<br> > 1 file changed, 38 insertions(+), 1 deletion(-)<br> > <br> > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c<br> > index bb039eb32d..9ecda6c0d0 100644<br> > --- a/accel/tcg/user-exec.c<br> > +++ b/accel/tcg/user-exec.c<br> > @@ -702,6 +702,10 @@ int cpu_signal_handler(int host_signum, void *pinfo,<br> > <br> > #elif defined(__mips__)<br> > <br> > +#if defined(__misp16) || defined(__mips_micromips)<br> > +#error "Unsupported encoding"<br> > +#endif<br> > +<br> > int cpu_signal_handler(int host_signum, void *pinfo,<br> > void *puc)<br> > {<br> > @@ -709,9 +713,42 @@ int cpu_signal_handler(int host_signum, void *pinfo,<br> > ucontext_t *uc = puc;<br> > greg_t pc = uc->uc_mcontext.pc;<br> > int is_write;<br> > + uint32_t insn;<br> > <br> > - /* XXX: compute is_write */<br> > + /* Detect all store instructions at program counter. */<br> > is_write = 0;<br> > + insn = *(uint32_t *)pc;<br> > + switch((insn >> 26) & 077) {<br> > + case 050: /* SB */<br> > + case 051: /* SH */<br> > + case 052: /* SWL */<br> > + case 053: /* SW */<br> > + case 054: /* SDL */<br> > + case 055: /* SDR */<br> > + case 056: /* SWR */<br> > + case 070: /* SC */<br> > + case 071: /* SWC1 */<br> > + case 074: /* SCD */<br> > + case 075: /* SDC1 */<br> > + case 077: /* SD */<br> > +#if !defined(__mips_isa_rev) || __mips_isa_rev < 6<br> > + case 072: /* SWC2 */<br> > + case 076: /* SDC2 */<br> > +#endif<br> > + is_write = 1;<br> > + break;<br> > + case 023: /* COP1X */<br> > + /* Required in all versions of MIPS64 since <br> > + MIPS64r1 and subsequent versions of MIPS32r2. */<br> > + switch (insn & 077) {<br> > + case 010: /* SWXC1 */<br> > + case 011: /* SDXC1 */<br> > + case 015: /* SDXC1 */<br> > + is_write = 1;<br> > + }<br> > + break;<br> > + }<br> > +<br> > return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);<br> > }<br> > <br> > <br> <br> </blockquote></div> </blockquote>
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); >>> > } >>> > >>> > >>> >>> <div dir="ltr">Thank you so much!<div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 28 Sep 2020 at 16:14, Aleksandar Markovic <<a href="mailto:aleksandar.qemu.devel@gmail.com">aleksandar.qemu.devel@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br><br>On Sunday, September 27, 2020, Kele Huang <<a href="mailto:kele.hwang@gmail.com" target="_blank">kele.hwang@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Sorry about that, I only got maintainers by './scripts/<a href="http://get_maintainer.pl" target="_blank">get_maintainer.pl</a> -f accel/tcg/user-exec.c' and missed your advice about maintainers. <div>In another words, I thought I had Cc'ed the TCG MIPS maintainers. 😅</div><div>And sorry to maintainers. 😅</div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br></blockquote></blockquote><div><br></div><div>This is fine, Kele. :)</div><div><br></div><div>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.</div><div><br></div><div>Thanks for the patch! :)</div><div><br></div><div>I expect Richard is going to include it in his next tcg queue.</div><div><br></div><div>Yours,</div><div>Aleksandar</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, 27 Sep 2020 at 16:41, Philippe Mathieu-Daudé <<a href="mailto:f4bug@amsat.org" target="_blank">f4bug@amsat.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 9/27/20 10:20 AM, Kele Huang wrote:<br> > Detect all MIPS store instructions in cpu_signal_handler for all available<br> > MIPS versions, and set is_write if encountering such store instructions.<br> > <br> > This fixed the error while dealing with self-modified code for MIPS.<br> > <br> > Reviewed-by: Richard Henderson <<a href="mailto:richard.henderson@linaro.org" target="_blank">richard.henderson@linaro.org</a>><br> > Signed-off-by: Kele Huang <<a href="mailto:kele.hwang@gmail.com" target="_blank">kele.hwang@gmail.com</a>><br> > Signed-off-by: Xu Zou <<a href="mailto:iwatchnima@gmail.com" target="_blank">iwatchnima@gmail.com</a>><br> <br> I already Cc'ed the TCG MIPS maintainers twice for you,<br> but you don't mind, so this time I won't insist.<br> <br> > ---<br> > accel/tcg/user-exec.c | 39 ++++++++++++++++++++++++++++++++++++++-<br> > 1 file changed, 38 insertions(+), 1 deletion(-)<br> > <br> > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c<br> > index bb039eb32d..9ecda6c0d0 100644<br> > --- a/accel/tcg/user-exec.c<br> > +++ b/accel/tcg/user-exec.c<br> > @@ -702,6 +702,10 @@ int cpu_signal_handler(int host_signum, void *pinfo,<br> > <br> > #elif defined(__mips__)<br> > <br> > +#if defined(__misp16) || defined(__mips_micromips)<br> > +#error "Unsupported encoding"<br> > +#endif<br> > +<br> > int cpu_signal_handler(int host_signum, void *pinfo,<br> > void *puc)<br> > {<br> > @@ -709,9 +713,42 @@ int cpu_signal_handler(int host_signum, void *pinfo,<br> > ucontext_t *uc = puc;<br> > greg_t pc = uc->uc_mcontext.pc;<br> > int is_write;<br> > + uint32_t insn;<br> > <br> > - /* XXX: compute is_write */<br> > + /* Detect all store instructions at program counter. */<br> > is_write = 0;<br> > + insn = *(uint32_t *)pc;<br> > + switch((insn >> 26) & 077) {<br> > + case 050: /* SB */<br> > + case 051: /* SH */<br> > + case 052: /* SWL */<br> > + case 053: /* SW */<br> > + case 054: /* SDL */<br> > + case 055: /* SDR */<br> > + case 056: /* SWR */<br> > + case 070: /* SC */<br> > + case 071: /* SWC1 */<br> > + case 074: /* SCD */<br> > + case 075: /* SDC1 */<br> > + case 077: /* SD */<br> > +#if !defined(__mips_isa_rev) || __mips_isa_rev < 6<br> > + case 072: /* SWC2 */<br> > + case 076: /* SDC2 */<br> > +#endif<br> > + is_write = 1;<br> > + break;<br> > + case 023: /* COP1X */<br> > + /* Required in all versions of MIPS64 since <br> > + MIPS64r1 and subsequent versions of MIPS32r2. */<br> > + switch (insn & 077) {<br> > + case 010: /* SWXC1 */<br> > + case 011: /* SDXC1 */<br> > + case 015: /* SDXC1 */<br> > + is_write = 1;<br> > + }<br> > + break;<br> > + }<br> > +<br> > return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);<br> > }<br> > <br> > <br> <br> </blockquote></div> </blockquote> </blockquote></div>
> + 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); >>>> > } >>>> > >>>> > >>>> >>>> <div dir="ltr"><div>> + case 015: /* SDXC1 */<br></div><div><br></div>I just found a comment mistake about SUXC1, and I have rectified it and resent a new patch.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 29 Sep 2020 at 09:59, Kele Huang <<a href="mailto:kele.hwang@gmail.com">kele.hwang@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Thank you so much!<div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 28 Sep 2020 at 16:14, Aleksandar Markovic <<a href="mailto:aleksandar.qemu.devel@gmail.com" target="_blank">aleksandar.qemu.devel@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br><br>On Sunday, September 27, 2020, Kele Huang <<a href="mailto:kele.hwang@gmail.com" target="_blank">kele.hwang@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Sorry about that, I only got maintainers by './scripts/<a href="http://get_maintainer.pl" target="_blank">get_maintainer.pl</a> -f accel/tcg/user-exec.c' and missed your advice about maintainers. <div>In another words, I thought I had Cc'ed the TCG MIPS maintainers. 😅</div><div>And sorry to maintainers. 😅</div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br></blockquote></blockquote><div><br></div><div>This is fine, Kele. :)</div><div><br></div><div>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.</div><div><br></div><div>Thanks for the patch! :)</div><div><br></div><div>I expect Richard is going to include it in his next tcg queue.</div><div><br></div><div>Yours,</div><div>Aleksandar</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, 27 Sep 2020 at 16:41, Philippe Mathieu-Daudé <<a href="mailto:f4bug@amsat.org" target="_blank">f4bug@amsat.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 9/27/20 10:20 AM, Kele Huang wrote:<br> > Detect all MIPS store instructions in cpu_signal_handler for all available<br> > MIPS versions, and set is_write if encountering such store instructions.<br> > <br> > This fixed the error while dealing with self-modified code for MIPS.<br> > <br> > Reviewed-by: Richard Henderson <<a href="mailto:richard.henderson@linaro.org" target="_blank">richard.henderson@linaro.org</a>><br> > Signed-off-by: Kele Huang <<a href="mailto:kele.hwang@gmail.com" target="_blank">kele.hwang@gmail.com</a>><br> > Signed-off-by: Xu Zou <<a href="mailto:iwatchnima@gmail.com" target="_blank">iwatchnima@gmail.com</a>><br> <br> I already Cc'ed the TCG MIPS maintainers twice for you,<br> but you don't mind, so this time I won't insist.<br> <br> > ---<br> > accel/tcg/user-exec.c | 39 ++++++++++++++++++++++++++++++++++++++-<br> > 1 file changed, 38 insertions(+), 1 deletion(-)<br> > <br> > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c<br> > index bb039eb32d..9ecda6c0d0 100644<br> > --- a/accel/tcg/user-exec.c<br> > +++ b/accel/tcg/user-exec.c<br> > @@ -702,6 +702,10 @@ int cpu_signal_handler(int host_signum, void *pinfo,<br> > <br> > #elif defined(__mips__)<br> > <br> > +#if defined(__misp16) || defined(__mips_micromips)<br> > +#error "Unsupported encoding"<br> > +#endif<br> > +<br> > int cpu_signal_handler(int host_signum, void *pinfo,<br> > void *puc)<br> > {<br> > @@ -709,9 +713,42 @@ int cpu_signal_handler(int host_signum, void *pinfo,<br> > ucontext_t *uc = puc;<br> > greg_t pc = uc->uc_mcontext.pc;<br> > int is_write;<br> > + uint32_t insn;<br> > <br> > - /* XXX: compute is_write */<br> > + /* Detect all store instructions at program counter. */<br> > is_write = 0;<br> > + insn = *(uint32_t *)pc;<br> > + switch((insn >> 26) & 077) {<br> > + case 050: /* SB */<br> > + case 051: /* SH */<br> > + case 052: /* SWL */<br> > + case 053: /* SW */<br> > + case 054: /* SDL */<br> > + case 055: /* SDR */<br> > + case 056: /* SWR */<br> > + case 070: /* SC */<br> > + case 071: /* SWC1 */<br> > + case 074: /* SCD */<br> > + case 075: /* SDC1 */<br> > + case 077: /* SD */<br> > +#if !defined(__mips_isa_rev) || __mips_isa_rev < 6<br> > + case 072: /* SWC2 */<br> > + case 076: /* SDC2 */<br> > +#endif<br> > + is_write = 1;<br> > + break;<br> > + case 023: /* COP1X */<br> > + /* Required in all versions of MIPS64 since <br> > + MIPS64r1 and subsequent versions of MIPS32r2. */<br> > + switch (insn & 077) {<br> > + case 010: /* SWXC1 */<br> > + case 011: /* SDXC1 */<br> > + case 015: /* SDXC1 */<br> > + is_write = 1;<br> > + }<br> > + break;<br> > + }<br> > +<br> > return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);<br> > }<br> > <br> > <br> <br> </blockquote></div> </blockquote> </blockquote></div> </blockquote></div>
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); }