Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail

[Eduardo Habkost]

On Wed, May 06, 2015 at 06:23:05PM +0200, Michael Mueller wrote:
> On Wed, 6 May 2015 08:23:32 -0300
> Eduardo Habkost <address@hidden> wrote:
[...]
> > > > >    cpudef_init();
> > > > > 
> > > > >    if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
> > > > >        list_cpus(stdout, &fprintf, cpu_model);
> > > > >        exit(0);
> > > > >    }
> > > > > 
> > > > > That is because the output does not solely depend on static 
> > > > > definitions
> > > > > but also on runtime context. Here the host machine type this instance 
> > > > > of
> > > > > QEMU is running on, at least for the KVM case.
> > > > 
> > > > Is this a required feature? I would prefer to have the main() code
> > > > simple even if it means not having runnable information in "-cpu ?" by
> > > > now (about possible ways to implement this without cpu_desc_avail(), see
> > > > below).
> > > 
> > > I think it is more than a desired feature because one might end up with a 
> > > failed
> > > CPU object instantiation although the help screen claims to CPU model to 
> > > be valid. 
> > 
> > I think you are more likely to confuse users by not showing information
> > on "-cpu ?" when -machine is not present. I believe most people use
> > "-cpu ?" with no other arguments, to see what the QEMU binary is capable
> > of.
> 
> I don't disagree with that, both cases are to some extend confusing...
> But the accelerator makes a big difference and a tended user should really be 
> aware
> of that.
> 
> Also that TCG is the default:
> 
> $ ./s390x-softmmu/qemu-system-s390x -cpu ?
> s390             host
> 
> And I don't see a way to make a user belief that all the defined CPU models 
> are available to a 
> TCG user in the S390 case where most of the CPU facilities are not 
> implemented.

Well, we could simply add a "KVM required" note (maybe just an asterisk beside
the CPU model description). But maybe we have a reasonable alternative below:

> 
> > 
> > Anyway, whatever we decide to do, I believe we should start with
> > something simple to get things working, and after that we can look for
> > ways improve the help output with "runnable" info.
> 
> I don't see how to solve this without cpu_desc_avail() or some other 
> comparable mechanism, the
> aliases e.g. are also dynamic...

What bothers me in cpu_desc_avail() is that it depends on global state that is
non-trivial (one needs to follow the whole KVM initialization path to find out
if cpu_desc_avail() will be true or false).

We could instead simply skip the cpu_list() call unconditionally on s390. e.g.:

target-s390x/cpu.h:
    /* Delete the existing cpu_list macro */

cpus.c:
    int list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
    {
    #if defined(cpu_list)
        cpu_list(f, cpu_fprintf);
        return 1;
    #else
        return 0;
    #endif
    }

vl.c:
    if (cpu_model && is_help_option(cpu_model)) {
        /* zero list_cpus() return value means "-cpu ?" will be
         * handled later by machine initialization code */
        if (list_cpus(stdout, &fprintf, cpu_model)) {
            exit(0);
        }
    }

[...]
> > 
> > About "-cpu ?": do we really want it to depend on -machine processing?
> > Today, help output shows what the QEMU binary is capable of, not just
> > what the host system and -machine option are capable of.
> 
> I think we have to take it into account because the available CPU models might
> deviate substantially as in the case for S390 for KVM and TCG.

That's true, on s390 the set of available CPU models is very different on both
cases. It breaks assumptions in the existing "-cpu ?" handling code in main().

>  
> > 
> > If we decide to change that assumption, let's do it in a generic way and
> > not as a arch-specific hack. The options I see are:
> 
> welcome
> 
> > 
> > 1) Continue with the current policy where "-cpu ?" does not depend on
> >    -machine arguments, and show all CPU models on "-cpu ?".
> > 2) Deciding that, yes, it is OK to make "-cpu ?" depend on -machine
> >    arguments, and move the list_cpus() call after machine initialization
> >    inside generic main() code for all arches.
> >    2.1) We could delay the list_cpus() call inside main() on all cases.
> >    2.2) We could delay the list_cpus() call inside main() only if
> >         an explicit -machine option is present.
> > 
> > I prefer (1) and my second choice would be (2.2), but the main point is
> > that none of the options above require making s390 special and
> > introducing cpu_desc_avail().
> 
> My take here is 2.1 because omitting option -machine is a decision to some
> defaults for machine type and accelerator type already.  

The problem with 2.1 is that some machine init functions require that
additional command-line parameters are set and will abort (e.g. mips machines).
So we can't do that unconditionally for all architectures.

The proposal above is like 2.1, but conditional: it will delay "-cpu ?"
handling only on architectures that don't define cpu_list().

-- 
Eduardo