Re: [PATCH v5 14/15] KVM: s390: add and wire function gib_alert_irq_handler()

[Michael Mueller]

On 03.01.19 16:16, Pierre Morel wrote:
> On 19/12/2018 20:17, Michael Mueller wrote:
> > The patch implements a handler for GIB alert interruptions
> > on the host. Its task is to alert guests that interrupts are
> > pending for them.
> >
> > A GIB alert interrupt statistic counter is added as well:
> >
> > $ cat /proc/interrupts
> > ÂÂÂÂÂÂÂÂÂÂ CPU0ÂÂÂÂÂÂ CPU1
> > ÂÂ ...
> > ÂÂ GAL:ÂÂÂÂÂ 23ÂÂÂÂÂÂÂÂ 37ÂÂ [I/O] GIB Alert
> > ÂÂ ...
> >
> > Signed-off-by: Michael Mueller <mimu@xxxxxxxxxxxxx>
> > ---
> > Â arch/s390/include/asm/irq.h |Â 1 +
> > Â arch/s390/include/asm/isc.h |Â 1 +
> > Â arch/s390/kernel/irq.cÂÂÂÂÂ |Â 1 +
> > Â arch/s390/kvm/interrupt.cÂÂ | 36 ++++++++++++++++++++++++++++++++++--
> > Â 4 files changed, 37 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/s390/include/asm/irq.h b/arch/s390/include/asm/irq.h
> > index 2f7f27e5493f..afaf5e3c57fd 100644
> > --- a/arch/s390/include/asm/irq.h
> > +++ b/arch/s390/include/asm/irq.h
> > @@ -62,6 +62,7 @@ enum interruption_class {
> > ÂÂÂÂÂ IRQIO_MSI,
> > ÂÂÂÂÂ IRQIO_VIR,
> > ÂÂÂÂÂ IRQIO_VAI,
> > +ÂÂÂ IRQIO_GAL,
> > ÂÂÂÂÂ NMI_NMI,
> > ÂÂÂÂÂ CPU_RST,
> > ÂÂÂÂÂ NR_ARCH_IRQS
> > diff --git a/arch/s390/include/asm/isc.h b/arch/s390/include/asm/isc.h
> > index 6cb9e2ed05b6..b2cc1ec78d06 100644
> > --- a/arch/s390/include/asm/isc.h
> > +++ b/arch/s390/include/asm/isc.h
> > @@ -21,6 +21,7 @@
> > Â /* Adapter interrupts. */
> > Â #define QDIO_AIRQ_ISC IO_SCH_ISCÂÂÂ /* I/O subchannel in qdio mode */
> > Â #define PCI_ISC 2ÂÂÂÂÂÂÂÂÂÂÂ /* PCI I/O subchannels */
> > +#define GAL_ISC 5ÂÂÂÂÂÂÂÂÂÂÂ /* GIB alert */
> > Â #define AP_ISC 6ÂÂÂÂÂÂÂÂÂÂÂ /* adjunct processor (crypto) devices */
> > Â /* Functions for registration of I/O interruption subclasses */
> > diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
> > index 0e8d68bac82c..0cd5a5f96729 100644
> > --- a/arch/s390/kernel/irq.c
> > +++ b/arch/s390/kernel/irq.c
> > @@ -88,6 +88,7 @@ static const struct irq_class irqclass_sub_desc[] = {
> >  {.irq = IRQIO_MSI, .name = "MSI", .desc = "[I/O] MSI Interrupt" },
> >  {.irq = IRQIO_VIR, .name = "VIR", .desc = "[I/O] Virtual I/O 
> > Devices"},
> >  {.irq = IRQIO_VAI, .name = "VAI", .desc = "[I/O] Virtual I/O 
> > Devices AI"},
> > + {.irq = IRQIO_GAL, .name = "GAL", .desc = "[I/O] GIB Alert"},
> > ÂÂÂÂÂ {.irq = NMI_NMI,ÂÂÂ .name = "NMI", .desc = "[NMI] Machine Check"},
> > ÂÂÂÂÂ {.irq = CPU_RST,ÂÂÂ .name = "RST", .desc = "[CPU] CPU Restart"},
> > Â };
> > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> > index 03e7ba4f215a..79b9c262479b 100644
> > --- a/arch/s390/kvm/interrupt.c
> > +++ b/arch/s390/kvm/interrupt.c
> > @@ -26,6 +26,7 @@
> > Â #include <asm/gmap.h>
> > Â #include <asm/switch_to.h>
> > Â #include <asm/nmi.h>
> > +#include <asm/airq.h>
> > Â #include "kvm-s390.h"
> > Â #include "gaccess.h"
> > Â #include "trace-s390.h"
> > @@ -3043,7 +3044,7 @@ static void __floating_airqs_kick(struct kvm *kvm)
> > Â #define NONE_GISA_ADDR 0x00000001UL
> > Â #define GISA_ADDR_MASK 0xfffff000UL
> > -static void __maybe_unused process_gib_alert_list(void)
> > +static void process_gib_alert_list(void)
> > Â {
> > ÂÂÂÂÂ u32 final, next_alert, origin = 0UL;
> > ÂÂÂÂÂ struct kvm_s390_gisa *gisa;
> > @@ -3091,7 +3092,10 @@ void kvm_s390_gisa_clear(struct kvm *kvm)
> > Â {
> > ÂÂÂÂÂ if (!kvm->arch.gisa)
> > ÂÂÂÂÂÂÂÂÂ return;
> > +ÂÂÂ if (set_iam(kvm->arch.gisa, 0) == -EBUSY)
> > +ÂÂÂÂÂÂÂ process_gib_alert_list();
>
> We call process_gib_alert_list() from different contexts shouldn't we 
> protect the calls?

That should not be necessary as the xcgh() guarantees that both
instances will work on gib alert lists with disjunctive gisas.

> > ÂÂÂÂÂ nullify_gisa(kvm->arch.gisa);
> > +ÂÂÂ set_iam(kvm->arch.gisa, kvm->arch.iam);
> > ÂÂÂÂÂ VM_EVENT(kvm, 3, "gisa 0x%pK cleared", kvm->arch.gisa);
> > Â }
> > @@ -3111,6 +3115,8 @@ void kvm_s390_gisa_destroy(struct kvm *kvm)
> > Â {
> > ÂÂÂÂÂ if (!kvm->arch.gisa)
> > ÂÂÂÂÂÂÂÂÂ return;
> > +ÂÂÂ if (set_iam(kvm->arch.gisa, 0) == -EBUSY)
> > +ÂÂÂÂÂÂÂ process_gib_alert_list();
>
> idem.
>
> > ÂÂÂÂÂ kvm->arch.gisa = NULL;
> > Â }
> > @@ -3159,11 +3165,23 @@ int kvm_s390_gisc_unregister(struct kvm *kvm, 
> > u32 gisc)
> > Â }
> > Â EXPORT_SYMBOL_GPL(kvm_s390_gisc_unregister);
> > +static void gib_alert_irq_handler(struct airq_struct *airq)
> > +{
> > +ÂÂÂ inc_irq_stat(IRQIO_GAL);
> > +ÂÂÂ process_gib_alert_list();
>
> idem.
>
> > +}
> > +
> > +static struct airq_struct gib_alert_irq = {
> > +ÂÂÂ .handler = gib_alert_irq_handler,
> > +ÂÂÂ .lsi_ptr = &gib_alert_irq.lsi_mask,
> > +};
> > +

>