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

[Niklas Schnelle <schnelle@linux.ibm.com>]

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 <mjrosato@linux.ibm.com>
> ---
>  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 <schnelle@linux.ibm.com>