Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <9a551f04c3878ecb3a26fed6aff2834fbfe41f18.camel@linux.ibm.com>
Subject: Re: [PATCH v5 14/21] KVM: s390: pci: provide routines for
 enabling/disabling interrupt forwarding
From:   Niklas Schnelle <schnelle@linux.ibm.com>
To:     Matthew Rosato <mjrosato@linux.ibm.com>, linux-s390@vger.kernel.org
Cc:     alex.williamson@redhat.com, cohuck@redhat.com,
        farman@linux.ibm.com, pmorel@linux.ibm.com,
        borntraeger@linux.ibm.com, hca@linux.ibm.com, gor@linux.ibm.com,
        gerald.schaefer@linux.ibm.com, agordeev@linux.ibm.com,
        svens@linux.ibm.com, frankja@linux.ibm.com, david@redhat.com,
        imbrenda@linux.ibm.com, vneethv@linux.ibm.com,
        oberpar@linux.ibm.com, freude@linux.ibm.com, thuth@redhat.com,
        pasic@linux.ibm.com, pbonzini@redhat.com, corbet@lwn.net,
        kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-doc@vger.kernel.org
Date:   Tue, 05 Apr 2022 15:39:19 +0200
In-Reply-To: <20220404174349.58530-15-mjrosato@linux.ibm.com>
References: <20220404174349.58530-1-mjrosato@linux.ibm.com>
         <20220404174349.58530-15-mjrosato@linux.ibm.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

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>