1. Patchew
  2. QEMU
  3. [PATCH v3 0/4] accel/tcg: Move perf and debuginfo support to tcg
  4. View patch

[PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c

Ilya Leoshkevich posted 4 patches 11 months, 3 weeks ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Laurent Vivier <laurent@vivier.eu>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
There is a newer version of this series
[PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c
Posted by Ilya Leoshkevich 11 months, 3 weeks ago 
Preparation for moving perf.c to tcg/.

This affects only profiling guest code, which has code in a non-0 based
segment, e.g., 16-bit code, which is not particularly important.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 accel/tcg/perf.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/accel/tcg/perf.c b/accel/tcg/perf.c
index ba75c1bbe45..68a46b1b524 100644
--- a/accel/tcg/perf.c
+++ b/accel/tcg/perf.c
@@ -337,10 +337,6 @@ void perf_report_code(uint64_t guest_pc, TranslationBlock *tb,
         q[insn].address = gen_insn_data[insn * start_words + 0];
         if (tb_cflags(tb) & CF_PCREL) {
             q[insn].address |= (guest_pc & qemu_target_page_mask());
-        } else {
-#if defined(TARGET_I386)
-            q[insn].address -= tb->cs_base;
-#endif
         }
         q[insn].flags = DEBUGINFO_SYMBOL | (jitdump ? DEBUGINFO_LINE : 0);
     }
-- 
2.43.0
Re: [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c
Posted by Richard Henderson 11 months, 3 weeks ago 
On 12/7/23 16:35, Ilya Leoshkevich wrote:
> Preparation for moving perf.c to tcg/.
> 
> This affects only profiling guest code, which has code in a non-0 based
> segment, e.g., 16-bit code, which is not particularly important.
> 
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   accel/tcg/perf.c | 4 ----
>   1 file changed, 4 deletions(-)
> 


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


r~
Re: [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c
Posted by Alex Bennée 11 months, 3 weeks ago 
Ilya Leoshkevich <iii@linux.ibm.com> writes:

> Preparation for moving perf.c to tcg/.
>
> This affects only profiling guest code, which has code in a non-0 based
> segment, e.g., 16-bit code, which is not particularly important.

I have no objection to removing the wart. Is it worth adding a note:: to
tcg.rst to say that profiles of 16-bit x64 code will be junk?

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  accel/tcg/perf.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/accel/tcg/perf.c b/accel/tcg/perf.c
> index ba75c1bbe45..68a46b1b524 100644
> --- a/accel/tcg/perf.c
> +++ b/accel/tcg/perf.c
> @@ -337,10 +337,6 @@ void perf_report_code(uint64_t guest_pc, TranslationBlock *tb,
>          q[insn].address = gen_insn_data[insn * start_words + 0];
>          if (tb_cflags(tb) & CF_PCREL) {
>              q[insn].address |= (guest_pc & qemu_target_page_mask());
> -        } else {
> -#if defined(TARGET_I386)
> -            q[insn].address -= tb->cs_base;
> -#endif
>          }
>          q[insn].flags = DEBUGINFO_SYMBOL | (jitdump ? DEBUGINFO_LINE : 0);
>      }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c
Posted by Ilya Leoshkevich 11 months, 3 weeks ago 
On Fri, 2023-12-08 at 09:53 +0000, Alex Bennée wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> 
> > Preparation for moving perf.c to tcg/.
> > 
> > This affects only profiling guest code, which has code in a non-0
> > based
> > segment, e.g., 16-bit code, which is not particularly important.
> 
> I have no objection to removing the wart. Is it worth adding a note::
> to
> tcg.rst to say that profiles of 16-bit x64 code will be junk?
> 
> Anyway:
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

I haven't tried them, but I don't think they work well anyway. With the
current logic, the samples are assigned to ip (without cs!), so if
there is code in different segments, there will be, to put it mildly,
ambiguities. Come to think of it, the patch even improves the
situation, since now the samples are assigned to cs*16+ip. In any
case, I don't think there is any good tooling to work with 16-bit
profiles.

> > Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  accel/tcg/perf.c | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/accel/tcg/perf.c b/accel/tcg/perf.c
> > index ba75c1bbe45..68a46b1b524 100644
> > --- a/accel/tcg/perf.c
> > +++ b/accel/tcg/perf.c
> > @@ -337,10 +337,6 @@ void perf_report_code(uint64_t guest_pc,
> > TranslationBlock *tb,
> >          q[insn].address = gen_insn_data[insn * start_words + 0];
> >          if (tb_cflags(tb) & CF_PCREL) {
> >              q[insn].address |= (guest_pc &
> > qemu_target_page_mask());
> > -        } else {
> > -#if defined(TARGET_I386)
> > -            q[insn].address -= tb->cs_base;
> > -#endif
> >          }
> >          q[insn].flags = DEBUGINFO_SYMBOL | (jitdump ?
> > DEBUGINFO_LINE : 0);
> >      }
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.