On 21/10/2019 11.54, David Hildenbrand wrote:
> On 21.10.19 11:52, Thomas Huth wrote:
>> 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 <[email protected]>
>>> Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing")
>>> Cc: Igor Mammedov <[email protected]>
>>> Signed-off-by: David Hildenbrand <[email protected]>
>>> ---
>>> 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" ?
>
> Can do, this was a copy and paste :)
>
>>
>>> 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?
>
> query-machines probes with the "none" machine all other machines.
> Current code only fixes up the active machine.
>
> (that's why you won't notice when starting a machine - you will always
> get "host" for the active one)
Ah, right, thanks for the explanation. I somehow read the patch
description that it is only fixed for the latest one (I should really
read more closely) ... but now it makes perfect sense.
Thomas
