[PATCH v2 0/1] Check for page crossings in use_goto_tb() for rx target

[PATCH v2 0/1] Check for page crossings in use_goto_tb() for rx target

[Ahmed Karaman]

Hi,

After discussion on v1 of this series, the conclusion was that page
crossings must be done for all targets and for both user and system
mode. This series deals with rx target, that is the only target that
does not perform this check in system mode.

In version two of this series, the original use_goto_tb() function in
the hppa target is left unchanged. For the rx target, it's modified to
check for the page crossings in both modes along with other minor
fixes.

First version of the series:
https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg05426.html

Best regards,
Ahmed Karaman

v1->v2:
- Skip the patch related to the use_goto_tb() of the hppa target.
- Apply the page crossings check in use_goto_tb() in both modes for
  the rx target.
- Add appropriate comments in rx use_goto_tb().

Ahmed Karaman (1):
  target/rx: Check for page crossings in use_goto_tb()

 target/rx/translate.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

-- 
2.17.1

[PATCH v2 1/1] target/rx: Check for page crossings in use_goto_tb()

[Ahmed Karaman]

Add the page crossings check in use_goto_tb(). If this check is not
applied, a number of bugs may occasionally occur during target rx
system mode emulation.
Also, this check is needed in user mode related to emulation of system
call mmap(). rx target does not currently support user mode, but it is
better to prepare use_goto_tb() in that sense in advance.

Rename parameter dc of type DisasContext* to the more common name ctx,
to keep consistency with other targets.

Add detailed comments.

Buglink: https://bugs.launchpad.net/qemu/+bug/1880763
Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
---
 target/rx/translate.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/target/rx/translate.c b/target/rx/translate.c
index 61e86653a4..85a884c7dd 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -143,12 +143,15 @@ void rx_cpu_dump_state(CPUState *cs, FILE *f, int flags)
     }
 }
 
-static bool use_goto_tb(DisasContext *dc, target_ulong dest)
+static bool use_goto_tb(DisasContext *ctx, target_ulong dest)
 {
-    if (unlikely(dc->base.singlestep_enabled)) {
+    /* No direct translation block linking in singlestep */
+    if (unlikely(ctx->base.singlestep_enabled)) {
         return false;
     } else {
-        return true;
+        /* Directly link translation blocks only within the same guest page */
+        return (ctx->base.tb->pc & TARGET_PAGE_MASK) ==
+               (dest & TARGET_PAGE_MASK);
     }
 }
 
-- 
2.17.1

Re: [PATCH v2 1/1] target/rx: Check for page crossings in use_goto_tb()

[Richard Henderson]

On 5/31/20 6:45 AM, Ahmed Karaman wrote:
> Add the page crossings check in use_goto_tb(). If this check is not
> applied, a number of bugs may occasionally occur during target rx
> system mode emulation.
> Also, this check is needed in user mode related to emulation of system
> call mmap(). rx target does not currently support user mode, but it is
> better to prepare use_goto_tb() in that sense in advance.
> 
> Rename parameter dc of type DisasContext* to the more common name ctx,
> to keep consistency with other targets.
> 
> Add detailed comments.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1880763
> Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> ---
>  target/rx/translate.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

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

Although I note that this failure is not currently visible because RX does not
have an MMU.  So there are no page permissions to change or fail.


r~

Re: [PATCH v2 1/1] target/rx: Check for page crossings in use_goto_tb()

[Aleksandar Markovic]

уто, 2. јун 2020. у 00:19 Richard Henderson
<richard.henderson@linaro.org> је написао/ла:
>
> On 5/31/20 6:45 AM, Ahmed Karaman wrote:
> > Add the page crossings check in use_goto_tb(). If this check is not
> > applied, a number of bugs may occasionally occur during target rx
> > system mode emulation.
> > Also, this check is needed in user mode related to emulation of system
> > call mmap(). rx target does not currently support user mode, but it is
> > better to prepare use_goto_tb() in that sense in advance.
> >
> > Rename parameter dc of type DisasContext* to the more common name ctx,
> > to keep consistency with other targets.
> >
> > Add detailed comments.
> >
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1880763
> > Signed-off-by: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
> > ---
> >  target/rx/translate.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>

If there is no objection, I plan to select this patch in the upcoming
"TCG Continuous Benchmarking" project queue.

Thanks to Richard and Ahmed!
Aleksandar

> Although I note that this failure is not currently visible because RX does not
> have an MMU.  So there are no page permissions to change or fail.
>
>
> r~
>