diff mbox series

[v1,5/7] docs: mark intention to deprecate TCG tracing functionality

Message ID 20210505092259.8202-6-alex.bennee@linaro.org (mailing list archive)
State New, archived
Headers show
Series plugins/next (windows, leaks, tcg tracing) | expand

Commit Message

Alex Bennée May 5, 2021, 9:22 a.m. UTC
Currently attempts to add a new TCG trace events results in failures
to build. Previous discussions have suggested maybe it's time to mark
the feature as deprecated and push people towards using plugins.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Luis Vilanova <vilanova@imperial.ac.uk>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/devel/tcg-plugins.rst |  2 ++
 docs/devel/tracing.rst     |  7 +++++++
 docs/system/deprecated.rst | 13 +++++++++++++
 3 files changed, 22 insertions(+)

Comments

Daniel P. Berrangé May 5, 2021, 9:33 a.m. UTC | #1
On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:
> Currently attempts to add a new TCG trace events results in failures
> to build. Previous discussions have suggested maybe it's time to mark
> the feature as deprecated and push people towards using plugins.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Luis Vilanova <vilanova@imperial.ac.uk>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  docs/devel/tcg-plugins.rst |  2 ++
>  docs/devel/tracing.rst     |  7 +++++++
>  docs/system/deprecated.rst | 13 +++++++++++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
> index 18c6581d85..edf04e3091 100644
> --- a/docs/devel/tcg-plugins.rst
> +++ b/docs/devel/tcg-plugins.rst
> @@ -3,6 +3,8 @@
>     Copyright (c) 2019, Linaro Limited
>     Written by Emilio Cota and Alex Bennée
>  
> +.. _tcgplugin-ref:
> +
>  ================
>  QEMU TCG Plugins
>  ================
> diff --git a/docs/devel/tracing.rst b/docs/devel/tracing.rst
> index ba83954899..6b0f46cd54 100644
> --- a/docs/devel/tracing.rst
> +++ b/docs/devel/tracing.rst
> @@ -414,6 +414,13 @@ disabled, this check will have no performance impact.
>  "tcg"
>  -----
>  
> +.. warning::
> +   The ability to add new TCG trace points relies on a having a good
> +   understanding of the TCG internals. In the meantime TCG plugins
> +   have been introduced which solve many of the same problems with
> +   more of a focus on analysing guest code. See :ref:`tcgplugin-ref`
> +   for more details.
> +
>  Guest code generated by TCG can be traced by defining an event with the "tcg"
>  event property. Internally, this property generates two events:
>  "<eventname>_trans" to trace the event at translation time, and
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 80cae86252..0c9d3c1e1e 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -312,6 +312,19 @@ The ``I7200`` guest CPU relies on the nanoMIPS ISA, which is deprecated
>  (the ISA has never been upstreamed to a compiler toolchain). Therefore
>  this CPU is also deprecated.
>  
> +TCG introspection features
> +--------------------------
> +
> +TCG trace-events (since 6.1)
> +''''''''''''''''''''''''''''
> +
> +The ability to add new TCG trace points has bit rotted and as the

When you say this "has bit rotted", just how bad is the situation ?

Is the TCG tracing still usable at all, or is is fully broken
already ?


> +feature can be replicated with TCG plugins it will be deprecated. If
> +any user is currently using this feature and needs help with
> +converting to using TCG plugins they should contact the qemu-devel
> +mailing list.
> +

Regards,
Daniel
Alex Bennée May 5, 2021, 10:35 a.m. UTC | #2
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:
>> Currently attempts to add a new TCG trace events results in failures
>> to build. Previous discussions have suggested maybe it's time to mark
>> the feature as deprecated and push people towards using plugins.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Luis Vilanova <vilanova@imperial.ac.uk>
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>  docs/devel/tcg-plugins.rst |  2 ++
>>  docs/devel/tracing.rst     |  7 +++++++
>>  docs/system/deprecated.rst | 13 +++++++++++++
>>  3 files changed, 22 insertions(+)
>> 
>> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
>> index 18c6581d85..edf04e3091 100644
>> --- a/docs/devel/tcg-plugins.rst
>> +++ b/docs/devel/tcg-plugins.rst
>> @@ -3,6 +3,8 @@
>>     Copyright (c) 2019, Linaro Limited
>>     Written by Emilio Cota and Alex Bennée
>>  
>> +.. _tcgplugin-ref:
>> +
>>  ================
>>  QEMU TCG Plugins
>>  ================
>> diff --git a/docs/devel/tracing.rst b/docs/devel/tracing.rst
>> index ba83954899..6b0f46cd54 100644
>> --- a/docs/devel/tracing.rst
>> +++ b/docs/devel/tracing.rst
>> @@ -414,6 +414,13 @@ disabled, this check will have no performance impact.
>>  "tcg"
>>  -----
>>  
>> +.. warning::
>> +   The ability to add new TCG trace points relies on a having a good
>> +   understanding of the TCG internals. In the meantime TCG plugins
>> +   have been introduced which solve many of the same problems with
>> +   more of a focus on analysing guest code. See :ref:`tcgplugin-ref`
>> +   for more details.
>> +
>>  Guest code generated by TCG can be traced by defining an event with the "tcg"
>>  event property. Internally, this property generates two events:
>>  "<eventname>_trans" to trace the event at translation time, and
>> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
>> index 80cae86252..0c9d3c1e1e 100644
>> --- a/docs/system/deprecated.rst
>> +++ b/docs/system/deprecated.rst
>> @@ -312,6 +312,19 @@ The ``I7200`` guest CPU relies on the nanoMIPS ISA, which is deprecated
>>  (the ISA has never been upstreamed to a compiler toolchain). Therefore
>>  this CPU is also deprecated.
>>  
>> +TCG introspection features
>> +--------------------------
>> +
>> +TCG trace-events (since 6.1)
>> +''''''''''''''''''''''''''''
>> +
>> +The ability to add new TCG trace points has bit rotted and as the
>
> When you say this "has bit rotted", just how bad is the situation ?
>
> Is the TCG tracing still usable at all, or is is fully broken
> already ?

Well patches 6/7 got it working for generic TCG things. I haven't been
able to get the architecture one working but I suspect that is some sort
of interaction between the per-arch trace header generation that I
haven't quite figured out yet.

It's currently broken without the included patches because it's not
really being exercised by anything.

>> +feature can be replicated with TCG plugins it will be deprecated. If
>> +any user is currently using this feature and needs help with
>> +converting to using TCG plugins they should contact the qemu-devel
>> +mailing list.
>> +
>
> Regards,
> Daniel
Alex Bennée May 5, 2021, 10:41 a.m. UTC | #3
Alex Bennée <alex.bennee@linaro.org> writes:

> Daniel P. Berrangé <berrange@redhat.com> writes:
>
>> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:
<snip>
>>> +TCG introspection features
>>> +--------------------------
>>> +
>>> +TCG trace-events (since 6.1)
>>> +''''''''''''''''''''''''''''
>>> +
>>> +The ability to add new TCG trace points has bit rotted and as the
>>
>> When you say this "has bit rotted", just how bad is the situation ?
>>
>> Is the TCG tracing still usable at all, or is is fully broken
>> already ?
>
> Well patches 6/7 got it working for generic TCG things. I haven't been
> able to get the architecture one working but I suspect that is some sort
> of interaction between the per-arch trace header generation that I
> haven't quite figured out yet.

Ahh it's since 7609ffb919 (trace: fix tcg tracing build breakage) which
limited tcg/vcpu events to the root trace-events file.
Daniel P. Berrangé May 5, 2021, 10:52 a.m. UTC | #4
On Wed, May 05, 2021 at 11:41:46AM +0100, Alex Bennée wrote:
> 
> Alex Bennée <alex.bennee@linaro.org> writes:
> 
> > Daniel P. Berrangé <berrange@redhat.com> writes:
> >
> >> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:
> <snip>
> >>> +TCG introspection features
> >>> +--------------------------
> >>> +
> >>> +TCG trace-events (since 6.1)
> >>> +''''''''''''''''''''''''''''
> >>> +
> >>> +The ability to add new TCG trace points has bit rotted and as the
> >>
> >> When you say this "has bit rotted", just how bad is the situation ?
> >>
> >> Is the TCG tracing still usable at all, or is is fully broken
> >> already ?
> >
> > Well patches 6/7 got it working for generic TCG things. I haven't been
> > able to get the architecture one working but I suspect that is some sort
> > of interaction between the per-arch trace header generation that I
> > haven't quite figured out yet.
> 
> Ahh it's since 7609ffb919 (trace: fix tcg tracing build breakage) which
> limited tcg/vcpu events to the root trace-events file.

That commit is from release 2.10.0.

The other commit mentioned in patch 6 (73ff061032) is from 2.12.0.

So no one has been able to use this feature for 3+ years already.

Is it actually worth fixing and then deprecating for 2 releases before
deleting, as opposed to just deleting the broken code today on basis
that it can't have any current users ?

Regards,
Daniel
Alex Bennée May 17, 2021, 10:47 a.m. UTC | #5
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, May 05, 2021 at 11:41:46AM +0100, Alex Bennée wrote:
>> 
>> Alex Bennée <alex.bennee@linaro.org> writes:
>> 
>> > Daniel P. Berrangé <berrange@redhat.com> writes:
>> >
>> >> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:
>> <snip>
>> >>> +TCG introspection features
>> >>> +--------------------------
>> >>> +
>> >>> +TCG trace-events (since 6.1)
>> >>> +''''''''''''''''''''''''''''
>> >>> +
>> >>> +The ability to add new TCG trace points has bit rotted and as the
>> >>
>> >> When you say this "has bit rotted", just how bad is the situation ?
>> >>
>> >> Is the TCG tracing still usable at all, or is is fully broken
>> >> already ?
>> >
>> > Well patches 6/7 got it working for generic TCG things. I haven't been
>> > able to get the architecture one working but I suspect that is some sort
>> > of interaction between the per-arch trace header generation that I
>> > haven't quite figured out yet.
>> 
>> Ahh it's since 7609ffb919 (trace: fix tcg tracing build breakage) which
>> limited tcg/vcpu events to the root trace-events file.
>
> That commit is from release 2.10.0.
>
> The other commit mentioned in patch 6 (73ff061032) is from 2.12.0.
>
> So no one has been able to use this feature for 3+ years already.
>
> Is it actually worth fixing and then deprecating for 2 releases before
> deleting, as opposed to just deleting the broken code today on basis
> that it can't have any current users ?

Well I can get it up and running with the aforementioned patches and it
seems reasonable to give some notice. I'm happy to defer to Stefan here
though as it's his sub-system.

>
> Regards,
> Daniel
Stefan Hajnoczi May 17, 2021, 1:44 p.m. UTC | #6
On Mon, May 17, 2021 at 11:47:11AM +0100, Alex Bennée wrote:
> 
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Wed, May 05, 2021 at 11:41:46AM +0100, Alex Bennée wrote:
> >> 
> >> Alex Bennée <alex.bennee@linaro.org> writes:
> >> 
> >> > Daniel P. Berrangé <berrange@redhat.com> writes:
> >> >
> >> >> On Wed, May 05, 2021 at 10:22:57AM +0100, Alex Bennée wrote:
> >> <snip>
> >> >>> +TCG introspection features
> >> >>> +--------------------------
> >> >>> +
> >> >>> +TCG trace-events (since 6.1)
> >> >>> +''''''''''''''''''''''''''''
> >> >>> +
> >> >>> +The ability to add new TCG trace points has bit rotted and as the
> >> >>
> >> >> When you say this "has bit rotted", just how bad is the situation ?
> >> >>
> >> >> Is the TCG tracing still usable at all, or is is fully broken
> >> >> already ?
> >> >
> >> > Well patches 6/7 got it working for generic TCG things. I haven't been
> >> > able to get the architecture one working but I suspect that is some sort
> >> > of interaction between the per-arch trace header generation that I
> >> > haven't quite figured out yet.
> >> 
> >> Ahh it's since 7609ffb919 (trace: fix tcg tracing build breakage) which
> >> limited tcg/vcpu events to the root trace-events file.
> >
> > That commit is from release 2.10.0.
> >
> > The other commit mentioned in patch 6 (73ff061032) is from 2.12.0.
> >
> > So no one has been able to use this feature for 3+ years already.
> >
> > Is it actually worth fixing and then deprecating for 2 releases before
> > deleting, as opposed to just deleting the broken code today on basis
> > that it can't have any current users ?
> 
> Well I can get it up and running with the aforementioned patches and it
> seems reasonable to give some notice. I'm happy to defer to Stefan here
> though as it's his sub-system.

Lluís Vilanova was the author and probably main user. He mentioned he's
been away from QEMU for a while.

If you want to drop the feature, I think that's fine since it has
already been broken for over 3 years. If someone wants it back then it
can be added via TCG plugins in the future.

Stefan
diff mbox series

Patch

diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 18c6581d85..edf04e3091 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -3,6 +3,8 @@ 
    Copyright (c) 2019, Linaro Limited
    Written by Emilio Cota and Alex Bennée
 
+.. _tcgplugin-ref:
+
 ================
 QEMU TCG Plugins
 ================
diff --git a/docs/devel/tracing.rst b/docs/devel/tracing.rst
index ba83954899..6b0f46cd54 100644
--- a/docs/devel/tracing.rst
+++ b/docs/devel/tracing.rst
@@ -414,6 +414,13 @@  disabled, this check will have no performance impact.
 "tcg"
 -----
 
+.. warning::
+   The ability to add new TCG trace points relies on a having a good
+   understanding of the TCG internals. In the meantime TCG plugins
+   have been introduced which solve many of the same problems with
+   more of a focus on analysing guest code. See :ref:`tcgplugin-ref`
+   for more details.
+
 Guest code generated by TCG can be traced by defining an event with the "tcg"
 event property. Internally, this property generates two events:
 "<eventname>_trans" to trace the event at translation time, and
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 80cae86252..0c9d3c1e1e 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -312,6 +312,19 @@  The ``I7200`` guest CPU relies on the nanoMIPS ISA, which is deprecated
 (the ISA has never been upstreamed to a compiler toolchain). Therefore
 this CPU is also deprecated.
 
+TCG introspection features
+--------------------------
+
+TCG trace-events (since 6.1)
+''''''''''''''''''''''''''''
+
+The ability to add new TCG trace points has bit rotted and as the
+feature can be replicated with TCG plugins it will be deprecated. If
+any user is currently using this feature and needs help with
+converting to using TCG plugins they should contact the qemu-devel
+mailing list.
+
+
 Related binaries
 ----------------