Re: [Qemu-devel] [PATCH/RFC 3/3] s390x/ais: disable ais for compat machines

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 09/26/2017 03:32 PM, Christian Borntraeger wrote:
> 
> 
> On 09/26/2017 03:00 PM, David Hildenbrand wrote:
> 
>>
>> Wonder when we can finally let go of these hacks. The host cpu model is
>> not migration safe, therefore such things are expected to not work. Just
>> think about trying to migrate from a kernel without AIS support to a
>> kernel with AIS support. It is broken.
>>
>> AIS is just one feature that actually tells you that you are currently
>> doing something evil. Other CPU features you lose on the way simply
>> don't result in an error, but still migration could break silently
>> afterwards, when the guest assumes it has certain CPU features.
>>
>>
>> The main problem is that we have machines <= 2.7 that had no CPU model
>> support. For these machines, migration should work just fine, as
>> 	s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION)
>> will always return false.
>> However, the guest will be presented the AIS bit, as soon as the
>> capability is enabled (as we then don't manually set the stfle bitmap
>> from QEMU), which is bad.
>>
>> So my point would be: don't turn on these new facilities if the cpu
>> model is not allowed (<=2.7), but don't add ais_allowed() compat
>> handling for any newer machines. If they use the host model, they have
>> to assume that migration can break.
>>
>> I think using cpu_model_allowed() would be just fine to be used instead
>> of ais_allowed().
> 
> Let me thing about that. I will send 2 (with fs->ais_supported gone) and 3
> and we can then decide if we only want to apply 2 or both.
> 
> Now: regarding your -cpu host comment:
> 
> I checked all the migration stuff now with -cpu z13 and it seems that
> migration is now broken for qemu 2.10->qemu 2.11 when using the s390-ccw-virtio-2.9
> machine due to the PCI fencing from Conny.
> 
> qemu-system-s390x: Unknown savevm section or instance 'PCIBUS' 0
> qemu-system-s390x: load of migration failed: Invalid argument
> 

A very dirty hack like this seems to do the trick

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 41b770a..3d49044 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -294,7 +294,7 @@ static void ccw_init(MachineState *machine)
                       machine->initrd_filename, "s390-ccw.img",
                       "s390-netboot.img", true);
 
-    if (s390_has_feat(S390_FEAT_ZPCI)) {
+    if (s390_has_feat(S390_FEAT_ZPCI) || !ais_allowed()) {
         DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
         object_property_add_child(qdev_get_machine(),
                                   TYPE_S390_PCI_HOST_BRIDGE,

Basic idea is that for compat_machines <= 2.10 we enable the PCI host bridge
(and I abused ais_allowed to check for <= 2.10). 
Not sure what to do for for CONFIG_PCI = off case, but the normal case is
broken right now.