2022-04-05 01:15:16

by Matthew Rosato

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

Note: in this version, all IOMMU changes have been removed to be pursued
as a follow-on / in conjunction with IOMMUFD. This series proposes to
add only exploitation of the interpretive execution facilities.

Enable interpretive execution of zPCI instructions + adapter interruption
forwarding for s390x KVM vfio-pci. This is done by triggering a routine
when the VFIO group is associated with the KVM guest, transmitting to
firmware a special token (GISA designation) to enable that specific guest
for interpretive execution on that zPCI device. Load/store interpreation
enablement is then controlled by userspace (based upon whether or not a
SHM bit is placed in the virtual function handle). Adapter Event
Notification interpretation is controlled from userspace via a new KVM
ioctl.

By allowing intepretation of zPCI instructions and firmware delivery of
interrupts to guests, we can reduce the frequency of guest SIE exits for
zPCI.

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.

Changelog v4->v5:
- Remove all IOMMU changes - this series will continue to use vfio type1
for mapping; RPCIT enhancements will be pursued as a follow-on in
coordination with the IOMMUFD project
- Remove most of the ops from the proposed KVM ioctl -- combine the notion
of 'start' and 'enable interpretation' into a single operation that
occurs in response to assigning a KVM association to the VFIO group
- a new kvm ioctl is still being used to register/unregister for adapter
event notification forwarding. But teach it to use pin_user_pages
instead of gfn_to_page and perform accounting.
- Because we now attempt to enable interpretation for all devices (even
if the virtual PCI device has a SHM bit on) avoid setting the GISA for
the device and the ECB bits for the guest on hardware without SHM.
- found a bug with "s390/pci: stash associated GISA designation", namely
that CLP SET PCI FN (disable) expects the gisa designation to always be
0. Now that we set the gisa earlier, vfio-pci can trip this when
triggering restore for ISM.
- Add a single routine to determine if interpretation support is usable
on this host, to be used in multiple spots.
- eliminated arch/s390/include/asm/kvm_pci.h, moving most elements into
arch/s390/kvm/pci.h with one exception to asm/kvm_host.h

Matthew Rosato (21):
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: stash dtsm and maxstbl
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 interrupt
forwarding
KVM: s390: pci: add routines to start/stop interpretive execution
KVM: vfio: add s390x hook to register KVM guest designation
vfio-pci/zdev: add function handle to clp base capability
vfio-pci/zdev: different maxstbl for interpreted devices
KVM: s390: add KVM_S390_ZPCI_OP to manage guest zPCI devices
KVM: s390: introduce CPU feature for zPCI Interpretation
MAINTAINERS: additional files related kvm s390 pci passthrough

Documentation/virt/kvm/api.rst | 45 +++
MAINTAINERS | 1 +
arch/s390/include/asm/airq.h | 7 +-
arch/s390/include/asm/kvm_host.h | 16 +
arch/s390/include/asm/pci.h | 10 +
arch/s390/include/asm/pci_clp.h | 9 +-
arch/s390/include/asm/pci_insn.h | 29 +-
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 | 1 +
arch/s390/kvm/interrupt.c | 95 ++++-
arch/s390/kvm/kvm-s390.c | 81 +++-
arch/s390/kvm/kvm-s390.h | 10 +
arch/s390/kvm/pci.c | 666 +++++++++++++++++++++++++++++++
arch/s390/kvm/pci.h | 86 ++++
arch/s390/pci/pci.c | 15 +
arch/s390/pci/pci_clp.c | 7 +
arch/s390/pci/pci_insn.c | 4 +-
arch/s390/pci/pci_irq.c | 48 ++-
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/vfio_pci_zdev.c | 11 +-
include/uapi/linux/kvm.h | 31 ++
include/uapi/linux/vfio_zdev.h | 7 +
virt/kvm/vfio.c | 35 +-
29 files changed, 1216 insertions(+), 53 deletions(-)
create mode 100644 arch/s390/kvm/pci.c
create mode 100644 arch/s390/kvm/pci.h

--
2.27.0


2022-04-05 01:18:22

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v5 11/21] KVM: s390: pci: do initial setup for AEN interpretation

Initial setup for Adapter Event Notification Interpretation for zPCI
passthrough devices. Specifically, allocate a structure for forwarding of
adapter events and pass the address of this structure to firmware.

Signed-off-by: Matthew Rosato <[email protected]>
---
arch/s390/include/asm/pci.h | 4 +
arch/s390/include/asm/pci_insn.h | 12 +++
arch/s390/kvm/interrupt.c | 14 +++
arch/s390/kvm/kvm-s390.c | 9 ++
arch/s390/kvm/pci.c | 153 +++++++++++++++++++++++++++++++
arch/s390/kvm/pci.h | 47 ++++++++++
arch/s390/pci/pci.c | 6 ++
7 files changed, 245 insertions(+)

diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
index 9eb20cebaa18..557b0ffb32d2 100644
--- a/arch/s390/include/asm/pci.h
+++ b/arch/s390/include/asm/pci.h
@@ -9,6 +9,7 @@
#include <asm-generic/pci.h>
#include <asm/pci_clp.h>
#include <asm/pci_debug.h>
+#include <asm/pci_insn.h>
#include <asm/sclp.h>

#define PCIBIOS_MIN_IO 0x1000
@@ -204,6 +205,9 @@ extern const struct attribute_group *zpci_attr_groups[];
extern unsigned int s390_pci_force_floating __initdata;
extern unsigned int s390_pci_no_rid;

+extern union zpci_sic_iib *zpci_aipb;
+extern struct airq_iv *zpci_aif_sbv;
+
/* -----------------------------------------------------------------------------
Prototypes
----------------------------------------------------------------------------- */
diff --git a/arch/s390/include/asm/pci_insn.h b/arch/s390/include/asm/pci_insn.h
index 5331082fa516..e5f57cfe1d45 100644
--- a/arch/s390/include/asm/pci_insn.h
+++ b/arch/s390/include/asm/pci_insn.h
@@ -101,6 +101,7 @@ struct zpci_fib {
/* Set Interruption Controls Operation Controls */
#define SIC_IRQ_MODE_ALL 0
#define SIC_IRQ_MODE_SINGLE 1
+#define SIC_SET_AENI_CONTROLS 2
#define SIC_IRQ_MODE_DIRECT 4
#define SIC_IRQ_MODE_D_ALL 16
#define SIC_IRQ_MODE_D_SINGLE 17
@@ -127,9 +128,20 @@ struct zpci_cdiib {
u64 : 64;
} __packed __aligned(8);

+/* adapter interruption parameters block */
+struct zpci_aipb {
+ u64 faisb;
+ u64 gait;
+ u16 : 13;
+ u16 afi : 3;
+ u32 : 32;
+ u16 faal;
+} __packed __aligned(8);
+
union zpci_sic_iib {
struct zpci_diib diib;
struct zpci_cdiib cdiib;
+ struct zpci_aipb aipb;
};

DECLARE_STATIC_KEY_FALSE(have_mio);
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 250119a26c60..57a27dfc85ea 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -32,6 +32,7 @@
#include "kvm-s390.h"
#include "gaccess.h"
#include "trace-s390.h"
+#include "pci.h"

#define PFAULT_INIT 0x0600
#define PFAULT_DONE 0x0680
@@ -3328,6 +3329,11 @@ void kvm_s390_gib_destroy(void)
{
if (!gib)
return;
+ if (kvm_s390_pci_interp_allowed() && aift) {
+ mutex_lock(&aift->aift_lock);
+ kvm_s390_pci_aen_exit();
+ mutex_unlock(&aift->aift_lock);
+ }
chsc_sgib(0);
unregister_adapter_interrupt(&gib_alert_irq);
free_page((unsigned long)gib);
@@ -3365,6 +3371,14 @@ int kvm_s390_gib_init(u8 nisc)
goto out_unreg_gal;
}

+ if (kvm_s390_pci_interp_allowed()) {
+ if (kvm_s390_pci_aen_init(nisc)) {
+ pr_err("Initializing AEN for PCI failed\n");
+ rc = -EIO;
+ goto out_unreg_gal;
+ }
+ }
+
KVM_EVENT(3, "gib 0x%pK (nisc=%d) initialized", gib, gib->nisc);
goto out;

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 156d1c25a3c1..9db6f8080f71 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -47,6 +47,7 @@
#include <asm/fpu/api.h>
#include "kvm-s390.h"
#include "gaccess.h"
+#include "pci.h"

#define CREATE_TRACE_POINTS
#include "trace.h"
@@ -502,6 +503,14 @@ int kvm_arch_init(void *opaque)
goto out;
}

+ if (kvm_s390_pci_interp_allowed()) {
+ rc = kvm_s390_pci_init();
+ if (rc) {
+ pr_err("Unable to allocate AIFT for PCI\n");
+ goto out;
+ }
+ }
+
rc = kvm_s390_gib_init(GAL_ISC);
if (rc)
goto out;
diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 213be236c05a..01bd8a2f503b 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -9,8 +9,149 @@

#include <linux/kvm_host.h>
#include <linux/pci.h>
+#include <asm/pci.h>
+#include <asm/pci_insn.h>
#include "pci.h"

+struct zpci_aift *aift;
+
+static inline int __set_irq_noiib(u16 ctl, u8 isc)
+{
+ union zpci_sic_iib iib = {{0}};
+
+ return zpci_set_irq_ctrl(ctl, isc, &iib);
+}
+
+/* Caller must hold the aift lock before calling this function */
+void kvm_s390_pci_aen_exit(void)
+{
+ unsigned long flags;
+ struct kvm_zdev **gait_kzdev;
+
+ /*
+ * Contents of the aipb remain registered for the life of the host
+ * kernel, the information preserved in zpci_aipb and zpci_aif_sbv
+ * in case we insert the KVM module again later. Clear the AIFT
+ * information and free anything not registered with underlying
+ * firmware.
+ */
+ spin_lock_irqsave(&aift->gait_lock, flags);
+ gait_kzdev = aift->kzdev;
+ aift->gait = 0;
+ aift->sbv = 0;
+ aift->kzdev = 0;
+ spin_unlock_irqrestore(&aift->gait_lock, flags);
+
+ kfree(gait_kzdev);
+}
+
+static int zpci_setup_aipb(u8 nisc)
+{
+ struct page *page;
+ int size, rc;
+
+ zpci_aipb = kzalloc(sizeof(union zpci_sic_iib), GFP_KERNEL);
+ if (!zpci_aipb)
+ return -ENOMEM;
+
+ aift->sbv = airq_iv_create(ZPCI_NR_DEVICES, AIRQ_IV_ALLOC, 0);
+ if (!aift->sbv) {
+ rc = -ENOMEM;
+ goto free_aipb;
+ }
+ zpci_aif_sbv = aift->sbv;
+ size = get_order(PAGE_ALIGN(ZPCI_NR_DEVICES *
+ sizeof(struct zpci_gaite)));
+ page = alloc_pages(GFP_KERNEL | __GFP_ZERO, size);
+ if (!page) {
+ rc = -ENOMEM;
+ goto free_sbv;
+ }
+ aift->gait = (struct zpci_gaite *)page_to_phys(page);
+
+ zpci_aipb->aipb.faisb = virt_to_phys(aift->sbv->vector);
+ zpci_aipb->aipb.gait = virt_to_phys(aift->gait);
+ zpci_aipb->aipb.afi = nisc;
+ zpci_aipb->aipb.faal = ZPCI_NR_DEVICES;
+
+ /* Setup Adapter Event Notification Interpretation */
+ if (zpci_set_irq_ctrl(SIC_SET_AENI_CONTROLS, 0, zpci_aipb)) {
+ rc = -EIO;
+ goto free_gait;
+ }
+
+ return 0;
+
+free_gait:
+ size = get_order(PAGE_ALIGN(ZPCI_NR_DEVICES *
+ sizeof(struct zpci_gaite)));
+ free_pages((unsigned long)aift->gait, size);
+free_sbv:
+ airq_iv_release(aift->sbv);
+ zpci_aif_sbv = 0;
+free_aipb:
+ kfree(zpci_aipb);
+ zpci_aipb = 0;
+
+ return rc;
+}
+
+static int zpci_reset_aipb(u8 nisc)
+{
+ /*
+ * AEN registration can only happen once per system boot. If
+ * an aipb already exists then AEN was already registered and
+ * we can re-use the aipb contents. This can only happen if
+ * the KVM module was removed and re-inserted.
+ */
+ if (zpci_aipb->aipb.faal != ZPCI_NR_DEVICES ||
+ zpci_aipb->aipb.afi != nisc) {
+ return -EINVAL;
+ }
+ aift->sbv = zpci_aif_sbv;
+ aift->gait = (struct zpci_gaite *)zpci_aipb->aipb.gait;
+
+ return 0;
+}
+
+int kvm_s390_pci_aen_init(u8 nisc)
+{
+ int rc = 0;
+
+ /* If already enabled for AEN, bail out now */
+ if (aift->gait || aift->sbv)
+ return -EPERM;
+
+ mutex_lock(&aift->aift_lock);
+ aift->kzdev = kcalloc(ZPCI_NR_DEVICES, sizeof(struct kvm_zdev),
+ GFP_KERNEL);
+ if (!aift->kzdev) {
+ rc = -ENOMEM;
+ goto unlock;
+ }
+
+ if (!zpci_aipb)
+ rc = zpci_setup_aipb(nisc);
+ else
+ rc = zpci_reset_aipb(nisc);
+ if (rc)
+ goto free_zdev;
+
+ /* Enable floating IRQs */
+ if (__set_irq_noiib(SIC_IRQ_MODE_SINGLE, nisc)) {
+ rc = -EIO;
+ kvm_s390_pci_aen_exit();
+ }
+
+ goto unlock;
+
+free_zdev:
+ kfree(aift->kzdev);
+unlock:
+ mutex_unlock(&aift->aift_lock);
+ return rc;
+}
+
int kvm_s390_pci_dev_open(struct zpci_dev *zdev)
{
struct kvm_zdev *kzdev;
@@ -36,3 +177,15 @@ void kvm_s390_pci_dev_release(struct zpci_dev *zdev)
kfree(kzdev);
}
EXPORT_SYMBOL_GPL(kvm_s390_pci_dev_release);
+
+int kvm_s390_pci_init(void)
+{
+ aift = kzalloc(sizeof(struct zpci_aift), GFP_KERNEL);
+ if (!aift)
+ return -ENOMEM;
+
+ spin_lock_init(&aift->gait_lock);
+ mutex_init(&aift->aift_lock);
+
+ return 0;
+}
diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
index ce93978e8913..a6a62db792b6 100644
--- a/arch/s390/kvm/pci.h
+++ b/arch/s390/kvm/pci.h
@@ -12,10 +12,57 @@

#include <linux/kvm_host.h>
#include <linux/pci.h>
+#include <linux/mutex.h>
+#include <asm/airq.h>
+#include <asm/cpu.h>

struct kvm_zdev {
struct zpci_dev *zdev;
struct kvm *kvm;
};

+struct zpci_gaite {
+ u32 gisa;
+ u8 gisc;
+ u8 count;
+ u8 reserved;
+ u8 aisbo;
+ u64 aisb;
+};
+
+struct zpci_aift {
+ struct zpci_gaite *gait;
+ struct airq_iv *sbv;
+ struct kvm_zdev **kzdev;
+ spinlock_t gait_lock; /* Protects the gait, used during AEN forward */
+ struct mutex aift_lock; /* Protects the other structures in aift */
+};
+
+extern struct zpci_aift *aift;
+
+int kvm_s390_pci_aen_init(u8 nisc);
+void kvm_s390_pci_aen_exit(void);
+
+int kvm_s390_pci_init(void);
+
+static inline bool kvm_s390_pci_interp_allowed(void)
+{
+ struct cpuid cpu_id;
+
+ get_cpu_id(&cpu_id);
+ switch (cpu_id.machine) {
+ case 0x2817:
+ case 0x2818:
+ case 0x2827:
+ case 0x2828:
+ case 0x2964:
+ case 0x2965:
+ /* No SHM on certain machines */
+ return false;
+ default:
+ return (IS_ENABLED(CONFIG_VFIO_PCI) && sclp.has_zpci_lsi &&
+ sclp.has_aeni && sclp.has_aisi && sclp.has_aisii);
+ }
+}
+
#endif /* __KVM_S390_PCI_H */
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index a86cd1cbb80e..f0a439c43395 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -61,6 +61,12 @@ DEFINE_STATIC_KEY_FALSE(have_mio);

static struct kmem_cache *zdev_fmb_cache;

+/* AEN structures that must be preserved over KVM module re-insertion */
+union zpci_sic_iib *zpci_aipb;
+EXPORT_SYMBOL_GPL(zpci_aipb);
+struct airq_iv *zpci_aif_sbv;
+EXPORT_SYMBOL_GPL(zpci_aif_sbv);
+
struct zpci_dev *get_zdev_by_fid(u32 fid)
{
struct zpci_dev *tmp, *zdev = NULL;
--
2.27.0

2022-04-05 01:43:51

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v5 05/21] s390/airq: pass more TPI info to airq handlers

A subsequent patch will introduce an airq handler that requires additional
TPI information beyond directed vs floating, so pass the entire tpi_info
structure via the handler. Only pci actually uses this information today,
for the other airq handlers this is effectively a no-op.

Reviewed-by: Eric Farman <[email protected]>
Reviewed-by: Claudio Imbrenda <[email protected]>
Reviewed-by: Pierre Morel <[email protected]>
Reviewed-by: Thomas Huth <[email protected]>
Acked-by: Christian Borntraeger <[email protected]>
Acked-by: Cornelia Huck <[email protected]>
Signed-off-by: Matthew Rosato <[email protected]>
---
arch/s390/include/asm/airq.h | 3 ++-
arch/s390/kvm/interrupt.c | 4 +++-
arch/s390/pci/pci_irq.c | 9 +++++++--
drivers/s390/cio/airq.c | 2 +-
drivers/s390/cio/qdio_thinint.c | 6 ++++--
drivers/s390/crypto/ap_bus.c | 9 ++++++---
drivers/s390/virtio/virtio_ccw.c | 4 +++-
7 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/arch/s390/include/asm/airq.h b/arch/s390/include/asm/airq.h
index 01936fdfaddb..7918a7d09028 100644
--- a/arch/s390/include/asm/airq.h
+++ b/arch/s390/include/asm/airq.h
@@ -12,10 +12,11 @@

#include <linux/bit_spinlock.h>
#include <linux/dma-mapping.h>
+#include <asm/tpi.h>

struct airq_struct {
struct hlist_node list; /* Handler queueing. */
- void (*handler)(struct airq_struct *airq, bool floating);
+ void (*handler)(struct airq_struct *airq, struct tpi_info *tpi_info);
u8 *lsi_ptr; /* Local-Summary-Indicator pointer */
u8 lsi_mask; /* Local-Summary-Indicator mask */
u8 isc; /* Interrupt-subclass */
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 9b30beac904d..250119a26c60 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -28,6 +28,7 @@
#include <asm/switch_to.h>
#include <asm/nmi.h>
#include <asm/airq.h>
+#include <asm/tpi.h>
#include "kvm-s390.h"
#include "gaccess.h"
#include "trace-s390.h"
@@ -3311,7 +3312,8 @@ 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, bool floating)
+static void gib_alert_irq_handler(struct airq_struct *airq,
+ struct tpi_info *tpi_info)
{
inc_irq_stat(IRQIO_GAL);
process_gib_alert_list();
diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index 500cd2dbdf53..b805c75252ed 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -11,6 +11,7 @@

#include <asm/isc.h>
#include <asm/airq.h>
+#include <asm/tpi.h>

static enum {FLOATING, DIRECTED} irq_delivery;

@@ -216,8 +217,11 @@ static void zpci_handle_fallback_irq(void)
}
}

-static void zpci_directed_irq_handler(struct airq_struct *airq, bool floating)
+static void zpci_directed_irq_handler(struct airq_struct *airq,
+ struct tpi_info *tpi_info)
{
+ bool floating = !tpi_info->directed_irq;
+
if (floating) {
inc_irq_stat(IRQIO_PCF);
zpci_handle_fallback_irq();
@@ -227,7 +231,8 @@ static void zpci_directed_irq_handler(struct airq_struct *airq, bool floating)
}
}

-static void zpci_floating_irq_handler(struct airq_struct *airq, bool floating)
+static void zpci_floating_irq_handler(struct airq_struct *airq,
+ struct tpi_info *tpi_info)
{
unsigned long si, ai;
struct airq_iv *aibv;
diff --git a/drivers/s390/cio/airq.c b/drivers/s390/cio/airq.c
index c0ed364bf446..230eb5b5b64e 100644
--- a/drivers/s390/cio/airq.c
+++ b/drivers/s390/cio/airq.c
@@ -99,7 +99,7 @@ static irqreturn_t do_airq_interrupt(int irq, void *dummy)
rcu_read_lock();
hlist_for_each_entry_rcu(airq, head, list)
if ((*airq->lsi_ptr & airq->lsi_mask) != 0)
- airq->handler(airq, !tpi_info->directed_irq);
+ airq->handler(airq, tpi_info);
rcu_read_unlock();

return IRQ_HANDLED;
diff --git a/drivers/s390/cio/qdio_thinint.c b/drivers/s390/cio/qdio_thinint.c
index 8e09bf3a2fcd..9b9335dd06db 100644
--- a/drivers/s390/cio/qdio_thinint.c
+++ b/drivers/s390/cio/qdio_thinint.c
@@ -15,6 +15,7 @@
#include <asm/qdio.h>
#include <asm/airq.h>
#include <asm/isc.h>
+#include <asm/tpi.h>

#include "cio.h"
#include "ioasm.h"
@@ -93,9 +94,10 @@ static inline u32 clear_shared_ind(void)
/**
* tiqdio_thinint_handler - thin interrupt handler for qdio
* @airq: pointer to adapter interrupt descriptor
- * @floating: flag to recognize floating vs. directed interrupts (unused)
+ * @tpi_info: interrupt information (e.g. floating vs directed -- unused)
*/
-static void tiqdio_thinint_handler(struct airq_struct *airq, bool floating)
+static void tiqdio_thinint_handler(struct airq_struct *airq,
+ struct tpi_info *tpi_info)
{
u64 irq_time = S390_lowcore.int_clock;
u32 si_used = clear_shared_ind();
diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c
index fdf16cb70881..6cb8a2802ef8 100644
--- a/drivers/s390/crypto/ap_bus.c
+++ b/drivers/s390/crypto/ap_bus.c
@@ -27,6 +27,7 @@
#include <linux/kthread.h>
#include <linux/mutex.h>
#include <asm/airq.h>
+#include <asm/tpi.h>
#include <linux/atomic.h>
#include <asm/isc.h>
#include <linux/hrtimer.h>
@@ -131,7 +132,8 @@ static int ap_max_adapter_id = 63;
static struct bus_type ap_bus_type;

/* Adapter interrupt definitions */
-static void ap_interrupt_handler(struct airq_struct *airq, bool floating);
+static void ap_interrupt_handler(struct airq_struct *airq,
+ struct tpi_info *tpi_info);

static bool ap_irq_flag;

@@ -452,9 +454,10 @@ static enum hrtimer_restart ap_poll_timeout(struct hrtimer *unused)
/**
* ap_interrupt_handler() - Schedule ap_tasklet on interrupt
* @airq: pointer to adapter interrupt descriptor
- * @floating: ignored
+ * @tpi_info: ignored
*/
-static void ap_interrupt_handler(struct airq_struct *airq, bool floating)
+static void ap_interrupt_handler(struct airq_struct *airq,
+ struct tpi_info *tpi_info)
{
inc_irq_stat(IRQIO_APB);
tasklet_schedule(&ap_tasklet);
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index d35e7a3f7067..52c376d15978 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -33,6 +33,7 @@
#include <asm/virtio-ccw.h>
#include <asm/isc.h>
#include <asm/airq.h>
+#include <asm/tpi.h>

/*
* virtio related functions
@@ -203,7 +204,8 @@ static void drop_airq_indicator(struct virtqueue *vq, struct airq_info *info)
write_unlock_irqrestore(&info->lock, flags);
}

-static void virtio_airq_handler(struct airq_struct *airq, bool floating)
+static void virtio_airq_handler(struct airq_struct *airq,
+ struct tpi_info *tpi_info)
{
struct airq_info *info = container_of(airq, struct airq_info, airq);
unsigned long ai;
--
2.27.0

2022-04-05 01:43:52

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v5 13/21] KVM: s390: mechanism to enable guest zPCI Interpretation

The guest must have access to certain facilities in order to allow
interpretive execution of zPCI instructions and adapter event
notifications. However, there are some cases where a guest might
disable interpretation -- provide a mechanism via which we can defer
enabling the associated zPCI interpretation facilities until the guest
indicates it wishes to use them.

Acked-by: Pierre Morel <[email protected]>
Signed-off-by: Matthew Rosato <[email protected]>
---
arch/s390/include/asm/kvm_host.h | 4 ++++
arch/s390/kvm/kvm-s390.c | 37 ++++++++++++++++++++++++++++++++
arch/s390/kvm/kvm-s390.h | 10 +++++++++
3 files changed, 51 insertions(+)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index c1518a505060..8e381603b6a7 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -254,7 +254,10 @@ struct kvm_s390_sie_block {
#define ECB2_IEP 0x20
#define ECB2_PFMFI 0x08
#define ECB2_ESCA 0x04
+#define ECB2_ZPCI_LSI 0x02
__u8 ecb2; /* 0x0062 */
+#define ECB3_AISI 0x20
+#define ECB3_AISII 0x10
#define ECB3_DEA 0x08
#define ECB3_AES 0x04
#define ECB3_RI 0x01
@@ -940,6 +943,7 @@ struct kvm_arch{
int use_cmma;
int use_pfmfi;
int use_skf;
+ int use_zpci_interp;
int user_cpu_state_ctrl;
int user_sigp;
int user_stsi;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index a9eb8b75af93..327649ddddce 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1029,6 +1029,41 @@ static int kvm_s390_vm_set_crypto(struct kvm *kvm, struct kvm_device_attr *attr)
return 0;
}

+static void kvm_s390_vcpu_pci_setup(struct kvm_vcpu *vcpu)
+{
+ /* Only set the ECB bits after guest requests zPCI interpretation */
+ if (!vcpu->kvm->arch.use_zpci_interp)
+ return;
+
+ vcpu->arch.sie_block->ecb2 |= ECB2_ZPCI_LSI;
+ vcpu->arch.sie_block->ecb3 |= ECB3_AISII + ECB3_AISI;
+}
+
+/* Must be called with kvm->lock held */
+void kvm_s390_vcpu_pci_enable_interp(struct kvm *kvm)
+{
+ struct kvm_vcpu *vcpu;
+ unsigned long i;
+
+ /*
+ * If host is configured for PCI and the necessary facilities are
+ * available, turn on interpretation for the life of this guest
+ */
+ if (!kvm_s390_pci_interp_allowed())
+ return;
+
+ kvm->arch.use_zpci_interp = 1;
+
+ kvm_s390_vcpu_block_all(kvm);
+
+ kvm_for_each_vcpu(i, vcpu, kvm) {
+ kvm_s390_vcpu_pci_setup(vcpu);
+ kvm_s390_sync_request(KVM_REQ_VSIE_RESTART, vcpu);
+ }
+
+ kvm_s390_vcpu_unblock_all(kvm);
+}
+
static void kvm_s390_sync_request_broadcast(struct kvm *kvm, int req)
{
unsigned long cx;
@@ -3329,6 +3364,8 @@ static int kvm_s390_vcpu_setup(struct kvm_vcpu *vcpu)

kvm_s390_vcpu_crypto_setup(vcpu);

+ kvm_s390_vcpu_pci_setup(vcpu);
+
mutex_lock(&vcpu->kvm->lock);
if (kvm_s390_pv_is_protected(vcpu->kvm)) {
rc = kvm_s390_pv_create_cpu(vcpu, &uvrc, &uvrrc);
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index 497d52a83c78..975cffaf13de 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -507,6 +507,16 @@ void kvm_s390_reinject_machine_check(struct kvm_vcpu *vcpu,
*/
void kvm_s390_vcpu_crypto_reset_all(struct kvm *kvm);

+/**
+ * kvm_s390_vcpu_pci_enable_interp
+ *
+ * Set the associated PCI attributes for each vcpu to allow for zPCI Load/Store
+ * interpretation as well as adapter interruption forwarding.
+ *
+ * @kvm: the KVM guest
+ */
+void kvm_s390_vcpu_pci_enable_interp(struct kvm *kvm);
+
/**
* diag9c_forwarding_hz
*
--
2.27.0

2022-04-05 02:04:29

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v5 14/21] KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding

These routines will be wired into a kvm ioctl in order to respond to
requests to enable / disable a device for Adapter Event Notifications /
Adapter Interuption Forwarding.

Signed-off-by: Matthew Rosato <[email protected]>
---
arch/s390/kvm/pci.c | 247 +++++++++++++++++++++++++++++++++++++++
arch/s390/kvm/pci.h | 1 +
arch/s390/pci/pci_insn.c | 1 +
3 files changed, 249 insertions(+)

diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
index 01bd8a2f503b..f0fd68569a9d 100644
--- a/arch/s390/kvm/pci.c
+++ b/arch/s390/kvm/pci.c
@@ -11,6 +11,7 @@
#include <linux/pci.h>
#include <asm/pci.h>
#include <asm/pci_insn.h>
+#include <asm/pci_io.h>
#include "pci.h"

struct zpci_aift *aift;
@@ -152,6 +153,252 @@ int kvm_s390_pci_aen_init(u8 nisc)
return rc;
}

+/* Modify PCI: Register floating adapter interruption forwarding */
+static int kvm_zpci_set_airq(struct zpci_dev *zdev)
+{
+ u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_REG_INT);
+ struct zpci_fib fib = {};
+ u8 status;
+
+ fib.fmt0.isc = zdev->kzdev->fib.fmt0.isc;
+ fib.fmt0.sum = 1; /* enable summary notifications */
+ fib.fmt0.noi = airq_iv_end(zdev->aibv);
+ fib.fmt0.aibv = virt_to_phys(zdev->aibv->vector);
+ fib.fmt0.aibvo = 0;
+ fib.fmt0.aisb = virt_to_phys(aift->sbv->vector + (zdev->aisb / 64) * 8);
+ fib.fmt0.aisbo = zdev->aisb & 63;
+ fib.gd = zdev->gisa;
+
+ return zpci_mod_fc(req, &fib, &status) ? -EIO : 0;
+}
+
+/* Modify PCI: Unregister floating adapter interruption forwarding */
+static int kvm_zpci_clear_airq(struct zpci_dev *zdev)
+{
+ u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_DEREG_INT);
+ struct zpci_fib fib = {};
+ u8 cc, status;
+
+ fib.gd = zdev->gisa;
+
+ cc = zpci_mod_fc(req, &fib, &status);
+ if (cc == 3 || (cc == 1 && status == 24))
+ /* Function already gone or IRQs already deregistered. */
+ cc = 0;
+
+ return cc ? -EIO : 0;
+}
+
+static inline void unaccount_mem(unsigned long nr_pages)
+{
+ struct user_struct *user = get_uid(current_user());
+
+ if (user)
+ atomic_long_sub(nr_pages, &user->locked_vm);
+ if (current->mm)
+ atomic64_sub(nr_pages, &current->mm->pinned_vm);
+}
+
+static inline int account_mem(unsigned long nr_pages)
+{
+ struct user_struct *user = get_uid(current_user());
+ unsigned long page_limit, cur_pages, new_pages;
+
+ page_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
+
+ do {
+ cur_pages = atomic_long_read(&user->locked_vm);
+ new_pages = cur_pages + nr_pages;
+ if (new_pages > page_limit)
+ return -ENOMEM;
+ } while (atomic_long_cmpxchg(&user->locked_vm, cur_pages,
+ new_pages) != cur_pages);
+
+ atomic64_add(nr_pages, &current->mm->pinned_vm);
+
+ return 0;
+}
+
+static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
+ bool assist)
+{
+ struct page *pages[1], *aibv_page, *aisb_page = NULL;
+ unsigned int msi_vecs, idx;
+ struct zpci_gaite *gaite;
+ unsigned long hva, bit;
+ struct kvm *kvm;
+ phys_addr_t gaddr;
+ int rc = 0, gisc, npages, pcount = 0;
+
+ /*
+ * Interrupt forwarding is only applicable if the device is already
+ * enabled for interpretation
+ */
+ if (zdev->gisa == 0)
+ return -EINVAL;
+
+ kvm = zdev->kzdev->kvm;
+ msi_vecs = min_t(unsigned int, fib->fmt0.noi, zdev->max_msi);
+
+ /* Get the associated forwarding ISC - if invalid, return the error */
+ gisc = kvm_s390_gisc_register(kvm, fib->fmt0.isc);
+ if (gisc < 0)
+ return gisc;
+
+ /* Replace AIBV address */
+ idx = srcu_read_lock(&kvm->srcu);
+ hva = gfn_to_hva(kvm, gpa_to_gfn((gpa_t)fib->fmt0.aibv));
+ npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM, pages);
+ srcu_read_unlock(&kvm->srcu, idx);
+ if (npages < 1) {
+ rc = -EIO;
+ goto out;
+ }
+ aibv_page = pages[0];
+ pcount++;
+ gaddr = page_to_phys(aibv_page) + (fib->fmt0.aibv & ~PAGE_MASK);
+ fib->fmt0.aibv = gaddr;
+
+ /* Pin the guest AISB if one was specified */
+ if (fib->fmt0.sum == 1) {
+ idx = srcu_read_lock(&kvm->srcu);
+ hva = gfn_to_hva(kvm, gpa_to_gfn((gpa_t)fib->fmt0.aisb));
+ npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM,
+ pages);
+ srcu_read_unlock(&kvm->srcu, idx);
+ if (npages < 1) {
+ rc = -EIO;
+ goto unpin1;
+ }
+ aisb_page = pages[0];
+ pcount++;
+ }
+
+ /* Account for pinned pages, roll back on failure */
+ if (account_mem(pcount))
+ goto unpin2;
+
+ /* AISB must be allocated before we can fill in GAITE */
+ mutex_lock(&aift->aift_lock);
+ bit = airq_iv_alloc_bit(aift->sbv);
+ if (bit == -1UL)
+ goto unlock;
+ zdev->aisb = bit; /* store the summary bit number */
+ zdev->aibv = airq_iv_create(msi_vecs, AIRQ_IV_DATA |
+ AIRQ_IV_BITLOCK |
+ AIRQ_IV_GUESTVEC,
+ phys_to_virt(fib->fmt0.aibv));
+
+ spin_lock_irq(&aift->gait_lock);
+ gaite = (struct zpci_gaite *)aift->gait + (zdev->aisb *
+ sizeof(struct zpci_gaite));
+
+ /* If assist not requested, host will get all alerts */
+ if (assist)
+ gaite->gisa = (u32)virt_to_phys(&kvm->arch.sie_page2->gisa);
+ else
+ gaite->gisa = 0;
+
+ gaite->gisc = fib->fmt0.isc;
+ gaite->count++;
+ gaite->aisbo = fib->fmt0.aisbo;
+ gaite->aisb = virt_to_phys(page_address(aisb_page) + (fib->fmt0.aisb &
+ ~PAGE_MASK));
+ aift->kzdev[zdev->aisb] = zdev->kzdev;
+ spin_unlock_irq(&aift->gait_lock);
+
+ /* Update guest FIB for re-issue */
+ fib->fmt0.aisbo = zdev->aisb & 63;
+ fib->fmt0.aisb = virt_to_phys(aift->sbv->vector + (zdev->aisb / 64) * 8);
+ fib->fmt0.isc = gisc;
+
+ /* Save some guest fib values in the host for later use */
+ zdev->kzdev->fib.fmt0.isc = fib->fmt0.isc;
+ zdev->kzdev->fib.fmt0.aibv = fib->fmt0.aibv;
+ mutex_unlock(&aift->aift_lock);
+
+ /* Issue the clp to setup the irq now */
+ rc = kvm_zpci_set_airq(zdev);
+ return rc;
+
+unlock:
+ mutex_unlock(&aift->aift_lock);
+unpin2:
+ if (fib->fmt0.sum == 1)
+ unpin_user_page(aisb_page);
+unpin1:
+ unpin_user_page(aibv_page);
+out:
+ return rc;
+}
+
+static int kvm_s390_pci_aif_disable(struct zpci_dev *zdev, bool force)
+{
+ struct kvm_zdev *kzdev = zdev->kzdev;
+ struct zpci_gaite *gaite;
+ struct page *vpage = NULL, *spage = NULL;
+ int rc, pcount = 0;
+ u8 isc;
+
+ if (zdev->gisa == 0)
+ return -EINVAL;
+
+ mutex_lock(&aift->aift_lock);
+
+ /*
+ * If the clear fails due to an error, leave now unless we know this
+ * device is about to go away (force) -- In that case clear the GAITE
+ * regardless.
+ */
+ rc = kvm_zpci_clear_airq(zdev);
+ if (rc && !force)
+ goto out;
+
+ if (zdev->kzdev->fib.fmt0.aibv == 0)
+ goto out;
+ spin_lock_irq(&aift->gait_lock);
+ gaite = (struct zpci_gaite *)aift->gait + (zdev->aisb *
+ sizeof(struct zpci_gaite));
+ isc = gaite->gisc;
+ gaite->count--;
+ if (gaite->count == 0) {
+ /* Release guest AIBV and AISB */
+ vpage = phys_to_page(kzdev->fib.fmt0.aibv);
+ if (gaite->aisb != 0)
+ spage = phys_to_page(gaite->aisb);
+ /* Clear the GAIT entry */
+ gaite->aisb = 0;
+ gaite->gisc = 0;
+ gaite->aisbo = 0;
+ gaite->gisa = 0;
+ aift->kzdev[zdev->aisb] = 0;
+ /* Clear zdev info */
+ airq_iv_free_bit(aift->sbv, zdev->aisb);
+ airq_iv_release(zdev->aibv);
+ zdev->aisb = 0;
+ zdev->aibv = NULL;
+ }
+ spin_unlock_irq(&aift->gait_lock);
+ kvm_s390_gisc_unregister(kzdev->kvm, isc);
+ kzdev->fib.fmt0.isc = 0;
+ kzdev->fib.fmt0.aibv = 0;
+
+ if (vpage) {
+ unpin_user_page(vpage);
+ pcount++;
+ }
+ if (spage) {
+ unpin_user_page(spage);
+ pcount++;
+ }
+ if (pcount > 0)
+ unaccount_mem(pcount);
+out:
+ mutex_unlock(&aift->aift_lock);
+
+ return rc;
+}
+
int kvm_s390_pci_dev_open(struct zpci_dev *zdev)
{
struct kvm_zdev *kzdev;
diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
index d4997e2236ef..b4bf3d1d4b66 100644
--- a/arch/s390/kvm/pci.h
+++ b/arch/s390/kvm/pci.h
@@ -20,6 +20,7 @@
struct kvm_zdev {
struct zpci_dev *zdev;
struct kvm *kvm;
+ struct zpci_fib fib;
};

struct zpci_gaite {
diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
index 4c6967b73932..cd9fb186a6be 100644
--- a/arch/s390/pci/pci_insn.c
+++ b/arch/s390/pci/pci_insn.c
@@ -60,6 +60,7 @@ u8 zpci_mod_fc(u64 req, struct zpci_fib *fib, u8 *status)

return cc;
}
+EXPORT_SYMBOL_GPL(zpci_mod_fc);

/* Refresh PCI Translations */
static inline u8 __rpcit(u64 fn, u64 addr, u64 range, u8 *status)
--
2.27.0

2022-04-05 02:25:07

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v5 18/21] vfio-pci/zdev: different maxstbl for interpreted devices

When doing load/store interpretation, the maximum store block length is
determined by the underlying firmware, not the host kernel API. Reflect
that in the associated Query PCI Function Group clp capability and let
userspace decide which is appropriate to present to the guest.

Signed-off-by: Matthew Rosato <[email protected]>
---
drivers/vfio/pci/vfio_pci_zdev.c | 6 ++++--
include/uapi/linux/vfio_zdev.h | 4 ++++
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c
index 4a653ce480c7..d3ca58d9d8ec 100644
--- a/drivers/vfio/pci/vfio_pci_zdev.c
+++ b/drivers/vfio/pci/vfio_pci_zdev.c
@@ -44,14 +44,16 @@ static int zpci_group_cap(struct zpci_dev *zdev, struct vfio_info_cap *caps)
{
struct vfio_device_info_cap_zpci_group cap = {
.header.id = VFIO_DEVICE_INFO_CAP_ZPCI_GROUP,
- .header.version = 1,
+ .header.version = 2,
.dasm = zdev->dma_mask,
.msi_addr = zdev->msi_addr,
.flags = VFIO_DEVICE_INFO_ZPCI_FLAG_REFRESH,
.mui = zdev->fmb_update,
.noi = zdev->max_msi,
.maxstbl = ZPCI_MAX_WRITE_SIZE,
- .version = zdev->version
+ .version = zdev->version,
+ .reserved = 0,
+ .imaxstbl = zdev->maxstbl
};

return vfio_info_add_capability(caps, &cap.header, sizeof(cap));
diff --git a/include/uapi/linux/vfio_zdev.h b/include/uapi/linux/vfio_zdev.h
index 78c022af3d29..77f2aff1f27e 100644
--- a/include/uapi/linux/vfio_zdev.h
+++ b/include/uapi/linux/vfio_zdev.h
@@ -50,6 +50,10 @@ struct vfio_device_info_cap_zpci_group {
__u16 noi; /* Maximum number of MSIs */
__u16 maxstbl; /* Maximum Store Block Length */
__u8 version; /* Supported PCI Version */
+ /* End of version 1 */
+ __u8 reserved;
+ __u16 imaxstbl; /* Maximum Interpreted Store Block Length */
+ /* End of version 2 */
};

/**
--
2.27.0

2022-04-05 02:26:07

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v5 01/21] s390/sclp: detect the zPCI load/store interpretation facility

Detect the zPCI Load/Store Interpretation facility.

Reviewed-by: Eric Farman <[email protected]>
Reviewed-by: Christian Borntraeger <[email protected]>
Reviewed-by: Claudio Imbrenda <[email protected]>
Signed-off-by: Matthew Rosato <[email protected]>
---
arch/s390/include/asm/sclp.h | 1 +
drivers/s390/char/sclp_early.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/arch/s390/include/asm/sclp.h b/arch/s390/include/asm/sclp.h
index 04cb1e7582a6..06c0b5eb8ef5 100644
--- a/arch/s390/include/asm/sclp.h
+++ b/arch/s390/include/asm/sclp.h
@@ -87,6 +87,7 @@ struct sclp_info {
unsigned char has_diag318 : 1;
unsigned char has_sipl : 1;
unsigned char has_dirq : 1;
+ unsigned char has_zpci_lsi : 1;
unsigned int ibc;
unsigned int mtid;
unsigned int mtid_cp;
diff --git a/drivers/s390/char/sclp_early.c b/drivers/s390/char/sclp_early.c
index e9943a86c361..b88dd0da1231 100644
--- a/drivers/s390/char/sclp_early.c
+++ b/drivers/s390/char/sclp_early.c
@@ -45,6 +45,7 @@ static void __init sclp_early_facilities_detect(void)
sclp.has_gisaf = !!(sccb->fac118 & 0x08);
sclp.has_hvs = !!(sccb->fac119 & 0x80);
sclp.has_kss = !!(sccb->fac98 & 0x01);
+ sclp.has_zpci_lsi = !!(sccb->fac118 & 0x01);
if (sccb->fac85 & 0x02)
S390_lowcore.machine_flags |= MACHINE_FLAG_ESOP;
if (sccb->fac91 & 0x40)
--
2.27.0

2022-04-05 02:27:38

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v5 03/21] s390/sclp: detect the AENI facility

Detect the Adapter Event Notification Interpretation facility.

Reviewed-by: Eric Farman <[email protected]>
Reviewed-by: Christian Borntraeger <[email protected]>
Reviewed-by: Claudio Imbrenda <[email protected]>
Signed-off-by: Matthew Rosato <[email protected]>
---
arch/s390/include/asm/sclp.h | 1 +
drivers/s390/char/sclp_early.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/arch/s390/include/asm/sclp.h b/arch/s390/include/asm/sclp.h
index be14644fe113..ff359df62154 100644
--- a/arch/s390/include/asm/sclp.h
+++ b/arch/s390/include/asm/sclp.h
@@ -89,6 +89,7 @@ struct sclp_info {
unsigned char has_dirq : 1;
unsigned char has_zpci_lsi : 1;
unsigned char has_aisii : 1;
+ unsigned char has_aeni : 1;
unsigned int ibc;
unsigned int mtid;
unsigned int mtid_cp;
diff --git a/drivers/s390/char/sclp_early.c b/drivers/s390/char/sclp_early.c
index 29fee179e197..e9af01b4c97a 100644
--- a/drivers/s390/char/sclp_early.c
+++ b/drivers/s390/char/sclp_early.c
@@ -46,6 +46,7 @@ static void __init sclp_early_facilities_detect(void)
sclp.has_hvs = !!(sccb->fac119 & 0x80);
sclp.has_kss = !!(sccb->fac98 & 0x01);
sclp.has_aisii = !!(sccb->fac118 & 0x40);
+ sclp.has_aeni = !!(sccb->fac118 & 0x20);
sclp.has_zpci_lsi = !!(sccb->fac118 & 0x01);
if (sccb->fac85 & 0x02)
S390_lowcore.machine_flags |= MACHINE_FLAG_ESOP;
--
2.27.0

2022-04-05 02:55:44

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v5 10/21] 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/pci.h | 3 +++
arch/s390/kvm/Makefile | 1 +
arch/s390/kvm/pci.c | 38 +++++++++++++++++++++++++++++++++++++
arch/s390/kvm/pci.h | 21 ++++++++++++++++++++
4 files changed, 63 insertions(+)
create mode 100644 arch/s390/kvm/pci.c
create mode 100644 arch/s390/kvm/pci.h

diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
index 4c5b8fbc2079..9eb20cebaa18 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 26f4a74e5ce4..00cf6853d93f 100644
--- a/arch/s390/kvm/Makefile
+++ b/arch/s390/kvm/Makefile
@@ -10,4 +10,5 @@ ccflags-y := -Ivirt/kvm -Iarch/s390/kvm
kvm-y += kvm-s390.o intercept.o interrupt.o priv.o sigp.o
kvm-y += 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..213be236c05a
--- /dev/null
+++ b/arch/s390/kvm/pci.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * s390 kvm PCI passthrough support
+ *
+ * Copyright IBM Corp. 2022
+ *
+ * Author(s): Matthew Rosato <[email protected]>
+ */
+
+#include <linux/kvm_host.h>
+#include <linux/pci.h>
+#include "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);
diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
new file mode 100644
index 000000000000..ce93978e8913
--- /dev/null
+++ b/arch/s390/kvm/pci.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * s390 kvm PCI passthrough support
+ *
+ * Copyright IBM Corp. 2022
+ *
+ * Author(s): Matthew Rosato <[email protected]>
+ */
+
+#ifndef __KVM_S390_PCI_H
+#define __KVM_S390_PCI_H
+
+#include <linux/kvm_host.h>
+#include <linux/pci.h>
+
+struct kvm_zdev {
+ struct zpci_dev *zdev;
+ struct kvm *kvm;
+};
+
+#endif /* __KVM_S390_PCI_H */
--
2.27.0

2022-04-05 02:57:34

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v5 07/21] s390/pci: externalize the SIC operation controls and routine

A subsequent patch will be issuing SIC from KVM -- export the necessary
routine and make the operation control definitions available from a header.
Because the routine will now be exported, let's rename __zpci_set_irq_ctrl
to zpci_set_irq_ctrl and get rid of the zero'd iib wrapper function of
the same name.

Reviewed-by: Niklas Schnelle <[email protected]>
Reviewed-by: Claudio Imbrenda <[email protected]>
Reviewed-by: Pierre Morel <[email protected]>
Signed-off-by: Matthew Rosato <[email protected]>
---
arch/s390/include/asm/pci_insn.h | 17 +++++++++--------
arch/s390/pci/pci_insn.c | 3 ++-
arch/s390/pci/pci_irq.c | 26 ++++++++++++--------------
3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/s390/include/asm/pci_insn.h b/arch/s390/include/asm/pci_insn.h
index 61cf9531f68f..5331082fa516 100644
--- a/arch/s390/include/asm/pci_insn.h
+++ b/arch/s390/include/asm/pci_insn.h
@@ -98,6 +98,14 @@ struct zpci_fib {
u32 gd;
} __packed __aligned(8);

+/* Set Interruption Controls Operation Controls */
+#define SIC_IRQ_MODE_ALL 0
+#define SIC_IRQ_MODE_SINGLE 1
+#define SIC_IRQ_MODE_DIRECT 4
+#define SIC_IRQ_MODE_D_ALL 16
+#define SIC_IRQ_MODE_D_SINGLE 17
+#define SIC_IRQ_MODE_SET_CPU 18
+
/* directed interruption information block */
struct zpci_diib {
u32 : 1;
@@ -134,13 +142,6 @@ int __zpci_store(u64 data, u64 req, u64 offset);
int zpci_store(const volatile void __iomem *addr, u64 data, unsigned long len);
int __zpci_store_block(const u64 *data, u64 req, u64 offset);
void zpci_barrier(void);
-int __zpci_set_irq_ctrl(u16 ctl, u8 isc, union zpci_sic_iib *iib);
-
-static inline int zpci_set_irq_ctrl(u16 ctl, u8 isc)
-{
- union zpci_sic_iib iib = {{0}};
-
- return __zpci_set_irq_ctrl(ctl, isc, &iib);
-}
+int zpci_set_irq_ctrl(u16 ctl, u8 isc, union zpci_sic_iib *iib);

#endif
diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
index 1710d006ee93..4c6967b73932 100644
--- a/arch/s390/pci/pci_insn.c
+++ b/arch/s390/pci/pci_insn.c
@@ -98,7 +98,7 @@ int zpci_refresh_trans(u64 fn, u64 addr, u64 range)
}

/* Set Interruption Controls */
-int __zpci_set_irq_ctrl(u16 ctl, u8 isc, union zpci_sic_iib *iib)
+int zpci_set_irq_ctrl(u16 ctl, u8 isc, union zpci_sic_iib *iib)
{
if (!test_facility(72))
return -EIO;
@@ -109,6 +109,7 @@ int __zpci_set_irq_ctrl(u16 ctl, u8 isc, union zpci_sic_iib *iib)

return 0;
}
+EXPORT_SYMBOL_GPL(zpci_set_irq_ctrl);

/* PCI Load */
static inline int ____pcilg(u64 *data, u64 req, u64 offset, u8 *status)
diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index 87c7d121c255..f2b3145b6697 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -15,13 +15,6 @@

static enum {FLOATING, DIRECTED} irq_delivery;

-#define SIC_IRQ_MODE_ALL 0
-#define SIC_IRQ_MODE_SINGLE 1
-#define SIC_IRQ_MODE_DIRECT 4
-#define SIC_IRQ_MODE_D_ALL 16
-#define SIC_IRQ_MODE_D_SINGLE 17
-#define SIC_IRQ_MODE_SET_CPU 18
-
/*
* summary bit vector
* FLOATING - summary bit per function
@@ -154,6 +147,7 @@ static struct irq_chip zpci_irq_chip = {
static void zpci_handle_cpu_local_irq(bool rescan)
{
struct airq_iv *dibv = zpci_ibv[smp_processor_id()];
+ union zpci_sic_iib iib = {{0}};
unsigned long bit;
int irqs_on = 0;

@@ -165,7 +159,7 @@ static void zpci_handle_cpu_local_irq(bool rescan)
/* End of second scan with interrupts on. */
break;
/* First scan complete, reenable interrupts. */
- if (zpci_set_irq_ctrl(SIC_IRQ_MODE_D_SINGLE, PCI_ISC))
+ if (zpci_set_irq_ctrl(SIC_IRQ_MODE_D_SINGLE, PCI_ISC, &iib))
break;
bit = 0;
continue;
@@ -193,6 +187,7 @@ static void zpci_handle_remote_irq(void *data)
static void zpci_handle_fallback_irq(void)
{
struct cpu_irq_data *cpu_data;
+ union zpci_sic_iib iib = {{0}};
unsigned long cpu;
int irqs_on = 0;

@@ -203,7 +198,7 @@ static void zpci_handle_fallback_irq(void)
/* End of second scan with interrupts on. */
break;
/* First scan complete, reenable interrupts. */
- if (zpci_set_irq_ctrl(SIC_IRQ_MODE_SINGLE, PCI_ISC))
+ if (zpci_set_irq_ctrl(SIC_IRQ_MODE_SINGLE, PCI_ISC, &iib))
break;
cpu = 0;
continue;
@@ -234,6 +229,7 @@ static void zpci_directed_irq_handler(struct airq_struct *airq,
static void zpci_floating_irq_handler(struct airq_struct *airq,
struct tpi_info *tpi_info)
{
+ union zpci_sic_iib iib = {{0}};
unsigned long si, ai;
struct airq_iv *aibv;
int irqs_on = 0;
@@ -247,7 +243,7 @@ static void zpci_floating_irq_handler(struct airq_struct *airq,
/* End of second scan with interrupts on. */
break;
/* First scan complete, reenable interrupts. */
- if (zpci_set_irq_ctrl(SIC_IRQ_MODE_SINGLE, PCI_ISC))
+ if (zpci_set_irq_ctrl(SIC_IRQ_MODE_SINGLE, PCI_ISC, &iib))
break;
si = 0;
continue;
@@ -407,11 +403,12 @@ static struct airq_struct zpci_airq = {
static void __init cpu_enable_directed_irq(void *unused)
{
union zpci_sic_iib iib = {{0}};
+ union zpci_sic_iib ziib = {{0}};

iib.cdiib.dibv_addr = (u64) zpci_ibv[smp_processor_id()]->vector;

- __zpci_set_irq_ctrl(SIC_IRQ_MODE_SET_CPU, 0, &iib);
- zpci_set_irq_ctrl(SIC_IRQ_MODE_D_SINGLE, PCI_ISC);
+ zpci_set_irq_ctrl(SIC_IRQ_MODE_SET_CPU, 0, &iib);
+ zpci_set_irq_ctrl(SIC_IRQ_MODE_D_SINGLE, PCI_ISC, &ziib);
}

static int __init zpci_directed_irq_init(void)
@@ -426,7 +423,7 @@ static int __init zpci_directed_irq_init(void)
iib.diib.isc = PCI_ISC;
iib.diib.nr_cpus = num_possible_cpus();
iib.diib.disb_addr = virt_to_phys(zpci_sbv->vector);
- __zpci_set_irq_ctrl(SIC_IRQ_MODE_DIRECT, 0, &iib);
+ zpci_set_irq_ctrl(SIC_IRQ_MODE_DIRECT, 0, &iib);

zpci_ibv = kcalloc(num_possible_cpus(), sizeof(*zpci_ibv),
GFP_KERNEL);
@@ -471,6 +468,7 @@ static int __init zpci_floating_irq_init(void)

int __init zpci_irq_init(void)
{
+ union zpci_sic_iib iib = {{0}};
int rc;

irq_delivery = sclp.has_dirq ? DIRECTED : FLOATING;
@@ -502,7 +500,7 @@ int __init zpci_irq_init(void)
* Enable floating IRQs (with suppression after one IRQ). When using
* directed IRQs this enables the fallback path.
*/
- zpci_set_irq_ctrl(SIC_IRQ_MODE_SINGLE, PCI_ISC);
+ zpci_set_irq_ctrl(SIC_IRQ_MODE_SINGLE, PCI_ISC, &iib);

return 0;
out_airq:
--
2.27.0

2022-04-05 03:14:29

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v5 04/21] s390/sclp: detect the AISI facility

Detect the Adapter Interruption Suppression Interpretation facility.

Reviewed-by: Eric Farman <[email protected]>
Reviewed-by: Christian Borntraeger <[email protected]>
Reviewed-by: Claudio Imbrenda <[email protected]>
Signed-off-by: Matthew Rosato <[email protected]>
---
arch/s390/include/asm/sclp.h | 1 +
drivers/s390/char/sclp_early.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/arch/s390/include/asm/sclp.h b/arch/s390/include/asm/sclp.h
index ff359df62154..bdd365407176 100644
--- a/arch/s390/include/asm/sclp.h
+++ b/arch/s390/include/asm/sclp.h
@@ -90,6 +90,7 @@ struct sclp_info {
unsigned char has_zpci_lsi : 1;
unsigned char has_aisii : 1;
unsigned char has_aeni : 1;
+ unsigned char has_aisi : 1;
unsigned int ibc;
unsigned int mtid;
unsigned int mtid_cp;
diff --git a/drivers/s390/char/sclp_early.c b/drivers/s390/char/sclp_early.c
index e9af01b4c97a..c13e55cc4a5d 100644
--- a/drivers/s390/char/sclp_early.c
+++ b/drivers/s390/char/sclp_early.c
@@ -47,6 +47,7 @@ static void __init sclp_early_facilities_detect(void)
sclp.has_kss = !!(sccb->fac98 & 0x01);
sclp.has_aisii = !!(sccb->fac118 & 0x40);
sclp.has_aeni = !!(sccb->fac118 & 0x20);
+ sclp.has_aisi = !!(sccb->fac118 & 0x10);
sclp.has_zpci_lsi = !!(sccb->fac118 & 0x01);
if (sccb->fac85 & 0x02)
S390_lowcore.machine_flags |= MACHINE_FLAG_ESOP;
--
2.27.0

2022-04-05 03:19:25

by Matthew Rosato

[permalink] [raw]
Subject: [PATCH v5 09/21] s390/pci: stash dtsm and maxstbl

Store information about what IOAT designation types are supported by
underlying hardware as well as the largest store block size allowed.
These values will be needed by passthrough.

Reviewed-by: Niklas Schnelle <[email protected]>
Reviewed-by: Pierre Morel <[email protected]>
Reviewed-by: Christian Borntraeger <[email protected]>
Signed-off-by: Matthew Rosato <[email protected]>
---
arch/s390/include/asm/pci.h | 2 ++
arch/s390/include/asm/pci_clp.h | 6 ++++--
arch/s390/pci/pci_clp.c | 2 ++
3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
index 42a4a312a6dd..4c5b8fbc2079 100644
--- a/arch/s390/include/asm/pci.h
+++ b/arch/s390/include/asm/pci.h
@@ -126,9 +126,11 @@ struct zpci_dev {
u32 gisa; /* GISA designation for passthrough */
u16 vfn; /* virtual function number */
u16 pchid; /* physical channel ID */
+ u16 maxstbl; /* Maximum store block size */
u8 pfgid; /* function group ID */
u8 pft; /* pci function type */
u8 port;
+ u8 dtsm; /* Supported DT mask */
u8 rid_available : 1;
u8 has_hp_slot : 1;
u8 has_resources : 1;
diff --git a/arch/s390/include/asm/pci_clp.h b/arch/s390/include/asm/pci_clp.h
index f3286bc5ba6e..d6189ed14f84 100644
--- a/arch/s390/include/asm/pci_clp.h
+++ b/arch/s390/include/asm/pci_clp.h
@@ -153,9 +153,11 @@ struct clp_rsp_query_pci_grp {
u8 : 6;
u8 frame : 1;
u8 refresh : 1; /* TLB refresh mode */
- u16 reserved2;
+ u16 : 3;
+ u16 maxstbl : 13; /* Maximum store block size */
u16 mui;
- u16 : 16;
+ u8 dtsm; /* Supported DT mask */
+ u8 reserved3;
u16 maxfaal;
u16 : 4;
u16 dnoi : 12;
diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c
index deed35edea14..1edb07ceb766 100644
--- a/arch/s390/pci/pci_clp.c
+++ b/arch/s390/pci/pci_clp.c
@@ -106,6 +106,8 @@ static void clp_store_query_pci_fngrp(struct zpci_dev *zdev,
zdev->max_msi = response->noi;
zdev->fmb_update = response->mui;
zdev->version = response->version;
+ zdev->maxstbl = response->maxstbl;
+ zdev->dtsm = response->dtsm;

switch (response->version) {
case 1:
--
2.27.0

2022-04-06 01:54:02

by Niklas Schnelle

[permalink] [raw]
Subject: Re: [PATCH v5 10/21] KVM: s390: pci: add basic kvm_zdev structure

On Mon, 2022-04-04 at 13:43 -0400, 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/pci.h | 3 +++
> arch/s390/kvm/Makefile | 1 +
> arch/s390/kvm/pci.c | 38 +++++++++++++++++++++++++++++++++++++
> arch/s390/kvm/pci.h | 21 ++++++++++++++++++++
> 4 files changed, 63 insertions(+)
> create mode 100644 arch/s390/kvm/pci.c
> create mode 100644 arch/s390/kvm/pci.h
>
> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
> index 4c5b8fbc2079..9eb20cebaa18 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 */
> };

The struct zpci_dev tries to use semantic groups in its formatting.
It's not perfect and we probably need to clean this up to remove some
holes in the future. For now let's put the new kzdev without a blank
line together with s390_domain and add a "section comment" like
"IOMMU and passthrough".
Also I'd drop the "... data" part of the line end comment or even drop
it entirely, the name is pretty clear already when combined with the
section comment.

With that Reviewed-by: Niklas Schnelle <[email protected]>

2022-04-06 11:08:25

by Matthew Rosato

[permalink] [raw]
Subject: Re: [PATCH v5 14/21] KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding

On 4/5/22 9:39 AM, Niklas Schnelle wrote:
> On Mon, 2022-04-04 at 13:43 -0400, Matthew Rosato wrote:
>> These routines will be wired into a kvm ioctl in order to respond to
>> requests to enable / disable a device for Adapter Event Notifications /
>> Adapter Interuption Forwarding.
>>
>> Signed-off-by: Matthew Rosato <[email protected]>
>> ---
>> arch/s390/kvm/pci.c | 247 +++++++++++++++++++++++++++++++++++++++
>> arch/s390/kvm/pci.h | 1 +
>> arch/s390/pci/pci_insn.c | 1 +
>> 3 files changed, 249 insertions(+)
>>
>> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
>> index 01bd8a2f503b..f0fd68569a9d 100644
>> --- a/arch/s390/kvm/pci.c
>> +++ b/arch/s390/kvm/pci.c
>> @@ -11,6 +11,7 @@
>> #include <linux/pci.h>
>> #include <asm/pci.h>
>> #include <asm/pci_insn.h>
>> +#include <asm/pci_io.h>
>> #include "pci.h"
>>
>> struct zpci_aift *aift;
>> @@ -152,6 +153,252 @@ int kvm_s390_pci_aen_init(u8 nisc)
>> return rc;
>> }
>>
>> +/* Modify PCI: Register floating adapter interruption forwarding */
>> +static int kvm_zpci_set_airq(struct zpci_dev *zdev)
>> +{
>> + u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_REG_INT);
>> + struct zpci_fib fib = {};
>
> Hmm this one uses '{}' as initializer while all current callers of
> zpci_mod_fc() use '{0}'. As far as I know the empty braces are a GNU
> extension so should work for the kernel but for consistency I'd go with
> '{0}' or possibly '{.foo = bar, ...}' where that is more readable.
> There too uninitialized fields will be set to 0. Unless of course there
> is a conflicting KVM convention that I don't know about.

No convention that I'm aware of, I previously had fib = {0} based on the
same rationale you describe and changed to fib = {} per review request
from Pierre a few versions back. I don't have a strong preference, but
I did not note any functional difference between the two and see a bunch
of examples of both methods throughout the kernel.

2022-04-06 11:51:30

by Matthew Rosato

[permalink] [raw]
Subject: Re: [PATCH v5 10/21] KVM: s390: pci: add basic kvm_zdev structure

On 4/5/22 4:20 AM, Niklas Schnelle wrote:
> On Mon, 2022-04-04 at 13:43 -0400, 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/pci.h | 3 +++
>> arch/s390/kvm/Makefile | 1 +
>> arch/s390/kvm/pci.c | 38 +++++++++++++++++++++++++++++++++++++
>> arch/s390/kvm/pci.h | 21 ++++++++++++++++++++
>> 4 files changed, 63 insertions(+)
>> create mode 100644 arch/s390/kvm/pci.c
>> create mode 100644 arch/s390/kvm/pci.h
>>
>> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
>> index 4c5b8fbc2079..9eb20cebaa18 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 */
>> };
>
> The struct zpci_dev tries to use semantic groups in its formatting.
> It's not perfect and we probably need to clean this up to remove some
> holes in the future. For now let's put the new kzdev without a blank
> line together with s390_domain and add a "section comment" like
> "IOMMU and passthrough".
> Also I'd drop the "... data" part of the line end comment or even drop
> it entirely, the name is pretty clear already when combined with the
> section comment.

Sure, will do

>
> With that Reviewed-by: Niklas Schnelle <[email protected]>
>

Thanks!

2022-04-06 12:02:20

by Pierre Morel

[permalink] [raw]
Subject: Re: [PATCH v5 14/21] KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding



On 4/5/22 15:48, Matthew Rosato wrote:
> On 4/5/22 9:39 AM, Niklas Schnelle wrote:
>> On Mon, 2022-04-04 at 13:43 -0400, Matthew Rosato wrote:
>>> These routines will be wired into a kvm ioctl in order to respond to
>>> requests to enable / disable a device for Adapter Event Notifications /
>>> Adapter Interuption Forwarding.
>>>
>>> Signed-off-by: Matthew Rosato <[email protected]>
>>> ---
>>>   arch/s390/kvm/pci.c      | 247 +++++++++++++++++++++++++++++++++++++++
>>>   arch/s390/kvm/pci.h      |   1 +
>>>   arch/s390/pci/pci_insn.c |   1 +
>>>   3 files changed, 249 insertions(+)
>>>
>>> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
>>> index 01bd8a2f503b..f0fd68569a9d 100644
>>> --- a/arch/s390/kvm/pci.c
>>> +++ b/arch/s390/kvm/pci.c
>>> @@ -11,6 +11,7 @@
>>>   #include <linux/pci.h>
>>>   #include <asm/pci.h>
>>>   #include <asm/pci_insn.h>
>>> +#include <asm/pci_io.h>
>>>   #include "pci.h"
>>>   struct zpci_aift *aift;
>>> @@ -152,6 +153,252 @@ int kvm_s390_pci_aen_init(u8 nisc)
>>>       return rc;
>>>   }
>>> +/* Modify PCI: Register floating adapter interruption forwarding */
>>> +static int kvm_zpci_set_airq(struct zpci_dev *zdev)
>>> +{
>>> +    u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_REG_INT);
>>> +    struct zpci_fib fib = {};
>>
>> Hmm this one uses '{}' as initializer while all current callers of
>> zpci_mod_fc() use '{0}'. As far as I know the empty braces are a GNU
>> extension so should work for the kernel but for consistency I'd go with
>> '{0}' or possibly '{.foo = bar, ...}' where that is more readable.
>> There too uninitialized fields will be set to 0. Unless of course there
>> is a conflicting KVM convention that I don't know about.
>
> No convention that I'm aware of, I previously had fib = {0} based on the
> same rationale you describe and changed to fib = {} per review request
> from Pierre a few versions back.  I don't have a strong preference, but
> I did not note any functional difference between the two and see a bunch
> of examples of both methods throughout the kernel.
>

Was stupid of me to comment that, as you said there are no difference,
so do as you want.


--
Pierre Morel
IBM Lab Boeblingen

2022-04-06 13:01:14

by Niklas Schnelle

[permalink] [raw]
Subject: Re: [PATCH v5 14/21] KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding

On Mon, 2022-04-04 at 13:43 -0400, Matthew Rosato wrote:
> These routines will be wired into a kvm ioctl in order to respond to
> requests to enable / disable a device for Adapter Event Notifications /
> Adapter Interuption Forwarding.
>
> Signed-off-by: Matthew Rosato <[email protected]>
> ---
> arch/s390/kvm/pci.c | 247 +++++++++++++++++++++++++++++++++++++++
> arch/s390/kvm/pci.h | 1 +
> arch/s390/pci/pci_insn.c | 1 +
> 3 files changed, 249 insertions(+)
>
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 01bd8a2f503b..f0fd68569a9d 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -11,6 +11,7 @@
> #include <linux/pci.h>
> #include <asm/pci.h>
> #include <asm/pci_insn.h>
> +#include <asm/pci_io.h>
> #include "pci.h"
>
> struct zpci_aift *aift;
> @@ -152,6 +153,252 @@ int kvm_s390_pci_aen_init(u8 nisc)
> return rc;
> }
>
> +/* Modify PCI: Register floating adapter interruption forwarding */
> +static int kvm_zpci_set_airq(struct zpci_dev *zdev)
> +{
> + u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_REG_INT);
> + struct zpci_fib fib = {};

Hmm this one uses '{}' as initializer while all current callers of
zpci_mod_fc() use '{0}'. As far as I know the empty braces are a GNU
extension so should work for the kernel but for consistency I'd go with
'{0}' or possibly '{.foo = bar, ...}' where that is more readable.
There too uninitialized fields will be set to 0. Unless of course there
is a conflicting KVM convention that I don't know about.

> + u8 status;
> +
> + fib.fmt0.isc = zdev->kzdev->fib.fmt0.isc;
> + fib.fmt0.sum = 1; /* enable summary notifications */
> + fib.fmt0.noi = airq_iv_end(zdev->aibv);
> + fib.fmt0.aibv = virt_to_phys(zdev->aibv->vector);
> + fib.fmt0.aibvo = 0;
> + fib.fmt0.aisb = virt_to_phys(aift->sbv->vector + (zdev->aisb / 64) * 8);
> + fib.fmt0.aisbo = zdev->aisb & 63;
> + fib.gd = zdev->gisa;
> +
> + return zpci_mod_fc(req, &fib, &status) ? -EIO : 0;
> +}
> +
> +/* Modify PCI: Unregister floating adapter interruption forwarding */
> +static int kvm_zpci_clear_airq(struct zpci_dev *zdev)
> +{
> + u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_DEREG_INT);
> + struct zpci_fib fib = {};

Same here

> + u8 cc, status;
> +
> + fib.gd = zdev->gisa;
> +
> + cc = zpci_mod_fc(req, &fib, &status);
> + if (cc == 3 || (cc == 1 && status == 24))
> + /* Function already gone or IRQs already deregistered. */
> + cc = 0;
> +
> + return cc ? -EIO : 0;
> +}
> +
>
---8<---
> int kvm_s390_pci_dev_open(struct zpci_dev *zdev)
> {
> struct kvm_zdev *kzdev;
> diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
> index d4997e2236ef..b4bf3d1d4b66 100644
> --- a/arch/s390/kvm/pci.h
> +++ b/arch/s390/kvm/pci.h
> @@ -20,6 +20,7 @@
> struct kvm_zdev {
> struct zpci_dev *zdev;
> struct kvm *kvm;
> + struct zpci_fib fib;
> };
>
> struct zpci_gaite {
> diff --git a/arch/s390/pci/pci_insn.c b/arch/s390/pci/pci_insn.c
> index 4c6967b73932..cd9fb186a6be 100644
> --- a/arch/s390/pci/pci_insn.c
> +++ b/arch/s390/pci/pci_insn.c
> @@ -60,6 +60,7 @@ u8 zpci_mod_fc(u64 req, struct zpci_fib *fib, u8 *status)
>
> return cc;
> }
> +EXPORT_SYMBOL_GPL(zpci_mod_fc);
>
> /* Refresh PCI Translations */
> static inline u8 __rpcit(u64 fn, u64 addr, u64 range, u8 *status)

Acked-by: Niklas Schnelle <[email protected]>


2022-04-10 20:32:57

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v5 14/21] KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding

On Tue, Apr 05, 2022 at 03:39:19PM +0200, Niklas Schnelle wrote:
> On Mon, 2022-04-04 at 13:43 -0400, Matthew Rosato wrote:
> > These routines will be wired into a kvm ioctl in order to respond to
> > requests to enable / disable a device for Adapter Event Notifications /
> > Adapter Interuption Forwarding.
> >
> > Signed-off-by: Matthew Rosato <[email protected]>
> > arch/s390/kvm/pci.c | 247 +++++++++++++++++++++++++++++++++++++++
> > arch/s390/kvm/pci.h | 1 +
> > arch/s390/pci/pci_insn.c | 1 +
> > 3 files changed, 249 insertions(+)
> >
> > diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> > index 01bd8a2f503b..f0fd68569a9d 100644
> > +++ b/arch/s390/kvm/pci.c
> > @@ -11,6 +11,7 @@
> > #include <linux/pci.h>
> > #include <asm/pci.h>
> > #include <asm/pci_insn.h>
> > +#include <asm/pci_io.h>
> > #include "pci.h"
> >
> > struct zpci_aift *aift;
> > @@ -152,6 +153,252 @@ int kvm_s390_pci_aen_init(u8 nisc)
> > return rc;
> > }
> >
> > +/* Modify PCI: Register floating adapter interruption forwarding */
> > +static int kvm_zpci_set_airq(struct zpci_dev *zdev)
> > +{
> > + u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_REG_INT);
> > + struct zpci_fib fib = {};
>
> Hmm this one uses '{}' as initializer while all current callers of
> zpci_mod_fc() use '{0}'. As far as I know the empty braces are a GNU
> extension so should work for the kernel but for consistency I'd go with
> '{0}' or possibly '{.foo = bar, ...}' where that is more readable.
> There too uninitialized fields will be set to 0. Unless of course there
> is a conflicting KVM convention that I don't know about.

{} is not a GNU extension, it is the preferred way to write it.

The standard has a weird distinction between {} and {0} that results
in different behavior.

Jason

2022-04-12 21:18:54

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH v5 14/21] KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding

On Fri, Apr 08, 2022 at 09:48:24AM -0300, Jason Gunthorpe wrote:
> On Tue, Apr 05, 2022 at 03:39:19PM +0200, Niklas Schnelle wrote:
> > On Mon, 2022-04-04 at 13:43 -0400, Matthew Rosato wrote:
> > > + struct zpci_fib fib = {};
> >
> > Hmm this one uses '{}' as initializer while all current callers of
> > zpci_mod_fc() use '{0}'. As far as I know the empty braces are a GNU
> > extension so should work for the kernel but for consistency I'd go with
> > '{0}' or possibly '{.foo = bar, ...}' where that is more readable.
> > There too uninitialized fields will be set to 0. Unless of course there
> > is a conflicting KVM convention that I don't know about.
>
> {} is not a GNU extension, it is the preferred way to write it.
>
> The standard has a weird distinction between {} and {0} that results
> in different behavior.

Whoever cares: details are described in "6.7.8 Initialization" within
the C standard.

2022-04-12 23:37:48

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v5 10/21] KVM: s390: pci: add basic kvm_zdev structure



Am 04.04.22 um 19:43 schrieb Matthew Rosato:
> This structure will be used to carry kvm passthrough information related to
> zPCI devices.
>
> Signed-off-by: Matthew Rosato <[email protected]>

Reviewed-by: Christian Borntraeger <[email protected]>

2022-04-14 14:41:18

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v5 11/21] KVM: s390: pci: do initial setup for AEN interpretation



Am 04.04.22 um 19:43 schrieb Matthew Rosato:
> Initial setup for Adapter Event Notification Interpretation for zPCI
> passthrough devices. Specifically, allocate a structure for forwarding of
> adapter events and pass the address of this structure to firmware.
>
> Signed-off-by: Matthew Rosato <[email protected]>
[...]
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 156d1c25a3c1..9db6f8080f71 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -47,6 +47,7 @@
> #include <asm/fpu/api.h>
> #include "kvm-s390.h"
> #include "gaccess.h"
> +#include "pci.h"
>
> #define CREATE_TRACE_POINTS
> #include "trace.h"
> @@ -502,6 +503,14 @@ int kvm_arch_init(void *opaque)
> goto out;
> }
>
> + if (kvm_s390_pci_interp_allowed()) {
> + rc = kvm_s390_pci_init();
> + if (rc) {
> + pr_err("Unable to allocate AIFT for PCI\n");
> + goto out;
> + }
> + }
> +
> rc = kvm_s390_gib_init(GAL_ISC);
> if (rc)
> goto out;

We would not free the aift that was allocated by kvm_s390_pci_init
in kvm_arch_exit.
Wouldnt we re-allocate a new aift when we unload/reload kvm forgetting about the old one?


> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
[...]
> +static int zpci_setup_aipb(u8 nisc)
[...]
> + size = get_order(PAGE_ALIGN(ZPCI_NR_DEVICES *
> + sizeof(struct zpci_gaite)));
[...]
> + if (zpci_set_irq_ctrl(SIC_SET_AENI_CONTROLS, 0, zpci_aipb)) {
> + rc = -EIO;
> + goto free_gait;
> + }
> +
> + return 0;
> +
> +free_gait:
> + size = get_order(PAGE_ALIGN(ZPCI_NR_DEVICES *
> + sizeof(struct zpci_gaite)));

size should still be valid here?

> + free_pages((unsigned long)aift->gait, size);
> +free_sbv:
> + airq_iv_release(aift->sbv);
> + zpci_aif_sbv = 0;
> +free_aipb:
> + kfree(zpci_aipb);
> + zpci_aipb = 0;
> +
> + return rc;
> +}
> +

The remaining parts look sane.

2022-04-14 18:59:32

by Matthew Rosato

[permalink] [raw]
Subject: Re: [PATCH v5 11/21] KVM: s390: pci: do initial setup for AEN interpretation

On 4/14/22 3:20 AM, Christian Borntraeger wrote:
>
>
> Am 04.04.22 um 19:43 schrieb Matthew Rosato:
>> Initial setup for Adapter Event Notification Interpretation for zPCI
>> passthrough devices.  Specifically, allocate a structure for
>> forwarding of
>> adapter events and pass the address of this structure to firmware.
>>
>> Signed-off-by: Matthew Rosato <[email protected]>
> [...]
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 156d1c25a3c1..9db6f8080f71 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -47,6 +47,7 @@
>>   #include <asm/fpu/api.h>
>>   #include "kvm-s390.h"
>>   #include "gaccess.h"
>> +#include "pci.h"
>>   #define CREATE_TRACE_POINTS
>>   #include "trace.h"
>> @@ -502,6 +503,14 @@ int kvm_arch_init(void *opaque)
>>           goto out;
>>       }
>> +    if (kvm_s390_pci_interp_allowed()) {
>> +        rc = kvm_s390_pci_init();
>> +        if (rc) {
>> +            pr_err("Unable to allocate AIFT for PCI\n");
>> +            goto out;
>> +        }
>> +    }
>> +
>>       rc = kvm_s390_gib_init(GAL_ISC);
>>       if (rc)
>>           goto out;
>
> We would not free the aift that was allocated by kvm_s390_pci_init
> in kvm_arch_exit.
> Wouldnt we re-allocate a new aift when we unload/reload kvm forgetting
> about the old one?

Oops, yes it looks like that's the case. We must back-pocket a certain
subset of firmware-shared structures (e.g. zpci_aipb and zpci_aif_sbv)
as these cannot change for the life of the system once registered with
firmware; but the aift is a kernel-only structure that should be safe to
free until next module load. I think this can be done at the end of
kvm_s390_pci_aen_exit (with some caller adjustments re: the aift mutex)
>
>
>> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> [...]
>> +static int zpci_setup_aipb(u8 nisc)
> [...]
>> +    size = get_order(PAGE_ALIGN(ZPCI_NR_DEVICES *
>> +                        sizeof(struct zpci_gaite)));
> [...]
>> +    if (zpci_set_irq_ctrl(SIC_SET_AENI_CONTROLS, 0, zpci_aipb)) {
>> +        rc = -EIO;
>> +        goto free_gait;
>> +    }
>> +
>> +    return 0;
>> +
>> +free_gait:
>> +    size = get_order(PAGE_ALIGN(ZPCI_NR_DEVICES *
>> +                    sizeof(struct zpci_gaite)));
>
> size should still be valid here?

Good point

>
>> +    free_pages((unsigned long)aift->gait, size);
>> +free_sbv:
>> +    airq_iv_release(aift->sbv);
>> +    zpci_aif_sbv = 0;
>> +free_aipb:
>> +    kfree(zpci_aipb);
>> +    zpci_aipb = 0;
>> +
>> +    return rc;
>> +}
>> +
>
> The remaining parts look sane.

2022-04-20 08:04:57

by Pierre Morel

[permalink] [raw]
Subject: Re: [PATCH v5 13/21] KVM: s390: mechanism to enable guest zPCI Interpretation



On 4/4/22 19:43, Matthew Rosato wrote:
> The guest must have access to certain facilities in order to allow
> interpretive execution of zPCI instructions and adapter event
> notifications. However, there are some cases where a guest might
> disable interpretation -- provide a mechanism via which we can defer
> enabling the associated zPCI interpretation facilities until the guest
> indicates it wishes to use them.
>
> Acked-by: Pierre Morel <[email protected]>
> Signed-off-by: Matthew Rosato <[email protected]>

You can change the ACK with a RB (I think it already was a RB)

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


> ---
> arch/s390/include/asm/kvm_host.h | 4 ++++
> arch/s390/kvm/kvm-s390.c | 37 ++++++++++++++++++++++++++++++++
> arch/s390/kvm/kvm-s390.h | 10 +++++++++
> 3 files changed, 51 insertions(+)
>
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index c1518a505060..8e381603b6a7 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -254,7 +254,10 @@ struct kvm_s390_sie_block {
> #define ECB2_IEP 0x20
> #define ECB2_PFMFI 0x08
> #define ECB2_ESCA 0x04
> +#define ECB2_ZPCI_LSI 0x02
> __u8 ecb2; /* 0x0062 */
> +#define ECB3_AISI 0x20
> +#define ECB3_AISII 0x10
> #define ECB3_DEA 0x08
> #define ECB3_AES 0x04
> #define ECB3_RI 0x01
> @@ -940,6 +943,7 @@ struct kvm_arch{
> int use_cmma;
> int use_pfmfi;
> int use_skf;
> + int use_zpci_interp;
> int user_cpu_state_ctrl;
> int user_sigp;
> int user_stsi;
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index a9eb8b75af93..327649ddddce 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1029,6 +1029,41 @@ static int kvm_s390_vm_set_crypto(struct kvm *kvm, struct kvm_device_attr *attr)
> return 0;
> }
>
> +static void kvm_s390_vcpu_pci_setup(struct kvm_vcpu *vcpu)
> +{
> + /* Only set the ECB bits after guest requests zPCI interpretation */
> + if (!vcpu->kvm->arch.use_zpci_interp)
> + return;
> +
> + vcpu->arch.sie_block->ecb2 |= ECB2_ZPCI_LSI;
> + vcpu->arch.sie_block->ecb3 |= ECB3_AISII + ECB3_AISI;
> +}
> +
> +/* Must be called with kvm->lock held */
> +void kvm_s390_vcpu_pci_enable_interp(struct kvm *kvm)
> +{
> + struct kvm_vcpu *vcpu;
> + unsigned long i;
> +
> + /*
> + * If host is configured for PCI and the necessary facilities are
> + * available, turn on interpretation for the life of this guest
> + */
> + if (!kvm_s390_pci_interp_allowed())
> + return;
> +
> + kvm->arch.use_zpci_interp = 1;
> +
> + kvm_s390_vcpu_block_all(kvm);
> +
> + kvm_for_each_vcpu(i, vcpu, kvm) {
> + kvm_s390_vcpu_pci_setup(vcpu);
> + kvm_s390_sync_request(KVM_REQ_VSIE_RESTART, vcpu);
> + }
> +
> + kvm_s390_vcpu_unblock_all(kvm);
> +}
> +
> static void kvm_s390_sync_request_broadcast(struct kvm *kvm, int req)
> {
> unsigned long cx;
> @@ -3329,6 +3364,8 @@ static int kvm_s390_vcpu_setup(struct kvm_vcpu *vcpu)
>
> kvm_s390_vcpu_crypto_setup(vcpu);
>
> + kvm_s390_vcpu_pci_setup(vcpu);
> +
> mutex_lock(&vcpu->kvm->lock);
> if (kvm_s390_pv_is_protected(vcpu->kvm)) {
> rc = kvm_s390_pv_create_cpu(vcpu, &uvrc, &uvrrc);
> diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
> index 497d52a83c78..975cffaf13de 100644
> --- a/arch/s390/kvm/kvm-s390.h
> +++ b/arch/s390/kvm/kvm-s390.h
> @@ -507,6 +507,16 @@ void kvm_s390_reinject_machine_check(struct kvm_vcpu *vcpu,
> */
> void kvm_s390_vcpu_crypto_reset_all(struct kvm *kvm);
>
> +/**
> + * kvm_s390_vcpu_pci_enable_interp
> + *
> + * Set the associated PCI attributes for each vcpu to allow for zPCI Load/Store
> + * interpretation as well as adapter interruption forwarding.
> + *
> + * @kvm: the KVM guest
> + */
> +void kvm_s390_vcpu_pci_enable_interp(struct kvm *kvm);
> +
> /**
> * diag9c_forwarding_hz
> *
>

--
Pierre Morel
IBM Lab Boeblingen

2022-04-21 05:13:58

by Pierre Morel

[permalink] [raw]
Subject: Re: [PATCH v5 10/21] KVM: s390: pci: add basic kvm_zdev structure



On 4/4/22 19:43, Matthew Rosato wrote:
> This structure will be used to carry kvm passthrough information related to
> zPCI devices.
>
> Signed-off-by: Matthew Rosato <[email protected]>


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

> ---
> arch/s390/include/asm/pci.h | 3 +++
> arch/s390/kvm/Makefile | 1 +
> arch/s390/kvm/pci.c | 38 +++++++++++++++++++++++++++++++++++++
> arch/s390/kvm/pci.h | 21 ++++++++++++++++++++
> 4 files changed, 63 insertions(+)
> create mode 100644 arch/s390/kvm/pci.c
> create mode 100644 arch/s390/kvm/pci.h
>
> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
> index 4c5b8fbc2079..9eb20cebaa18 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 26f4a74e5ce4..00cf6853d93f 100644
> --- a/arch/s390/kvm/Makefile
> +++ b/arch/s390/kvm/Makefile
> @@ -10,4 +10,5 @@ ccflags-y := -Ivirt/kvm -Iarch/s390/kvm
> kvm-y += kvm-s390.o intercept.o interrupt.o priv.o sigp.o
> kvm-y += 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..213be236c05a
> --- /dev/null
> +++ b/arch/s390/kvm/pci.c
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * s390 kvm PCI passthrough support
> + *
> + * Copyright IBM Corp. 2022
> + *
> + * Author(s): Matthew Rosato <[email protected]>
> + */
> +
> +#include <linux/kvm_host.h>
> +#include <linux/pci.h>
> +#include "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);
> diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
> new file mode 100644
> index 000000000000..ce93978e8913
> --- /dev/null
> +++ b/arch/s390/kvm/pci.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * s390 kvm PCI passthrough support
> + *
> + * Copyright IBM Corp. 2022
> + *
> + * Author(s): Matthew Rosato <[email protected]>
> + */
> +
> +#ifndef __KVM_S390_PCI_H
> +#define __KVM_S390_PCI_H
> +
> +#include <linux/kvm_host.h>
> +#include <linux/pci.h>
> +
> +struct kvm_zdev {
> + struct zpci_dev *zdev;
> + struct kvm *kvm;
> +};
> +
> +#endif /* __KVM_S390_PCI_H */
>

--
Pierre Morel
IBM Lab Boeblingen

2022-04-22 17:29:32

by Matthew Rosato

[permalink] [raw]
Subject: Re: [PATCH v5 11/21] KVM: s390: pci: do initial setup for AEN interpretation

On 4/19/22 4:16 AM, Pierre Morel wrote:
>
>
> On 4/4/22 19:43, Matthew Rosato wrote:
>> Initial setup for Adapter Event Notification Interpretation for zPCI
>> passthrough devices.  Specifically, allocate a structure for
>> forwarding of
>> adapter events and pass the address of this structure to firmware.
>>
>> Signed-off-by: Matthew Rosato <[email protected]>
>> ---

...

>> +
>> +static int zpci_reset_aipb(u8 nisc)
>> +{
>> +    /*
>> +     * AEN registration can only happen once per system boot.  If
>> +     * an aipb already exists then AEN was already registered and
>> +     * we can re-use the aipb contents.  This can only happen if
>> +     * the KVM module was removed and re-inserted.
>> +     */
>> +    if (zpci_aipb->aipb.faal != ZPCI_NR_DEVICES ||
>> +        zpci_aipb->aipb.afi != nisc) {
>> +        return -EINVAL;
>> +    }
>
> I do not understand how faal cound be different of ZPCI_NR_DEVICES if
> aipb has been already initialised.
> Same for afi.
> Can you please explain?

Well, my concern was along the lines of 'what if we rmmod kvm and then
insmod a different version of the kvm module' -- These are really sanity
checks.

Now, ZPCI_NR_DEVICES/faal is built in with PCI, so yeah this check is
probably unnecessary as we shouldn't be able to change this value
without a new kernel.

afi is however derived from nisc, which was passed in all the way from
kvm_s390_gib_init during kvm_arch_init. Today, this is hard-coded as
GAL_ISC; but the point is that this is hard-coded within the kvm module,
so we can't be quite sure that it's the same value every time we insmod
kvm. In an (admittedly, far-fetched) scenario where we insmod kvm,
initialize AEN with GAL_ISC, rmmod kvm, then insmod a kvm where, for
example, GAL_ISC was changed to a different number, we would need to
trigger a failure here because we have no way to update the forwarding
isc with firmware until the kernel is rebooted.

2022-04-22 18:51:06

by Pierre Morel

[permalink] [raw]
Subject: Re: [PATCH v5 11/21] KVM: s390: pci: do initial setup for AEN interpretation



On 4/4/22 19:43, Matthew Rosato wrote:
> Initial setup for Adapter Event Notification Interpretation for zPCI
> passthrough devices. Specifically, allocate a structure for forwarding of
> adapter events and pass the address of this structure to firmware.
>
> Signed-off-by: Matthew Rosato <[email protected]>
> ---
> arch/s390/include/asm/pci.h | 4 +
> arch/s390/include/asm/pci_insn.h | 12 +++
> arch/s390/kvm/interrupt.c | 14 +++
> arch/s390/kvm/kvm-s390.c | 9 ++
> arch/s390/kvm/pci.c | 153 +++++++++++++++++++++++++++++++
> arch/s390/kvm/pci.h | 47 ++++++++++
> arch/s390/pci/pci.c | 6 ++
> 7 files changed, 245 insertions(+)
>
> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
> index 9eb20cebaa18..557b0ffb32d2 100644
> --- a/arch/s390/include/asm/pci.h
> +++ b/arch/s390/include/asm/pci.h
> @@ -9,6 +9,7 @@
> #include <asm-generic/pci.h>
> #include <asm/pci_clp.h>
> #include <asm/pci_debug.h>
> +#include <asm/pci_insn.h>
> #include <asm/sclp.h>
>
> #define PCIBIOS_MIN_IO 0x1000
> @@ -204,6 +205,9 @@ extern const struct attribute_group *zpci_attr_groups[];
> extern unsigned int s390_pci_force_floating __initdata;
> extern unsigned int s390_pci_no_rid;
>
> +extern union zpci_sic_iib *zpci_aipb;
> +extern struct airq_iv *zpci_aif_sbv;
> +
> /* -----------------------------------------------------------------------------
> Prototypes
> ----------------------------------------------------------------------------- */
> diff --git a/arch/s390/include/asm/pci_insn.h b/arch/s390/include/asm/pci_insn.h
> index 5331082fa516..e5f57cfe1d45 100644
> --- a/arch/s390/include/asm/pci_insn.h
> +++ b/arch/s390/include/asm/pci_insn.h
> @@ -101,6 +101,7 @@ struct zpci_fib {
> /* Set Interruption Controls Operation Controls */
> #define SIC_IRQ_MODE_ALL 0
> #define SIC_IRQ_MODE_SINGLE 1
> +#define SIC_SET_AENI_CONTROLS 2
> #define SIC_IRQ_MODE_DIRECT 4
> #define SIC_IRQ_MODE_D_ALL 16
> #define SIC_IRQ_MODE_D_SINGLE 17
> @@ -127,9 +128,20 @@ struct zpci_cdiib {
> u64 : 64;
> } __packed __aligned(8);
>
> +/* adapter interruption parameters block */
> +struct zpci_aipb {
> + u64 faisb;
> + u64 gait;
> + u16 : 13;
> + u16 afi : 3;
> + u32 : 32;
> + u16 faal;
> +} __packed __aligned(8);
> +
> union zpci_sic_iib {
> struct zpci_diib diib;
> struct zpci_cdiib cdiib;
> + struct zpci_aipb aipb;
> };
>
> DECLARE_STATIC_KEY_FALSE(have_mio);
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 250119a26c60..57a27dfc85ea 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -32,6 +32,7 @@
> #include "kvm-s390.h"
> #include "gaccess.h"
> #include "trace-s390.h"
> +#include "pci.h"
>
> #define PFAULT_INIT 0x0600
> #define PFAULT_DONE 0x0680
> @@ -3328,6 +3329,11 @@ void kvm_s390_gib_destroy(void)
> {
> if (!gib)
> return;
> + if (kvm_s390_pci_interp_allowed() && aift) {
> + mutex_lock(&aift->aift_lock);
> + kvm_s390_pci_aen_exit();
> + mutex_unlock(&aift->aift_lock);
> + }
> chsc_sgib(0);
> unregister_adapter_interrupt(&gib_alert_irq);
> free_page((unsigned long)gib);
> @@ -3365,6 +3371,14 @@ int kvm_s390_gib_init(u8 nisc)
> goto out_unreg_gal;
> }
>
> + if (kvm_s390_pci_interp_allowed()) {
> + if (kvm_s390_pci_aen_init(nisc)) {
> + pr_err("Initializing AEN for PCI failed\n");
> + rc = -EIO;
> + goto out_unreg_gal;
> + }
> + }
> +
> KVM_EVENT(3, "gib 0x%pK (nisc=%d) initialized", gib, gib->nisc);
> goto out;
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 156d1c25a3c1..9db6f8080f71 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -47,6 +47,7 @@
> #include <asm/fpu/api.h>
> #include "kvm-s390.h"
> #include "gaccess.h"
> +#include "pci.h"
>
> #define CREATE_TRACE_POINTS
> #include "trace.h"
> @@ -502,6 +503,14 @@ int kvm_arch_init(void *opaque)
> goto out;
> }
>
> + if (kvm_s390_pci_interp_allowed()) {
> + rc = kvm_s390_pci_init();
> + if (rc) {
> + pr_err("Unable to allocate AIFT for PCI\n");
> + goto out;
> + }
> + }
> +
> rc = kvm_s390_gib_init(GAL_ISC);
> if (rc)
> goto out;
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 213be236c05a..01bd8a2f503b 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -9,8 +9,149 @@
>
> #include <linux/kvm_host.h>
> #include <linux/pci.h>
> +#include <asm/pci.h>
> +#include <asm/pci_insn.h>
> #include "pci.h"
>
> +struct zpci_aift *aift;
> +
> +static inline int __set_irq_noiib(u16 ctl, u8 isc)
> +{
> + union zpci_sic_iib iib = {{0}};
> +
> + return zpci_set_irq_ctrl(ctl, isc, &iib);
> +}
> +
> +/* Caller must hold the aift lock before calling this function */
> +void kvm_s390_pci_aen_exit(void)
> +{
> + unsigned long flags;
> + struct kvm_zdev **gait_kzdev;
> +
> + /*
> + * Contents of the aipb remain registered for the life of the host
> + * kernel, the information preserved in zpci_aipb and zpci_aif_sbv
> + * in case we insert the KVM module again later. Clear the AIFT
> + * information and free anything not registered with underlying
> + * firmware.
> + */
> + spin_lock_irqsave(&aift->gait_lock, flags);
> + gait_kzdev = aift->kzdev;
> + aift->gait = 0;
> + aift->sbv = 0;
> + aift->kzdev = 0;
> + spin_unlock_irqrestore(&aift->gait_lock, flags);
> +
> + kfree(gait_kzdev);
> +}
> +
> +static int zpci_setup_aipb(u8 nisc)
> +{
> + struct page *page;
> + int size, rc;
> +
> + zpci_aipb = kzalloc(sizeof(union zpci_sic_iib), GFP_KERNEL);
> + if (!zpci_aipb)
> + return -ENOMEM;
> +
> + aift->sbv = airq_iv_create(ZPCI_NR_DEVICES, AIRQ_IV_ALLOC, 0);
> + if (!aift->sbv) {
> + rc = -ENOMEM;
> + goto free_aipb;
> + }
> + zpci_aif_sbv = aift->sbv;
> + size = get_order(PAGE_ALIGN(ZPCI_NR_DEVICES *
> + sizeof(struct zpci_gaite)));
> + page = alloc_pages(GFP_KERNEL | __GFP_ZERO, size);
> + if (!page) {
> + rc = -ENOMEM;
> + goto free_sbv;
> + }
> + aift->gait = (struct zpci_gaite *)page_to_phys(page);
> +
> + zpci_aipb->aipb.faisb = virt_to_phys(aift->sbv->vector);
> + zpci_aipb->aipb.gait = virt_to_phys(aift->gait);
> + zpci_aipb->aipb.afi = nisc;
> + zpci_aipb->aipb.faal = ZPCI_NR_DEVICES;
> +
> + /* Setup Adapter Event Notification Interpretation */
> + if (zpci_set_irq_ctrl(SIC_SET_AENI_CONTROLS, 0, zpci_aipb)) {
> + rc = -EIO;
> + goto free_gait;
> + }
> +
> + return 0;
> +
> +free_gait:
> + size = get_order(PAGE_ALIGN(ZPCI_NR_DEVICES *
> + sizeof(struct zpci_gaite)));
> + free_pages((unsigned long)aift->gait, size);
> +free_sbv:
> + airq_iv_release(aift->sbv);
> + zpci_aif_sbv = 0;
> +free_aipb:
> + kfree(zpci_aipb);
> + zpci_aipb = 0;
> +
> + return rc;
> +}
> +
> +static int zpci_reset_aipb(u8 nisc)
> +{
> + /*
> + * AEN registration can only happen once per system boot. If
> + * an aipb already exists then AEN was already registered and
> + * we can re-use the aipb contents. This can only happen if
> + * the KVM module was removed and re-inserted.
> + */
> + if (zpci_aipb->aipb.faal != ZPCI_NR_DEVICES ||
> + zpci_aipb->aipb.afi != nisc) {
> + return -EINVAL;
> + }

I do not understand how faal cound be different of ZPCI_NR_DEVICES if
aipb has been already initialised.
Same for afi.
Can you please explain?


> + aift->sbv = zpci_aif_sbv;
> + aift->gait = (struct zpci_gaite *)zpci_aipb->aipb.gait;
> +
> + return 0;
> +}
> +
> +int kvm_s390_pci_aen_init(u8 nisc)
> +{
> + int rc = 0;
> +
> + /* If already enabled for AEN, bail out now */
> + if (aift->gait || aift->sbv)
> + return -EPERM;
> +
> + mutex_lock(&aift->aift_lock);
> + aift->kzdev = kcalloc(ZPCI_NR_DEVICES, sizeof(struct kvm_zdev),
> + GFP_KERNEL);
> + if (!aift->kzdev) {
> + rc = -ENOMEM;
> + goto unlock;
> + }
> +
> + if (!zpci_aipb)
> + rc = zpci_setup_aipb(nisc);
> + else
> + rc = zpci_reset_aipb(nisc);
> + if (rc)
> + goto free_zdev;
> +
> + /* Enable floating IRQs */
> + if (__set_irq_noiib(SIC_IRQ_MODE_SINGLE, nisc)) {
> + rc = -EIO;
> + kvm_s390_pci_aen_exit();
> + }
> +
> + goto unlock;
> +
> +free_zdev:
> + kfree(aift->kzdev);
> +unlock:
> + mutex_unlock(&aift->aift_lock);
> + return rc;
> +}
> +
> int kvm_s390_pci_dev_open(struct zpci_dev *zdev)
> {
> struct kvm_zdev *kzdev;
> @@ -36,3 +177,15 @@ void kvm_s390_pci_dev_release(struct zpci_dev *zdev)
> kfree(kzdev);
> }
> EXPORT_SYMBOL_GPL(kvm_s390_pci_dev_release);
> +
> +int kvm_s390_pci_init(void)
> +{
> + aift = kzalloc(sizeof(struct zpci_aift), GFP_KERNEL);
> + if (!aift)
> + return -ENOMEM;
> +
> + spin_lock_init(&aift->gait_lock);
> + mutex_init(&aift->aift_lock);
> +
> + return 0;
> +}
> diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
> index ce93978e8913..a6a62db792b6 100644
> --- a/arch/s390/kvm/pci.h
> +++ b/arch/s390/kvm/pci.h
> @@ -12,10 +12,57 @@
>
> #include <linux/kvm_host.h>
> #include <linux/pci.h>
> +#include <linux/mutex.h>
> +#include <asm/airq.h>
> +#include <asm/cpu.h>
>
> struct kvm_zdev {
> struct zpci_dev *zdev;
> struct kvm *kvm;
> };
>
> +struct zpci_gaite {
> + u32 gisa;
> + u8 gisc;
> + u8 count;
> + u8 reserved;
> + u8 aisbo;
> + u64 aisb;
> +};
> +
> +struct zpci_aift {
> + struct zpci_gaite *gait;
> + struct airq_iv *sbv;
> + struct kvm_zdev **kzdev;
> + spinlock_t gait_lock; /* Protects the gait, used during AEN forward */
> + struct mutex aift_lock; /* Protects the other structures in aift */
> +};
> +
> +extern struct zpci_aift *aift;
> +
> +int kvm_s390_pci_aen_init(u8 nisc);
> +void kvm_s390_pci_aen_exit(void);
> +
> +int kvm_s390_pci_init(void);
> +
> +static inline bool kvm_s390_pci_interp_allowed(void)
> +{
> + struct cpuid cpu_id;
> +
> + get_cpu_id(&cpu_id);
> + switch (cpu_id.machine) {
> + case 0x2817:
> + case 0x2818:
> + case 0x2827:
> + case 0x2828:
> + case 0x2964:
> + case 0x2965:
> + /* No SHM on certain machines */
> + return false;
> + default:
> + return (IS_ENABLED(CONFIG_VFIO_PCI) && sclp.has_zpci_lsi &&
> + sclp.has_aeni && sclp.has_aisi && sclp.has_aisii);
> + }
> +}
> +
> #endif /* __KVM_S390_PCI_H */
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index a86cd1cbb80e..f0a439c43395 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -61,6 +61,12 @@ DEFINE_STATIC_KEY_FALSE(have_mio);
>
> static struct kmem_cache *zdev_fmb_cache;
>
> +/* AEN structures that must be preserved over KVM module re-insertion */
> +union zpci_sic_iib *zpci_aipb;
> +EXPORT_SYMBOL_GPL(zpci_aipb);
> +struct airq_iv *zpci_aif_sbv;
> +EXPORT_SYMBOL_GPL(zpci_aif_sbv);
> +
> struct zpci_dev *get_zdev_by_fid(u32 fid)
> {
> struct zpci_dev *tmp, *zdev = NULL;
>

--
Pierre Morel
IBM Lab Boeblingen

2022-04-22 21:07:06

by Pierre Morel

[permalink] [raw]
Subject: Re: [PATCH v5 18/21] vfio-pci/zdev: different maxstbl for interpreted devices



On 4/4/22 19:43, Matthew Rosato wrote:
> When doing load/store interpretation, the maximum store block length is
> determined by the underlying firmware, not the host kernel API. Reflect
> that in the associated Query PCI Function Group clp capability and let
> userspace decide which is appropriate to present to the guest.
>
> Signed-off-by: Matthew Rosato <[email protected]>


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


> ---
> drivers/vfio/pci/vfio_pci_zdev.c | 6 ++++--
> include/uapi/linux/vfio_zdev.h | 4 ++++
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c
> index 4a653ce480c7..d3ca58d9d8ec 100644
> --- a/drivers/vfio/pci/vfio_pci_zdev.c
> +++ b/drivers/vfio/pci/vfio_pci_zdev.c
> @@ -44,14 +44,16 @@ static int zpci_group_cap(struct zpci_dev *zdev, struct vfio_info_cap *caps)
> {
> struct vfio_device_info_cap_zpci_group cap = {
> .header.id = VFIO_DEVICE_INFO_CAP_ZPCI_GROUP,
> - .header.version = 1,
> + .header.version = 2,
> .dasm = zdev->dma_mask,
> .msi_addr = zdev->msi_addr,
> .flags = VFIO_DEVICE_INFO_ZPCI_FLAG_REFRESH,
> .mui = zdev->fmb_update,
> .noi = zdev->max_msi,
> .maxstbl = ZPCI_MAX_WRITE_SIZE,
> - .version = zdev->version
> + .version = zdev->version,
> + .reserved = 0,
> + .imaxstbl = zdev->maxstbl
> };
>
> return vfio_info_add_capability(caps, &cap.header, sizeof(cap));
> diff --git a/include/uapi/linux/vfio_zdev.h b/include/uapi/linux/vfio_zdev.h
> index 78c022af3d29..77f2aff1f27e 100644
> --- a/include/uapi/linux/vfio_zdev.h
> +++ b/include/uapi/linux/vfio_zdev.h
> @@ -50,6 +50,10 @@ struct vfio_device_info_cap_zpci_group {
> __u16 noi; /* Maximum number of MSIs */
> __u16 maxstbl; /* Maximum Store Block Length */
> __u8 version; /* Supported PCI Version */
> + /* End of version 1 */
> + __u8 reserved;
> + __u16 imaxstbl; /* Maximum Interpreted Store Block Length */
> + /* End of version 2 */
> };
>
> /**
>

--
Pierre Morel
IBM Lab Boeblingen