On 10.12.2012, at 11:09, Cornelia Huck wrote:

> On Mon, 10 Dec 2012 08:33:10 +0100
> Alexander Graf <ag...@suse.de> wrote:
> 
>> 
>> On 07.12.2012, at 13:30, Cornelia Huck wrote:
>> 
>>> Add support for handling I/O interrupts (standard, subchannel-related
>>> ones and rudimentary adapter interrupts).
>>> 
>>> The subchannel-identifying parameters are encoded into the interrupt
>>> type.
>>> 
>>> I/O interrupts are floating, so they can't be injected on a specific
>>> vcpu.
>>> 
>>> Reviewed-by: Marcelo Tosatti <mtosa...@redhat.com>
>>> Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com>
>>> ---
>>> Documentation/virtual/kvm/api.txt |   4 ++
>>> arch/s390/include/asm/kvm_host.h  |   2 +
>>> arch/s390/kvm/interrupt.c         | 115 
>>> ++++++++++++++++++++++++++++++++++++--
>>> include/uapi/linux/kvm.h          |   6 ++
>>> 4 files changed, 122 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/Documentation/virtual/kvm/api.txt 
>>> b/Documentation/virtual/kvm/api.txt
>>> index 6671fdc..e298a72 100644
>>> --- a/Documentation/virtual/kvm/api.txt
>>> +++ b/Documentation/virtual/kvm/api.txt
>>> @@ -2068,6 +2068,10 @@ KVM_S390_INT_VIRTIO (vm) - virtio external 
>>> interrupt; external interrupt
>>> KVM_S390_INT_SERVICE (vm) - sclp external interrupt; sclp parameter in parm
>>> KVM_S390_INT_EMERGENCY (vcpu) - sigp emergency; source cpu in parm
>>> KVM_S390_INT_EXTERNAL_CALL (vcpu) - sigp external call; source cpu in parm
>>> +KVM_S390_INT_IO(ai,cssid,ssid,schid) (vm) - compound value to indicate an
>>> +    I/O interrupt (ai - adapter interrupt; cssid,ssid,schid - subchannel);
>>> +    I/O interruption parameters in parm (subchannel) and parm64 (intparm,
>>> +    interruption subclass)
>>> 
>>> Note that the vcpu ioctl is asynchronous to vcpu execution.
>>> 
>>> diff --git a/arch/s390/include/asm/kvm_host.h 
>>> b/arch/s390/include/asm/kvm_host.h
>>> index b784154..e47f697 100644
>>> --- a/arch/s390/include/asm/kvm_host.h
>>> +++ b/arch/s390/include/asm/kvm_host.h
>>> @@ -76,6 +76,7 @@ struct kvm_s390_sie_block {
>>>     __u64   epoch;                  /* 0x0038 */
>>>     __u8    reserved40[4];          /* 0x0040 */
>>> #define LCTL_CR0    0x8000
>>> +#define LCTL_CR6   0x0200
>>>     __u16   lctl;                   /* 0x0044 */
>>>     __s16   icpua;                  /* 0x0046 */
>>>     __u32   ictl;                   /* 0x0048 */
>>> @@ -127,6 +128,7 @@ struct kvm_vcpu_stat {
>>>     u32 deliver_prefix_signal;
>>>     u32 deliver_restart_signal;
>>>     u32 deliver_program_int;
>>> +   u32 deliver_io_int;
>>>     u32 exit_wait_state;
>>>     u32 instruction_stidp;
>>>     u32 instruction_spx;
>>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>>> index c30615e..070ba22 100644
>>> --- a/arch/s390/kvm/interrupt.c
>>> +++ b/arch/s390/kvm/interrupt.c
>>> @@ -21,11 +21,26 @@
>>> #include "gaccess.h"
>>> #include "trace-s390.h"
>>> 
>>> +#define IOINT_SCHID_MASK 0x0000ffff
>>> +#define IOINT_SSID_MASK 0x00030000
>>> +#define IOINT_CSSID_MASK 0x03fc0000
>>> +#define IOINT_AI_MASK 0x04000000
>>> +
>>> +static int is_ioint(u64 type)
>>> +{
>>> +   return ((type & 0xfffe0000u) != 0xfffe0000u);
>>> +}
>>> +
>>> static int psw_extint_disabled(struct kvm_vcpu *vcpu)
>>> {
>>>     return !(vcpu->arch.sie_block->gpsw.mask & PSW_MASK_EXT);
>>> }
>>> 
>>> +static int psw_ioint_disabled(struct kvm_vcpu *vcpu)
>>> +{
>>> +   return !(vcpu->arch.sie_block->gpsw.mask & PSW_MASK_IO);
>>> +}
>>> +
>>> static int psw_interrupts_disabled(struct kvm_vcpu *vcpu)
>>> {
>>>     if ((vcpu->arch.sie_block->gpsw.mask & PSW_MASK_PER) ||
>>> @@ -68,7 +83,18 @@ static int __interrupt_is_deliverable(struct kvm_vcpu 
>>> *vcpu,
>>>     case KVM_S390_RESTART:
>>>             return 1;
>>>     default:
>>> -           BUG();
>>> +           if (is_ioint(inti->type)) {
>> 
>> Though I usually like if (...) { positive) } else { abort(); } coding style 
>> in general, it makes code quite hard to read when you are limited to 80 
>> characters per line :)
>> 
>> I think it'd really help readability if you instead would write
>> 
>> if (!is_ioint(...)) {
>>    BUG();
>> }
>> 
>> and then continue without indent. That problem gets even more obvious 
>> further down the file.
> 
> Hm, "bad state last" seems to parse, though.

Fine with me. Extract the io bits into a separate function then :). Or maybe 
better even just use gcc switch magic:

  case 0x0 ... 0xfffdffff:


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to