[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
From: |
Michael Mueller |
Subject: |
Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail |
Date: |
Thu, 7 May 2015 09:35:01 +0200 |
On Wed, 6 May 2015 14:06:16 -0300
Eduardo Habkost <address@hidden> wrote:
> 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);
> }
> }
That approach is will do the job as well. I will prepare a patch for the next
version.
Thanks!
>
> [...]
> > >
> > > 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().
perfect.
Michael
>