On Mon, 10 Dec 2012 08:33:10 +0100 Alexander Graf <[email protected]> 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 <[email protected]> > > Signed-off-by: Cornelia Huck <[email protected]> > > --- > > 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. > > > + if (psw_ioint_disabled(vcpu)) > > + return 0; > > + if (vcpu->arch.sie_block->gcr[6] & > > + inti->io.io_int_word) > > + return 1; > > + return 0; > > + } else { > > + printk(KERN_WARNING "illegal interrupt type %llx\n", > > + inti->type); > > + BUG(); > > + } > > } > > return 0; > > } > > @@ -117,6 +143,13 @@ static void __set_intercept_indicator(struct kvm_vcpu > > *vcpu, > > __set_cpuflag(vcpu, CPUSTAT_STOP_INT); > > break; > > default: > > + if (is_ioint(inti->type)) { > > + if (psw_ioint_disabled(vcpu)) > > + __set_cpuflag(vcpu, CPUSTAT_IO_INT); > > + else > > + vcpu->arch.sie_block->lctl |= LCTL_CR6; > > + break; > > + } > > BUG(); > > Please reverse the logic here, just like you did above :). Hm, they're both the same? > > > } > > } > > @@ -298,7 +331,49 @@ static void __do_deliver_interrupt(struct kvm_vcpu > > *vcpu, > > break; > > > > default: > > - BUG(); > > + if (is_ioint(inti->type)) { > > + __u32 param0 = ((__u32)inti->io.subchannel_id << 16) | > > + inti->io.subchannel_nr; > > + __u64 param1 = ((__u64)inti->io.io_int_parm << 32) | > > + inti->io.io_int_word; > > + VCPU_EVENT(vcpu, 4, > > + "interrupt: I/O %llx", inti->type); > > + vcpu->stat.deliver_io_int++; > > + trace_kvm_s390_deliver_interrupt(vcpu->vcpu_id, > > inti->type, > > + param0, param1); > > + rc = put_guest_u16(vcpu, __LC_SUBCHANNEL_ID, > > + inti->io.subchannel_id); > > + if (rc == -EFAULT) > > + exception = 1; > > + > > + rc = put_guest_u16(vcpu, __LC_SUBCHANNEL_NR, > > + inti->io.subchannel_nr); > > + if (rc == -EFAULT) > > + exception = 1; > > + > > + rc = put_guest_u32(vcpu, __LC_IO_INT_PARM, > > + inti->io.io_int_parm); > > + if (rc == -EFAULT) > > + exception = 1; > > + > > + rc = put_guest_u32(vcpu, __LC_IO_INT_WORD, > > + inti->io.io_int_word); > > + if (rc == -EFAULT) > > + exception = 1; > > + > > + rc = copy_to_guest(vcpu, __LC_IO_OLD_PSW, > > + &vcpu->arch.sie_block->gpsw, > > + sizeof(psw_t)); > > + if (rc == -EFAULT) > > + exception = 1; > > + > > + rc = copy_from_guest(vcpu, &vcpu->arch.sie_block->gpsw, > > + __LC_IO_NEW_PSW, sizeof(psw_t)); > > + if (rc == -EFAULT) > > + exception = 1; > > + break; > > + } else > > + BUG(); > > } > > if (exception) { > > printk("kvm: The guest lowcore is not mapped during interrupt " > > @@ -545,7 +620,7 @@ int kvm_s390_inject_vm(struct kvm *kvm, > > { > > struct kvm_s390_local_interrupt *li; > > struct kvm_s390_float_interrupt *fi; > > - struct kvm_s390_interrupt_info *inti; > > + struct kvm_s390_interrupt_info *inti, *iter; > > int sigcpu; > > > > inti = kzalloc(sizeof(*inti), GFP_KERNEL); > > @@ -569,9 +644,26 @@ int kvm_s390_inject_vm(struct kvm *kvm, > > case KVM_S390_SIGP_STOP: > > case KVM_S390_INT_EXTERNAL_CALL: > > case KVM_S390_INT_EMERGENCY: > > - default: > > kfree(inti); > > return -EINVAL; > > + default: > > + if (!is_ioint(s390int->type)) { > > + kfree(inti); > > + return -EINVAL; > > + } > > + if (s390int->type & IOINT_AI_MASK) > > + VM_EVENT(kvm, 5, "%s", "inject: I/O (AI)"); > > + else > > + VM_EVENT(kvm, 5, "inject: I/O css %x ss %x schid %04x", > > + s390int->type & IOINT_CSSID_MASK, > > + s390int->type & IOINT_SSID_MASK, > > + s390int->type & IOINT_SCHID_MASK); > > + inti->type = s390int->type; > > + inti->io.subchannel_id = s390int->parm >> 16; > > + inti->io.subchannel_nr = s390int->parm & 0x0000ffffu; > > + inti->io.io_int_parm = s390int->parm64 >> 32; > > + inti->io.io_int_word = s390int->parm64 & 0x00000000ffffffffull; > > + break; > > } > > trace_kvm_s390_inject_vm(s390int->type, s390int->parm, s390int->parm64, > > 2); > > @@ -579,7 +671,19 @@ int kvm_s390_inject_vm(struct kvm *kvm, > > mutex_lock(&kvm->lock); > > fi = &kvm->arch.float_int; > > spin_lock(&fi->lock); > > - list_add_tail(&inti->list, &fi->list); > > + if (!is_ioint(inti->type)) > > + list_add_tail(&inti->list, &fi->list); > > + else { > > + /* Keep I/O interrupts sorted in isc order. */ > > + list_for_each_entry(iter, &fi->list, list) { > > + if (!is_ioint(iter->type)) > > + continue; > > + if (iter->io.io_int_word <= inti->io.io_int_word) > > + continue; > > + break; > > + } > > + list_add_tail(&inti->list, &iter->list); > > + } > > atomic_set(&fi->active, 1); > > sigcpu = find_first_bit(fi->idle_mask, KVM_MAX_VCPUS); > > if (sigcpu == KVM_MAX_VCPUS) { > > @@ -654,6 +758,7 @@ int kvm_s390_inject_vcpu(struct kvm_vcpu *vcpu, > > case KVM_S390_INT_VIRTIO: > > case KVM_S390_INT_SERVICE: > > default: > > + /* also includes IOINT */ > > kfree(inti); > > return -EINVAL; > > } > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > > index 494a84c..e676940 100644 > > --- a/include/uapi/linux/kvm.h > > +++ b/include/uapi/linux/kvm.h > > @@ -401,6 +401,12 @@ struct kvm_s390_psw { > > #define KVM_S390_INT_SERVICE 0xffff2401u > > #define KVM_S390_INT_EMERGENCY 0xffff1201u > > #define KVM_S390_INT_EXTERNAL_CALL 0xffff1202u > > +#define KVM_S390_INT_IO(ai,cssid,ssid,schid) \ > > This one wants a comment saying that the namespace for everything not > starting with 0xfffe/0xffff is taken by INT_IO. Sounds reasonable. > > > Alex > > > + (((schid)) | \ > > + ((ssid) << 16) | \ > > + ((cssid) << 18) | \ > > + ((ai) << 26)) > > + > > > > struct kvm_s390_interrupt { > > __u32 type; > > -- > > 1.7.12.4 > > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
