Messages in this thread Patch in this message | | | Date | Tue, 25 Jan 2022 09:16:51 -0500 | Subject | Re: [PATCH v2 26/30] vfio-pci/zdev: wire up zPCI adapter interrupt forwarding support | From | Matthew Rosato <> |
| |
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 */
...
>> 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 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
| |