* [PATCH v1] s390x/kvm: Set default cpu model for all machine classes
@ 2019-10-21 9:34 David Hildenbrand
2019-10-21 9:52 ` Thomas Huth
0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2019-10-21 9:34 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Janosch Frank, David Hildenbrand, Cornelia Huck,
Richard Henderson, Halil Pasic, Christian Borntraeger,
qemu-s390x, Igor Mammedov, Jiri Denemark
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 <jdenemar@redhat.com>
Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing")
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
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);
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 - "
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1] s390x/kvm: Set default cpu model for all machine classes
2019-10-21 9:34 [PATCH v1] s390x/kvm: Set default cpu model for all machine classes David Hildenbrand
@ 2019-10-21 9:52 ` Thomas Huth
2019-10-21 9:54 ` David Hildenbrand
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2019-10-21 9:52 UTC (permalink / raw)
To: David Hildenbrand, qemu-devel
Cc: Janosch Frank, Cornelia Huck, Richard Henderson, Halil Pasic,
Christian Borntraeger, qemu-s390x, Igor Mammedov, Jiri Denemark
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 <jdenemar@redhat.com>
> Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing")
> Cc: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> 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 <thuth@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] s390x/kvm: Set default cpu model for all machine classes
2019-10-21 9:52 ` Thomas Huth
@ 2019-10-21 9:54 ` David Hildenbrand
2019-10-21 9:58 ` Thomas Huth
2019-10-21 9:58 ` David Hildenbrand
0 siblings, 2 replies; 6+ messages in thread
From: David Hildenbrand @ 2019-10-21 9:54 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Janosch Frank, Cornelia Huck, Richard Henderson, Halil Pasic,
Christian Borntraeger, qemu-s390x, Igor Mammedov, Jiri Denemark
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 <jdenemar@redhat.com>
>> Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing")
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> 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)
>
> Anyway, your patch sounds fine, so (with "klass" replaced by "oc"):
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] s390x/kvm: Set default cpu model for all machine classes
2019-10-21 9:54 ` David Hildenbrand
@ 2019-10-21 9:58 ` Thomas Huth
2019-10-21 9:58 ` David Hildenbrand
1 sibling, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2019-10-21 9:58 UTC (permalink / raw)
To: David Hildenbrand, qemu-devel
Cc: Janosch Frank, Cornelia Huck, Richard Henderson, Halil Pasic,
Christian Borntraeger, qemu-s390x, Igor Mammedov, Jiri Denemark
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 <jdenemar@redhat.com>
>>> Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing")
>>> Cc: Igor Mammedov <imammedo@redhat.com>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] s390x/kvm: Set default cpu model for all machine classes
2019-10-21 9:54 ` David Hildenbrand
2019-10-21 9:58 ` Thomas Huth
@ 2019-10-21 9:58 ` David Hildenbrand
2019-10-21 10:01 ` Cornelia Huck
1 sibling, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2019-10-21 9:58 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Janosch Frank, Cornelia Huck, Richard Henderson, Halil Pasic,
Christian Borntraeger, qemu-s390x, Igor Mammedov, Jiri Denemark
> query-machines probes with the "none" machine all other machines.
> Current code only fixes up the active machine.
>
To be more precise, libvirt probes with "-machine none,accel=kvm:tcg"
all other machines.
> (that's why you won't notice when starting a machine - you will always
> get "host" for the active one)
>
>>
>> Anyway, your patch sounds fine, so (with "klass" replaced by "oc"):
>>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>
>
>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] s390x/kvm: Set default cpu model for all machine classes
2019-10-21 9:58 ` David Hildenbrand
@ 2019-10-21 10:01 ` Cornelia Huck
0 siblings, 0 replies; 6+ messages in thread
From: Cornelia Huck @ 2019-10-21 10:01 UTC (permalink / raw)
To: David Hildenbrand
Cc: Thomas Huth, Janosch Frank, Richard Henderson, qemu-devel,
Halil Pasic, Christian Borntraeger, qemu-s390x, Igor Mammedov,
Jiri Denemark
On Mon, 21 Oct 2019 11:58:35 +0200
David Hildenbrand <david@redhat.com> wrote:
> > query-machines probes with the "none" machine all other machines.
> > Current code only fixes up the active machine.
> >
>
> To be more precise, libvirt probes with "-machine none,accel=kvm:tcg"
> all other machines.
Add that to the patch description?
>
> > (that's why you won't notice when starting a machine - you will always
> > get "host" for the active one)
> >
> >>
> >> Anyway, your patch sounds fine, so (with "klass" replaced by "oc"):
Respin with that? Makes it easier to pick up :)
> >>
> >> Reviewed-by: Thomas Huth <thuth@redhat.com>
> >>
> >
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-21 10:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-21 9:34 [PATCH v1] s390x/kvm: Set default cpu model for all machine classes David Hildenbrand
2019-10-21 9:52 ` Thomas Huth
2019-10-21 9:54 ` David Hildenbrand
2019-10-21 9:58 ` Thomas Huth
2019-10-21 9:58 ` David Hildenbrand
2019-10-21 10:01 ` Cornelia Huck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.