Re: [PATCH v4 4/4] meson: Warn when TCI is selected but TCG backend is available

[Stefan Weil]

Am 25.01.21 um 18:05 schrieb Philippe Mathieu-Daudé:

> On 1/25/21 5:47 PM, Daniel P. Berrangé wrote:
> > On Mon, Jan 25, 2021 at 03:45:30PM +0100, Philippe Mathieu-Daudé wrote:
> > > Some new users get confused with 'TCG' and 'TCI', and enable TCI
> > > support expecting to enable TCG.
> > >
> > > Emit a warning when native TCG backend is available on the
> > > host architecture, mentioning this is a suboptimal configuration.
> > >
> > > Reviewed-by: Stefan Weil <sw@weilnetz.de>
> > > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > Nitpick, the text printed is completely rewritten from what they
> > reviewed, so I would probably have dropped their R-b for that
> > scenario.
> I thought about it, and assumed their review tag was for the logical
> change of adding a warning, not particularly the warning content.
>
> I agree this it would have been better to ask them to review again.
> Next time I will reset the tags.


You are right, I would not have signed that new text and either used the 
original text (which was sufficient in my opinion) or used a different one:

Use of the TCG interpretor is not recommended on this host architecture for 
most users because there is a native TCG execution backend available which 
provides substantially better performance.

I have no evidence that TCI is less reliable than TCG, so I would not 
write that.

And there are people who have good reasons to use TCI. So why should I 
recommend them to stop that?

Stefan

> > > Suggested-by: Daniel Berrangé <berrange@redhat.com>
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > ---
> > >   meson.build | 7 +++++++
> > >   1 file changed, 7 insertions(+)
> > >
> > > diff --git a/meson.build b/meson.build
> > > index 16b2560e7e7..f675c54e636 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -228,6 +228,13 @@
> > >       else
> > >         error('Unsupported CPU @0@, try --enable-tcg-interpreter'.format(cpu))
> > >       endif
> > > +  elif get_option('tcg_interpreter')
> > > +    warning('Use of the TCG interpretor is not recommended on this host')
> > > +    warning('architecture. There is a native TCG execution backend available')
> > > +    warning('which provides substantially better performance and reliability.')
> > > +    warning('It is strongly recommended to remove the 
> > > --enable-tcg-interpreter')
> > > +    warning('configuration option on this architecture to use the native')
> > > +    warning('backend.')