lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 17 Jan 2022 18:38:23 +0100 From: Pierre Morel <pmorel@...ux.ibm.com> To: Matthew Rosato <mjrosato@...ux.ibm.com>, linux-s390@...r.kernel.org Cc: alex.williamson@...hat.com, cohuck@...hat.com, schnelle@...ux.ibm.com, farman@...ux.ibm.com, borntraeger@...ux.ibm.com, hca@...ux.ibm.com, gor@...ux.ibm.com, gerald.schaefer@...ux.ibm.com, agordeev@...ux.ibm.com, frankja@...ux.ibm.com, david@...hat.com, imbrenda@...ux.ibm.com, vneethv@...ux.ibm.com, oberpar@...ux.ibm.com, freude@...ux.ibm.com, thuth@...hat.com, pasic@...ux.ibm.com, kvm@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v2 16/30] KVM: s390: pci: enable host forwarding of Adapter Event Notifications On 1/14/22 21:31, 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@...ux.ibm.com> > --- > arch/s390/include/asm/kvm_host.h | 1 + > arch/s390/include/asm/tpi.h | 13 ++++++ > arch/s390/kvm/interrupt.c | 76 +++++++++++++++++++++++++++++++- > arch/s390/kvm/kvm-s390.c | 3 +- > arch/s390/kvm/pci.h | 9 ++++ > 5 files changed, 100 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..f76e5fdff23a 100644 > --- a/arch/s390/include/asm/tpi.h > +++ b/arch/s390/include/asm/tpi.h > @@ -19,6 +19,19 @@ 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 aism:8; > + u32 :22; > + 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 a591b8cd662f..07743c6a67c4 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(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; > + > + 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; AFAIU this code is VFIO interpretation specific code and facility 12 is a precondition for it, so I think this break will never occur. If I am right we should not test the return value which will make the code clearer. > + first = found = false; > + } else { > + /* Interrupts on and all bits processed */ > + break; > + } May be add a comment: "rescan after re-enabling interrupts" > + found = false; > + si = 0; > + continue; > + } > + found = true; > + aen_host_forward(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 (IS_ENABLED(CONFIG_PCI) && (info->forward || info->error)) { > + aen_process_gait(info->isc); > + if (info->aism != 0) > + process_gib_alert_list(); > + } else > + process_gib_alert_list(); NIT: I think we need braces around this statement > } > > static struct airq_struct gib_alert_irq = { > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 01dc3f6883d0..ab8b56deed11 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 b2000ed7b8c3..387b637863c9 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> > > @@ -34,6 +35,14 @@ struct zpci_aift { > > extern struct zpci_aift *aift; > > +static inline struct kvm *kvm_s390_pci_si_to_kvm(struct zpci_aift *aift, > + unsigned long si) > +{ > + if (!IS_ENABLED(CONFIG_PCI) || aift->kzdev == 0 || aift->kzdev[si] == 0) Shouldn't it be better CONFIG_VFIO_PCI ? > + return 0; > + return aift->kzdev[si]->kvm; > +}; > + > int kvm_s390_pci_aen_init(u8 nisc); > void kvm_s390_pci_aen_exit(void); > > -- Pierre Morel IBM Lab Boeblingen
Powered by blists - more mailing lists