diff mbox series

[15/32] KVM: s390: pci: enable host forwarding of Adapter Event Notifications

Message ID 20211207205743.150299-16-mjrosato@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series KVM: s390: enable zPCI for interpretive execution | expand

Commit Message

Matthew Rosato Dec. 7, 2021, 8:57 p.m. UTC
In cases where interrupts are not forwarded to the guest via firmware,
KVM is responsible for ensuring delivery.  When an interrupt presents
with the forwarding bit, we must process the forwarding tables until
all interrupts are delivered.

Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |  1 +
 arch/s390/include/asm/tpi.h      | 14 ++++++
 arch/s390/kvm/interrupt.c        | 76 +++++++++++++++++++++++++++++++-
 arch/s390/kvm/kvm-s390.c         |  3 +-
 arch/s390/kvm/pci.h              |  9 ++++
 5 files changed, 101 insertions(+), 2 deletions(-)

Comments

Eric Farman Dec. 10, 2021, 9:51 p.m. UTC | #1
On Tue, 2021-12-07 at 15:57 -0500, Matthew Rosato wrote:
> In cases where interrupts are not forwarded to the guest via
> firmware,
> KVM is responsible for ensuring delivery.  When an interrupt presents
> with the forwarding bit, we must process the forwarding tables until
> all interrupts are delivered.
> 
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>

Nits below regarding 0-vs-NULL, but otherwise:

Reviewed-by: Eric Farman <farman@linux.ibm.com>

> ---
>  arch/s390/include/asm/kvm_host.h |  1 +
>  arch/s390/include/asm/tpi.h      | 14 ++++++
>  arch/s390/kvm/interrupt.c        | 76
> +++++++++++++++++++++++++++++++-
>  arch/s390/kvm/kvm-s390.c         |  3 +-
>  arch/s390/kvm/pci.h              |  9 ++++
>  5 files changed, 101 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h
> b/arch/s390/include/asm/kvm_host.h
> index a604d51acfc8..3f147b8d050b 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -757,6 +757,7 @@ struct kvm_vm_stat {
>  	u64 inject_pfault_done;
>  	u64 inject_service_signal;
>  	u64 inject_virtio;
> +	u64 aen_forward;
>  };
>  
>  struct kvm_arch_memory_slot {
> diff --git a/arch/s390/include/asm/tpi.h
> b/arch/s390/include/asm/tpi.h
> index 1ac538b8cbf5..47a531fdb15b 100644
> --- a/arch/s390/include/asm/tpi.h
> +++ b/arch/s390/include/asm/tpi.h
> @@ -19,6 +19,20 @@ struct tpi_info {
>  	u32 :12;
>  } __packed __aligned(4);
>  
> +/* I/O-Interruption Code as stored by TPI for an Adapter I/O */
> +struct tpi_adapter_info {
> +	u32 :1;
> +	u32 pci:1;
> +	u32 :28;
> +	u32 error:1;
> +	u32 forward:1;
> +	u32 reserved;
> +	u32 adapter_IO:1;
> +	u32 directed_irq:1;
> +	u32 isc:3;
> +	u32 :27;
> +} __packed __aligned(4);
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* _ASM_S390_TPI_H */
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 4efe0e95a40f..c6ff871a6ed1 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -3263,11 +3263,85 @@ int kvm_s390_gisc_unregister(struct kvm *kvm,
> u32 gisc)
>  }
>  EXPORT_SYMBOL_GPL(kvm_s390_gisc_unregister);
>  
> +static void aen_host_forward(struct zpci_aift *aift, unsigned long
> si)
> +{
> +	struct kvm_s390_gisa_interrupt *gi;
> +	struct zpci_gaite *gaite;
> +	struct kvm *kvm;
> +
> +	gaite = (struct zpci_gaite *)aift->gait +
> +		(si * sizeof(struct zpci_gaite));
> +	if (gaite->count == 0)
> +		return;
> +	if (gaite->aisb != 0)
> +		set_bit_inv(gaite->aisbo, (unsigned long *)gaite-
> >aisb);
> +
> +	kvm = kvm_s390_pci_si_to_kvm(aift, si);
> +	if (kvm == 0)

if (!kvm)

> +		return;
> +	gi = &kvm->arch.gisa_int;
> +
> +	if (!(gi->origin->g1.simm & AIS_MODE_MASK(gaite->gisc)) ||
> +	    !(gi->origin->g1.nimm & AIS_MODE_MASK(gaite->gisc))) {
> +		gisa_set_ipm_gisc(gi->origin, gaite->gisc);
> +		if (hrtimer_active(&gi->timer))
> +			hrtimer_cancel(&gi->timer);
> +		hrtimer_start(&gi->timer, 0, HRTIMER_MODE_REL);
> +		kvm->stat.aen_forward++;
> +	}
> +}
> +
> +static void aen_process_gait(u8 isc)
> +{
> +	bool found = false, first = true;
> +	union zpci_sic_iib iib = {{0}};
> +	unsigned long si, flags;
> +	struct zpci_aift *aift;
> +
> +	aift = kvm_s390_pci_get_aift();
> +	spin_lock_irqsave(&aift->gait_lock, flags);
> +
> +	if (!aift->gait) {
> +		spin_unlock_irqrestore(&aift->gait_lock, flags);
> +		return;
> +	}
> +
> +	for (si = 0;;) {
> +		/* Scan adapter summary indicator bit vector */
> +		si = airq_iv_scan(aift->sbv, si, airq_iv_end(aift-
> >sbv));
> +		if (si == -1UL) {
> +			if (first || found) {
> +				/* Reenable interrupts. */
> +				if
> (zpci_set_irq_ctrl(SIC_IRQ_MODE_SINGLE, isc,
> +						      &iib))
> +					break;
> +				first = found = false;
> +			} else {
> +				/* Interrupts on and all bits processed
> */
> +				break;
> +			}
> +			found = false;
> +			si = 0;
> +			continue;
> +		}
> +		found = true;
> +		aen_host_forward(aift, si);
> +	}
> +
> +	spin_unlock_irqrestore(&aift->gait_lock, flags);
> +}
> +
>  static void gib_alert_irq_handler(struct airq_struct *airq,
>  				  struct tpi_info *tpi_info)
>  {
> +	struct tpi_adapter_info *info = (struct tpi_adapter_info
> *)tpi_info;
> +
>  	inc_irq_stat(IRQIO_GAL);
> -	process_gib_alert_list();
> +
> +	if (info->forward || info->error)
> +		aen_process_gait(info->isc);
> +	else
> +		process_gib_alert_list();
>  }
>  
>  static struct airq_struct gib_alert_irq = {
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 9cd3c8eb59e8..c8fe9b7c2395 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -65,7 +65,8 @@ const struct _kvm_stats_desc kvm_vm_stats_desc[] =
> {
>  	STATS_DESC_COUNTER(VM, inject_float_mchk),
>  	STATS_DESC_COUNTER(VM, inject_pfault_done),
>  	STATS_DESC_COUNTER(VM, inject_service_signal),
> -	STATS_DESC_COUNTER(VM, inject_virtio)
> +	STATS_DESC_COUNTER(VM, inject_virtio),
> +	STATS_DESC_COUNTER(VM, aen_forward)
>  };
>  
>  const struct kvm_stats_header kvm_vm_stats_header = {
> diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
> index 74b06d39be3b..776b2745c675 100644
> --- a/arch/s390/kvm/pci.h
> +++ b/arch/s390/kvm/pci.h
> @@ -12,6 +12,7 @@
>  
>  #include <linux/pci.h>
>  #include <linux/mutex.h>
> +#include <linux/kvm_host.h>
>  #include <asm/airq.h>
>  #include <asm/kvm_pci.h>
>  
> @@ -32,6 +33,14 @@ struct zpci_aift {
>  	struct mutex lock; /* Protects the other structures in aift */
>  };
>  
> +static inline struct kvm *kvm_s390_pci_si_to_kvm(struct zpci_aift
> *aift,
> +						 unsigned long si)
> +{
> +	if (aift->kzdev == 0 || aift->kzdev[si] == 0)
> +		return 0;

Check/return NULL

> +	return aift->kzdev[si]->kvm;
> +};
> +
>  struct zpci_aift *kvm_s390_pci_get_aift(void);
>  
>  int kvm_s390_pci_aen_init(u8 nisc);
Christian Borntraeger Dec. 17, 2021, 4:56 p.m. UTC | #2
Am 07.12.21 um 21:57 schrieb Matthew Rosato:
> In cases where interrupts are not forwarded to the guest via firmware,
> KVM is responsible for ensuring delivery.  When an interrupt presents
> with the forwarding bit, we must process the forwarding tables until
> all interrupts are delivered.
> 
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---
[...]

> +static void aen_host_forward(struct zpci_aift *aift, unsigned long si)
> +{
> +	struct kvm_s390_gisa_interrupt *gi;
> +	struct zpci_gaite *gaite;
> +	struct kvm *kvm;
> +
> +	gaite = (struct zpci_gaite *)aift->gait +
> +		(si * sizeof(struct zpci_gaite));
> +	if (gaite->count == 0)
> +		return;
> +	if (gaite->aisb != 0)
> +		set_bit_inv(gaite->aisbo, (unsigned long *)gaite->aisb);
> +
> +	kvm = kvm_s390_pci_si_to_kvm(aift, si);
> +	if (kvm == 0)
> +		return;
> +	gi = &kvm->arch.gisa_int;
> +
> +	if (!(gi->origin->g1.simm & AIS_MODE_MASK(gaite->gisc)) ||
> +	    !(gi->origin->g1.nimm & AIS_MODE_MASK(gaite->gisc))) {
> +		gisa_set_ipm_gisc(gi->origin, gaite->gisc);
> +		if (hrtimer_active(&gi->timer))
> +			hrtimer_cancel(&gi->timer);
> +		hrtimer_start(&gi->timer, 0, HRTIMER_MODE_REL);
> +		kvm->stat.aen_forward++;
> +	}
> +}
> +
> +static void aen_process_gait(u8 isc)
> +{
> +	bool found = false, first = true;
> +	union zpci_sic_iib iib = {{0}};
> +	unsigned long si, flags;
> +	struct zpci_aift *aift;
> +
> +	aift = kvm_s390_pci_get_aift();
> +	spin_lock_irqsave(&aift->gait_lock, flags);
> +
> +	if (!aift->gait) {
> +		spin_unlock_irqrestore(&aift->gait_lock, flags);
> +		return;
> +	}
> +
> +	for (si = 0;;) {
> +		/* Scan adapter summary indicator bit vector */
> +		si = airq_iv_scan(aift->sbv, si, airq_iv_end(aift->sbv));
> +		if (si == -1UL) {
> +			if (first || found) {
> +				/* Reenable interrupts. */
> +				if (zpci_set_irq_ctrl(SIC_IRQ_MODE_SINGLE, isc,
> +						      &iib))
> +					break;
> +				first = found = false;
> +			} else {
> +				/* Interrupts on and all bits processed */
> +				break;
> +			}
> +			found = false;
> +			si = 0;
> +			continue;
> +		}
> +		found = true;
> +		aen_host_forward(aift, si);
> +	}
> +
> +	spin_unlock_irqrestore(&aift->gait_lock, flags);
> +}
> +
>   static void gib_alert_irq_handler(struct airq_struct *airq,
>   				  struct tpi_info *tpi_info)
>   {
> +	struct tpi_adapter_info *info = (struct tpi_adapter_info *)tpi_info;
> +
>   	inc_irq_stat(IRQIO_GAL);
> -	process_gib_alert_list();
> +
> +	if (info->forward || info->error)
> +		aen_process_gait(info->isc);
> +	else
> +		process_gib_alert_list();
>   }

Not sure, would it make sense to actually do both after an alert interrupt or do we always get a separate interrupt for event vs. irq?
[..]
Matthew Rosato Dec. 17, 2021, 5:42 p.m. UTC | #3
On 12/17/21 11:56 AM, Christian Borntraeger wrote:
> Am 07.12.21 um 21:57 schrieb Matthew Rosato:
>> In cases where interrupts are not forwarded to the guest via firmware,
>> KVM is responsible for ensuring delivery.  When an interrupt presents
>> with the forwarding bit, we must process the forwarding tables until
>> all interrupts are delivered.
>>
>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
>> ---
> [...]
> 
>> +static void aen_host_forward(struct zpci_aift *aift, unsigned long si)
>> +{
>> +    struct kvm_s390_gisa_interrupt *gi;
>> +    struct zpci_gaite *gaite;
>> +    struct kvm *kvm;
>> +
>> +    gaite = (struct zpci_gaite *)aift->gait +
>> +        (si * sizeof(struct zpci_gaite));
>> +    if (gaite->count == 0)
>> +        return;
>> +    if (gaite->aisb != 0)
>> +        set_bit_inv(gaite->aisbo, (unsigned long *)gaite->aisb);
>> +
>> +    kvm = kvm_s390_pci_si_to_kvm(aift, si);
>> +    if (kvm == 0)
>> +        return;
>> +    gi = &kvm->arch.gisa_int;
>> +
>> +    if (!(gi->origin->g1.simm & AIS_MODE_MASK(gaite->gisc)) ||
>> +        !(gi->origin->g1.nimm & AIS_MODE_MASK(gaite->gisc))) {
>> +        gisa_set_ipm_gisc(gi->origin, gaite->gisc);
>> +        if (hrtimer_active(&gi->timer))
>> +            hrtimer_cancel(&gi->timer);
>> +        hrtimer_start(&gi->timer, 0, HRTIMER_MODE_REL);
>> +        kvm->stat.aen_forward++;
>> +    }
>> +}
>> +
>> +static void aen_process_gait(u8 isc)
>> +{
>> +    bool found = false, first = true;
>> +    union zpci_sic_iib iib = {{0}};
>> +    unsigned long si, flags;
>> +    struct zpci_aift *aift;
>> +
>> +    aift = kvm_s390_pci_get_aift();
>> +    spin_lock_irqsave(&aift->gait_lock, flags);
>> +
>> +    if (!aift->gait) {
>> +        spin_unlock_irqrestore(&aift->gait_lock, flags);
>> +        return;
>> +    }
>> +
>> +    for (si = 0;;) {
>> +        /* Scan adapter summary indicator bit vector */
>> +        si = airq_iv_scan(aift->sbv, si, airq_iv_end(aift->sbv));
>> +        if (si == -1UL) {
>> +            if (first || found) {
>> +                /* Reenable interrupts. */
>> +                if (zpci_set_irq_ctrl(SIC_IRQ_MODE_SINGLE, isc,
>> +                              &iib))
>> +                    break;
>> +                first = found = false;
>> +            } else {
>> +                /* Interrupts on and all bits processed */
>> +                break;
>> +            }
>> +            found = false;
>> +            si = 0;
>> +            continue;
>> +        }
>> +        found = true;
>> +        aen_host_forward(aift, si);
>> +    }
>> +
>> +    spin_unlock_irqrestore(&aift->gait_lock, flags);
>> +}
>> +
>>   static void gib_alert_irq_handler(struct airq_struct *airq,
>>                     struct tpi_info *tpi_info)
>>   {
>> +    struct tpi_adapter_info *info = (struct tpi_adapter_info *)tpi_info;
>> +
>>       inc_irq_stat(IRQIO_GAL);
>> -    process_gib_alert_list();
>> +
>> +    if (info->forward || info->error)
>> +        aen_process_gait(info->isc);
>> +    else
>> +        process_gib_alert_list();
>>   }
> 
> Not sure, would it make sense to actually do both after an alert 
> interrupt or do we always get a separate interrupt for event vs. irq?
> [..]

Good point - I thought this was an either/or scenario but I went back 
and doubled checked -- looks like it is indeed possible to get a single 
interrupt that indicates processing of both AEN events and the alert 
list is required.  (It is also possible to get interrupts that indicate 
processing of only one or the other is required).  So, my code above is 
wrong.

However, we also don't need to call process_gib_alert_list() 
unconditionally after handling AEN -- there is more information we can 
check in tpi_adapter_info to decide whether that is necessary (aism);  I 
will add this.
diff mbox series

Patch

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index a604d51acfc8..3f147b8d050b 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -757,6 +757,7 @@  struct kvm_vm_stat {
 	u64 inject_pfault_done;
 	u64 inject_service_signal;
 	u64 inject_virtio;
+	u64 aen_forward;
 };
 
 struct kvm_arch_memory_slot {
diff --git a/arch/s390/include/asm/tpi.h b/arch/s390/include/asm/tpi.h
index 1ac538b8cbf5..47a531fdb15b 100644
--- a/arch/s390/include/asm/tpi.h
+++ b/arch/s390/include/asm/tpi.h
@@ -19,6 +19,20 @@  struct tpi_info {
 	u32 :12;
 } __packed __aligned(4);
 
+/* I/O-Interruption Code as stored by TPI for an Adapter I/O */
+struct tpi_adapter_info {
+	u32 :1;
+	u32 pci:1;
+	u32 :28;
+	u32 error:1;
+	u32 forward:1;
+	u32 reserved;
+	u32 adapter_IO:1;
+	u32 directed_irq:1;
+	u32 isc:3;
+	u32 :27;
+} __packed __aligned(4);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_S390_TPI_H */
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 4efe0e95a40f..c6ff871a6ed1 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -3263,11 +3263,85 @@  int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc)
 }
 EXPORT_SYMBOL_GPL(kvm_s390_gisc_unregister);
 
+static void aen_host_forward(struct zpci_aift *aift, unsigned long si)
+{
+	struct kvm_s390_gisa_interrupt *gi;
+	struct zpci_gaite *gaite;
+	struct kvm *kvm;
+
+	gaite = (struct zpci_gaite *)aift->gait +
+		(si * sizeof(struct zpci_gaite));
+	if (gaite->count == 0)
+		return;
+	if (gaite->aisb != 0)
+		set_bit_inv(gaite->aisbo, (unsigned long *)gaite->aisb);
+
+	kvm = kvm_s390_pci_si_to_kvm(aift, si);
+	if (kvm == 0)
+		return;
+	gi = &kvm->arch.gisa_int;
+
+	if (!(gi->origin->g1.simm & AIS_MODE_MASK(gaite->gisc)) ||
+	    !(gi->origin->g1.nimm & AIS_MODE_MASK(gaite->gisc))) {
+		gisa_set_ipm_gisc(gi->origin, gaite->gisc);
+		if (hrtimer_active(&gi->timer))
+			hrtimer_cancel(&gi->timer);
+		hrtimer_start(&gi->timer, 0, HRTIMER_MODE_REL);
+		kvm->stat.aen_forward++;
+	}
+}
+
+static void aen_process_gait(u8 isc)
+{
+	bool found = false, first = true;
+	union zpci_sic_iib iib = {{0}};
+	unsigned long si, flags;
+	struct zpci_aift *aift;
+
+	aift = kvm_s390_pci_get_aift();
+	spin_lock_irqsave(&aift->gait_lock, flags);
+
+	if (!aift->gait) {
+		spin_unlock_irqrestore(&aift->gait_lock, flags);
+		return;
+	}
+
+	for (si = 0;;) {
+		/* Scan adapter summary indicator bit vector */
+		si = airq_iv_scan(aift->sbv, si, airq_iv_end(aift->sbv));
+		if (si == -1UL) {
+			if (first || found) {
+				/* Reenable interrupts. */
+				if (zpci_set_irq_ctrl(SIC_IRQ_MODE_SINGLE, isc,
+						      &iib))
+					break;
+				first = found = false;
+			} else {
+				/* Interrupts on and all bits processed */
+				break;
+			}
+			found = false;
+			si = 0;
+			continue;
+		}
+		found = true;
+		aen_host_forward(aift, si);
+	}
+
+	spin_unlock_irqrestore(&aift->gait_lock, flags);
+}
+
 static void gib_alert_irq_handler(struct airq_struct *airq,
 				  struct tpi_info *tpi_info)
 {
+	struct tpi_adapter_info *info = (struct tpi_adapter_info *)tpi_info;
+
 	inc_irq_stat(IRQIO_GAL);
-	process_gib_alert_list();
+
+	if (info->forward || info->error)
+		aen_process_gait(info->isc);
+	else
+		process_gib_alert_list();
 }
 
 static struct airq_struct gib_alert_irq = {
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 9cd3c8eb59e8..c8fe9b7c2395 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -65,7 +65,8 @@  const struct _kvm_stats_desc kvm_vm_stats_desc[] = {
 	STATS_DESC_COUNTER(VM, inject_float_mchk),
 	STATS_DESC_COUNTER(VM, inject_pfault_done),
 	STATS_DESC_COUNTER(VM, inject_service_signal),
-	STATS_DESC_COUNTER(VM, inject_virtio)
+	STATS_DESC_COUNTER(VM, inject_virtio),
+	STATS_DESC_COUNTER(VM, aen_forward)
 };
 
 const struct kvm_stats_header kvm_vm_stats_header = {
diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
index 74b06d39be3b..776b2745c675 100644
--- a/arch/s390/kvm/pci.h
+++ b/arch/s390/kvm/pci.h
@@ -12,6 +12,7 @@ 
 
 #include <linux/pci.h>
 #include <linux/mutex.h>
+#include <linux/kvm_host.h>
 #include <asm/airq.h>
 #include <asm/kvm_pci.h>
 
@@ -32,6 +33,14 @@  struct zpci_aift {
 	struct mutex lock; /* Protects the other structures in aift */
 };
 
+static inline struct kvm *kvm_s390_pci_si_to_kvm(struct zpci_aift *aift,
+						 unsigned long si)
+{
+	if (aift->kzdev == 0 || aift->kzdev[si] == 0)
+		return 0;
+	return aift->kzdev[si]->kvm;
+};
+
 struct zpci_aift *kvm_s390_pci_get_aift(void);
 
 int kvm_s390_pci_aen_init(u8 nisc);