| Message ID | 20180206141743.24497-1-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 6 Feb 2018 15:17:43 +0100 David Hildenbrand <david@redhat.com> wrote: > Just like for the interception handlers, let's also use a switch-case > in our interrupt delivery code. Amusingly, we did have a switch/case in the past :) Nice that we can get rid of the I/O interrupt special casing. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > arch/s390/kvm/interrupt.c | 84 ++++++++++++++++++++++++++++------------------- > 1 file changed, 50 insertions(+), 34 deletions(-) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
I see a minimal regression for uperf 1byte ping pong between two guests (~3%) Probably because the old code first handled IO interrupts and then did the remaing stuff. Not sure if its worth to keep the old io_ioirq hack. Other than that this looks good. On 02/06/2018 03:17 PM, David Hildenbrand wrote: > Just like for the interception handlers, let's also use a switch-case > in our interrupt delivery code. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > arch/s390/kvm/interrupt.c | 84 ++++++++++++++++++++++++++++------------------- > 1 file changed, 50 insertions(+), 34 deletions(-) > > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c > index aabf46f5f883..3ea9cfa31b16 100644 > --- a/arch/s390/kvm/interrupt.c > +++ b/arch/s390/kvm/interrupt.c > @@ -187,12 +187,6 @@ static int cpu_timer_irq_pending(struct kvm_vcpu *vcpu) > return kvm_s390_get_cpu_timer(vcpu) >> 63; > } > > -static inline int is_ioirq(unsigned long irq_type) > -{ > - return ((irq_type >= IRQ_PEND_IO_ISC_7) && > - (irq_type <= IRQ_PEND_IO_ISC_0)); > -} > - > static uint64_t isc_to_isc_bits(int isc) > { > return (0x80 >> isc) << 24; > @@ -1011,24 +1005,6 @@ static int __must_check __deliver_io(struct kvm_vcpu *vcpu, > return rc; > } > > -typedef int (*deliver_irq_t)(struct kvm_vcpu *vcpu); > - > -static const deliver_irq_t deliver_irq_funcs[] = { > - [IRQ_PEND_MCHK_EX] = __deliver_machine_check, > - [IRQ_PEND_MCHK_REP] = __deliver_machine_check, > - [IRQ_PEND_PROG] = __deliver_prog, > - [IRQ_PEND_EXT_EMERGENCY] = __deliver_emergency_signal, > - [IRQ_PEND_EXT_EXTERNAL] = __deliver_external_call, > - [IRQ_PEND_EXT_CLOCK_COMP] = __deliver_ckc, > - [IRQ_PEND_EXT_CPU_TIMER] = __deliver_cpu_timer, > - [IRQ_PEND_RESTART] = __deliver_restart, > - [IRQ_PEND_SET_PREFIX] = __deliver_set_prefix, > - [IRQ_PEND_PFAULT_INIT] = __deliver_pfault_init, > - [IRQ_PEND_EXT_SERVICE] = __deliver_service, > - [IRQ_PEND_PFAULT_DONE] = __deliver_pfault_done, > - [IRQ_PEND_VIRTIO] = __deliver_virtio, > -}; > - > /* Check whether an external call is pending (deliverable or not) */ > int kvm_s390_ext_call_pending(struct kvm_vcpu *vcpu) > { > @@ -1192,7 +1168,6 @@ void kvm_s390_clear_local_irqs(struct kvm_vcpu *vcpu) > int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) > { > struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int; > - deliver_irq_t func; > int rc = 0; > unsigned long irq_type; > unsigned long irqs; > @@ -1212,16 +1187,57 @@ int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) > while ((irqs = deliverable_irqs(vcpu)) && !rc) { > /* bits are in the reverse order of interrupt priority */ > irq_type = find_last_bit(&irqs, IRQ_PEND_COUNT); > - if (is_ioirq(irq_type)) { > + switch (irq_type) { > + case IRQ_PEND_IO_ISC_0: > + case IRQ_PEND_IO_ISC_1: > + case IRQ_PEND_IO_ISC_2: > + case IRQ_PEND_IO_ISC_3: > + case IRQ_PEND_IO_ISC_4: > + case IRQ_PEND_IO_ISC_5: > + case IRQ_PEND_IO_ISC_6: > + case IRQ_PEND_IO_ISC_7: > rc = __deliver_io(vcpu, irq_type); > - } else { > - func = deliver_irq_funcs[irq_type]; > - if (!func) { > - WARN_ON_ONCE(func == NULL); > - clear_bit(irq_type, &li->pending_irqs); > - continue; > - } > - rc = func(vcpu); > + break; > + case IRQ_PEND_MCHK_EX: > + case IRQ_PEND_MCHK_REP: > + rc = __deliver_machine_check(vcpu); > + break; > + case IRQ_PEND_PROG: > + rc = __deliver_prog(vcpu); > + break; > + case IRQ_PEND_EXT_EMERGENCY: > + rc = __deliver_emergency_signal(vcpu); > + break; > + case IRQ_PEND_EXT_EXTERNAL: > + rc = __deliver_external_call(vcpu); > + break; > + case IRQ_PEND_EXT_CLOCK_COMP: > + rc = __deliver_ckc(vcpu); > + break; > + case IRQ_PEND_EXT_CPU_TIMER: > + rc = __deliver_cpu_timer(vcpu); > + break; > + case IRQ_PEND_RESTART: > + rc = __deliver_restart(vcpu); > + break; > + case IRQ_PEND_SET_PREFIX: > + rc = __deliver_set_prefix(vcpu); > + break; > + case IRQ_PEND_PFAULT_INIT: > + rc = __deliver_pfault_init(vcpu); > + break; > + case IRQ_PEND_EXT_SERVICE: > + rc = __deliver_service(vcpu); > + break; > + case IRQ_PEND_PFAULT_DONE: > + rc = __deliver_pfault_done(vcpu); > + break; > + case IRQ_PEND_VIRTIO: > + rc = __deliver_virtio(vcpu); > + break; > + default: > + WARN_ONCE(1, "Unknown pending irq type %ld", irq_type); > + clear_bit(irq_type, &li->pending_irqs); > } > } >
On Wed, 7 Feb 2018 19:28:04 +0100 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > I see a minimal regression for uperf 1byte ping pong between two guests (~3%) > Probably because the old code first handled IO interrupts and then did the remaing > stuff. Not sure if its worth to keep the old io_ioirq hack. Hm, that confuses me a bit. We search the pending bit map, which should give us the irq with the highest priority, and the switch/case still starts out with I/O interrupts. > > Other than that this looks good. > > > On 02/06/2018 03:17 PM, David Hildenbrand wrote: > > Just like for the interception handlers, let's also use a switch-case > > in our interrupt delivery code. > > > > Signed-off-by: David Hildenbrand <david@redhat.com> > > --- > > arch/s390/kvm/interrupt.c | 84 ++++++++++++++++++++++++++++------------------- > > 1 file changed, 50 insertions(+), 34 deletions(-) > > > > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c > > index aabf46f5f883..3ea9cfa31b16 100644 > > --- a/arch/s390/kvm/interrupt.c > > +++ b/arch/s390/kvm/interrupt.c > > @@ -187,12 +187,6 @@ static int cpu_timer_irq_pending(struct kvm_vcpu *vcpu) > > return kvm_s390_get_cpu_timer(vcpu) >> 63; > > } > > > > -static inline int is_ioirq(unsigned long irq_type) > > -{ > > - return ((irq_type >= IRQ_PEND_IO_ISC_7) && > > - (irq_type <= IRQ_PEND_IO_ISC_0)); > > -} > > - > > static uint64_t isc_to_isc_bits(int isc) > > { > > return (0x80 >> isc) << 24; > > @@ -1011,24 +1005,6 @@ static int __must_check __deliver_io(struct kvm_vcpu *vcpu, > > return rc; > > } > > > > -typedef int (*deliver_irq_t)(struct kvm_vcpu *vcpu); > > - > > -static const deliver_irq_t deliver_irq_funcs[] = { > > - [IRQ_PEND_MCHK_EX] = __deliver_machine_check, > > - [IRQ_PEND_MCHK_REP] = __deliver_machine_check, > > - [IRQ_PEND_PROG] = __deliver_prog, > > - [IRQ_PEND_EXT_EMERGENCY] = __deliver_emergency_signal, > > - [IRQ_PEND_EXT_EXTERNAL] = __deliver_external_call, > > - [IRQ_PEND_EXT_CLOCK_COMP] = __deliver_ckc, > > - [IRQ_PEND_EXT_CPU_TIMER] = __deliver_cpu_timer, > > - [IRQ_PEND_RESTART] = __deliver_restart, > > - [IRQ_PEND_SET_PREFIX] = __deliver_set_prefix, > > - [IRQ_PEND_PFAULT_INIT] = __deliver_pfault_init, > > - [IRQ_PEND_EXT_SERVICE] = __deliver_service, > > - [IRQ_PEND_PFAULT_DONE] = __deliver_pfault_done, > > - [IRQ_PEND_VIRTIO] = __deliver_virtio, > > -}; > > - > > /* Check whether an external call is pending (deliverable or not) */ > > int kvm_s390_ext_call_pending(struct kvm_vcpu *vcpu) > > { > > @@ -1192,7 +1168,6 @@ void kvm_s390_clear_local_irqs(struct kvm_vcpu *vcpu) > > int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) > > { > > struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int; > > - deliver_irq_t func; > > int rc = 0; > > unsigned long irq_type; > > unsigned long irqs; > > @@ -1212,16 +1187,57 @@ int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) > > while ((irqs = deliverable_irqs(vcpu)) && !rc) { > > /* bits are in the reverse order of interrupt priority */ > > irq_type = find_last_bit(&irqs, IRQ_PEND_COUNT); > > - if (is_ioirq(irq_type)) { > > + switch (irq_type) { > > + case IRQ_PEND_IO_ISC_0: > > + case IRQ_PEND_IO_ISC_1: > > + case IRQ_PEND_IO_ISC_2: > > + case IRQ_PEND_IO_ISC_3: > > + case IRQ_PEND_IO_ISC_4: > > + case IRQ_PEND_IO_ISC_5: > > + case IRQ_PEND_IO_ISC_6: > > + case IRQ_PEND_IO_ISC_7: > > rc = __deliver_io(vcpu, irq_type); > > - } else { > > - func = deliver_irq_funcs[irq_type]; > > - if (!func) { > > - WARN_ON_ONCE(func == NULL); > > - clear_bit(irq_type, &li->pending_irqs); > > - continue; > > - } > > - rc = func(vcpu); > > + break; > > + case IRQ_PEND_MCHK_EX: > > + case IRQ_PEND_MCHK_REP: > > + rc = __deliver_machine_check(vcpu); > > + break; > > + case IRQ_PEND_PROG: > > + rc = __deliver_prog(vcpu); > > + break; > > + case IRQ_PEND_EXT_EMERGENCY: > > + rc = __deliver_emergency_signal(vcpu); > > + break; > > + case IRQ_PEND_EXT_EXTERNAL: > > + rc = __deliver_external_call(vcpu); > > + break; > > + case IRQ_PEND_EXT_CLOCK_COMP: > > + rc = __deliver_ckc(vcpu); > > + break; > > + case IRQ_PEND_EXT_CPU_TIMER: > > + rc = __deliver_cpu_timer(vcpu); > > + break; > > + case IRQ_PEND_RESTART: > > + rc = __deliver_restart(vcpu); > > + break; > > + case IRQ_PEND_SET_PREFIX: > > + rc = __deliver_set_prefix(vcpu); > > + break; > > + case IRQ_PEND_PFAULT_INIT: > > + rc = __deliver_pfault_init(vcpu); > > + break; > > + case IRQ_PEND_EXT_SERVICE: > > + rc = __deliver_service(vcpu); > > + break; > > + case IRQ_PEND_PFAULT_DONE: > > + rc = __deliver_pfault_done(vcpu); > > + break; > > + case IRQ_PEND_VIRTIO: > > + rc = __deliver_virtio(vcpu); > > + break; > > + default: > > + WARN_ONCE(1, "Unknown pending irq type %ld", irq_type); > > + clear_bit(irq_type, &li->pending_irqs); > > } > > } > > >
On 06.02.2018 15:17, David Hildenbrand wrote: > Just like for the interception handlers, let's also use a switch-case > in our interrupt delivery code. > > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> > --- > arch/s390/kvm/interrupt.c | 84 ++++++++++++++++++++++++++++------------------- > 1 file changed, 50 insertions(+), 34 deletions(-) > > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c > index aabf46f5f883..3ea9cfa31b16 100644 > --- a/arch/s390/kvm/interrupt.c > +++ b/arch/s390/kvm/interrupt.c > @@ -187,12 +187,6 @@ static int cpu_timer_irq_pending(struct kvm_vcpu *vcpu) > return kvm_s390_get_cpu_timer(vcpu) >> 63; > } > > -static inline int is_ioirq(unsigned long irq_type) > -{ > - return ((irq_type >= IRQ_PEND_IO_ISC_7) && > - (irq_type <= IRQ_PEND_IO_ISC_0)); > -} > - > static uint64_t isc_to_isc_bits(int isc) > { > return (0x80 >> isc) << 24; > @@ -1011,24 +1005,6 @@ static int __must_check __deliver_io(struct kvm_vcpu *vcpu, > return rc; > } > > -typedef int (*deliver_irq_t)(struct kvm_vcpu *vcpu); > - > -static const deliver_irq_t deliver_irq_funcs[] = { > - [IRQ_PEND_MCHK_EX] = __deliver_machine_check, > - [IRQ_PEND_MCHK_REP] = __deliver_machine_check, > - [IRQ_PEND_PROG] = __deliver_prog, > - [IRQ_PEND_EXT_EMERGENCY] = __deliver_emergency_signal, > - [IRQ_PEND_EXT_EXTERNAL] = __deliver_external_call, > - [IRQ_PEND_EXT_CLOCK_COMP] = __deliver_ckc, > - [IRQ_PEND_EXT_CPU_TIMER] = __deliver_cpu_timer, > - [IRQ_PEND_RESTART] = __deliver_restart, > - [IRQ_PEND_SET_PREFIX] = __deliver_set_prefix, > - [IRQ_PEND_PFAULT_INIT] = __deliver_pfault_init, > - [IRQ_PEND_EXT_SERVICE] = __deliver_service, > - [IRQ_PEND_PFAULT_DONE] = __deliver_pfault_done, > - [IRQ_PEND_VIRTIO] = __deliver_virtio, > -}; > - > /* Check whether an external call is pending (deliverable or not) */ > int kvm_s390_ext_call_pending(struct kvm_vcpu *vcpu) > { > @@ -1192,7 +1168,6 @@ void kvm_s390_clear_local_irqs(struct kvm_vcpu *vcpu) > int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) > { > struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int; > - deliver_irq_t func; > int rc = 0; > unsigned long irq_type; > unsigned long irqs; > @@ -1212,16 +1187,57 @@ int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) > while ((irqs = deliverable_irqs(vcpu)) && !rc) { > /* bits are in the reverse order of interrupt priority */ > irq_type = find_last_bit(&irqs, IRQ_PEND_COUNT); > - if (is_ioirq(irq_type)) { > + switch (irq_type) { > + case IRQ_PEND_IO_ISC_0: > + case IRQ_PEND_IO_ISC_1: > + case IRQ_PEND_IO_ISC_2: > + case IRQ_PEND_IO_ISC_3: > + case IRQ_PEND_IO_ISC_4: > + case IRQ_PEND_IO_ISC_5: > + case IRQ_PEND_IO_ISC_6: > + case IRQ_PEND_IO_ISC_7: > rc = __deliver_io(vcpu, irq_type); > - } else { > - func = deliver_irq_funcs[irq_type]; > - if (!func) { > - WARN_ON_ONCE(func == NULL); > - clear_bit(irq_type, &li->pending_irqs); > - continue; > - } > - rc = func(vcpu); > + break; > + case IRQ_PEND_MCHK_EX: > + case IRQ_PEND_MCHK_REP: > + rc = __deliver_machine_check(vcpu); > + break; > + case IRQ_PEND_PROG: > + rc = __deliver_prog(vcpu); > + break; > + case IRQ_PEND_EXT_EMERGENCY: > + rc = __deliver_emergency_signal(vcpu); > + break; > + case IRQ_PEND_EXT_EXTERNAL: > + rc = __deliver_external_call(vcpu); > + break; > + case IRQ_PEND_EXT_CLOCK_COMP: > + rc = __deliver_ckc(vcpu); > + break; > + case IRQ_PEND_EXT_CPU_TIMER: > + rc = __deliver_cpu_timer(vcpu); > + break; > + case IRQ_PEND_RESTART: > + rc = __deliver_restart(vcpu); > + break; > + case IRQ_PEND_SET_PREFIX: > + rc = __deliver_set_prefix(vcpu); > + break; > + case IRQ_PEND_PFAULT_INIT: > + rc = __deliver_pfault_init(vcpu); > + break; > + case IRQ_PEND_EXT_SERVICE: > + rc = __deliver_service(vcpu); > + break; > + case IRQ_PEND_PFAULT_DONE: > + rc = __deliver_pfault_done(vcpu); > + break; > + case IRQ_PEND_VIRTIO: > + rc = __deliver_virtio(vcpu); > + break; > + default: > + WARN_ONCE(1, "Unknown pending irq type %ld", irq_type); > + clear_bit(irq_type, &li->pending_irqs); > } > } >
On 02/08/2018 09:14 AM, Cornelia Huck wrote: > On Wed, 7 Feb 2018 19:28:04 +0100 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > >> I see a minimal regression for uperf 1byte ping pong between two guests (~3%) >> Probably because the old code first handled IO interrupts and then did the remaing >> stuff. Not sure if its worth to keep the old io_ioirq hack. > > Hm, that confuses me a bit. We search the pending bit map, which should > give us the irq with the highest priority, and the switch/case still > starts out with I/O interrupts. gcc does not obey the order of the case statements. It uses several heuristics depending on the size and others. So gcc might fall back to jump tables for large switches, or it uses bisecting or it might even split the search into a jump table and several relative branches if there are strange distributions. Quite often the default case is evaulated first.
On Thu, 8 Feb 2018 10:53:00 +0100 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 02/08/2018 09:14 AM, Cornelia Huck wrote: > > On Wed, 7 Feb 2018 19:28:04 +0100 > > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > > > >> I see a minimal regression for uperf 1byte ping pong between two guests (~3%) > >> Probably because the old code first handled IO interrupts and then did the remaing > >> stuff. Not sure if its worth to keep the old io_ioirq hack. > > > > Hm, that confuses me a bit. We search the pending bit map, which should > > give us the irq with the highest priority, and the switch/case still > > starts out with I/O interrupts. > > gcc does not obey the order of the case statements. It uses several heuristics depending > on the size and others. So gcc might fall back to jump tables for large switches, or > it uses bisecting or it might even split the search into a jump table and several > relative branches if there are strange distributions. Quite often the default > case is evaulated first. But should we really try to optimize something that may change with a different compiler anyway? The important thing is the priority in the bitmap.
On 02/08/2018 11:00 AM, Cornelia Huck wrote: > On Thu, 8 Feb 2018 10:53:00 +0100 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > >> On 02/08/2018 09:14 AM, Cornelia Huck wrote: >>> On Wed, 7 Feb 2018 19:28:04 +0100 >>> Christian Borntraeger <borntraeger@de.ibm.com> wrote: >>> >>>> I see a minimal regression for uperf 1byte ping pong between two guests (~3%) >>>> Probably because the old code first handled IO interrupts and then did the remaing >>>> stuff. Not sure if its worth to keep the old io_ioirq hack. >>> >>> Hm, that confuses me a bit. We search the pending bit map, which should >>> give us the irq with the highest priority, and the switch/case still >>> starts out with I/O interrupts. >> >> gcc does not obey the order of the case statements. It uses several heuristics depending >> on the size and others. So gcc might fall back to jump tables for large switches, or >> it uses bisecting or it might even split the search into a jump table and several >> relative branches if there are strange distributions. Quite often the default >> case is evaulated first. > > But should we really try to optimize something that may change with a > different compiler anyway? The important thing is the priority in the > bitmap. An if before the switch would always prefer that condition. But I agree,we should probably not go down this path of micro optimization.
On 02/06/2018 03:17 PM, David Hildenbrand wrote: > Just like for the interception handlers, let's also use a switch-case > in our interrupt delivery code. > > Signed-off-by: David Hildenbrand <david@redhat.com> Thanks applied. > --- > arch/s390/kvm/interrupt.c | 84 ++++++++++++++++++++++++++++------------------- > 1 file changed, 50 insertions(+), 34 deletions(-) > > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c > index aabf46f5f883..3ea9cfa31b16 100644 > --- a/arch/s390/kvm/interrupt.c > +++ b/arch/s390/kvm/interrupt.c > @@ -187,12 +187,6 @@ static int cpu_timer_irq_pending(struct kvm_vcpu *vcpu) > return kvm_s390_get_cpu_timer(vcpu) >> 63; > } > > -static inline int is_ioirq(unsigned long irq_type) > -{ > - return ((irq_type >= IRQ_PEND_IO_ISC_7) && > - (irq_type <= IRQ_PEND_IO_ISC_0)); > -} > - > static uint64_t isc_to_isc_bits(int isc) > { > return (0x80 >> isc) << 24; > @@ -1011,24 +1005,6 @@ static int __must_check __deliver_io(struct kvm_vcpu *vcpu, > return rc; > } > > -typedef int (*deliver_irq_t)(struct kvm_vcpu *vcpu); > - > -static const deliver_irq_t deliver_irq_funcs[] = { > - [IRQ_PEND_MCHK_EX] = __deliver_machine_check, > - [IRQ_PEND_MCHK_REP] = __deliver_machine_check, > - [IRQ_PEND_PROG] = __deliver_prog, > - [IRQ_PEND_EXT_EMERGENCY] = __deliver_emergency_signal, > - [IRQ_PEND_EXT_EXTERNAL] = __deliver_external_call, > - [IRQ_PEND_EXT_CLOCK_COMP] = __deliver_ckc, > - [IRQ_PEND_EXT_CPU_TIMER] = __deliver_cpu_timer, > - [IRQ_PEND_RESTART] = __deliver_restart, > - [IRQ_PEND_SET_PREFIX] = __deliver_set_prefix, > - [IRQ_PEND_PFAULT_INIT] = __deliver_pfault_init, > - [IRQ_PEND_EXT_SERVICE] = __deliver_service, > - [IRQ_PEND_PFAULT_DONE] = __deliver_pfault_done, > - [IRQ_PEND_VIRTIO] = __deliver_virtio, > -}; > - > /* Check whether an external call is pending (deliverable or not) */ > int kvm_s390_ext_call_pending(struct kvm_vcpu *vcpu) > { > @@ -1192,7 +1168,6 @@ void kvm_s390_clear_local_irqs(struct kvm_vcpu *vcpu) > int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) > { > struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int; > - deliver_irq_t func; > int rc = 0; > unsigned long irq_type; > unsigned long irqs; > @@ -1212,16 +1187,57 @@ int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) > while ((irqs = deliverable_irqs(vcpu)) && !rc) { > /* bits are in the reverse order of interrupt priority */ > irq_type = find_last_bit(&irqs, IRQ_PEND_COUNT); > - if (is_ioirq(irq_type)) { > + switch (irq_type) { > + case IRQ_PEND_IO_ISC_0: > + case IRQ_PEND_IO_ISC_1: > + case IRQ_PEND_IO_ISC_2: > + case IRQ_PEND_IO_ISC_3: > + case IRQ_PEND_IO_ISC_4: > + case IRQ_PEND_IO_ISC_5: > + case IRQ_PEND_IO_ISC_6: > + case IRQ_PEND_IO_ISC_7: > rc = __deliver_io(vcpu, irq_type); > - } else { > - func = deliver_irq_funcs[irq_type]; > - if (!func) { > - WARN_ON_ONCE(func == NULL); > - clear_bit(irq_type, &li->pending_irqs); > - continue; > - } > - rc = func(vcpu); > + break; > + case IRQ_PEND_MCHK_EX: > + case IRQ_PEND_MCHK_REP: > + rc = __deliver_machine_check(vcpu); > + break; > + case IRQ_PEND_PROG: > + rc = __deliver_prog(vcpu); > + break; > + case IRQ_PEND_EXT_EMERGENCY: > + rc = __deliver_emergency_signal(vcpu); > + break; > + case IRQ_PEND_EXT_EXTERNAL: > + rc = __deliver_external_call(vcpu); > + break; > + case IRQ_PEND_EXT_CLOCK_COMP: > + rc = __deliver_ckc(vcpu); > + break; > + case IRQ_PEND_EXT_CPU_TIMER: > + rc = __deliver_cpu_timer(vcpu); > + break; > + case IRQ_PEND_RESTART: > + rc = __deliver_restart(vcpu); > + break; > + case IRQ_PEND_SET_PREFIX: > + rc = __deliver_set_prefix(vcpu); > + break; > + case IRQ_PEND_PFAULT_INIT: > + rc = __deliver_pfault_init(vcpu); > + break; > + case IRQ_PEND_EXT_SERVICE: > + rc = __deliver_service(vcpu); > + break; > + case IRQ_PEND_PFAULT_DONE: > + rc = __deliver_pfault_done(vcpu); > + break; > + case IRQ_PEND_VIRTIO: > + rc = __deliver_virtio(vcpu); > + break; > + default: > + WARN_ONCE(1, "Unknown pending irq type %ld", irq_type); > + clear_bit(irq_type, &li->pending_irqs); > } > } >
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c index aabf46f5f883..3ea9cfa31b16 100644 --- a/arch/s390/kvm/interrupt.c +++ b/arch/s390/kvm/interrupt.c @@ -187,12 +187,6 @@ static int cpu_timer_irq_pending(struct kvm_vcpu *vcpu) return kvm_s390_get_cpu_timer(vcpu) >> 63; } -static inline int is_ioirq(unsigned long irq_type) -{ - return ((irq_type >= IRQ_PEND_IO_ISC_7) && - (irq_type <= IRQ_PEND_IO_ISC_0)); -} - static uint64_t isc_to_isc_bits(int isc) { return (0x80 >> isc) << 24; @@ -1011,24 +1005,6 @@ static int __must_check __deliver_io(struct kvm_vcpu *vcpu, return rc; } -typedef int (*deliver_irq_t)(struct kvm_vcpu *vcpu); - -static const deliver_irq_t deliver_irq_funcs[] = { - [IRQ_PEND_MCHK_EX] = __deliver_machine_check, - [IRQ_PEND_MCHK_REP] = __deliver_machine_check, - [IRQ_PEND_PROG] = __deliver_prog, - [IRQ_PEND_EXT_EMERGENCY] = __deliver_emergency_signal, - [IRQ_PEND_EXT_EXTERNAL] = __deliver_external_call, - [IRQ_PEND_EXT_CLOCK_COMP] = __deliver_ckc, - [IRQ_PEND_EXT_CPU_TIMER] = __deliver_cpu_timer, - [IRQ_PEND_RESTART] = __deliver_restart, - [IRQ_PEND_SET_PREFIX] = __deliver_set_prefix, - [IRQ_PEND_PFAULT_INIT] = __deliver_pfault_init, - [IRQ_PEND_EXT_SERVICE] = __deliver_service, - [IRQ_PEND_PFAULT_DONE] = __deliver_pfault_done, - [IRQ_PEND_VIRTIO] = __deliver_virtio, -}; - /* Check whether an external call is pending (deliverable or not) */ int kvm_s390_ext_call_pending(struct kvm_vcpu *vcpu) { @@ -1192,7 +1168,6 @@ void kvm_s390_clear_local_irqs(struct kvm_vcpu *vcpu) int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) { struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int; - deliver_irq_t func; int rc = 0; unsigned long irq_type; unsigned long irqs; @@ -1212,16 +1187,57 @@ int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) while ((irqs = deliverable_irqs(vcpu)) && !rc) { /* bits are in the reverse order of interrupt priority */ irq_type = find_last_bit(&irqs, IRQ_PEND_COUNT); - if (is_ioirq(irq_type)) { + switch (irq_type) { + case IRQ_PEND_IO_ISC_0: + case IRQ_PEND_IO_ISC_1: + case IRQ_PEND_IO_ISC_2: + case IRQ_PEND_IO_ISC_3: + case IRQ_PEND_IO_ISC_4: + case IRQ_PEND_IO_ISC_5: + case IRQ_PEND_IO_ISC_6: + case IRQ_PEND_IO_ISC_7: rc = __deliver_io(vcpu, irq_type); - } else { - func = deliver_irq_funcs[irq_type]; - if (!func) { - WARN_ON_ONCE(func == NULL); - clear_bit(irq_type, &li->pending_irqs); - continue; - } - rc = func(vcpu); + break; + case IRQ_PEND_MCHK_EX: + case IRQ_PEND_MCHK_REP: + rc = __deliver_machine_check(vcpu); + break; + case IRQ_PEND_PROG: + rc = __deliver_prog(vcpu); + break; + case IRQ_PEND_EXT_EMERGENCY: + rc = __deliver_emergency_signal(vcpu); + break; + case IRQ_PEND_EXT_EXTERNAL: + rc = __deliver_external_call(vcpu); + break; + case IRQ_PEND_EXT_CLOCK_COMP: + rc = __deliver_ckc(vcpu); + break; + case IRQ_PEND_EXT_CPU_TIMER: + rc = __deliver_cpu_timer(vcpu); + break; + case IRQ_PEND_RESTART: + rc = __deliver_restart(vcpu); + break; + case IRQ_PEND_SET_PREFIX: + rc = __deliver_set_prefix(vcpu); + break; + case IRQ_PEND_PFAULT_INIT: + rc = __deliver_pfault_init(vcpu); + break; + case IRQ_PEND_EXT_SERVICE: + rc = __deliver_service(vcpu); + break; + case IRQ_PEND_PFAULT_DONE: + rc = __deliver_pfault_done(vcpu); + break; + case IRQ_PEND_VIRTIO: + rc = __deliver_virtio(vcpu); + break; + default: + WARN_ONCE(1, "Unknown pending irq type %ld", irq_type); + clear_bit(irq_type, &li->pending_irqs); } }
Just like for the interception handlers, let's also use a switch-case in our interrupt delivery code. Signed-off-by: David Hildenbrand <david@redhat.com> --- arch/s390/kvm/interrupt.c | 84 ++++++++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 34 deletions(-)