2022-01-14 23:07:21

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v2 00/30] KVM: s390: enable zPCI for interpretive execution

Enable interpretive execution of zPCI instructions + adapter interruption
forwarding for s390x KVM vfio-pci. This is done by introducing a series
of new vfio-pci feature ioctls that are unique vfio-pci-zdev (s390x) and
are used to negotiate the various aspects of zPCI interpretation setup.
By allowing intepretation of zPCI instructions and firmware delivery of
interrupts to guests, we can significantly reduce the frequency of guest
SIE exits for zPCI. We then see additional gains by handling a hot-path
instruction that can still intercept to the hypervisor (RPCIT) directly
in kvm.

From the perspective of guest configuration, you passthrough zPCI devices
in the same manner as before, with intepretation support being used by
default if available in kernel+qemu.

Will reply with a link to the associated QEMU series.

Changes v1->v2:
- s/has_zpci_interp/has_zpci_lsi/ (Christian)
- Added many R-bs / ACKs (Thanks!)
- Re-work zpci_set_irq_ctrl (Niklas)
- Simplify changes made for zpci_get_mdd (Niklas, Christian)
- 'KVM: s390: pci: add basic kvm_zdev structure' changes (Pierre)
- only build s390/kvm/pci.o when CONFIG_PCI
- Related to the above, add some more checks for
IS_ENABLED(CONFIG_PCI) (Pierre)
- Drop set_kvm_facility until VSIE support (Christian)
- Use sclp check instead of stfle when setting ECB (Christian)
- remove unnecessary externs from header (Pierre)
- macro for checkling if shadow ioat is initialized (Pierre)
- Fix interrupt case where we have both AEN and alert list (Christian)
- Re-work AEN setup to satisfy firmware requirements on all supported
platforms
- V!=R changes (Niklas)
- vifo_pci_zdev_feat_* - check argz against data size (Pierre, Alex)
- vfio_pci_zdev_{open,release} switch to return void (Alex)
- Related to the above, make kvm_s390_pci_*_probe functions return error
if KVM is not registered for the device (Alex)
- Fix my probe implementation to ignore GET|SET, as these don't change
the result of the probe. I was erroneously performing the GET or SET
operation if specified along with PROBE.
- A few additional fixes regarding ioctl implementation. Return EINVAL
if none of PROBE|GET|SET are specified. Return EINVAL if both GET
and SET are specified (without PROBE).
- New patch to return status from zpci_refresh_trans (Pierre, Niklas)
- And use that status when possible for KVM rpcit intercept (Pierre)

Matthew Rosato (30):
s390/sclp: detect the zPCI load/store interpretation facility
s390/sclp: detect the AISII facility
s390/sclp: detect the AENI facility
s390/sclp: detect the AISI facility
s390/airq: pass more TPI info to airq handlers
s390/airq: allow for airq structure that uses an input vector
s390/pci: externalize the SIC operation controls and routine
s390/pci: stash associated GISA designation
s390/pci: export some routines related to RPCIT processing
s390/pci: stash dtsm and maxstbl
s390/pci: add helper function to find device by handle
s390/pci: get SHM information from list pci
s390/pci: return status from zpci_refresh_trans
KVM: s390: pci: add basic kvm_zdev structure
KVM: s390: pci: do initial setup for AEN interpretation
KVM: s390: pci: enable host forwarding of Adapter Event Notifications
KVM: s390: mechanism to enable guest zPCI Interpretation
KVM: s390: pci: provide routines for enabling/disabling interpretation
KVM: s390: pci: provide routines for enabling/disabling interrupt
forwarding
KVM: s390: pci: provide routines for enabling/disabling IOAT assist
KVM: s390: pci: handle refresh of PCI translations
KVM: s390: intercept the rpcit instruction
vfio/pci: re-introduce CONFIG_VFIO_PCI_ZDEV
vfio-pci/zdev: wire up group notifier
vfio-pci/zdev: wire up zPCI interpretive execution support
vfio-pci/zdev: wire up zPCI adapter interrupt forwarding support
vfio-pci/zdev: wire up zPCI IOAT assist support
vfio-pci/zdev: add DTSM to clp group capability
KVM: s390: introduce CPU feature for zPCI Interpretation
MAINTAINERS: additional files related kvm s390 pci passthrough

MAINTAINERS | 2 +
arch/s390/include/asm/airq.h | 7 +-
arch/s390/include/asm/kvm_host.h | 5 +
arch/s390/include/asm/kvm_pci.h | 62 +++
arch/s390/include/asm/pci.h | 12 +
arch/s390/include/asm/pci_clp.h | 11 +-
arch/s390/include/asm/pci_dma.h | 3 +
arch/s390/include/asm/pci_insn.h | 31 +-
arch/s390/include/asm/sclp.h | 4 +
arch/s390/include/asm/tpi.h | 13 +
arch/s390/include/uapi/asm/kvm.h | 1 +
arch/s390/kvm/Makefile | 2 +-
arch/s390/kvm/interrupt.c | 94 +++-
arch/s390/kvm/kvm-s390.c | 56 ++-
arch/s390/kvm/kvm-s390.h | 10 +
arch/s390/kvm/pci.c | 837 +++++++++++++++++++++++++++++++
arch/s390/kvm/pci.h | 59 +++
arch/s390/kvm/priv.c | 46 ++
arch/s390/pci/pci.c | 31 ++
arch/s390/pci/pci_clp.c | 31 +-
arch/s390/pci/pci_dma.c | 7 +-
arch/s390/pci/pci_insn.c | 15 +-
arch/s390/pci/pci_irq.c | 48 +-
drivers/iommu/s390-iommu.c | 4 +-
drivers/s390/char/sclp_early.c | 4 +
drivers/s390/cio/airq.c | 12 +-
drivers/s390/cio/qdio_thinint.c | 6 +-
drivers/s390/crypto/ap_bus.c | 9 +-
drivers/s390/virtio/virtio_ccw.c | 6 +-
drivers/vfio/pci/Kconfig | 11 +
drivers/vfio/pci/Makefile | 2 +-
drivers/vfio/pci/vfio_pci_core.c | 8 +
drivers/vfio/pci/vfio_pci_zdev.c | 290 ++++++++++-
include/linux/vfio_pci_core.h | 42 +-
include/uapi/linux/vfio.h | 22 +
include/uapi/linux/vfio_zdev.h | 51 ++
36 files changed, 1788 insertions(+), 66 deletions(-)
create mode 100644 arch/s390/include/asm/kvm_pci.h
create mode 100644 arch/s390/kvm/pci.c
create mode 100644 arch/s390/kvm/pci.h

--
2.27.0


2022-01-14 23:07:29

by Matthew Rosato

[permalink] [raw]
Subject: Re: [PATCH v2 00/30] KVM: s390: enable zPCI for interpretive execution

On 1/14/22 3:31 PM, Matthew Rosato wrote:
> Enable interpretive execution of zPCI instructions + adapter interruption
> forwarding for s390x KVM vfio-pci. This is done by introducing a series
> of new vfio-pci feature ioctls that are unique vfio-pci-zdev (s390x) and
> are used to negotiate the various aspects of zPCI interpretation setup.
> By allowing intepretation of zPCI instructions and firmware delivery of
> interrupts to guests, we can significantly reduce the frequency of guest
> SIE exits for zPCI. We then see additional gains by handling a hot-path
> instruction that can still intercept to the hypervisor (RPCIT) directly
> in kvm.
>
> From the perspective of guest configuration, you passthrough zPCI devices
> in the same manner as before, with intepretation support being used by
> default if available in kernel+qemu.
>
> Will reply with a link to the associated QEMU series.

https://lore.kernel.org/qemu-devel/[email protected]/

2022-01-14 23:07:45

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v2 16/30] KVM: s390: pci: enable host forwarding of Adapter Event Notifications

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 <[email protected]>
---
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;
+ first = found = false;
+ } else {
+ /* Interrupts on and all bits processed */
+ break;
+ }
+ 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();
}

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)
+ return 0;
+ return aift->kzdev[si]->kvm;
+};
+
int kvm_s390_pci_aen_init(u8 nisc);
void kvm_s390_pci_aen_exit(void);

--
2.27.0

2022-01-14 23:07:51

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v2 30/30] MAINTAINERS: additional files related kvm s390 pci passthrough

Add entries from the s390 kvm subdirectory related to pci passthrough.

Acked-by: Christian Borntraeger <[email protected]>
Signed-off-by: Matthew Rosato <[email protected]>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5d0cd537803a..1b52acd74cfd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16874,6 +16874,8 @@ M: Eric Farman <[email protected]>
L: [email protected]
L: [email protected]
S: Supported
+F: arch/s390/include/asm/kvm_pci.h
+F: arch/s390/kvm/pci*
F: drivers/vfio/pci/vfio_pci_zdev.c
F: include/uapi/linux/vfio_zdev.h

--
2.27.0

2022-01-14 23:07:59

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v2 14/30] KVM: s390: pci: add basic kvm_zdev structure

This structure will be used to carry kvm passthrough information related to
zPCI devices.

Signed-off-by: Matthew Rosato <[email protected]>
---
arch/s390/include/asm/kvm_pci.h | 29 +++++++++++++++++++++
arch/s390/include/asm/pci.h | 3 +++
arch/s390/kvm/Makefile | 2 +-
arch/s390/kvm/pci.c | 46 +++++++++++++++++++++++++++++++++
4 files changed, 79 insertions(+), 1 deletion(-)
create mode 100644 arch/s390/include/asm/kvm_pci.h
create mode 100644 arch/s390/kvm/pci.c

diff --git a/arch/s390/include/asm/kvm_pci.h b/arch/s390/include/asm/kvm_pci.h
new file mode 100644
index 000000000000..aafee2976929
--- /dev/null
+++ b/arch/s390/include/asm/kvm_pci.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KVM PCI Passthrough for virtual machines on s390
+ *
+ * Copyright IBM Corp. 2021
+ *
+ * Author(s): Matthew Rosato <[email protected]>
+ */
+
+
+#ifndef ASM_KVM_PCI_H
+#define ASM_KVM_PCI_H
+
+#include <linux/types.h>
+#include <linux/kvm_types.h>
+#include <linux/kvm_host.h>
+#include <linux/kvm.h>
+#include <linux/pci.h>
+
+struct kvm_zdev {
+ struct zpci_dev *zdev;
+ struct kvm *kvm;
+};
+
+int kvm_s390_pci_dev_open(struct zpci_dev *zdev);
+void kvm_s390_pci_dev_release(struct zpci_dev *zdev);
+void kvm_s390_pci_attach_kvm(struct zpci_dev *zdev, struct kvm *kvm);
+
+#endif /* ASM_KVM_PCI_H */
diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
index f3cd2da8128c..9b6c657d8d31 100644
--- a/arch/s390/include/asm/pci.h
+++ b/arch/s390/include/asm/pci.h
@@ -97,6 +97,7 @@ struct zpci_bar_struct {
};

struct s390_domain;
+struct kvm_zdev;

#define ZPCI_FUNCTIONS_PER_BUS 256
struct zpci_bus {
@@ -190,6 +191,8 @@ struct zpci_dev {
struct dentry *debugfs_dev;

struct s390_domain *s390_domain; /* s390 IOMMU domain data */
+
+ struct kvm_zdev *kzdev; /* passthrough data */
};

static inline bool zdev_enabled(struct zpci_dev *zdev)
diff --git a/arch/s390/kvm/Makefile b/arch/s390/kvm/Makefile
index b3aaadc60ead..a26f4fe7b680 100644
--- a/arch/s390/kvm/Makefile
+++ b/arch/s390/kvm/Makefile
@@ -11,5 +11,5 @@ ccflags-y := -Ivirt/kvm -Iarch/s390/kvm

kvm-objs := $(common-objs) kvm-s390.o intercept.o interrupt.o priv.o sigp.o
kvm-objs += diag.o gaccess.o guestdbg.o vsie.o pv.o
-
+kvm-$(CONFIG_PCI) += pci.o
obj-$(CONFIG_KVM) += kvm.o
diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
new file mode 100644
index 000000000000..1c33bc7bf2bd
--- /dev/null
+++ b/arch/s390/kvm/pci.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * s390 kvm PCI passthrough support
+ *
+ * Copyright IBM Corp. 2021
+ *
+ * Author(s): Matthew Rosato <[email protected]>
+ */
+
+#include <linux/kvm_host.h>
+#include <linux/pci.h>
+#include <asm/kvm_pci.h>
+
+int kvm_s390_pci_dev_open(struct zpci_dev *zdev)
+{
+ struct kvm_zdev *kzdev;
+
+ kzdev = kzalloc(sizeof(struct kvm_zdev), GFP_KERNEL);
+ if (!kzdev)
+ return -ENOMEM;
+
+ kzdev->zdev = zdev;
+ zdev->kzdev = kzdev;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(kvm_s390_pci_dev_open);
+
+void kvm_s390_pci_dev_release(struct zpci_dev *zdev)
+{
+ struct kvm_zdev *kzdev;
+
+ kzdev = zdev->kzdev;
+ WARN_ON(kzdev->zdev != zdev);
+ zdev->kzdev = 0;
+ kfree(kzdev);
+}
+EXPORT_SYMBOL_GPL(kvm_s390_pci_dev_release);
+
+void kvm_s390_pci_attach_kvm(struct zpci_dev *zdev, struct kvm *kvm)
+{
+ struct kvm_zdev *kzdev = zdev->kzdev;
+
+ kzdev->kvm = kvm;
+}
+EXPORT_SYMBOL_GPL(kvm_s390_pci_attach_kvm);
--
2.27.0

2022-01-18 02:38:47

by Pierre Morel

[permalink] [raw]
Subject: Re: [PATCH v2 14/30] KVM: s390: pci: add basic kvm_zdev structure



On 1/14/22 21:31, Matthew Rosato wrote:
> This structure will be used to carry kvm passthrough information related to
> zPCI devices.
>
> Signed-off-by: Matthew Rosato <[email protected]>
> ---
> arch/s390/include/asm/kvm_pci.h | 29 +++++++++++++++++++++
> arch/s390/include/asm/pci.h | 3 +++
> arch/s390/kvm/Makefile | 2 +-
> arch/s390/kvm/pci.c | 46 +++++++++++++++++++++++++++++++++
> 4 files changed, 79 insertions(+), 1 deletion(-)
> create mode 100644 arch/s390/include/asm/kvm_pci.h
> create mode 100644 arch/s390/kvm/pci.c
>
> diff --git a/arch/s390/include/asm/kvm_pci.h b/arch/s390/include/asm/kvm_pci.h
> new file mode 100644
> index 000000000000..aafee2976929
> --- /dev/null
> +++ b/arch/s390/include/asm/kvm_pci.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * KVM PCI Passthrough for virtual machines on s390
> + *
> + * Copyright IBM Corp. 2021
> + *
> + * Author(s): Matthew Rosato <[email protected]>
> + */
> +
> +

One blank line too much.

Otherwise, look good to me.

Reviewed-by: Pierre Morel <[email protected]>

> +#ifndef ASM_KVM_PCI_H
> +#define ASM_KVM_PCI_H
> +
> +#include <linux/types.h>
> +#include <linux/kvm_types.h>
> +#include <linux/kvm_host.h>
> +#include <linux/kvm.h>
> +#include <linux/pci.h>
> +
> +struct kvm_zdev {
> + struct zpci_dev *zdev;
> + struct kvm *kvm;
> +};
> +
> +int kvm_s390_pci_dev_open(struct zpci_dev *zdev);
> +void kvm_s390_pci_dev_release(struct zpci_dev *zdev);
> +void kvm_s390_pci_attach_kvm(struct zpci_dev *zdev, struct kvm *kvm);
> +
> +#endif /* ASM_KVM_PCI_H */
> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
> index f3cd2da8128c..9b6c657d8d31 100644
> --- a/arch/s390/include/asm/pci.h
> +++ b/arch/s390/include/asm/pci.h
> @@ -97,6 +97,7 @@ struct zpci_bar_struct {
> };
>
> struct s390_domain;
> +struct kvm_zdev;
>
> #define ZPCI_FUNCTIONS_PER_BUS 256
> struct zpci_bus {
> @@ -190,6 +191,8 @@ struct zpci_dev {
> struct dentry *debugfs_dev;
>
> struct s390_domain *s390_domain; /* s390 IOMMU domain data */
> +
> + struct kvm_zdev *kzdev; /* passthrough data */
> };
>
> static inline bool zdev_enabled(struct zpci_dev *zdev)
> diff --git a/arch/s390/kvm/Makefile b/arch/s390/kvm/Makefile
> index b3aaadc60ead..a26f4fe7b680 100644
> --- a/arch/s390/kvm/Makefile
> +++ b/arch/s390/kvm/Makefile
> @@ -11,5 +11,5 @@ ccflags-y := -Ivirt/kvm -Iarch/s390/kvm
>
> kvm-objs := $(common-objs) kvm-s390.o intercept.o interrupt.o priv.o sigp.o
> kvm-objs += diag.o gaccess.o guestdbg.o vsie.o pv.o
> -
> +kvm-$(CONFIG_PCI) += pci.o
> obj-$(CONFIG_KVM) += kvm.o
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> new file mode 100644
> index 000000000000..1c33bc7bf2bd
> --- /dev/null
> +++ b/arch/s390/kvm/pci.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * s390 kvm PCI passthrough support
> + *
> + * Copyright IBM Corp. 2021
> + *
> + * Author(s): Matthew Rosato <[email protected]>
> + */
> +
> +#include <linux/kvm_host.h>
> +#include <linux/pci.h>
> +#include <asm/kvm_pci.h>
> +
> +int kvm_s390_pci_dev_open(struct zpci_dev *zdev)
> +{
> + struct kvm_zdev *kzdev;
> +
> + kzdev = kzalloc(sizeof(struct kvm_zdev), GFP_KERNEL);
> + if (!kzdev)
> + return -ENOMEM;
> +
> + kzdev->zdev = zdev;
> + zdev->kzdev = kzdev;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(kvm_s390_pci_dev_open);
> +
> +void kvm_s390_pci_dev_release(struct zpci_dev *zdev)
> +{
> + struct kvm_zdev *kzdev;
> +
> + kzdev = zdev->kzdev;
> + WARN_ON(kzdev->zdev != zdev);
> + zdev->kzdev = 0;
> + kfree(kzdev);
> +}
> +EXPORT_SYMBOL_GPL(kvm_s390_pci_dev_release);
> +
> +void kvm_s390_pci_attach_kvm(struct zpci_dev *zdev, struct kvm *kvm)
> +{
> + struct kvm_zdev *kzdev = zdev->kzdev;
> +
> + kzdev->kvm = kvm;
> +}
> +EXPORT_SYMBOL_GPL(kvm_s390_pci_attach_kvm);
>

--
Pierre Morel
IBM Lab Boeblingen

2022-01-18 02:54:21

by Pierre Morel

[permalink] [raw]
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 <[email protected]>
> ---
> 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

2022-01-20 19:16:22

by Matthew Rosato

[permalink] [raw]
Subject: Re: [PATCH v2 16/30] KVM: s390: pci: enable host forwarding of Adapter Event Notifications

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.



2022-01-20 20:21:24

by Pierre Morel

[permalink] [raw]
Subject: Re: [PATCH v2 14/30] KVM: s390: pci: add basic kvm_zdev structure



On 1/17/22 17:25, Pierre Morel wrote:
>
>
> On 1/14/22 21:31, Matthew Rosato wrote:
>> This structure will be used to carry kvm passthrough information
>> related to
>> zPCI devices.
>>
>> Signed-off-by: Matthew Rosato <[email protected]>
>> ---
>>   arch/s390/include/asm/kvm_pci.h | 29 +++++++++++++++++++++
>>   arch/s390/include/asm/pci.h     |  3 +++
>>   arch/s390/kvm/Makefile          |  2 +-
>>   arch/s390/kvm/pci.c             | 46 +++++++++++++++++++++++++++++++++
>>   4 files changed, 79 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/s390/include/asm/kvm_pci.h
>>   create mode 100644 arch/s390/kvm/pci.c
>>
>> diff --git a/arch/s390/include/asm/kvm_pci.h
>> b/arch/s390/include/asm/kvm_pci.h
>> new file mode 100644
>> index 000000000000..aafee2976929
>> --- /dev/null
>> +++ b/arch/s390/include/asm/kvm_pci.h
>> @@ -0,0 +1,29 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * KVM PCI Passthrough for virtual machines on s390
>> + *
>> + * Copyright IBM Corp. 2021
>> + *
>> + *    Author(s): Matthew Rosato <[email protected]>
>> + */
>> +
>> +
>
> One blank line too much.
>
> Otherwise, look good to me.
>
> Reviewed-by: Pierre Morel <[email protected]>
>
>> +#ifndef ASM_KVM_PCI_H
>> +#define ASM_KVM_PCI_H
>> +
>> +#include <linux/types.h>
>> +#include <linux/kvm_types.h>
>> +#include <linux/kvm_host.h>
>> +#include <linux/kvm.h>
>> +#include <linux/pci.h>
>> +
>> +struct kvm_zdev {
>> +    struct zpci_dev *zdev;
>> +    struct kvm *kvm;
>> +};
>> +
>> +int kvm_s390_pci_dev_open(struct zpci_dev *zdev);
>> +void kvm_s390_pci_dev_release(struct zpci_dev *zdev);
>> +void kvm_s390_pci_attach_kvm(struct zpci_dev *zdev, struct kvm *kvm);
>> +
>> +#endif /* ASM_KVM_PCI_H */
>> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
>> index f3cd2da8128c..9b6c657d8d31 100644
>> --- a/arch/s390/include/asm/pci.h
>> +++ b/arch/s390/include/asm/pci.h
>> @@ -97,6 +97,7 @@ struct zpci_bar_struct {
>>   };
>>   struct s390_domain;
>> +struct kvm_zdev;
>>   #define ZPCI_FUNCTIONS_PER_BUS 256
>>   struct zpci_bus {
>> @@ -190,6 +191,8 @@ struct zpci_dev {
>>       struct dentry    *debugfs_dev;
>>       struct s390_domain *s390_domain; /* s390 IOMMU domain data */
>> +
>> +    struct kvm_zdev *kzdev; /* passthrough data */
>>   };
>>   static inline bool zdev_enabled(struct zpci_dev *zdev)
>> diff --git a/arch/s390/kvm/Makefile b/arch/s390/kvm/Makefile
>> index b3aaadc60ead..a26f4fe7b680 100644
>> --- a/arch/s390/kvm/Makefile
>> +++ b/arch/s390/kvm/Makefile
>> @@ -11,5 +11,5 @@ ccflags-y := -Ivirt/kvm -Iarch/s390/kvm
>>   kvm-objs := $(common-objs) kvm-s390.o intercept.o interrupt.o priv.o
>> sigp.o
>>   kvm-objs += diag.o gaccess.o guestdbg.o vsie.o pv.o
>> -
>> +kvm-$(CONFIG_PCI) += pci.o
>>   obj-$(CONFIG_KVM) += kvm.o
>> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
>> new file mode 100644
>> index 000000000000..1c33bc7bf2bd
>> --- /dev/null
>> +++ b/arch/s390/kvm/pci.c
>> @@ -0,0 +1,46 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * s390 kvm PCI passthrough support
>> + *
>> + * Copyright IBM Corp. 2021
>> + *
>> + *    Author(s): Matthew Rosato <[email protected]>
>> + */
>> +
>> +#include <linux/kvm_host.h>
>> +#include <linux/pci.h>
>> +#include <asm/kvm_pci.h>
>> +
>> +int kvm_s390_pci_dev_open(struct zpci_dev *zdev)
>> +{
>> +    struct kvm_zdev *kzdev;
>> +
>> +    kzdev = kzalloc(sizeof(struct kvm_zdev), GFP_KERNEL);
>> +    if (!kzdev)
>> +        return -ENOMEM;
>> +
>> +    kzdev->zdev = zdev;
>> +    zdev->kzdev = kzdev;
>> +
>> +    return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_s390_pci_dev_open);
>> +
>> +void kvm_s390_pci_dev_release(struct zpci_dev *zdev)
>> +{
>> +    struct kvm_zdev *kzdev;
>> +
>> +    kzdev = zdev->kzdev;
>> +    WARN_ON(kzdev->zdev != zdev);
>> +    zdev->kzdev = 0;
>> +    kfree(kzdev);
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_s390_pci_dev_release);
>> +
>> +void kvm_s390_pci_attach_kvm(struct zpci_dev *zdev, struct kvm *kvm)
>> +{
>> +    struct kvm_zdev *kzdev = zdev->kzdev;
>> +
>> +    kzdev->kvm = kvm;
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_s390_pci_attach_kvm);
>>
>

Working now on patch 24, I am not sure that this function is necessary.
the only purpose seems to set kzdev->kvm = kvm while we already know
kzdev in the caller.



--
Pierre Morel
IBM Lab Boeblingen

2022-01-20 23:23:51

by Matthew Rosato

[permalink] [raw]
Subject: Re: [PATCH v2 14/30] KVM: s390: pci: add basic kvm_zdev structure

On 1/18/22 12:32 PM, Pierre Morel wrote:
>
>
> On 1/17/22 17:25, Pierre Morel wrote:
>>
>>
>> On 1/14/22 21:31, Matthew Rosato wrote:
>>> This structure will be used to carry kvm passthrough information
>>> related to
>>> zPCI devices.
>>>
>>> Signed-off-by: Matthew Rosato <[email protected]>
>>> ---
>>>   arch/s390/include/asm/kvm_pci.h | 29 +++++++++++++++++++++
>>>   arch/s390/include/asm/pci.h     |  3 +++
>>>   arch/s390/kvm/Makefile          |  2 +-
>>>   arch/s390/kvm/pci.c             | 46 +++++++++++++++++++++++++++++++++
>>>   4 files changed, 79 insertions(+), 1 deletion(-)
>>>   create mode 100644 arch/s390/include/asm/kvm_pci.h
>>>   create mode 100644 arch/s390/kvm/pci.c
>>>
>>> diff --git a/arch/s390/include/asm/kvm_pci.h
>>> b/arch/s390/include/asm/kvm_pci.h
>>> new file mode 100644
>>> index 000000000000..aafee2976929
>>> --- /dev/null
>>> +++ b/arch/s390/include/asm/kvm_pci.h
>>> @@ -0,0 +1,29 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * KVM PCI Passthrough for virtual machines on s390
>>> + *
>>> + * Copyright IBM Corp. 2021
>>> + *
>>> + *    Author(s): Matthew Rosato <[email protected]>
>>> + */
>>> +
>>> +
>>
>> One blank line too much.
>>
>> Otherwise, look good to me.
>>
>> Reviewed-by: Pierre Morel <[email protected]>
>>
>>> +#ifndef ASM_KVM_PCI_H
>>> +#define ASM_KVM_PCI_H
>>> +
>>> +#include <linux/types.h>
>>> +#include <linux/kvm_types.h>
>>> +#include <linux/kvm_host.h>
>>> +#include <linux/kvm.h>
>>> +#include <linux/pci.h>
>>> +
>>> +struct kvm_zdev {
>>> +    struct zpci_dev *zdev;
>>> +    struct kvm *kvm;
>>> +};
>>> +
>>> +int kvm_s390_pci_dev_open(struct zpci_dev *zdev);
>>> +void kvm_s390_pci_dev_release(struct zpci_dev *zdev);
>>> +void kvm_s390_pci_attach_kvm(struct zpci_dev *zdev, struct kvm *kvm);
>>> +
>>> +#endif /* ASM_KVM_PCI_H */
>>> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
>>> index f3cd2da8128c..9b6c657d8d31 100644
>>> --- a/arch/s390/include/asm/pci.h
>>> +++ b/arch/s390/include/asm/pci.h
>>> @@ -97,6 +97,7 @@ struct zpci_bar_struct {
>>>   };
>>>   struct s390_domain;
>>> +struct kvm_zdev;
>>>   #define ZPCI_FUNCTIONS_PER_BUS 256
>>>   struct zpci_bus {
>>> @@ -190,6 +191,8 @@ struct zpci_dev {
>>>       struct dentry    *debugfs_dev;
>>>       struct s390_domain *s390_domain; /* s390 IOMMU domain data */
>>> +
>>> +    struct kvm_zdev *kzdev; /* passthrough data */
>>>   };
>>>   static inline bool zdev_enabled(struct zpci_dev *zdev)
>>> diff --git a/arch/s390/kvm/Makefile b/arch/s390/kvm/Makefile
>>> index b3aaadc60ead..a26f4fe7b680 100644
>>> --- a/arch/s390/kvm/Makefile
>>> +++ b/arch/s390/kvm/Makefile
>>> @@ -11,5 +11,5 @@ ccflags-y := -Ivirt/kvm -Iarch/s390/kvm
>>>   kvm-objs := $(common-objs) kvm-s390.o intercept.o interrupt.o
>>> priv.o sigp.o
>>>   kvm-objs += diag.o gaccess.o guestdbg.o vsie.o pv.o
>>> -
>>> +kvm-$(CONFIG_PCI) += pci.o

As discussed in other threads, I will look at changing this to
kvm-$(CONFIG_VFIO_PCI_ZDEV) += pci.o
Along with other IS_ENABLE(CONFIG_PCI) ->
IS_ENABLED(CONFIG_VFIO_PCI_ZDDEV) changes

>>>   obj-$(CONFIG_KVM) += kvm.o
>>> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
>>> new file mode 100644
>>> index 000000000000..1c33bc7bf2bd
>>> --- /dev/null
>>> +++ b/arch/s390/kvm/pci.c
>>> @@ -0,0 +1,46 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * s390 kvm PCI passthrough support
>>> + *
>>> + * Copyright IBM Corp. 2021
>>> + *
>>> + *    Author(s): Matthew Rosato <[email protected]>
>>> + */
>>> +
>>> +#include <linux/kvm_host.h>
>>> +#include <linux/pci.h>
>>> +#include <asm/kvm_pci.h>
>>> +
>>> +int kvm_s390_pci_dev_open(struct zpci_dev *zdev)
>>> +{
>>> +    struct kvm_zdev *kzdev;
>>> +
>>> +    kzdev = kzalloc(sizeof(struct kvm_zdev), GFP_KERNEL);
>>> +    if (!kzdev)
>>> +        return -ENOMEM;
>>> +
>>> +    kzdev->zdev = zdev;
>>> +    zdev->kzdev = kzdev;
>>> +
>>> +    return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(kvm_s390_pci_dev_open);
>>> +
>>> +void kvm_s390_pci_dev_release(struct zpci_dev *zdev)
>>> +{
>>> +    struct kvm_zdev *kzdev;
>>> +
>>> +    kzdev = zdev->kzdev;
>>> +    WARN_ON(kzdev->zdev != zdev);
>>> +    zdev->kzdev = 0;
>>> +    kfree(kzdev);
>>> +}
>>> +EXPORT_SYMBOL_GPL(kvm_s390_pci_dev_release);
>>> +
>>> +void kvm_s390_pci_attach_kvm(struct zpci_dev *zdev, struct kvm *kvm)
>>> +{
>>> +    struct kvm_zdev *kzdev = zdev->kzdev;
>>> +
>>> +    kzdev->kvm = kvm;
>>> +}
>>> +EXPORT_SYMBOL_GPL(kvm_s390_pci_attach_kvm);
>>>
>>
>
> Working now on patch 24, I am not sure that this function is necessary.
> the only purpose seems to set kzdev->kvm = kvm while we already know
> kzdev in the caller.
>

Yep, as mentioned in the patch 24 thread I will drop this function and
set kzdev->kvm = kvm directly in patch 24.

2022-01-21 19:54:59

by Pierre Morel

[permalink] [raw]
Subject: Re: [PATCH v2 00/30] KVM: s390: enable zPCI for interpretive execution



On 1/14/22 21:31, Matthew Rosato wrote:
> Enable interpretive execution of zPCI instructions + adapter interruption
> forwarding for s390x KVM vfio-pci. This is done by introducing a series
> of new vfio-pci feature ioctls that are unique vfio-pci-zdev (s390x) and
> are used to negotiate the various aspects of zPCI interpretation setup.
> By allowing intepretation of zPCI instructions and firmware delivery of
> interrupts to guests, we can significantly reduce the frequency of guest
> SIE exits for zPCI. We then see additional gains by handling a hot-path
> instruction that can still intercept to the hypervisor (RPCIT) directly
> in kvm.
>
> From the perspective of guest configuration, you passthrough zPCI devices
> in the same manner as before, with intepretation support being used by
> default if available in kernel+qemu.
>
> Will reply with a link to the associated QEMU series.

I did the comment in a patch but I think that centralizing it here is
clearer: I think having a documentation in Documentation/S390 like we
have already for VFIO AP and VFIO CCW would be a good thing.

--
Pierre Morel
IBM Lab Boeblingen