| Message ID | 20220404174349.58530-15-mjrosato@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: enable zPCI for interpretive execution | expand |
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>
On 4/5/22 9:39 AM, Niklas Schnelle wrote: > 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. No convention that I'm aware of, I previously had fib = {0} based on the same rationale you describe and changed to fib = {} per review request from Pierre a few versions back. I don't have a strong preference, but I did not note any functional difference between the two and see a bunch of examples of both methods throughout the kernel.
On 4/5/22 15:48, Matthew Rosato wrote: > On 4/5/22 9:39 AM, Niklas Schnelle wrote: >> 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. > > No convention that I'm aware of, I previously had fib = {0} based on the > same rationale you describe and changed to fib = {} per review request > from Pierre a few versions back. I don't have a strong preference, but > I did not note any functional difference between the two and see a bunch > of examples of both methods throughout the kernel. > Was stupid of me to comment that, as you said there are no difference, so do as you want.
On Tue, Apr 05, 2022 at 03:39:19PM +0200, Niklas Schnelle wrote: > 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 > > +++ 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. {} is not a GNU extension, it is the preferred way to write it. The standard has a weird distinction between {} and {0} that results in different behavior. Jason
On Fri, Apr 08, 2022 at 09:48:24AM -0300, Jason Gunthorpe wrote: > On Tue, Apr 05, 2022 at 03:39:19PM +0200, Niklas Schnelle wrote: > > On Mon, 2022-04-04 at 13:43 -0400, Matthew Rosato wrote: > > > + 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. > > {} is not a GNU extension, it is the preferred way to write it. > > The standard has a weird distinction between {} and {0} that results > in different behavior. Whoever cares: details are described in "6.7.8 Initialization" within the C standard.
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 = {}; + 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 = {}; + 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; +} + +static inline void unaccount_mem(unsigned long nr_pages) +{ + struct user_struct *user = get_uid(current_user()); + + if (user) + atomic_long_sub(nr_pages, &user->locked_vm); + if (current->mm) + atomic64_sub(nr_pages, ¤t->mm->pinned_vm); +} + +static inline int account_mem(unsigned long nr_pages) +{ + struct user_struct *user = get_uid(current_user()); + unsigned long page_limit, cur_pages, new_pages; + + page_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT; + + do { + cur_pages = atomic_long_read(&user->locked_vm); + new_pages = cur_pages + nr_pages; + if (new_pages > page_limit) + return -ENOMEM; + } while (atomic_long_cmpxchg(&user->locked_vm, cur_pages, + new_pages) != cur_pages); + + atomic64_add(nr_pages, ¤t->mm->pinned_vm); + + return 0; +} + +static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib, + bool assist) +{ + struct page *pages[1], *aibv_page, *aisb_page = NULL; + unsigned int msi_vecs, idx; + struct zpci_gaite *gaite; + unsigned long hva, bit; + struct kvm *kvm; + phys_addr_t gaddr; + int rc = 0, gisc, npages, pcount = 0; + + /* + * Interrupt forwarding is only applicable if the device is already + * enabled for interpretation + */ + if (zdev->gisa == 0) + return -EINVAL; + + kvm = zdev->kzdev->kvm; + msi_vecs = min_t(unsigned int, fib->fmt0.noi, zdev->max_msi); + + /* Get the associated forwarding ISC - if invalid, return the error */ + gisc = kvm_s390_gisc_register(kvm, fib->fmt0.isc); + if (gisc < 0) + return gisc; + + /* Replace AIBV address */ + idx = srcu_read_lock(&kvm->srcu); + hva = gfn_to_hva(kvm, gpa_to_gfn((gpa_t)fib->fmt0.aibv)); + npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM, pages); + srcu_read_unlock(&kvm->srcu, idx); + if (npages < 1) { + rc = -EIO; + goto out; + } + aibv_page = pages[0]; + pcount++; + gaddr = page_to_phys(aibv_page) + (fib->fmt0.aibv & ~PAGE_MASK); + fib->fmt0.aibv = gaddr; + + /* Pin the guest AISB if one was specified */ + if (fib->fmt0.sum == 1) { + idx = srcu_read_lock(&kvm->srcu); + hva = gfn_to_hva(kvm, gpa_to_gfn((gpa_t)fib->fmt0.aisb)); + npages = pin_user_pages_fast(hva, 1, FOLL_WRITE | FOLL_LONGTERM, + pages); + srcu_read_unlock(&kvm->srcu, idx); + if (npages < 1) { + rc = -EIO; + goto unpin1; + } + aisb_page = pages[0]; + pcount++; + } + + /* Account for pinned pages, roll back on failure */ + if (account_mem(pcount)) + goto unpin2; + + /* AISB must be allocated before we can fill in GAITE */ + mutex_lock(&aift->aift_lock); + bit = airq_iv_alloc_bit(aift->sbv); + if (bit == -1UL) + goto unlock; + zdev->aisb = bit; /* store the summary bit number */ + zdev->aibv = airq_iv_create(msi_vecs, AIRQ_IV_DATA | + AIRQ_IV_BITLOCK | + AIRQ_IV_GUESTVEC, + phys_to_virt(fib->fmt0.aibv)); + + spin_lock_irq(&aift->gait_lock); + gaite = (struct zpci_gaite *)aift->gait + (zdev->aisb * + sizeof(struct zpci_gaite)); + + /* If assist not requested, host will get all alerts */ + if (assist) + gaite->gisa = (u32)virt_to_phys(&kvm->arch.sie_page2->gisa); + else + gaite->gisa = 0; + + gaite->gisc = fib->fmt0.isc; + gaite->count++; + gaite->aisbo = fib->fmt0.aisbo; + gaite->aisb = virt_to_phys(page_address(aisb_page) + (fib->fmt0.aisb & + ~PAGE_MASK)); + aift->kzdev[zdev->aisb] = zdev->kzdev; + spin_unlock_irq(&aift->gait_lock); + + /* Update guest FIB for re-issue */ + fib->fmt0.aisbo = zdev->aisb & 63; + fib->fmt0.aisb = virt_to_phys(aift->sbv->vector + (zdev->aisb / 64) * 8); + fib->fmt0.isc = gisc; + + /* Save some guest fib values in the host for later use */ + zdev->kzdev->fib.fmt0.isc = fib->fmt0.isc; + zdev->kzdev->fib.fmt0.aibv = fib->fmt0.aibv; + mutex_unlock(&aift->aift_lock); + + /* Issue the clp to setup the irq now */ + rc = kvm_zpci_set_airq(zdev); + return rc; + +unlock: + mutex_unlock(&aift->aift_lock); +unpin2: + if (fib->fmt0.sum == 1) + unpin_user_page(aisb_page); +unpin1: + unpin_user_page(aibv_page); +out: + return rc; +} + +static int kvm_s390_pci_aif_disable(struct zpci_dev *zdev, bool force) +{ + struct kvm_zdev *kzdev = zdev->kzdev; + struct zpci_gaite *gaite; + struct page *vpage = NULL, *spage = NULL; + int rc, pcount = 0; + u8 isc; + + if (zdev->gisa == 0) + return -EINVAL; + + mutex_lock(&aift->aift_lock); + + /* + * If the clear fails due to an error, leave now unless we know this + * device is about to go away (force) -- In that case clear the GAITE + * regardless. + */ + rc = kvm_zpci_clear_airq(zdev); + if (rc && !force) + goto out; + + if (zdev->kzdev->fib.fmt0.aibv == 0) + goto out; + spin_lock_irq(&aift->gait_lock); + gaite = (struct zpci_gaite *)aift->gait + (zdev->aisb * + sizeof(struct zpci_gaite)); + isc = gaite->gisc; + gaite->count--; + if (gaite->count == 0) { + /* Release guest AIBV and AISB */ + vpage = phys_to_page(kzdev->fib.fmt0.aibv); + if (gaite->aisb != 0) + spage = phys_to_page(gaite->aisb); + /* Clear the GAIT entry */ + gaite->aisb = 0; + gaite->gisc = 0; + gaite->aisbo = 0; + gaite->gisa = 0; + aift->kzdev[zdev->aisb] = 0; + /* Clear zdev info */ + airq_iv_free_bit(aift->sbv, zdev->aisb); + airq_iv_release(zdev->aibv); + zdev->aisb = 0; + zdev->aibv = NULL; + } + spin_unlock_irq(&aift->gait_lock); + kvm_s390_gisc_unregister(kzdev->kvm, isc); + kzdev->fib.fmt0.isc = 0; + kzdev->fib.fmt0.aibv = 0; + + if (vpage) { + unpin_user_page(vpage); + pcount++; + } + if (spage) { + unpin_user_page(spage); + pcount++; + } + if (pcount > 0) + unaccount_mem(pcount); +out: + mutex_unlock(&aift->aift_lock); + + return rc; +} + 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)
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(+)