[libvirt] Question about the host-model CPU mode

[Christian Borntraeger]

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 :-)