Re: [Qemu-devel] [PATCH v2 0/2] KVM: s390: add floating irq controller

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

On 05/10/13 01:54, Alexander Graf wrote:
> 
> On 06.09.2013, at 15:30, Christian Borntraeger wrote:
> 
>> On 06/09/13 14:19, Jens Freimann wrote:> This series adds a kvm_device that acts as a irq controller for floating
>>> interrupts.  As a first step it implements functionality to retrieve and inject
>>> interrupts for the purpose of migration and for hardening the reset code by
>>> allowing user space to explicitly remove all pending floating interrupts.
>>>
>>> PFAULT patches will also use this device for enabling/disabling pfault, therefore
>>> the pfault patch series will be reworked to use this device.
>>>
>>> * Patch 1/2 adds a new data structure to hold interrupt information. The current
>>>  one (struct kvm_s390_interrupt) does not allow to inject every kind of interrupt,
>>>  e.g. some data for program interrupts and machine check interruptions were
>>>  missing.
>>>
>>> * Patch 2/2 adds a kvm_device which supports getting/setting currently pending
>>>  floating interrupts as well as deleting all currently pending interrupts
>>>
>>>
>>> Jens Freimann (2):
>>>  KVM: s390: add and extend interrupt information data structs
>>>  KVM: s390: add floating irq controller
>>>
>>> Documentation/virtual/kvm/devices/s390_flic.txt |  36 +++
>>> arch/s390/include/asm/kvm_host.h                |  35 +--
>>> arch/s390/include/uapi/asm/kvm.h                |   5 +
>>> arch/s390/kvm/interrupt.c                       | 304 ++++++++++++++++++++----
>>> arch/s390/kvm/kvm-s390.c                        |   1 +
>>> include/linux/kvm_host.h                        |   1 +
>>> include/uapi/linux/kvm.h                        |  65 +++++
>>> virt/kvm/kvm_main.c                             |   5 +
>>> 8 files changed, 368 insertions(+), 84 deletions(-)
>>> create mode 100644 Documentation/virtual/kvm/devices/s390_flic.txt
>>>
>>
>>
>> Gleb, Paolo,
>>
>> since the qemu part relies on a kernel header file, it makes sense to not only let the kernel
>> part go via the kvm tree, but also the qemu part. I want Alex to Ack the interface, and if he
>> agrees then I am fine with applying the whole series.
> 
> I think the interface works. My comments are almost exclusively on internal code structure which can follow up on a later patch. The only thing that definitely needs fixing now is the unnamed union.

Will send a fixed version. Jens is working on patches to totally rework the internal structure 
for a while now. They will use a per-cpu bitfield for interrupt types as well as floating int
bitfield. Each cpu will OR both bitfields and obey the architectures priority. These patches
 will take some more time and testing, so it makes sense to start with flic and let the other
patches mature a bit.

> 
> 
> Alex
> 
>>
>> If nothing else comes up, feel free to apply the small change request from Peter yourself or
>> ask Jens for a resend.
>>
>> ------snip----
>>
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -908,7 +908,7 @@ struct kvm_device_attr {
>> #define KVM_DEV_TYPE_FSL_MPIC_20       1
>> #define KVM_DEV_TYPE_FSL_MPIC_42       2
>> #define KVM_DEV_TYPE_XICS              3
>> -#define KVM_DEV_TYPE_FLIC              4
>> +#define KVM_DEV_TYPE_FLIC              5
>>
>> /*
>>  * ioctls for VM fds
>>
>> ------snip----
>>
>