Re: [PATCH] hw/ppc/spapr: Test whether TCG is enabled with tcg_enabled()

[Philippe Mathieu-Daudé]

On 20/6/23 16:48, Greg Kurz wrote:
> On Tue, 20 Jun 2023 09:55:49 +0200
> Claudio Fontana <cfontana@suse.de> wrote:
>
> > On 6/20/23 09:48, Philippe Mathieu-Daudé wrote:
> > > Although the PPC target only supports the TCG and KVM
> > > accelerators, QEMU supports more. We can no assume that
> > > '!kvm == tcg', so test for the correct accelerator. This
> > > also eases code review, because here we don't care about
> > > KVM, we really want to test for TCG.
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >
> > I don't remember anymore, but what about qtest ? It is usually the forgotten 
> > case in these kind of tests... so much complexity :-)

Good to think about this, since such changes indeed usually break,
qtests :)

> This check was added with TCG in mind because it is a known limitation.
> I don't see any reason to prevent qtest from being used with the rest
> of this function though.

We don't have any CPU core when using qtest accelerator, so this
check is irrelevant IMHO (not reachable with '-accel qtest').

> > > ---
> > >   hw/ppc/spapr.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index dcb7f1c70a..c4b666587b 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -2524,7 +2524,7 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, 
> > > Error **errp)
> > >       int ret;
> > >       unsigned int smp_threads = ms->smp.threads;
> > >   
> > > -    if (!kvm_enabled() && (smp_threads > 1)) {
> > > +    if (tcg_enabled() && (smp_threads > 1)) {
>
> Bonjour Philippe,
>
> Please drop the unneeded parens in the second check.

I wanted to do that but then thought someone would ask me to do one
change at once ;)

> With this fixed,
>
> Reviewed-by: Greg Kurz <groug@kaod.org>

Thanks!