On 1/25/22 15:16, Matthew Rosato wrote:
On 1/25/22 7:36 AM, Pierre Morel wrote:
On 1/14/22 21:31, Matthew Rosato wrote:
Introduce support for VFIO_DEVICE_FEATURE_ZPCI_AIF, which is a new
VFIO_DEVICE_FEATURE ioctl. This interface is used to indicate that an
s390x vfio-pci device wishes to enable/disable zPCI adapter interrupt
forwarding, which allows underlying firmware to deliver interrupts
directly to the associated kvm guest.
Signed-off-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
---
arch/s390/include/asm/kvm_pci.h | 2 +
drivers/vfio/pci/vfio_pci_core.c | 2 +
drivers/vfio/pci/vfio_pci_zdev.c | 98 +++++++++++++++++++++++++++++++-
include/linux/vfio_pci_core.h | 10 ++++
include/uapi/linux/vfio.h | 7 +++
include/uapi/linux/vfio_zdev.h | 20 +++++++
6 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/arch/s390/include/asm/kvm_pci.h
b/arch/s390/include/asm/kvm_pci.h
index dc00c3f27a00..dbab349a4a75 100644
--- a/arch/s390/include/asm/kvm_pci.h
+++ b/arch/s390/include/asm/kvm_pci.h
@@ -36,6 +36,8 @@ struct kvm_zdev {
struct zpci_fib fib;
struct notifier_block nb;
bool interp;
+ bool aif;
+ bool fhost;
Can we please have a comment on these booleans? > Can we have explicit
naming to be able to follow their usage more easily?
May be aif_float and aif_host to match with the VFIO feature?
Sure, rename would be fine.
As for a comment, maybe something like
bool aif_float; /* Enabled for floating interrupt assist */
bool aif_host; /* Require host delivery */
good for me.
...
diff --git a/include/uapi/linux/vfio_zdev.h
b/include/uapi/linux/vfio_zdev.h
index 575f0410dc66..c574e23f9385 100644
--- a/include/uapi/linux/vfio_zdev.h
+++ b/include/uapi/linux/vfio_zdev.h
@@ -90,4 +90,24 @@ struct vfio_device_zpci_interp {
__u32 fh; /* Host device function handle */
};
+/**
+ * VFIO_DEVICE_FEATURE_ZPCI_AIF
+ *
+ * This feature is used for enabling forwarding of adapter
interrupts directly
+ * from firmware to the guest. When setting this feature, the flags
indicate
+ * whether to enable/disable the feature and the structure defined
below is
+ * used to setup the forwarding structures. When getting this
feature, only
+ * the flags are used to indicate the current state.
+ */
+struct vfio_device_zpci_aif {
+ __u64 flags;
+#define VFIO_DEVICE_ZPCI_FLAG_AIF_FLOAT 1
+#define VFIO_DEVICE_ZPCI_FLAG_AIF_HOST 2
I think we need more information on these flags.
What does AIF_FLOAT and what does AIF_HOST ?
You actually asked for this already on Jan 19 :), here's a copy of that
response inline here:
:) I forgot
I can add a small line comment for each, like:
AIF_FLOAT 1 /* Floating interrupts enabled */
AIF_HOST 2 /* Host delivery forced */
But here's a bit more detail:
On SET:
AIF_FLOAT = 1 means enable the interrupt forwarding assist for floating
interrupt delivery
AIF_FLOAT = 0 means to disable it.
AIF_HOST = 1 means the assist will always deliver the interrupt to the
host and let the host inject it
AIF_HOST = 0 host only gets interrupts when firmware can't deliver
on GET, we just indicate the current settings from the most recent SET,
meaning:
AIF_FLOAT = 1 interrupt forwarding assist is currently active
AIF_FLOAT = 0 interrupt forwarding assist is not currently active
AIF_HOST = 1 interrupt forwarding will always go through host
AIF_HOST = 0 interrupt forwarding will only go through the host when
necessary
My thought would be add the line comments in this patch and then the
additional detail in a follow-on patch that adds vfio zPCI to
Documentation/S390
good for me.
--
Pierre Morel
IBM Lab Boeblingen
