| Message ID | 20211207205743.150299-21-mjrosato@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: enable zPCI for interpretive execution | expand |
On Tue, 2021-12-07 at 15:57 -0500, Matthew Rosato wrote: > These routines will be wired into the vfio_pci_zdev ioctl handlers to > respond to requests to enable / disable a device for zPCI Load/Store > interpretation. > > The first time such a request is received, enable the necessary facilities > for the guest. > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > --- > arch/s390/include/asm/kvm_pci.h | 4 ++ > arch/s390/kvm/pci.c | 91 +++++++++++++++++++++++++++++++++ > arch/s390/pci/pci.c | 3 ++ > 3 files changed, 98 insertions(+) > > diff --git a/arch/s390/include/asm/kvm_pci.h b/arch/s390/include/asm/kvm_pci.h > index 3e491a39704c..5d6283acb54c 100644 > --- a/arch/s390/include/asm/kvm_pci.h > ---8<--- > return rc; > + } > + > + /* > + * Store information about the identity of the kvm guest allowed to > + * access this device via interpretation to be used by host CLP > + */ > + zdev->gd = gd; > + > + rc = zpci_enable_device(zdev); > + if (rc) > + goto err; > + > + /* Re-register the IOMMU that was already created */ > + rc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, > + (u64)zdev->dma_table); The zdev->dma_table is a virtual address but we need an absolute address in the MPCIFC so the above should use virt_to_phys(zdev->dma_table) to be compatible with future V != R kernel memory. As of now since virtual and absolute kernel addresses are the same this is not a bug and we've had this (wrong) pattern in the rest of the code but let's get it righht here from the start. See also my commit "s390/pci: use physical addresses in DMA tables" that is currently in the s390 feature branch. > + if (rc) > + goto err; > + > + return rc; > + > +err: > + zdev->gd = 0; > + return rc; > +} > +EXPORT_SYMBOL_GPL(kvm_s390_pci_interp_enable); > + > +int kvm_s390_pci_interp_disable(struct zpci_dev *zdev) > +{ > + int rc; > + > + if (zdev->gd == 0) > + return -EINVAL; > + > + /* Remove the host CLP guest designation */ > + zdev->gd = 0; > + > + if (zdev_enabled(zdev)) { > + rc = zpci_disable_device(zdev); > + if (rc) > + return rc; > + } > + > + rc = zpci_enable_device(zdev); > + if (rc) > + return rc; > + > + /* Re-register the IOMMU that was already created */ > + rc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, > + (u64)zdev->dma_table); Same as above > + > + return rc; > +} > +EXPORT_SYMBOL_GPL(kvm_s390_pci_interp_disable); > + > ---8<---
On 12/8/21 4:44 AM, Niklas Schnelle wrote: > On Tue, 2021-12-07 at 15:57 -0500, Matthew Rosato wrote: >> These routines will be wired into the vfio_pci_zdev ioctl handlers to >> respond to requests to enable / disable a device for zPCI Load/Store >> interpretation. >> >> The first time such a request is received, enable the necessary facilities >> for the guest. >> >> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >> --- >> arch/s390/include/asm/kvm_pci.h | 4 ++ >> arch/s390/kvm/pci.c | 91 +++++++++++++++++++++++++++++++++ >> arch/s390/pci/pci.c | 3 ++ >> 3 files changed, 98 insertions(+) >> >> diff --git a/arch/s390/include/asm/kvm_pci.h b/arch/s390/include/asm/kvm_pci.h >> index 3e491a39704c..5d6283acb54c 100644 >> --- a/arch/s390/include/asm/kvm_pci.h >> > ---8<--- >> return rc; >> + } >> + >> + /* >> + * Store information about the identity of the kvm guest allowed to >> + * access this device via interpretation to be used by host CLP >> + */ >> + zdev->gd = gd; >> + >> + rc = zpci_enable_device(zdev); >> + if (rc) >> + goto err; >> + >> + /* Re-register the IOMMU that was already created */ >> + rc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, >> + (u64)zdev->dma_table); > > The zdev->dma_table is a virtual address but we need an absolute > address in the MPCIFC so the above should use > virt_to_phys(zdev->dma_table) to be compatible with future V != R > kernel memory. As of now since virtual and absolute kernel addresses > are the same this is not a bug and we've had this (wrong) pattern in > the rest of the code but let's get it righht here from the start. > > See also my commit "s390/pci: use physical addresses in DMA tables" > that is currently in the s390 feature branch. You're right of course -- I saw those changes happening as I prepared this series but I didn't want to delay getting comments any longer, what with the holidays approaching. Of course, I didn't realize they were already out on the feature branch. I suspect there is some more of this also in the code related to handling RPCIT. AEN setup too. > >> + if (rc) >> + goto err; >> + >> + return rc; >> + >> +err: >> + zdev->gd = 0; >> + return rc; >> +} >> +EXPORT_SYMBOL_GPL(kvm_s390_pci_interp_enable); >> + >> +int kvm_s390_pci_interp_disable(struct zpci_dev *zdev) >> +{ >> + int rc; >> + >> + if (zdev->gd == 0) >> + return -EINVAL; >> + >> + /* Remove the host CLP guest designation */ >> + zdev->gd = 0; >> + >> + if (zdev_enabled(zdev)) { >> + rc = zpci_disable_device(zdev); >> + if (rc) >> + return rc; >> + } >> + >> + rc = zpci_enable_device(zdev); >> + if (rc) >> + return rc; >> + >> + /* Re-register the IOMMU that was already created */ >> + rc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, >> + (u64)zdev->dma_table); > > Same as above > >> + >> + return rc; >> +} >> +EXPORT_SYMBOL_GPL(kvm_s390_pci_interp_disable); >> + >> > ---8<--- >
On 12/7/21 21:57, Matthew Rosato wrote: > These routines will be wired into the vfio_pci_zdev ioctl handlers to > respond to requests to enable / disable a device for zPCI Load/Store > interpretation. > > The first time such a request is received, enable the necessary facilities > for the guest. > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > --- > arch/s390/include/asm/kvm_pci.h | 4 ++ > arch/s390/kvm/pci.c | 91 +++++++++++++++++++++++++++++++++ > arch/s390/pci/pci.c | 3 ++ > 3 files changed, 98 insertions(+) > > diff --git a/arch/s390/include/asm/kvm_pci.h b/arch/s390/include/asm/kvm_pci.h > index 3e491a39704c..5d6283acb54c 100644 > --- a/arch/s390/include/asm/kvm_pci.h > +++ b/arch/s390/include/asm/kvm_pci.h > @@ -26,4 +26,8 @@ extern int kvm_s390_pci_dev_open(struct zpci_dev *zdev); > extern void kvm_s390_pci_dev_release(struct zpci_dev *zdev); > extern int kvm_s390_pci_attach_kvm(struct zpci_dev *zdev, struct kvm *kvm); > > +extern int kvm_s390_pci_interp_probe(struct zpci_dev *zdev); > +extern int kvm_s390_pci_interp_enable(struct zpci_dev *zdev); > +extern int kvm_s390_pci_interp_disable(struct zpci_dev *zdev); extern prototypes should be avoided in .h files > + > #endif /* ASM_KVM_PCI_H */ > diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c > index f0e5386ff943..57cbe3827ea6 100644 > --- a/arch/s390/kvm/pci.c > +++ b/arch/s390/kvm/pci.c > @@ -10,7 +10,9 @@ > #include <linux/kvm_host.h> > #include <linux/pci.h> > #include <asm/kvm_pci.h> > +#include <asm/sclp.h> > #include "pci.h" > +#include "kvm-s390.h" > > static struct zpci_aift aift; > > @@ -118,6 +120,95 @@ int kvm_s390_pci_aen_init(u8 nisc) > return rc; > } > > +int kvm_s390_pci_interp_probe(struct zpci_dev *zdev) > +{ > + if (!(sclp.has_zpci_interp && test_facility(69))) > + return -EINVAL; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(kvm_s390_pci_interp_probe); > + > +int kvm_s390_pci_interp_enable(struct zpci_dev *zdev) > +{ > + u32 gd; > + int rc; > + > + /* > + * If this is the first request to use an interpreted device, make the > + * necessary vcpu changes > + */ > + if (!zdev->kzdev->kvm->arch.use_zpci_interp) > + kvm_s390_vcpu_pci_enable_interp(zdev->kzdev->kvm); > + > + /* > + * In the event of a system reset in userspace, the GISA designation > + * may still be assigned because the device is still enabled. > + * Verify it's the same guest before proceeding. > + */ > + gd = (u32)(u64)&zdev->kzdev->kvm->arch.sie_page2->gisa; > + if (zdev->gd != 0 && zdev->gd != gd) > + return -EPERM; > + > + if (zdev_enabled(zdev)) { > + zdev->gd = 0; > + rc = zpci_disable_device(zdev); > + if (rc) > + return rc; > + } > + > + /* > + * Store information about the identity of the kvm guest allowed to > + * access this device via interpretation to be used by host CLP > + */ > + zdev->gd = gd; > + > + rc = zpci_enable_device(zdev); > + if (rc) > + goto err; > + > + /* Re-register the IOMMU that was already created */ > + rc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, > + (u64)zdev->dma_table); > + if (rc) > + goto err; > + > + return rc; > + > +err: > + zdev->gd = 0; > + return rc; > +} > +EXPORT_SYMBOL_GPL(kvm_s390_pci_interp_enable); > + > +int kvm_s390_pci_interp_disable(struct zpci_dev *zdev) > +{ > + int rc; > + > + if (zdev->gd == 0) > + return -EINVAL; > + > + /* Remove the host CLP guest designation */ > + zdev->gd = 0; > + > + if (zdev_enabled(zdev)) { > + rc = zpci_disable_device(zdev); > + if (rc) > + return rc; > + } > + > + rc = zpci_enable_device(zdev); > + if (rc) > + return rc; > + > + /* Re-register the IOMMU that was already created */ > + rc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, > + (u64)zdev->dma_table); > + > + return rc; > +} > +EXPORT_SYMBOL_GPL(kvm_s390_pci_interp_disable); > + > int kvm_s390_pci_dev_open(struct zpci_dev *zdev) > { > struct kvm_zdev *kzdev; > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c > index 175854c861cd..0eac84387f3c 100644 > --- a/arch/s390/pci/pci.c > +++ b/arch/s390/pci/pci.c > @@ -141,6 +141,7 @@ int zpci_register_ioat(struct zpci_dev *zdev, u8 dmaas, > zpci_dbg(3, "reg ioat fid:%x, cc:%d, status:%d\n", zdev->fid, cc, status); > return cc; > } > +EXPORT_SYMBOL_GPL(zpci_register_ioat); > > /* Modify PCI: Unregister I/O address translation parameters */ > int zpci_unregister_ioat(struct zpci_dev *zdev, u8 dmaas) > @@ -740,6 +741,7 @@ int zpci_enable_device(struct zpci_dev *zdev) > zpci_update_fh(zdev, fh); > return rc; > } > +EXPORT_SYMBOL_GPL(zpci_enable_device); > > int zpci_disable_device(struct zpci_dev *zdev) > { > @@ -763,6 +765,7 @@ int zpci_disable_device(struct zpci_dev *zdev) > } > return rc; > } > +EXPORT_SYMBOL_GPL(zpci_disable_device); > > /** > * zpci_hot_reset_device - perform a reset of the given zPCI function >
diff --git a/arch/s390/include/asm/kvm_pci.h b/arch/s390/include/asm/kvm_pci.h index 3e491a39704c..5d6283acb54c 100644 --- a/arch/s390/include/asm/kvm_pci.h +++ b/arch/s390/include/asm/kvm_pci.h @@ -26,4 +26,8 @@ extern int kvm_s390_pci_dev_open(struct zpci_dev *zdev); extern void kvm_s390_pci_dev_release(struct zpci_dev *zdev); extern int kvm_s390_pci_attach_kvm(struct zpci_dev *zdev, struct kvm *kvm); +extern int kvm_s390_pci_interp_probe(struct zpci_dev *zdev); +extern int kvm_s390_pci_interp_enable(struct zpci_dev *zdev); +extern int kvm_s390_pci_interp_disable(struct zpci_dev *zdev); + #endif /* ASM_KVM_PCI_H */ diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c index f0e5386ff943..57cbe3827ea6 100644 --- a/arch/s390/kvm/pci.c +++ b/arch/s390/kvm/pci.c @@ -10,7 +10,9 @@ #include <linux/kvm_host.h> #include <linux/pci.h> #include <asm/kvm_pci.h> +#include <asm/sclp.h> #include "pci.h" +#include "kvm-s390.h" static struct zpci_aift aift; @@ -118,6 +120,95 @@ int kvm_s390_pci_aen_init(u8 nisc) return rc; } +int kvm_s390_pci_interp_probe(struct zpci_dev *zdev) +{ + if (!(sclp.has_zpci_interp && test_facility(69))) + return -EINVAL; + + return 0; +} +EXPORT_SYMBOL_GPL(kvm_s390_pci_interp_probe); + +int kvm_s390_pci_interp_enable(struct zpci_dev *zdev) +{ + u32 gd; + int rc; + + /* + * If this is the first request to use an interpreted device, make the + * necessary vcpu changes + */ + if (!zdev->kzdev->kvm->arch.use_zpci_interp) + kvm_s390_vcpu_pci_enable_interp(zdev->kzdev->kvm); + + /* + * In the event of a system reset in userspace, the GISA designation + * may still be assigned because the device is still enabled. + * Verify it's the same guest before proceeding. + */ + gd = (u32)(u64)&zdev->kzdev->kvm->arch.sie_page2->gisa; + if (zdev->gd != 0 && zdev->gd != gd) + return -EPERM; + + if (zdev_enabled(zdev)) { + zdev->gd = 0; + rc = zpci_disable_device(zdev); + if (rc) + return rc; + } + + /* + * Store information about the identity of the kvm guest allowed to + * access this device via interpretation to be used by host CLP + */ + zdev->gd = gd; + + rc = zpci_enable_device(zdev); + if (rc) + goto err; + + /* Re-register the IOMMU that was already created */ + rc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, + (u64)zdev->dma_table); + if (rc) + goto err; + + return rc; + +err: + zdev->gd = 0; + return rc; +} +EXPORT_SYMBOL_GPL(kvm_s390_pci_interp_enable); + +int kvm_s390_pci_interp_disable(struct zpci_dev *zdev) +{ + int rc; + + if (zdev->gd == 0) + return -EINVAL; + + /* Remove the host CLP guest designation */ + zdev->gd = 0; + + if (zdev_enabled(zdev)) { + rc = zpci_disable_device(zdev); + if (rc) + return rc; + } + + rc = zpci_enable_device(zdev); + if (rc) + return rc; + + /* Re-register the IOMMU that was already created */ + rc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, + (u64)zdev->dma_table); + + return rc; +} +EXPORT_SYMBOL_GPL(kvm_s390_pci_interp_disable); + int kvm_s390_pci_dev_open(struct zpci_dev *zdev) { struct kvm_zdev *kzdev; diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index 175854c861cd..0eac84387f3c 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -141,6 +141,7 @@ int zpci_register_ioat(struct zpci_dev *zdev, u8 dmaas, zpci_dbg(3, "reg ioat fid:%x, cc:%d, status:%d\n", zdev->fid, cc, status); return cc; } +EXPORT_SYMBOL_GPL(zpci_register_ioat); /* Modify PCI: Unregister I/O address translation parameters */ int zpci_unregister_ioat(struct zpci_dev *zdev, u8 dmaas) @@ -740,6 +741,7 @@ int zpci_enable_device(struct zpci_dev *zdev) zpci_update_fh(zdev, fh); return rc; } +EXPORT_SYMBOL_GPL(zpci_enable_device); int zpci_disable_device(struct zpci_dev *zdev) { @@ -763,6 +765,7 @@ int zpci_disable_device(struct zpci_dev *zdev) } return rc; } +EXPORT_SYMBOL_GPL(zpci_disable_device); /** * zpci_hot_reset_device - perform a reset of the given zPCI function
These routines will be wired into the vfio_pci_zdev ioctl handlers to respond to requests to enable / disable a device for zPCI Load/Store interpretation. The first time such a request is received, enable the necessary facilities for the guest. Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> --- arch/s390/include/asm/kvm_pci.h | 4 ++ arch/s390/kvm/pci.c | 91 +++++++++++++++++++++++++++++++++ arch/s390/pci/pci.c | 3 ++ 3 files changed, 98 insertions(+)