Re: [PATCH v1] s390x/kvm: Set default cpu model for all machine classes

[Thomas Huth]

On 21/10/2019 11.34, David Hildenbrand wrote:
> We have to set the default model of all machine classes, not just for
> the active one. Otherwise, "query-machines" will indicate the wrong
> CPU model ("qemu-s390x-cpu" instead of "host-s390x-cpu") as
> "default-cpu-type".
> 
> Doing a
>     {"execute":"query-machines"}
> under KVM now results in
>     {"return": [
>         {
>             "hotpluggable-cpus": true,
>             "name": "s390-ccw-virtio-4.0",
>             "numa-mem-supported": false,
>             "default-cpu-type": "host-s390x-cpu",
>             "cpu-max": 248,
>             "deprecated": false},
>         {
>             "hotpluggable-cpus": true,
>             "name": "s390-ccw-virtio-2.7",
>             "numa-mem-supported": false,
>             "default-cpu-type": "host-s390x-cpu",
>             "cpu-max": 248,
>             "deprecated": false
>         } ...
> 
> Reported-by: Jiri Denemark <address@hidden>
> Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing")
> Cc: Igor Mammedov <address@hidden>
> Signed-off-by: David Hildenbrand <address@hidden>
> ---
>  target/s390x/kvm.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index c24c869e77..5966ab0d37 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -320,11 +320,17 @@ void kvm_s390_set_max_pagesize(uint64_t pagesize, Error 
> **errp)
>      cap_hpage_1m = 1;
>  }
>  
> -int kvm_arch_init(MachineState *ms, KVMState *s)
> +static void ccw_machine_class_foreach(ObjectClass *klass, void *opaque)
>  {
> -    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +    MachineClass *mc = MACHINE_CLASS(klass);

I think we rather wanted to avoid using "klass" in new code... maybe use
"oc" instead of "klass" ?

>      mc->default_cpu_type = S390_CPU_TYPE_NAME("host");
> +}
> +
> +int kvm_arch_init(MachineState *ms, KVMState *s)
> +{
> +    object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,
> +                         false, NULL);
>  
>      if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
>          error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "
> 

Weird, if you start an older machine, you still get the "host" CPU
without your patch, too:

echo -e "info qom-tree \n quit" | \
 qemu-system-s390x -display none -monitor stdio -no-shutdown \
   -accel kvm -M s390-ccw-virtio-3.0 | grep s390x-cpu

Results in:

    /device[0] (host-s390x-cpu)

... so I wonder why that differs from the "query-machines" command?

Anyway, your patch sounds fine, so (with "klass" replaced by "oc"):

Reviewed-by: Thomas Huth <address@hidden>