On Tue, May 05, 2015 at 06:12:16PM +0200, Michael Mueller wrote:
> On Tue, 5 May 2015 10:55:47 -0300
> Eduardo Habkost <ehabk...@redhat.com> 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()
> */
(Side note: I believe the above outdated, I will send a patch to update
it.)
> 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).
>
> 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);
> ...
> }
> }
In other words, you just need to ensure that s390_cpu_list() run after
kvm_setup_cpu_classes().
Can't you simply call s390_setup_cpu_classes(ACCEL_TEMP) inside
s390_init_cpus(), just like arch_query_cpu_definitions()? You could even
share code between both functions.
(In the future, we should be able to implement "-cpu ?" by simply
calling the query-cpu-definitions implementation.)
>
> >
> > 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)
>
>
[...]
--
Eduardo
