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

[Michael Mueller]

On Tue, 5 May 2015 10:55:47 -0300
Eduardo Habkost <address@hidden> wrote:

> On Mon, Apr 27, 2015 at 04:53:15PM +0200, Michael Mueller wrote:
> > This patch introduces the function cpu_desc_avail() which returns by
> > default true if not architecture specific implemented. Its intention
> > is to indicate if the cpu model description is available for display
> > by list_cpus(). This change allows cpu model descriptions to become
> > dynamically created by evaluating the runtime context instead of
> > putting static cpu model information at display.
> 
> Why are you deliberately breaking "-cpu ?" when cpu_desc_avail() is
> false?

In the s390x case cpu_desc_avail() is per se false in this code section of vl.c:

  /* Init CPU def lists, based on config                         
   * - Must be called after all the qemu_read_config_file() calls
   * - Must be called before list_cpu()
   * - Must be called before machine->init()
   */
   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.

Once the accelerator has been initialized AND the S390 cpu classes have
been setup by means of the following code:

static void kvm_setup_cpu_classes(KVMState *s)
{
    S390MachineProps mach;

    if (!kvm_s390_get_machine_props(s, &mach)) {
        s390_setup_cpu_classes(ACCEL_CURRENT, &mach,
                               s390_current_fac_list_mask());
        s390_setup_cpu_aliases();
        cpu_classes_initialized = true;
    }
}

cpu_desc_avail() becomes true. In case the selceted mode was "?"
the list_cpu() is now done right before the cpu model is used as part
of the cpu initialization (hw/s390-virtio.c):

void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
{
    int i;

    if (cpu_model == NULL) {
        cpu_model = "none";
    }

    if (is_help_option(cpu_model)) {
        list_cpus(stdout, &fprintf, cpu_model);
        exit(0);
    }

    ...
    for (i = 0; i < smp_cpus; i++) {
        ...
        cpu = cpu_s390x_init(cpu_model);
        ...
    }
}

> 
> What exactly could cause cpu_desc_avail() to be false? If CPU model
> information is not yet available when cpu_list() is called, it is a bug.
> 

Here an example output that shows only runnable cpu models:

$ ./s390x-softmmu/qemu-system-s390x -machine s390,accel=kvm -cpu ?
s390 none       
s390 2064-ga1   IBM zSeries 900 GA1
s390 2064-ga2   IBM zSeries 900 GA2
s390 2064-ga3   IBM zSeries 900 GA3
s390 2064       (alias for 2064-ga3)
s390 z900       (alias for 2064-ga3)
s390 2066-ga1   IBM zSeries 800 GA1
s390 2066       (alias for 2066-ga1)
s390 z800       (alias for 2066-ga1)
s390 2084-ga1   IBM zSeries 990 GA1
s390 2084-ga2   IBM zSeries 990 GA2
s390 2084-ga3   IBM zSeries 990 GA3
s390 2084-ga4   IBM zSeries 990 GA4
s390 2084-ga5   IBM zSeries 990 GA5
s390 2084       (alias for 2084-ga5)
s390 z990       (alias for 2084-ga5)
s390 2086-ga1   IBM zSeries 890 GA1
s390 2086-ga2   IBM zSeries 890 GA2
s390 2086-ga3   IBM zSeries 890 GA3
s390 2086       (alias for 2086-ga3)
s390 z890       (alias for 2086-ga3)
s390 2094-ga1   IBM System z9 EC GA1
s390 z9-109     (alias for 2094-ga1)
s390 2094-ga2   IBM System z9 EC GA2
s390 2094-ga3   IBM System z9 EC GA3
s390 2094       (alias for 2094-ga3)
s390 z9         (alias for 2094-ga3)
s390 z9-ec      (alias for 2094-ga3)
s390 2096-ga1   IBM System z9 BC GA1
s390 2096-ga2   IBM System z9 BC GA2
s390 2096       (alias for 2096-ga2)
s390 z9-bc      (alias for 2096-ga2)
s390 2097-ga1   IBM System z10 EC GA1
s390 2097-ga2   IBM System z10 EC GA2
s390 2097-ga3   IBM System z10 EC GA3
s390 2097       (alias for 2097-ga3)
s390 z10        (alias for 2097-ga3)
s390 z10-ec     (alias for 2097-ga3)
s390 2098-ga1   IBM System z10 BC GA1
s390 2098-ga2   IBM System z10 BC GA2
s390 2098       (alias for 2098-ga2)
s390 z10-bc     (alias for 2098-ga2)
s390 2817-ga1   IBM zEnterprise 196 GA1
s390 2817-ga2   IBM zEnterprise 196 GA2
s390 2817       (alias for 2817-ga2)
s390 z196       (alias for 2817-ga2)
s390 2818-ga1   IBM zEnterprise 114 GA1
s390 2818       (alias for 2818-ga1)
s390 z114       (alias for 2818-ga1)
s390 2827-ga1   IBM zEnterprise EC12 GA1
s390 2827-ga2   IBM zEnterprise EC12 GA2
s390 2827       (alias for 2827-ga2)
s390 zEC12      (alias for 2827-ga2)
s390 host       (alias for 2827-ga2)
s390 2828-ga1   IBM zEnterprise BC12 GA1
s390 2828       (alias for 2828-ga1)
s390 zBC12      (alias for 2828-ga1)


> > 
> > Signed-off-by: Michael Mueller <address@hidden>
> > Reviewed-by: Thomas Huth <address@hidden>
> > Acked-by: Christian Borntraeger <address@hidden>
> > ---
> >  include/qemu-common.h  | 2 ++
> >  stubs/Makefile.objs    | 1 +
> >  stubs/cpu-desc-avail.c | 6 ++++++
> >  vl.c                   | 2 +-
> >  4 files changed, 10 insertions(+), 1 deletion(-)
> >  create mode 100644 stubs/cpu-desc-avail.c
> > 
> > diff --git a/include/qemu-common.h b/include/qemu-common.h
> > index 1b5cffb..386750f 100644
> > --- a/include/qemu-common.h
> > +++ b/include/qemu-common.h
> > @@ -484,4 +484,6 @@ int parse_debug_env(const char *name, int max, int 
> > initial);
> >  
> >  const char *qemu_ether_ntoa(const MACAddr *mac);
> >  
> > +bool cpu_desc_avail(void);
> > +
> >  #endif
> > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> > index 8beff4c..dce9cd2 100644
> > --- a/stubs/Makefile.objs
> > +++ b/stubs/Makefile.objs
> > @@ -39,3 +39,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
> >  stub-obj-y += cpus.o
> >  stub-obj-y += kvm.o
> >  stub-obj-y += qmp_pc_dimm_device_list.o
> > +stub-obj-y += cpu-desc-avail.o
> > diff --git a/stubs/cpu-desc-avail.c b/stubs/cpu-desc-avail.c
> > new file mode 100644
> > index 0000000..0cd594e
> > --- /dev/null
> > +++ b/stubs/cpu-desc-avail.c
> > @@ -0,0 +1,6 @@
> > +#include "qemu-common.h"
> > +
> > +bool cpu_desc_avail(void)
> > +{
> > +    return true;
> > +}
> > diff --git a/vl.c b/vl.c
> > index 74c2681..c552561 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -3820,7 +3820,7 @@ int main(int argc, char **argv, char **envp)
> >       */
> >      cpudef_init();
> >  
> > -    if (cpu_model && is_help_option(cpu_model)) {
> > +    if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
> >          list_cpus(stdout, &fprintf, cpu_model);
> >          exit(0);
> >      }
> > -- 
> > 1.8.3.1
> > 
>