[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target/hppa: Check for page crossings in use_go
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [PATCH] target/hppa: Check for page crossings in use_goto_tb |
Date: |
Fri, 8 Mar 2019 11:15:00 -0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
On 3/8/19 11:04 AM, Peter Maydell wrote:
> On Fri, 8 Mar 2019 at 19:00, Richard Henderson
> <address@hidden> wrote:
>>
>> We got away with eliding this check when target/hppa was user-only,
>> but missed adding this check when adding system support.
>>
>> Fixes an early crash in the HP-UX 11 installer.
>>
>> Reported-by: Sven Schnelle <address@hidden>
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>> target/hppa/translate.c | 10 ++++------
>> 1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
>> index dc5636fe94..6c815e05c2 100644
>> --- a/target/hppa/translate.c
>> +++ b/target/hppa/translate.c
>> @@ -816,12 +816,10 @@ static bool gen_illegal(DisasContext *ctx)
>>
>> static bool use_goto_tb(DisasContext *ctx, target_ureg dest)
>> {
>> - /* Suppress goto_tb in the case of single-steping and IO. */
>> - if ((tb_cflags(ctx->base.tb) & CF_LAST_IO)
>> - || ctx->base.singlestep_enabled) {
>> - return false;
>> - }
>> - return true;
>> + /* Suppress goto_tb for page crossing, IO, or single-steping. */
>
> "stepping"
Oops.
>> + return !(((ctx->base.pc_first ^ dest) & TARGET_PAGE_MASK)
>> + || (tb_cflags(ctx->base.tb) & CF_LAST_IO)
>> + || ctx->base.singlestep_enabled);
>> }
>
> I note that (a) this isn't the way every other port phrases
> the "same page" check -- they generally use something like
> (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK)
This should be the same result.
ctx->base.pc_first was initialized from tb->pc. I find the xor expression more
compact and usually fits on a line, where repeating TARGET_PAGE_MASK doesn't.
> and (b) the other ports generally keep that check inside an
> ifndef CONFIG_USER_ONLY.
I've wondered about that. It certainly works for normal executables, but I
wonder if there are jit-like cases that fail when eliding that check.
Here, I think I was just a bit lazy.
Thoughts?
r~