[Qemu-devel] Weird behavior while using the instruction counter

[Qemu-devel] Weird behavior while using the instruction counter

[Luis Pureza]

Hi,

I'm using the instruction counter to execute N instructions at a time.
With very small values of N (say, N < 10), I observed the following
behavior:

1. A new TB is generated and execution starts there;
2. The instruction counter timer expires and cpu_exec_nocache() is called;
3. cpu_exec_nocache() generates a new TB for the same PC and starts to
execute it;
4. Some instruction inside the TB turns out to be an I/O instruction.
Thus, cpu_io_recompile() gets called
5; cpu_io_recompile() regenerates the TB and longjmps back to the
beginning of cpu_exec()
6. on cpu_exec(), tb_find_fast() returns the first TB, instead of the
one generated by cpu_io_recompile()
7. Endless loop!

Actually, for some reason beyond my comprehension, the loop is not
really infinite: after a few seconds it actually executes the block
and moves on. However, as you can imagine, this is too slow.

I think I fixed the problem by appending CF_LAST_IO to the cflags of
the TB generated by cpu_exec_nocache(). This way, cpu_io_recompile()
won't be called for this TB. Certainly not the best solution, though.

I was wondering if my thinking makes sense or am I just completely wrong :-)

Cheers,

Luis Pureza

Re: [Qemu-devel] Weird behavior while using the instruction counter

[Paul Brook]

On Thursday 24 July 2008, Luis Pureza wrote:
> Hi,
>
> I'm using the instruction counter to execute N instructions at a time.
> With very small values of N (say, N < 10), I observed the following
> behavior:
>
> 1. A new TB is generated and execution starts there;
> 2. The instruction counter timer expires and cpu_exec_nocache() is called;
> 3. cpu_exec_nocache() generates a new TB for the same PC and starts to
> execute it;
> 4. Some instruction inside the TB turns out to be an I/O instruction.
> Thus, cpu_io_recompile() gets called
> 5; cpu_io_recompile() regenerates the TB and longjmps back to the
> beginning of cpu_exec()
> 6. on cpu_exec(), tb_find_fast() returns the first TB, instead of the
> one generated by cpu_io_recompile()
> 7. Endless loop!

I think I can see how this could happen, but only when the IO instruction is 
the first instruction in the block.  For any other TB you probably get 
run+fault first.

> Actually, for some reason beyond my comprehension, the loop is not
> really infinite: after a few seconds it actually executes the block
> and moves on. However, as you can imagine, this is too slow.

You need to figure out what's actually happening. Either it's an infinite loop 
or it's not.

Instruction counter expiry and the first IO trap are both fairly expensive 
operation. Having the counter expire every few instructionswill make qemu go 
extremely slowly.  Are you sure it's not just running very slowly?

> I think I fixed the problem by appending CF_LAST_IO to the cflags of
> the TB generated by cpu_exec_nocache(). This way, cpu_io_recompile()
> won't be called for this TB.

No. You're assuming the IO trap occurs on the last instruction, which not 
true.  The problem is that cpu_exec_nocache introduces a second TB with the 
same lookup key(pc+flags). cpu_io_recompile (and possibly other places) 
assume the currently executing TB is the only tb that matches. It needs to 
invalidate the original TB (if it exists) as well as the uncached one.

A related issue is that we don't invalidate the cached TB if a fault occurs 
while it is being executed.

Paul

Re: [Qemu-devel] Weird behavior while using the instruction counter

[Luis Pureza]

On Thu, Jul 24, 2008 at 1:44 PM, Paul Brook <paul@codesourcery.com> wrote:
> On Thursday 24 July 2008, Luis Pureza wrote:
>> Hi,
>>
>> I'm using the instruction counter to execute N instructions at a time.
>> With very small values of N (say, N < 10), I observed the following
>> behavior:
>>
>> 1. A new TB is generated and execution starts there;
>> 2. The instruction counter timer expires and cpu_exec_nocache() is called;
>> 3. cpu_exec_nocache() generates a new TB for the same PC and starts to
>> execute it;
>> 4. Some instruction inside the TB turns out to be an I/O instruction.
>> Thus, cpu_io_recompile() gets called
>> 5; cpu_io_recompile() regenerates the TB and longjmps back to the
>> beginning of cpu_exec()
>> 6. on cpu_exec(), tb_find_fast() returns the first TB, instead of the
>> one generated by cpu_io_recompile()
>> 7. Endless loop!
>
> I think I can see how this could happen, but only when the IO instruction is
> the first instruction in the block.  For any other TB you probably get
> run+fault first.
>
>> Actually, for some reason beyond my comprehension, the loop is not
>> really infinite: after a few seconds it actually executes the block
>> and moves on. However, as you can imagine, this is too slow.
>
> You need to figure out what's actually happening. Either it's an infinite loop
> or it's not.

I'm not sure of this, but I think I know what was happening. Since the
first TB was never cleared, the number of TBs would grow forever...
until it filled the code generation buffer. That's when tb_find_pc()
would actually find the correct TB instead of the old one and execute
it. And it started to move again.

> Instruction counter expiry and the first IO trap are both fairly expensive
> operation. Having the counter expire every few instructionswill make qemu go
> extremely slowly.  Are you sure it's not just running very slowly?
>
>> I think I fixed the problem by appending CF_LAST_IO to the cflags of
>> the TB generated by cpu_exec_nocache(). This way, cpu_io_recompile()
>> won't be called for this TB.
>
> No. You're assuming the IO trap occurs on the last instruction, which not
> true.  The problem is that cpu_exec_nocache introduces a second TB with the
> same lookup key(pc+flags). cpu_io_recompile (and possibly other places)
> assume the currently executing TB is the only tb that matches. It needs to
> invalidate the original TB (if it exists) as well as the uncached one.

Obviously, you're right. I was testing with blocks of a single
instruction. What do you think of this:

 if (tb != &tbs[0] && (tb - 1)->pc == tb->pc) {
        tb_phys_invalidate(tb - 1, -1);
 }

added to cpu_io_recompile() just after the if (!tb) check?

Seems to do the trick for me...

Thanks,

Luis Pureza

Re: [Qemu-devel] Weird behavior while using the instruction counter

[Paul Brook]

> > No. You're assuming the IO trap occurs on the last instruction, which not
> > true.  The problem is that cpu_exec_nocache introduces a second TB with
> > the same lookup key(pc+flags). cpu_io_recompile (and possibly other
> > places) assume the currently executing TB is the only tb that matches. It
> > needs to invalidate the original TB (if it exists) as well as the
> > uncached one.
>
> Obviously, you're right. I was testing with blocks of a single
> instruction. What do you think of this:
>
>  if (tb != &tbs[0] && (tb - 1)->pc == tb->pc) {
>         tb_phys_invalidate(tb - 1, -1);
>  }

No. There's no guarantee that the TBs are consecutive.

Paul

Fwd: [Qemu-devel] Weird behavior while using the instruction counter

[Luis Pureza]

By mistake I sent the following message to Paul Brook's private address.
Apologies for that.

Luis Pureza

---------- Forwarded message ----------
From: Luis Pureza <pureza@student.dei.uc.pt>
Date: Thu, Jul 24, 2008 at 3:42 PM
Subject: Re: [Qemu-devel] Weird behavior while using the instruction counter
To: Paul Brook <paul@codesourcery.com>


On Thu, Jul 24, 2008 at 3:02 PM, Paul Brook <paul@codesourcery.com> wrote:
>> > No. You're assuming the IO trap occurs on the last instruction, which not
>> > true.  The problem is that cpu_exec_nocache introduces a second TB with
>> > the same lookup key(pc+flags). cpu_io_recompile (and possibly other
>> > places) assume the currently executing TB is the only tb that matches. It
>> > needs to invalidate the original TB (if it exists) as well as the
>> > uncached one.
>>
>> Obviously, you're right. I was testing with blocks of a single
>> instruction. What do you think of this:
>>
>>  if (tb != &tbs[0] && (tb - 1)->pc == tb->pc) {
>>         tb_phys_invalidate(tb - 1, -1);
>>  }
>
> No. There's no guarantee that the TBs are consecutive.

What about this then:

tb2 = env->tb_jmp_cache[tb_jmp_cache_hash_func(tb->pc)];
if (tb2) {
   tb_phys_invalidate(tb2, -1);
}

added to the same place. This way, tb_find_fast() will never return
the old TB again.

Luis Pureza

Re: [Qemu-devel] Weird behavior while using the instruction counter

[Luis Pureza]

On Thu, Jul 24, 2008 at 3:56 PM, Paul Brook <paul@codesourcery.com> wrote:
>> > >cpu_io_recompile (and possibly other
>> > > places) assume the currently executing TB is the only tb that matches.
>> > > It needs to invalidate the original TB (if it exists) as well as the
>> > > uncached one.
>>
>> What about this then:
>>
>> tb2 = env->tb_jmp_cache[tb_jmp_cache_hash_func(tb->pc)];
>> if (tb2) {
>>     tb_phys_invalidate(tb2, -1);
>> }
>
> I don't think this is correct. If we have chained TBs then the jump cache
> entry may have been overwritten by a different TB.
> It's also inefficient. If we didn't get here via cpu_exec_nocache then we can
> invalidate a TB unnecessarily.
>
> Anywhere that uses tb_find_pc/cpu_restore_state is probably broken and will
> need auditing for the problems I mentioned above.

Ok. In the meanwhile, I'll stick with

tb2 = env->tb_jmp_cache[tb_jmp_cache_hash_func(tb->pc)];
    if (tb2 && tb2 != tb && tb2->pc == tb->pc && tb2->cs_base == tb->cs_base
    && tb2->flags == tb->flags) {
    tb_phys_invalidate(tb2, -1);
}

Unless, of course, you tell me there's something horribly wrong with this :-)
Thank you,

Luis Pureza

Re: [Qemu-devel] Weird behavior while using the instruction counter

[Paul Brook]

> > I don't think this is correct. If we have chained TBs then the jump cache
> > entry may have been overwritten by a different TB.
> > It's also inefficient. If we didn't get here via cpu_exec_nocache then we
> > can invalidate a TB unnecessarily.
> >
> > Anywhere that uses tb_find_pc/cpu_restore_state is probably broken and
> > will need auditing for the problems I mentioned above.
>
> Ok. In the meanwhile, I'll stick with
>
> tb2 = env->tb_jmp_cache[tb_jmp_cache_hash_func(tb->pc)];
>     if (tb2 && tb2 != tb && tb2->pc == tb->pc && tb2->cs_base ==
> tb->cs_base && tb2->flags == tb->flags) {
>     tb_phys_invalidate(tb2, -1);
> }
>
> Unless, of course, you tell me there's something horribly wrong with this
> :-) Thank you,

If by "horribly wrong" you mean won't work reliably and is just making the bug 
a bit harder to find, then it's no better then the previous change.
It just avoids a few of the unnecessary (but harmless) invalidations.

Paul

Re: [Qemu-devel] Weird behavior while using the instruction counter

[Luis Pureza]

On Thu, Jul 24, 2008 at 5:02 PM, Paul Brook <paul@codesourcery.com> wrote:
>> > I don't think this is correct. If we have chained TBs then the jump cache
>> > entry may have been overwritten by a different TB.
>> > It's also inefficient. If we didn't get here via cpu_exec_nocache then we
>> > can invalidate a TB unnecessarily.
>> >
>> > Anywhere that uses tb_find_pc/cpu_restore_state is probably broken and
>> > will need auditing for the problems I mentioned above.
>>
>> Ok. In the meanwhile, I'll stick with
>>
>> tb2 = env->tb_jmp_cache[tb_jmp_cache_hash_func(tb->pc)];
>>     if (tb2 && tb2 != tb && tb2->pc == tb->pc && tb2->cs_base ==
>> tb->cs_base && tb2->flags == tb->flags) {
>>     tb_phys_invalidate(tb2, -1);
>> }
>>
>> Unless, of course, you tell me there's something horribly wrong with this
>> :-) Thank you,
>
> If by "horribly wrong" you mean won't work reliably and is just making the bug
> a bit harder to find, then it's no better then the previous change.
> It just avoids a few of the unnecessary (but harmless) invalidations.

I see.

One last try then... Would the invalidation of the original TB by
cpu_exec_nocache() solve the problem (despite being terribly
inefficient)?

Thanks,

Luis Pureza

Re: [Qemu-devel] Weird behavior while using the instruction counter

[Paul Brook]

> One last try then... Would the invalidation of the original TB by
> cpu_exec_nocache() solve the problem (despite being terribly
> inefficient)?

Yes.

Paul