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

[Alexander Graf]

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.


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