| Message ID | 20210505092259.8202-6-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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 -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
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
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. -- Alex Bennée
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 -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
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 -- Alex Bennée
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 --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 ----------------
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(+) -- 2.20.1