Re: [Qemu-devel] [PATCH/RFC 2/3] s390x/ais: enable ais when migration is available

[Halil Pasic <pasic@linux.vnet.ibm.com>]

On 09/22/2017 04:07 PM, Christian Borntraeger wrote:
> 
> 
> On 09/22/2017 04:02 PM, Pierre Morel wrote:
>> On 22/09/2017 14:40, Christian Borntraeger wrote:
>>>
>>>
>>> On 09/22/2017 02:13 PM, Pierre Morel wrote:
>>>> On 22/09/2017 10:38, Christian Borntraeger wrote:
>>>>> Instead of unconditionally enabling the KVM AIS capability
>>>>> in the kvm arch init function, do this in the flic realize function
>>>>> when we know if migration is available. This requires to initialize
>>>>> flic before the CPUs.
>>>>
>>>> I am not sure to agree.
>>>>
>>>> AIS facility is used for PCI (currently only PCI)
>>>> We want to support PCI emulation and PCI VFIO
>>>>
>>>>
>>>> Not having AIS support in the host kernel or not supporting AISM in the host kernel does not affect the emulation.
>>>> Neither virtio-pci nor TCG.
>>>> The only devices, (currently), which can not work without AIS and is not migratable without AISM are PCI VFIO devices.
>>>
>>> This patch enable the conditional enablement facility for the KVM host mask. The cpu model enablement is done
>>> differently for KVM and TCG anyway.
>>> Right now AIS is only enabled for KVM. For TCG AIS is not implemented at all and disabled. So for whenever this
>>> is fixed in TCG it can be handled then.
>>>
>>> And for emulated devices under KVM you still need the kernel support - otherwise migration is really broken for
>>> the nimm/dimm values.
>>>
>>
>> Yes, that is right.
>> I do not pretend that it is working directly.
>>
>> To support AIS emulation we have some work to do there (the migration), in kvm_s390_inject_airq() and other functions using ais_supported, to use emulated values.
>>
>> Don't you think that we could use emulation instead of faulting when the kernel can not handle values needed for pass-through?
>>
>> We can fault at the moment a VFIO device is realized.
>> Migration will be stopped on realizing VFIO PCI devices instead of flic.
>>
>> I can miss something, is there a reason not to do so?
> 
> The problem is not vfio vs emulation. The problem is that for KVM the kernel is handling the interrupts.
> and it holds the state of the interrupt controller (nimm/dimm) and without an interface to read/write these, 
> the PCI core will be in a wrong state after migration regarding AIS.
> 
> Really, this patch just moves the supported host kernel from >=4.12 to >=4.13. It makes absolutely no
> sense to start having a mixed interrupt stack (qemu + kernel) just to make this work on 4.12.
> 

I tend to agree with Christian. I don't really understand Pierre's
proposal. I'm sure a patch would help clarify things. If it's too
complicated for writing up a patch, I think it's too complicated for
discussing too.

About the wrong state. Back then I did some reasoning about how bad this
bad state actually is. In case of AIS starting with the initial state
after the migration might not be fatal (all cases considered). I was
asking myself is there any problem beyond getting more irqs delivered
than necessary, and does that only affect the performance of the guest,
or could it lead to correctness issues?  I found the later question
difficult to answer.

The whole AIS stuff was a while ago. From what I see what Christian is
trying to do is both sane and conservative. Would need to revisit
everything to say more.

One thing I would find very helpful is what do we expect to work and not
work for which version. Kind of a matrix. For instance should vfio pci
work for versions prior 2.11. I think in the not so distant past we
changed how SIC works (so it complains when we don't have ais).

Btw nimm/dimm is simm/nimm. And I would really like to hear something
form the original author(s) too.

Regards,
Halil