[Qemu-devel] icount and tb chaining

[Qemu-devel] icount and tb chaining

[James Greensky]

Hello all, I have a question about icount and tb chaining that I hope
somebody can clear up.  In cpu-exec.c, when the icount_decr.u16.low
counter expires, it passes back the current tb as the next_tb and add
a jump with the least significant bits = 2. This falls through to tb
add jump, which then updates the jmp_first field of the current tb.
why is this done? Thanks -Jim

[Qemu-devel] icount and tb chaining

[James Greensky]

Hello all, I have a question about icount and tb chaining that I hope
somebody can clear up.  In cpu-exec.c, when the icount_decr.u16.low
counter expires, it passes back the current tb as the next_tb and add
a jump with the least significant bits = 2. This falls through to tb
add jump, which then updates the jmp_first field of the current tb.
why is this done? Thanks -Jim

Re: [Qemu-devel] icount and tb chaining

[陳韋任]

On Thu, Jan 12, 2012 at 11:00:43AM -0800, James Greensky wrote:
> Hello all, I have a question about icount and tb chaining that I hope
> somebody can clear up.  In cpu-exec.c, when the icount_decr.u16.low
> counter expires, it passes back the current tb as the next_tb and add
> a jump with the least significant bits = 2. This falls through to tb
> add jump, which then updates the jmp_first field of the current tb.
> why is this done? Thanks -Jim

  Could you elaborate on the whole sequence? I am not sure where should
I look into to answer your question. Thanks.

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-devel] icount and tb chaining

[James Greensky]

Sure, usually a tb chain is setup after a subsequent tb is
found/constructed in the loop in cpu_exec when a tb returns.
Taken/non-taken branch chaining is implemented by indicating the
branch direction by the two least significant digits of the the
previously returned tb. This is usually 0/1. When running icount, you
can also get a 2 value in these least significant digits, indicating
that the translation block was restarted due to the
icount_decr.u16.low field being exhausted but having instructions left
to execute in icount_extra. This 2 value falls through to tb_add_jump,
which then updates the tb's jmp_first field, as both tb and next_tb
refer to the same translation block. My question is why is this
necessary, why not do nothing, and leave the previous chaining intact?
I hope this is clearer and thanks for the response. -Jim

On Thu, Jan 12, 2012 at 7:41 PM, 陳韋任 <chenwj@iis.sinica.edu.tw> wrote:
> On Thu, Jan 12, 2012 at 11:00:43AM -0800, James Greensky wrote:
>> Hello all, I have a question about icount and tb chaining that I hope
>> somebody can clear up.  In cpu-exec.c, when the icount_decr.u16.low
>> counter expires, it passes back the current tb as the next_tb and add
>> a jump with the least significant bits = 2. This falls through to tb
>> add jump, which then updates the jmp_first field of the current tb.
>> why is this done? Thanks -Jim
>
>  Could you elaborate on the whole sequence? I am not sure where should
> I look into to answer your question. Thanks.
>
> Regards,
> chenwj
>
> --
> Wei-Ren Chen (陳韋任)
> Computer Systems Lab, Institute of Information Science,
> Academia Sinica, Taiwan (R.O.C.)
> Tel:886-2-2788-3799 #1667
> Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-devel] icount and tb chaining

[陳韋任]

> a jump with the least significant bits = 2. This falls through to tb
> add jump, which then updates the jmp_first field of the current tb.

  I don't know if tb_add_jump's second parameter will be two or not, but
look at TranslationBlock (exec-all.h),

struct TranslationBlock {

  struct TranslationBlock *jmp_next[2];

}

and tb_add_jump (exec-all.h).

static inline void tb_add_jump(TranslationBlock *tb, int n,
                               TranslationBlock *tb_next)
{
    /* NOTE: this test is only needed for thread safety */
    if (!tb->jmp_next[n]) { <--- what if n is 2? 
    }
}

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-devel] icount and tb chaining

[Peter Maydell]

2012/1/13 James Greensky <gsky51@gmail.com>:
> Sure, usually a tb chain is setup after a subsequent tb is
> found/constructed in the loop in cpu_exec when a tb returns.
> Taken/non-taken branch chaining is implemented by indicating the
> branch direction by the two least significant digits of the the
> previously returned tb. This is usually 0/1. When running icount, you
> can also get a 2 value in these least significant digits, indicating
> that the translation block was restarted due to the
> icount_decr.u16.low field being exhausted but having instructions left
> to execute in icount_extra. This 2 value falls through to tb_add_jump,
> which then updates the tb's jmp_first field, as both tb and next_tb
> refer to the same translation block. My question is why is this
> necessary, why not do nothing, and leave the previous chaining intact?

This looks suspiciously like a bug to me, although the code
is a bit opaque to me. Calling tb_add_jump() with n==2 seems like
it's not going to work since tb_set_jmp_target() is going to fall
off the end of either tb_jmp_offset[] or tb_next[], so even if we're
playing clever games with jmp_first being treatable as jmp_next[2] for
some purposes there's going to be a problem.

I thought the 2 return from a TB execution was only used as a flag
value, which would suggest that we should clear next_tb in the
'refill decrementer' code path too. I'm not sure I'm not missing
something subtle, though.

-- PMM

Re: [Qemu-devel] icount and tb chaining

[Laurent Desnogues]

On Tue, Jan 17, 2012 at 7:50 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> 2012/1/13 James Greensky <gsky51@gmail.com>:
>> Sure, usually a tb chain is setup after a subsequent tb is
>> found/constructed in the loop in cpu_exec when a tb returns.
>> Taken/non-taken branch chaining is implemented by indicating the
>> branch direction by the two least significant digits of the the
>> previously returned tb. This is usually 0/1. When running icount, you
>> can also get a 2 value in these least significant digits, indicating
>> that the translation block was restarted due to the
>> icount_decr.u16.low field being exhausted but having instructions left
>> to execute in icount_extra. This 2 value falls through to tb_add_jump,
>> which then updates the tb's jmp_first field, as both tb and next_tb
>> refer to the same translation block. My question is why is this
>> necessary, why not do nothing, and leave the previous chaining intact?
>
> This looks suspiciously like a bug to me, although the code
> is a bit opaque to me. Calling tb_add_jump() with n==2 seems like
> it's not going to work since tb_set_jmp_target() is going to fall
> off the end of either tb_jmp_offset[] or tb_next[], so even if we're
> playing clever games with jmp_first being treatable as jmp_next[2] for
> some purposes there's going to be a problem.
>
> I thought the 2 return from a TB execution was only used as a flag
> value, which would suggest that we should clear next_tb in the
> 'refill decrementer' code path too. I'm not sure I'm not missing
> something subtle, though.

Hmm I wonder if I didn't miss something in the discussion
because the only call to tb_add_jmp clears the lower two
bits so I fail to see the issue.


Laurent

Re: [Qemu-devel] icount and tb chaining

[Laurent Desnogues]

On Tue, Jan 17, 2012 at 8:06 PM, Laurent Desnogues
<laurent.desnogues@gmail.com> wrote:
> On Tue, Jan 17, 2012 at 7:50 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> 2012/1/13 James Greensky <gsky51@gmail.com>:
>>> Sure, usually a tb chain is setup after a subsequent tb is
>>> found/constructed in the loop in cpu_exec when a tb returns.
>>> Taken/non-taken branch chaining is implemented by indicating the
>>> branch direction by the two least significant digits of the the
>>> previously returned tb. This is usually 0/1. When running icount, you
>>> can also get a 2 value in these least significant digits, indicating
>>> that the translation block was restarted due to the
>>> icount_decr.u16.low field being exhausted but having instructions left
>>> to execute in icount_extra. This 2 value falls through to tb_add_jump,
>>> which then updates the tb's jmp_first field, as both tb and next_tb
>>> refer to the same translation block. My question is why is this
>>> necessary, why not do nothing, and leave the previous chaining intact?
>>
>> This looks suspiciously like a bug to me, although the code
>> is a bit opaque to me. Calling tb_add_jump() with n==2 seems like
>> it's not going to work since tb_set_jmp_target() is going to fall
>> off the end of either tb_jmp_offset[] or tb_next[], so even if we're
>> playing clever games with jmp_first being treatable as jmp_next[2] for
>> some purposes there's going to be a problem.
>>
>> I thought the 2 return from a TB execution was only used as a flag
>> value, which would suggest that we should clear next_tb in the
>> 'refill decrementer' code path too. I'm not sure I'm not missing
>> something subtle, though.
>
> Hmm I wonder if I didn't miss something in the discussion
> because the only call to tb_add_jmp clears the lower two
> bits so I fail to see the issue.

Hmm I knew I was missing something, forget this :-)


Laurent

Re: [Qemu-devel] icount and tb chaining

[James Greensky]

On Tue, Jan 17, 2012 at 7:06 AM, 陳韋任 <chenwj@iis.sinica.edu.tw> wrote:
>> a jump with the least significant bits = 2. This falls through to tb
>> add jump, which then updates the jmp_first field of the current tb.
>
>  I don't know if tb_add_jump's second parameter will be two or not, but
> look at TranslationBlock (exec-all.h),
>
> struct TranslationBlock {
>
>  struct TranslationBlock *jmp_next[2];
>
> }
>
> and tb_add_jump (exec-all.h).
>
> static inline void tb_add_jump(TranslationBlock *tb, int n,
>                               TranslationBlock *tb_next)
> {
>    /* NOTE: this test is only needed for thread safety */
>    if (!tb->jmp_next[n]) { <--- what if n is 2?
>    }
> }
>
> Regards,
> chenwj
>
> --
> Wei-Ren Chen (陳韋任)
> Computer Systems Lab, Institute of Information Science,
> Academia Sinica, Taiwan (R.O.C.)
> Tel:886-2-2788-3799 #1667
> Homepage: http://people.cs.nctu.edu.tw/~chenwj

if (!tb->jmp_next[n]) { <--- what if n is 2?

This is my question, if n is two, it would actually be checking the
jmp_first field immediatedly following the jmp_next array in the tb
structure.  This function only updates the jmp_first field and doesn't
touch jmp_next when n is 2. Does anybody know why this is? It seems
like you would like to leave the tb chaining as it was, because you
are going to execute the same translation block again, it just jumped
out to replenish the icount_decr counter. -Jim

Re: [Qemu-devel] icount and tb chaining

[James Greensky]

On Tue, Jan 17, 2012 at 10:50 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> 2012/1/13 James Greensky <gsky51@gmail.com>:
>> Sure, usually a tb chain is setup after a subsequent tb is
>> found/constructed in the loop in cpu_exec when a tb returns.
>> Taken/non-taken branch chaining is implemented by indicating the
>> branch direction by the two least significant digits of the the
>> previously returned tb. This is usually 0/1. When running icount, you
>> can also get a 2 value in these least significant digits, indicating
>> that the translation block was restarted due to the
>> icount_decr.u16.low field being exhausted but having instructions left
>> to execute in icount_extra. This 2 value falls through to tb_add_jump,
>> which then updates the tb's jmp_first field, as both tb and next_tb
>> refer to the same translation block. My question is why is this
>> necessary, why not do nothing, and leave the previous chaining intact?
>
> This looks suspiciously like a bug to me, although the code
> is a bit opaque to me. Calling tb_add_jump() with n==2 seems like
> it's not going to work since tb_set_jmp_target() is going to fall
> off the end of either tb_jmp_offset[] or tb_next[], so even if we're
> playing clever games with jmp_first being treatable as jmp_next[2] for
> some purposes there's going to be a problem.
>
> I thought the 2 return from a TB execution was only used as a flag
> value, which would suggest that we should clear next_tb in the
> 'refill decrementer' code path too. I'm not sure I'm not missing
> something subtle, though.
>
> -- PMM

I agree, seems like a bug, but wanted to verify with the author or
somebody that understands that section of the code whether this is
intentional or not. -Jim

Re: [Qemu-devel] icount and tb chaining

[陳韋任]

> if (!tb->jmp_next[n]) { <--- what if n is 2?
> 
> This is my question, if n is two, it would actually be checking the
> jmp_first field immediatedly following the jmp_next array in the tb
> structure.  This function only updates the jmp_first field and doesn't
> touch jmp_next when n is 2. Does anybody know why this is? It seems
> like you would like to leave the tb chaining as it was, because you
> are going to execute the same translation block again, it just jumped
> out to replenish the icount_decr counter. -Jim

  I set a conditional breakpoint at tb_add_jump to watch what happened when
n is 2. Look at tb_link_page (exec.c) first, which initializes some fields of
a TranslatioBlock.

void tb_link_page(TranslationBlock *tb, ...)
{
    tb->jmp_first = (TranslationBlock *)((long)tb | 2);
    tb->jmp_next[0] = NULL;
    tb->jmp_next[1] = NULL;
}

  And look at struct TranslationBlock (exec-all.h),

struct TranslationBlock {
    /* list of TBs jumping to this one. This is a circular list using
       the two least significant bits of the pointers to tell what is
       the next pointer: 0 = jmp_next[0], 1 = jmp_next[1], 2 =
       jmp_first */
    struct TranslationBlock *jmp_next[2];
    struct TranslationBlock *jmp_first;
}

I think those numbers (0, 1, and 2) means tb_add_jump 2nd parameter here, so
it's intentionally to check jmp_first when n is 2. Since tb->jmp_first is never
to be zero, then condition is always false (do nothing) when n is 2. You said

  "This function only updates the jmp_first field and doesn't touch jmp_next
   when n is 2"

Do you mean in some case (tb->jmp_first == 0) it might execute the following
code when n is 2? Because I don't see tb_add_jump updates jmp_first and leave
jmp_next alone when n is 2, it just do nothing.

    tb_set_jmp_target(tb, n, (unsigned long)tb_next->tc_ptr);
    tb->jmp_next[n] = tb_next->jmp_first;
    tb_next->jmp_first = (TranslationBlock *)((long)(tb) | (n));

Your guess makes sense to me,

  "It seems like you would like to leave the tb chaining as it was, because you
   are going to execute the same translation block again, it just jumped out to
   replenish the icount_decr counter"

And talk about the meaning of last significant bits of TranslationBlock*. AFAIK,
if last significant bits of TranslationBlock* are set to 2, then it means the
circular list of block chaining is stopped at this TranslationBlock. For
example, if tb1 links to tb2 (tb1 -> tb2), then

  tb1->jmp_next = tb2 | 2
  tb2->jmp_first = tb1 | 1

You can check the slide below if you want. It illustates how those fields
(jmp_first and jmp_next) are used in block chaining.

  http://www.hellogcc.org/wp-content/uploads/2011/10/QEMU-block-chaining.ppt

Regards,
chenwj
 
-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-devel] icount and tb chaining

[陳韋任]

> previously returned tb. This is usually 0/1. When running icount, you
> can also get a 2 value in these least significant digits, indicating
> that the translation block was restarted due to the
> icount_decr.u16.low field being exhausted but having instructions left
> to execute in icount_extra. This 2 value falls through to tb_add_jump,
> which then updates the tb's jmp_first field, as both tb and next_tb
> refer to the same translation block. My question is why is this
> necessary, why not do nothing, and leave the previous chaining intact?
> I hope this is clearer and thanks for the response. -Jim

  I have a question here. Look at gen_icount_start and gen_icount_end in 
gen-icount.h, I think the least significant bits are set to 2 by following
code.

  tcg_gen_exit_tb((tcg_target_long)tb + 2);

And you said,

  "indicating that the translation block was restarted due to the
   icount_decr.u16.low field being exhausted but having instructions left
   to execute in icount_extra."

>From the code snipt of gen_icount_start below, I can only figure out something
(icount_decr.u32?) is exhausted so it will jump to label set by gen_icount_end
and execute "tcg_gen_exit_tb((tcg_target_long)tb + 2)". I only see icount_extra
and icount_decr.u16.low are dealt in cpu_exec (cpu-exec.c). Do I miss something
or misunderstand what you said? Thanks. 

---
    icount_label = gen_new_label();
    count = tcg_temp_local_new_i32();
    tcg_gen_ld_i32(count, cpu_env, offsetof(CPUState, icount_decr.u32));
    /* This is a horrid hack to allow fixing up the value later.  */
    icount_arg = gen_opparam_ptr + 1;
    tcg_gen_subi_i32(count, count, 0xdeadbeef);

    tcg_gen_brcondi_i32(TCG_COND_LT, count, 0, icount_label);
---

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-devel] icount and tb chaining

[James Greensky]

On Tue, Jan 17, 2012 at 7:03 PM, 陳韋任 <chenwj@iis.sinica.edu.tw> wrote:
>> if (!tb->jmp_next[n]) { <--- what if n is 2?
>>
>> This is my question, if n is two, it would actually be checking the
>> jmp_first field immediatedly following the jmp_next array in the tb
>> structure.  This function only updates the jmp_first field and doesn't
>> touch jmp_next when n is 2. Does anybody know why this is? It seems
>> like you would like to leave the tb chaining as it was, because you
>> are going to execute the same translation block again, it just jumped
>> out to replenish the icount_decr counter. -Jim
>
>  I set a conditional breakpoint at tb_add_jump to watch what happened when
> n is 2. Look at tb_link_page (exec.c) first, which initializes some fields of
> a TranslatioBlock.
>
> void tb_link_page(TranslationBlock *tb, ...)
> {
>    tb->jmp_first = (TranslationBlock *)((long)tb | 2);
>    tb->jmp_next[0] = NULL;
>    tb->jmp_next[1] = NULL;
> }
>
>  And look at struct TranslationBlock (exec-all.h),
>
> struct TranslationBlock {
>    /* list of TBs jumping to this one. This is a circular list using
>       the two least significant bits of the pointers to tell what is
>       the next pointer: 0 = jmp_next[0], 1 = jmp_next[1], 2 =
>       jmp_first */
>    struct TranslationBlock *jmp_next[2];
>    struct TranslationBlock *jmp_first;
> }
>
> I think those numbers (0, 1, and 2) means tb_add_jump 2nd parameter here, so
> it's intentionally to check jmp_first when n is 2. Since tb->jmp_first is never
> to be zero, then condition is always false (do nothing) when n is 2. You said
>
>  "This function only updates the jmp_first field and doesn't touch jmp_next
>   when n is 2"
>
> Do you mean in some case (tb->jmp_first == 0) it might execute the following
> code when n is 2? Because I don't see tb_add_jump updates jmp_first and leave
> jmp_next alone when n is 2, it just do nothing.
>
>    tb_set_jmp_target(tb, n, (unsigned long)tb_next->tc_ptr);
>    tb->jmp_next[n] = tb_next->jmp_first;
>    tb_next->jmp_first = (TranslationBlock *)((long)(tb) | (n));
>
> Your guess makes sense to me,
>
>  "It seems like you would like to leave the tb chaining as it was, because you
>   are going to execute the same translation block again, it just jumped out to
>   replenish the icount_decr counter"
>
> And talk about the meaning of last significant bits of TranslationBlock*. AFAIK,
> if last significant bits of TranslationBlock* are set to 2, then it means the
> circular list of block chaining is stopped at this TranslationBlock. For
> example, if tb1 links to tb2 (tb1 -> tb2), then
>
>  tb1->jmp_next = tb2 | 2
>  tb2->jmp_first = tb1 | 1
>
> You can check the slide below if you want. It illustates how those fields
> (jmp_first and jmp_next) are used in block chaining.
>
>  http://www.hellogcc.org/wp-content/uploads/2011/10/QEMU-block-chaining.ppt
>
> Regards,
> chenwj
>
> --
> Wei-Ren Chen (陳韋任)
> Computer Systems Lab, Institute of Information Science,
> Academia Sinica, Taiwan (R.O.C.)
> Tel:886-2-2788-3799 #1667
> Homepage: http://people.cs.nctu.edu.tw/~chenwj

Chenwj, you are correct, the check bypasses the whole function, I was
not paying enough attention to that check, it should never be null, in
which case the code doesn't get executed and the chaining remains
unchanged. Thanks. -Jim

Re: [Qemu-devel] icount and tb chaining

[James Greensky]

On Tue, Jan 17, 2012 at 7:22 PM, 陳韋任 <chenwj@iis.sinica.edu.tw> wrote:
>> previously returned tb. This is usually 0/1. When running icount, you
>> can also get a 2 value in these least significant digits, indicating
>> that the translation block was restarted due to the
>> icount_decr.u16.low field being exhausted but having instructions left
>> to execute in icount_extra. This 2 value falls through to tb_add_jump,
>> which then updates the tb's jmp_first field, as both tb and next_tb
>> refer to the same translation block. My question is why is this
>> necessary, why not do nothing, and leave the previous chaining intact?
>> I hope this is clearer and thanks for the response. -Jim
>
>  I have a question here. Look at gen_icount_start and gen_icount_end in
> gen-icount.h, I think the least significant bits are set to 2 by following
> code.
>
>  tcg_gen_exit_tb((tcg_target_long)tb + 2);
>
> And you said,
>
>  "indicating that the translation block was restarted due to the
>   icount_decr.u16.low field being exhausted but having instructions left
>   to execute in icount_extra."
>
> From the code snipt of gen_icount_start below, I can only figure out something
> (icount_decr.u32?) is exhausted so it will jump to label set by gen_icount_end
> and execute "tcg_gen_exit_tb((tcg_target_long)tb + 2)". I only see icount_extra
> and icount_decr.u16.low are dealt in cpu_exec (cpu-exec.c). Do I miss something
> or misunderstand what you said? Thanks.
>
> ---
>    icount_label = gen_new_label();
>    count = tcg_temp_local_new_i32();
>    tcg_gen_ld_i32(count, cpu_env, offsetof(CPUState, icount_decr.u32));
>    /* This is a horrid hack to allow fixing up the value later.  */
>    icount_arg = gen_opparam_ptr + 1;
>    tcg_gen_subi_i32(count, count, 0xdeadbeef);
>
>    tcg_gen_brcondi_i32(TCG_COND_LT, count, 0, icount_label);
> ---
>
> Regards,
> chenwj
>
> --
> Wei-Ren Chen (陳韋任)
> Computer Systems Lab, Institute of Information Science,
> Academia Sinica, Taiwan (R.O.C.)
> Tel:886-2-2788-3799 #1667
> Homepage: http://people.cs.nctu.edu.tw/~chenwj

What i mean here is that in gen-icount.h, the icount_decr.u32 field is
exhausted and jumps out to the cpu-exec loop. This is where you would
fall into tb_add_jump with the second argument being 2, and only if
icount_extra was greater than zero. This is what I meant, and
previously you showed me that the chaining is left intact by the check
in tb_add_jump. Hope this is clearer. -Jim

Re: [Qemu-devel] icount and tb chaining

[Peter Maydell]

On 18 January 2012 19:43, James Greensky <gsky51@gmail.com> wrote:
> On Tue, Jan 17, 2012 at 7:03 PM, 陳韋任 <chenwj@iis.sinica.edu.tw> wrote:
>> I think those numbers (0, 1, and 2) means tb_add_jump 2nd parameter here, so
>> it's intentionally to check jmp_first when n is 2. Since tb->jmp_first is never
>> to be zero, then condition is always false (do nothing) when n is 2.

> Chenwj, you are correct, the check bypasses the whole function, I was
> not paying enough attention to that check, it should never be null, in
> which case the code doesn't get executed and the chaining remains
> unchanged.

The question that occurs to me is, is this working like this by design,
or is it a bug that just happens to have no ill effects? The fact that
the condition in tb_add_jump() is commented as "only needed for thread
safety" suggests the latter to me...

-- PMM

Re: [Qemu-devel] icount and tb chaining

[陳韋任]

On Wed, Jan 18, 2012 at 07:50:19PM +0000, Peter Maydell wrote:
> On 18 January 2012 19:43, James Greensky <gsky51@gmail.com> wrote:
> > On Tue, Jan 17, 2012 at 7:03 PM, 陳韋任 <chenwj@iis.sinica.edu.tw> wrote:
> >> I think those numbers (0, 1, and 2) means tb_add_jump 2nd parameter here, so
> >> it's intentionally to check jmp_first when n is 2. Since tb->jmp_first is never
> >> to be zero, then condition is always false (do nothing) when n is 2.
> 
> > Chenwj, you are correct, the check bypasses the whole function, I was
> > not paying enough attention to that check, it should never be null, in
> > which case the code doesn't get executed and the chaining remains
> > unchanged.
> 
> The question that occurs to me is, is this working like this by design,
> or is it a bug that just happens to have no ill effects? The fact that
> the condition in tb_add_jump() is commented as "only needed for thread
> safety" suggests the latter to me...

  `git blame` shows Fabrice wrote that comment, so maybe only he knows the real
reason.

  Is the "thread" in the comment the same as in multi-thread?

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-devel] icount and tb chaining

[陳韋任]

> What i mean here is that in gen-icount.h, the icount_decr.u32 field is
> exhausted and jumps out to the cpu-exec loop. This is where you would
> fall into tb_add_jump with the second argument being 2, and only if
> icount_extra was greater than zero. This is what I meant, and
> previously you showed me that the chaining is left intact by the check
> in tb_add_jump. Hope this is clearer. -Jim

  I am not familiar with icount stuff, so some dunmp questions here. In 
gen_icount_start (gen-icount.h), it loads icount_decr.u32 into count,
then subtracts 0xdeadbeef from count, finally stores count back to
icount_decr.u16.low.

  1) Why subtract 0xdeadbeef from count? Does 0xdeadbeef have any meaning?

  2) Why store count back to icount_decr.u16.low rather than icount_decr.u32? 

In cpu_exec (cpu-exec.c),

  // Does insns_left means how many guest instructions left in tb and needed
  // to be executed?
  insns_left = env->icount_decr.u32;

  // what the if-branch means, in particular icount_extra? I guess it has
  // something to do with Qemu timer, but not sure.
  if (env->icount_extra && insns_left >= 0) {

    // I cannot figure out what's going on here. Could you shed light on it? 

  } else {
  }


  Many thanks! :)

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-devel] icount and tb chaining

[James Greensky]

On Thu, Jan 19, 2012 at 2:32 AM, 陳韋任 <chenwj@iis.sinica.edu.tw> wrote:
>> What i mean here is that in gen-icount.h, the icount_decr.u32 field is
>> exhausted and jumps out to the cpu-exec loop. This is where you would
>> fall into tb_add_jump with the second argument being 2, and only if
>> icount_extra was greater than zero. This is what I meant, and
>> previously you showed me that the chaining is left intact by the check
>> in tb_add_jump. Hope this is clearer. -Jim
>
>  I am not familiar with icount stuff, so some dunmp questions here. In
> gen_icount_start (gen-icount.h), it loads icount_decr.u32 into count,
> then subtracts 0xdeadbeef from count, finally stores count back to
> icount_decr.u16.low.
>
>  1) Why subtract 0xdeadbeef from count? Does 0xdeadbeef have any meaning?
>
>  2) Why store count back to icount_decr.u16.low rather than icount_decr.u32?
>
> In cpu_exec (cpu-exec.c),
>
>  // Does insns_left means how many guest instructions left in tb and needed
>  // to be executed?
>  insns_left = env->icount_decr.u32;
>
>  // what the if-branch means, in particular icount_extra? I guess it has
>  // something to do with Qemu timer, but not sure.
>  if (env->icount_extra && insns_left >= 0) {
>
>    // I cannot figure out what's going on here. Could you shed light on it?
>
>  } else {
>  }
>
>
>  Many thanks! :)
>
> Regards,
> chenwj
>
> --
> Wei-Ren Chen (陳韋任)
> Computer Systems Lab, Institute of Information Science,
> Academia Sinica, Taiwan (R.O.C.)
> Tel:886-2-2788-3799 #1667
> Homepage: http://people.cs.nctu.edu.tw/~chenwj

1) 0xdeadbeef is the argument value that will be overwritten in
gen_icount_end with the real number of instruction translated in this
block, num_insns. If you notice, the argument is saved in icount_arg
and it value is replaced.

2) Only the low bits are used for the instruction counting, once the
low bits are exhausted, they will be refilled in the cpu_exec loop.
The high bits are used to indicate an interrupt has occurred, you can
see them set in cpu_interrup of exec.c. They indicate an interrupt is
waiting, and the test in the beginning of a translation block can jump
out of the loop with a simple check to see if the value is below zero,
as the count is a signed value.

>  // Does insns_left means how many guest instructions left in tb and needed
>  // to be executed?

No, insns_left are number of instruction left in qemu_icount over what
can be stored in the low bits of icount_decr. look at qemu_cpu_exec of
cpus.c

This is my understanding. -Jim