Messages in this thread | | | Date | Wed, 26 Jan 2022 09:24:39 +0100 | Subject | Re: [PATCH v2 26/30] vfio-pci/zdev: wire up zPCI adapter interrupt forwarding support | From | Pierre Morel <> |
| |
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@linux.ibm.com> >>> --- >>> 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
| |