On 10/20/2017 04:12 PM, Christian Borntraeger wrote:
On 10/20/2017 04:06 PM, David Hildenbrand wrote:
On 20.10.2017 16:02, Christian Borntraeger wrote:
On 10/20/2017 03:51 PM, David Hildenbrand wrote: [...]
The problem goes much further. A fresh guest with
<os> <type arch='s390x' machine='s390-ccw-virtio-2.9'>hvm</type> </os> <cpu mode='host-model'/>
does not start. No migration from an older system is necessary.
Yes, as stated in the documentation "copying host CPU definition from capabilities XML" this can not work. And it works just as documented. Not saying this is a nice thing :)
I think we should try to fix gs_allowed (if possible) and avoid something like that in the future. This would avoid other complexity involved when suddenly having X host models.
Maybe this one is really a proper fix. It will allow the guest to start and on migration the cpu model will complain if the target cannot provide gs. Similar things can happen if - for example - the host kernel lacks some features.
Right, just what I had in mind.
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 77169bb..97a08fa 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -430,7 +430,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) s390mc->ri_allowed = true; s390mc->cpu_model_allowed = true; s390mc->css_migration_enabled = true; - s390mc->gs_allowed = true; mc->init = ccw_init; mc->reset = s390_machine_reset; mc->hot_add_cpu = s390_hot_add_cpu; @@ -509,12 +508,6 @@ bool cpu_model_allowed(void) return get_machine_class()->cpu_model_allowed; }
-bool gs_allowed(void) -{ - /* for "none" machine this results in true */ - return get_machine_class()->gs_allowed; -} - static char *machine_get_loadparm(Object *obj, Error **errp) { S390CcwMachineState *ms = S390_CCW_MACHINE(obj); @@ -757,7 +750,6 @@ static void ccw_machine_2_9_class_options(MachineClass *mc) { S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
- s390mc->gs_allowed = false; ccw_machine_2_10_class_options(mc); SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9); s390mc->css_migration_enabled = false; diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h index a9a90c2..1de53b0 100644 --- a/include/hw/s390x/s390-virtio-ccw.h +++ b/include/hw/s390x/s390-virtio-ccw.h @@ -40,7 +40,6 @@ typedef struct S390CcwMachineClass { bool ri_allowed; bool cpu_model_allowed; bool css_migration_enabled; - bool gs_allowed; } S390CcwMachineClass;
/* runtime-instrumentation allowed by the machine */ diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index a0d5052..3f13fc2 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -362,7 +362,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) cap_ri = 1; } } - if (gs_allowed()) { + if (cpu_model_allowed()) { if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) { cap_gs = 1; }
And the last hunk makes sure we keep same handling for machines without CPU model support and therefore no way to mask support. For all recent machines, we expect CPU model checks to be in place.
Will have to think about this a bit more. Will you send this as a proper patch?
After thinking about it :-)
I intend to put some brain-power in this too. Probably next week. My general impression is, that I have a at places different understanding of how things should work compared to David. Especially when it comes to this concept of persistent copying, and also an end-user-digestible definition of what host-model does. (I think this with copying capabilities from whatever xml which is subject to convoluted caching is a bit too hard to digest for an end user not involved in the development of qemu and libvirt). I had a conversation with Boris a couple of hours ago, and it seems, things are pretty convoluted. If I understand the train of thought here (David) it can be summarized like this: For a certain QEMU binary no aspect of the cpu-models may depend on the machine type. In particular, compat properties and compat handling is not alowed to alter cpu-models (whether the available cpu models nor the capabilities of each of these). This we would have to make a part of the external interface. That way one could be sure that the 'cpu capabilities' are machine-type independent (that is, the same for all the machine types). Or did I get this completely wrong? (Based on the answer branches my think). Halil