On 1/17/22 12:38 PM, Pierre Morel wrote:
...
+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.
Yep, you are correct; we can just ignore the return value here.
+ first = found = false;
+ } else {
+ /* Interrupts on and all bits processed */
+ break;
+ }
May be add a comment: "rescan after re-enabling interrupts"
OK
+ 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
OK
}
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 ?
While it's true that we can't be doing interpretation without
CONFIG_VFIO_PCI=y|m, the reason I'm using CONFIG_PCI here and elsewhere
in the code is because CONFIG_PCI is what is being used to determine
whether or not we build arch/s390/kvm/pci.o in patch 14 (and thus
whether or not the aift exists) -- And the reason we use this is because
this is where the code dependencies exist (examples include
ZPCI_NR_DEVICES, the AEN pieces that must be preserved over KVM module
remove/insert in patch 15)
If we for some reason have a case where CONFIG_KVM=y|m && CONFIG_PCI=y|m
&& CONFIG_VFIO_PCI=n, this will still work: aift and aift->kzdev will
exist (kvm/pci.o is linked) but we will never actually drive this
routine anyway because we'll never register a device for AEN forwarding
without CONFIG_VFIO_PCI.
