| Message ID | 20220818164652.269336-1-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: pci: Hook to access KVM lowlevel from VFIO | expand |
On 8/18/22 12:46 PM, Pierre Morel wrote: > We have a cross dependency between KVM and VFIO when using > s390 vfio_pci_zdev extensions for PCI passthrough > To be able to keep both subsystem modular we add a registering > hook inside the S390 core code. > > This fixes a build problem when VFIO is built-in and KVM is built > as a module. > > Reported-by: Randy Dunlap <rdunlap@infradead.org> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > Fixes: 09340b2fca007 ("KVM: s390: pci: add routines to start/stop inter..") > Cc: <stable@vger.kernel.org> > --- > arch/s390/include/asm/kvm_host.h | 17 ++++++----------- > arch/s390/kvm/pci.c | 10 ++++++---- > arch/s390/pci/Makefile | 2 ++ > arch/s390/pci/pci_kvm_hook.c | 11 +++++++++++ > drivers/vfio/pci/vfio_pci_zdev.c | 8 ++++++-- > 5 files changed, 31 insertions(+), 17 deletions(-) > create mode 100644 arch/s390/pci/pci_kvm_hook.c > > diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h > index f39092e0ceaa..b1e98a9ed152 100644 > --- a/arch/s390/include/asm/kvm_host.h > +++ b/arch/s390/include/asm/kvm_host.h > @@ -1038,16 +1038,11 @@ static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {} > #define __KVM_HAVE_ARCH_VM_FREE > void kvm_arch_free_vm(struct kvm *kvm); > > -#ifdef CONFIG_VFIO_PCI_ZDEV_KVM > -int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm); > -void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev); > -#else > -static inline int kvm_s390_pci_register_kvm(struct zpci_dev *dev, > - struct kvm *kvm) > -{ > - return -EPERM; > -} > -static inline void kvm_s390_pci_unregister_kvm(struct zpci_dev *dev) {} > -#endif > +struct zpci_kvm_hook { > + int (*kvm_register)(void *opaque, struct kvm *kvm); > + void (*kvm_unregister)(void *opaque); > +}; > + > +extern struct zpci_kvm_hook zpci_kvm_hook; > > #endif > diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c > index 4946fb7757d6..22c025538323 100644 > --- a/arch/s390/kvm/pci.c > +++ b/arch/s390/kvm/pci.c > @@ -431,8 +431,9 @@ static void kvm_s390_pci_dev_release(struct zpci_dev *zdev) > * available, enable them and let userspace indicate whether or not they will > * be used (specify SHM bit to disable). > */ > -int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm) > +static int kvm_s390_pci_register_kvm(void *opaque, struct kvm *kvm) > { > + struct zpci_dev *zdev = opaque; > int rc; > > if (!zdev) > @@ -510,10 +511,10 @@ int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm) > kvm_put_kvm(kvm); > return rc; > } > -EXPORT_SYMBOL_GPL(kvm_s390_pci_register_kvm); > > -void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev) > +static void kvm_s390_pci_unregister_kvm(void *opaque) > { > + struct zpci_dev *zdev = opaque; > struct kvm *kvm; > > if (!zdev) > @@ -566,7 +567,6 @@ void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev) > > kvm_put_kvm(kvm); > } > -EXPORT_SYMBOL_GPL(kvm_s390_pci_unregister_kvm); > > void kvm_s390_pci_init_list(struct kvm *kvm) > { > @@ -678,6 +678,8 @@ int kvm_s390_pci_init(void) > > spin_lock_init(&aift->gait_lock); > mutex_init(&aift->aift_lock); > + zpci_kvm_hook.kvm_register = kvm_s390_pci_register_kvm; > + zpci_kvm_hook.kvm_unregister = kvm_s390_pci_unregister_kvm; > > return 0; > } You should also set these to NULL in kvm_s390_pci_exit (which is called from kvm_arch_exit). In practice, the kvm module would need to be loaded again before we have a nonzero vdev->vdev.kvm so it should never be an issue - but we should clean up anyway when the module is removed. With that change: Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> > diff --git a/arch/s390/pci/Makefile b/arch/s390/pci/Makefile > index bf557a1b789c..c02dbfb415d9 100644 > --- a/arch/s390/pci/Makefile > +++ b/arch/s390/pci/Makefile > @@ -7,3 +7,5 @@ obj-$(CONFIG_PCI) += pci.o pci_irq.o pci_dma.o pci_clp.o pci_sysfs.o \ > pci_event.o pci_debug.o pci_insn.o pci_mmio.o \ > pci_bus.o > obj-$(CONFIG_PCI_IOV) += pci_iov.o > + > +obj-y += pci_kvm_hook.o > diff --git a/arch/s390/pci/pci_kvm_hook.c b/arch/s390/pci/pci_kvm_hook.c > new file mode 100644 > index 000000000000..ff34baf50a3e > --- /dev/null > +++ b/arch/s390/pci/pci_kvm_hook.c > @@ -0,0 +1,11 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * VFIO ZPCI devices support > + * > + * Copyright (C) IBM Corp. 2022. All rights reserved. > + * Author(s): Pierre Morel <pmorel@linux.ibm.com> > + */ > +#include <linux/kvm_host.h> > + > +struct zpci_kvm_hook zpci_kvm_hook; > +EXPORT_SYMBOL_GPL(zpci_kvm_hook); > diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c > index e163aa9f6144..0cbdcd14f1c8 100644 > --- a/drivers/vfio/pci/vfio_pci_zdev.c > +++ b/drivers/vfio/pci/vfio_pci_zdev.c > @@ -151,7 +151,10 @@ int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev) > if (!vdev->vdev.kvm) > return 0; > > - return kvm_s390_pci_register_kvm(zdev, vdev->vdev.kvm); > + if (zpci_kvm_hook.kvm_register) > + return zpci_kvm_hook.kvm_register(zdev, vdev->vdev.kvm); > + > + return -ENOENT; > } > > void vfio_pci_zdev_close_device(struct vfio_pci_core_device *vdev) > @@ -161,5 +164,6 @@ void vfio_pci_zdev_close_device(struct vfio_pci_core_device *vdev) > if (!zdev || !vdev->vdev.kvm) > return; > > - kvm_s390_pci_unregister_kvm(zdev); > + if (zpci_kvm_hook.kvm_unregister) > + zpci_kvm_hook.kvm_unregister(zdev); > }
On 8/18/22 22:38, Matthew Rosato wrote: > On 8/18/22 12:46 PM, Pierre Morel wrote: >> We have a cross dependency between KVM and VFIO when using >> s390 vfio_pci_zdev extensions for PCI passthrough >> To be able to keep both subsystem modular we add a registering >> hook inside the S390 core code. >> >> This fixes a build problem when VFIO is built-in and KVM is built >> as a module. >> >> Reported-by: Randy Dunlap <rdunlap@infradead.org> >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> Fixes: 09340b2fca007 ("KVM: s390: pci: add routines to start/stop inter..") >> Cc: <stable@vger.kernel.org> >> --- >> arch/s390/include/asm/kvm_host.h | 17 ++++++----------- >> arch/s390/kvm/pci.c | 10 ++++++---- >> arch/s390/pci/Makefile | 2 ++ >> arch/s390/pci/pci_kvm_hook.c | 11 +++++++++++ >> drivers/vfio/pci/vfio_pci_zdev.c | 8 ++++++-- >> 5 files changed, 31 insertions(+), 17 deletions(-) >> create mode 100644 arch/s390/pci/pci_kvm_hook.c >> >> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h >> index f39092e0ceaa..b1e98a9ed152 100644 >> --- a/arch/s390/include/asm/kvm_host.h >> +++ b/arch/s390/include/asm/kvm_host.h >> @@ -1038,16 +1038,11 @@ static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {} >> #define __KVM_HAVE_ARCH_VM_FREE >> void kvm_arch_free_vm(struct kvm *kvm); >> >> -#ifdef CONFIG_VFIO_PCI_ZDEV_KVM >> -int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm); >> -void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev); >> -#else >> -static inline int kvm_s390_pci_register_kvm(struct zpci_dev *dev, >> - struct kvm *kvm) >> -{ >> - return -EPERM; >> -} >> -static inline void kvm_s390_pci_unregister_kvm(struct zpci_dev *dev) {} >> -#endif >> +struct zpci_kvm_hook { >> + int (*kvm_register)(void *opaque, struct kvm *kvm); >> + void (*kvm_unregister)(void *opaque); >> +}; >> + >> +extern struct zpci_kvm_hook zpci_kvm_hook; >> >> #endif >> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c >> index 4946fb7757d6..22c025538323 100644 >> --- a/arch/s390/kvm/pci.c >> +++ b/arch/s390/kvm/pci.c >> @@ -431,8 +431,9 @@ static void kvm_s390_pci_dev_release(struct zpci_dev *zdev) >> * available, enable them and let userspace indicate whether or not they will >> * be used (specify SHM bit to disable). >> */ >> -int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm) >> +static int kvm_s390_pci_register_kvm(void *opaque, struct kvm *kvm) >> { >> + struct zpci_dev *zdev = opaque; >> int rc; >> >> if (!zdev) >> @@ -510,10 +511,10 @@ int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm) >> kvm_put_kvm(kvm); >> return rc; >> } >> -EXPORT_SYMBOL_GPL(kvm_s390_pci_register_kvm); >> >> -void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev) >> +static void kvm_s390_pci_unregister_kvm(void *opaque) >> { >> + struct zpci_dev *zdev = opaque; >> struct kvm *kvm; >> >> if (!zdev) >> @@ -566,7 +567,6 @@ void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev) >> >> kvm_put_kvm(kvm); >> } >> -EXPORT_SYMBOL_GPL(kvm_s390_pci_unregister_kvm); >> >> void kvm_s390_pci_init_list(struct kvm *kvm) >> { >> @@ -678,6 +678,8 @@ int kvm_s390_pci_init(void) >> >> spin_lock_init(&aift->gait_lock); >> mutex_init(&aift->aift_lock); >> + zpci_kvm_hook.kvm_register = kvm_s390_pci_register_kvm; >> + zpci_kvm_hook.kvm_unregister = kvm_s390_pci_unregister_kvm; >> >> return 0; >> } > > You should also set these to NULL in kvm_s390_pci_exit (which is called from kvm_arch_exit). In practice, the kvm module would need to be loaded again before we have a nonzero vdev->vdev.kvm so it should never be an issue - but we should clean up anyway when the module is removed. > > With that change: > > Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> > Yes indeed, will do. Thanks Pierre
On Thu, 2022-08-18 at 18:46 +0200, Pierre Morel wrote: > We have a cross dependency between KVM and VFIO when using > s390 vfio_pci_zdev extensions for PCI passthrough > To be able to keep both subsystem modular we add a registering > hook inside the S390 core code. > > This fixes a build problem when VFIO is built-in and KVM is built > as a module. > > Reported-by: Randy Dunlap <rdunlap@infradead.org> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > Fixes: 09340b2fca007 ("KVM: s390: pci: add routines to start/stop inter..") Please don't shorten the Fixes tag, the subject line is likely also checked by some automated tools. It's okay for this line to be over the column limit and checkpatch.pl --strict also accepts it. > Cc: <stable@vger.kernel.org> > --- > arch/s390/include/asm/kvm_host.h | 17 ++++++----------- > arch/s390/kvm/pci.c | 10 ++++++---- > arch/s390/pci/Makefile | 2 ++ > arch/s390/pci/pci_kvm_hook.c | 11 +++++++++++ > drivers/vfio/pci/vfio_pci_zdev.c | 8 ++++++-- > 5 files changed, 31 insertions(+), 17 deletions(-) > create mode 100644 arch/s390/pci/pci_kvm_hook.c > > ---8<--- > > kvm_put_kvm(kvm); > } > -EXPORT_SYMBOL_GPL(kvm_s390_pci_unregister_kvm); > > void kvm_s390_pci_init_list(struct kvm *kvm) > { > @@ -678,6 +678,8 @@ int kvm_s390_pci_init(void) > > spin_lock_init(&aift->gait_lock); > mutex_init(&aift->aift_lock); > + zpci_kvm_hook.kvm_register = kvm_s390_pci_register_kvm; > + zpci_kvm_hook.kvm_unregister = kvm_s390_pci_unregister_kvm; > > return 0; > } > diff --git a/arch/s390/pci/Makefile b/arch/s390/pci/Makefile > index bf557a1b789c..c02dbfb415d9 100644 > --- a/arch/s390/pci/Makefile > +++ b/arch/s390/pci/Makefile > @@ -7,3 +7,5 @@ obj-$(CONFIG_PCI) += pci.o pci_irq.o pci_dma.o pci_clp.o pci_sysfs.o \ > pci_event.o pci_debug.o pci_insn.o pci_mmio.o \ > pci_bus.o > obj-$(CONFIG_PCI_IOV) += pci_iov.o > + > +obj-y += pci_kvm_hook.o I thought we wanted to compile this only for CONFIG_PCI? > diff --git a/arch/s390/pci/pci_kvm_hook.c b/arch/s390/pci/pci_kvm_hook.c > new file mode 100644 > index 000000000000..ff34baf50a3e ---8<---
On 8/19/22 09:14, Niklas Schnelle wrote: > On Thu, 2022-08-18 at 18:46 +0200, Pierre Morel wrote: >> We have a cross dependency between KVM and VFIO when using >> s390 vfio_pci_zdev extensions for PCI passthrough >> To be able to keep both subsystem modular we add a registering >> hook inside the S390 core code. >> >> This fixes a build problem when VFIO is built-in and KVM is built >> as a module. >> >> Reported-by: Randy Dunlap <rdunlap@infradead.org> >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> Fixes: 09340b2fca007 ("KVM: s390: pci: add routines to start/stop inter..") > > Please don't shorten the Fixes tag, the subject line is likely also > checked by some automated tools. It's okay for this line to be over the > column limit and checkpatch.pl --strict also accepts it. > OK >> Cc: <stable@vger.kernel.org> >> --- >> arch/s390/include/asm/kvm_host.h | 17 ++++++----------- >> arch/s390/kvm/pci.c | 10 ++++++---- >> arch/s390/pci/Makefile | 2 ++ >> arch/s390/pci/pci_kvm_hook.c | 11 +++++++++++ >> drivers/vfio/pci/vfio_pci_zdev.c | 8 ++++++-- >> 5 files changed, 31 insertions(+), 17 deletions(-) >> create mode 100644 arch/s390/pci/pci_kvm_hook.c >> >> > ---8<--- >> >> kvm_put_kvm(kvm); >> } >> -EXPORT_SYMBOL_GPL(kvm_s390_pci_unregister_kvm); >> >> void kvm_s390_pci_init_list(struct kvm *kvm) >> { >> @@ -678,6 +678,8 @@ int kvm_s390_pci_init(void) >> >> spin_lock_init(&aift->gait_lock); >> mutex_init(&aift->aift_lock); >> + zpci_kvm_hook.kvm_register = kvm_s390_pci_register_kvm; >> + zpci_kvm_hook.kvm_unregister = kvm_s390_pci_unregister_kvm; >> >> return 0; >> } >> diff --git a/arch/s390/pci/Makefile b/arch/s390/pci/Makefile >> index bf557a1b789c..c02dbfb415d9 100644 >> --- a/arch/s390/pci/Makefile >> +++ b/arch/s390/pci/Makefile >> @@ -7,3 +7,5 @@ obj-$(CONFIG_PCI) += pci.o pci_irq.o pci_dma.o pci_clp.o pci_sysfs.o \ >> pci_event.o pci_debug.o pci_insn.o pci_mmio.o \ >> pci_bus.o >> obj-$(CONFIG_PCI_IOV) += pci_iov.o >> + >> +obj-y += pci_kvm_hook.o > > I thought we wanted to compile this only for CONFIG_PCI? Ah sorry, that is indeed what I understood with Matt but then I misunderstood your own answer from yesterday. I change to obj-$(CONFIG_PCI) += pci_kvm_hook.o > >> diff --git a/arch/s390/pci/pci_kvm_hook.c b/arch/s390/pci/pci_kvm_hook.c >> new file mode 100644 >> index 000000000000..ff34baf50a3e > ---8<--- >
On Fri, 2022-08-19 at 10:44 +0200, Pierre Morel wrote: > > On 8/19/22 09:14, Niklas Schnelle wrote: > > On Thu, 2022-08-18 at 18:46 +0200, Pierre Morel wrote: > > > We have a cross dependency between KVM and VFIO when using > > > s390 vfio_pci_zdev extensions for PCI passthrough > > > To be able to keep both subsystem modular we add a registering > > > hook inside the S390 core code. > > > > > > This fixes a build problem when VFIO is built-in and KVM is built > > > as a module. > > > > > > Reported-by: Randy Dunlap <rdunlap@infradead.org> > > > Reported-by: kernel test robot <lkp@intel.com> > > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > > Fixes: 09340b2fca007 ("KVM: s390: pci: add routines to start/stop inter..") > > > > Please don't shorten the Fixes tag, the subject line is likely also > > checked by some automated tools. It's okay for this line to be over the > > column limit and checkpatch.pl --strict also accepts it. > > > > OK > > > > Cc: <stable@vger.kernel.org> > > > --- > > > arch/s390/include/asm/kvm_host.h | 17 ++++++----------- > > > arch/s390/kvm/pci.c | 10 ++++++---- > > > arch/s390/pci/Makefile | 2 ++ > > > arch/s390/pci/pci_kvm_hook.c | 11 +++++++++++ > > > drivers/vfio/pci/vfio_pci_zdev.c | 8 ++++++-- > > > 5 files changed, 31 insertions(+), 17 deletions(-) > > > create mode 100644 arch/s390/pci/pci_kvm_hook.c > > > > > > > > ---8<--- > > > > > > kvm_put_kvm(kvm); > > > } > > > -EXPORT_SYMBOL_GPL(kvm_s390_pci_unregister_kvm); > > > > > > void kvm_s390_pci_init_list(struct kvm *kvm) > > > { > > > @@ -678,6 +678,8 @@ int kvm_s390_pci_init(void) > > > > > > spin_lock_init(&aift->gait_lock); > > > mutex_init(&aift->aift_lock); > > > + zpci_kvm_hook.kvm_register = kvm_s390_pci_register_kvm; > > > + zpci_kvm_hook.kvm_unregister = kvm_s390_pci_unregister_kvm; > > > > > > return 0; > > > } > > > diff --git a/arch/s390/pci/Makefile b/arch/s390/pci/Makefile > > > index bf557a1b789c..c02dbfb415d9 100644 > > > --- a/arch/s390/pci/Makefile > > > +++ b/arch/s390/pci/Makefile > > > @@ -7,3 +7,5 @@ obj-$(CONFIG_PCI) += pci.o pci_irq.o pci_dma.o pci_clp.o pci_sysfs.o \ > > > pci_event.o pci_debug.o pci_insn.o pci_mmio.o \ > > > pci_bus.o > > > obj-$(CONFIG_PCI_IOV) += pci_iov.o > > > + > > > +obj-y += pci_kvm_hook.o > > > > I thought we wanted to compile this only for CONFIG_PCI? > > Ah sorry, that is indeed what I understood with Matt but then I > misunderstood your own answer from yesterday. > I change to > obj-$(CONFIG_PCI) += pci_kvm_hook.o > > > > diff --git a/arch/s390/pci/pci_kvm_hook.c b/arch/s390/pci/pci_kvm_hook.c > > > new file mode 100644 > > > index 000000000000..ff34baf50a3e > > ---8<--- > > Ok with the two things above plus the comment by Matt incorporated: Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
On 8/19/22 12:42, Niklas Schnelle wrote: > On Fri, 2022-08-19 at 10:44 +0200, Pierre Morel wrote: >> >> On 8/19/22 09:14, Niklas Schnelle wrote: >>> On Thu, 2022-08-18 at 18:46 +0200, Pierre Morel wrote: >>>> We have a cross dependency between KVM and VFIO when using >>>> s390 vfio_pci_zdev extensions for PCI passthrough >>>> To be able to keep both subsystem modular we add a registering >>>> hook inside the S390 core code. >>>> >>>> This fixes a build problem when VFIO is built-in and KVM is built >>>> as a module. >>>> >>>> Reported-by: Randy Dunlap <rdunlap@infradead.org> >>>> Reported-by: kernel test robot <lkp@intel.com> >>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>>> Fixes: 09340b2fca007 ("KVM: s390: pci: add routines to start/stop inter..") >>> >>> Please don't shorten the Fixes tag, the subject line is likely also >>> checked by some automated tools. It's okay for this line to be over the >>> column limit and checkpatch.pl --strict also accepts it. >>> >> >> OK >> >>>> Cc: <stable@vger.kernel.org> >>>> --- >>>> arch/s390/include/asm/kvm_host.h | 17 ++++++----------- >>>> arch/s390/kvm/pci.c | 10 ++++++---- >>>> arch/s390/pci/Makefile | 2 ++ >>>> arch/s390/pci/pci_kvm_hook.c | 11 +++++++++++ >>>> drivers/vfio/pci/vfio_pci_zdev.c | 8 ++++++-- >>>> 5 files changed, 31 insertions(+), 17 deletions(-) >>>> create mode 100644 arch/s390/pci/pci_kvm_hook.c >>>> >>>> >>> ---8<--- >>>> >>>> kvm_put_kvm(kvm); >>>> } >>>> -EXPORT_SYMBOL_GPL(kvm_s390_pci_unregister_kvm); >>>> >>>> void kvm_s390_pci_init_list(struct kvm *kvm) >>>> { >>>> @@ -678,6 +678,8 @@ int kvm_s390_pci_init(void) >>>> >>>> spin_lock_init(&aift->gait_lock); >>>> mutex_init(&aift->aift_lock); >>>> + zpci_kvm_hook.kvm_register = kvm_s390_pci_register_kvm; >>>> + zpci_kvm_hook.kvm_unregister = kvm_s390_pci_unregister_kvm; >>>> >>>> return 0; >>>> } >>>> diff --git a/arch/s390/pci/Makefile b/arch/s390/pci/Makefile >>>> index bf557a1b789c..c02dbfb415d9 100644 >>>> --- a/arch/s390/pci/Makefile >>>> +++ b/arch/s390/pci/Makefile >>>> @@ -7,3 +7,5 @@ obj-$(CONFIG_PCI) += pci.o pci_irq.o pci_dma.o pci_clp.o pci_sysfs.o \ >>>> pci_event.o pci_debug.o pci_insn.o pci_mmio.o \ >>>> pci_bus.o >>>> obj-$(CONFIG_PCI_IOV) += pci_iov.o >>>> + >>>> +obj-y += pci_kvm_hook.o >>> >>> I thought we wanted to compile this only for CONFIG_PCI? >> >> Ah sorry, that is indeed what I understood with Matt but then I >> misunderstood your own answer from yesterday. >> I change to >> obj-$(CONFIG_PCI) += pci_kvm_hook.o >> >>>> diff --git a/arch/s390/pci/pci_kvm_hook.c b/arch/s390/pci/pci_kvm_hook.c >>>> new file mode 100644 >>>> index 000000000000..ff34baf50a3e >>> ---8<--- >>> > > Ok with the two things above plus the comment by Matt incorporated: > > Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com> > Just a little correction, it changes nothing if the pci_kvm_hook.c goes on same lines as other CONFIG_PCI depending files. So I put it on the same line. Thanks Pierre
---8<--- > > > > > diff --git a/arch/s390/pci/Makefile b/arch/s390/pci/Makefile > > > > > index bf557a1b789c..c02dbfb415d9 100644 > > > > > --- a/arch/s390/pci/Makefile > > > > > +++ b/arch/s390/pci/Makefile > > > > > @@ -7,3 +7,5 @@ obj-$(CONFIG_PCI) += pci.o pci_irq.o pci_dma.o pci_clp.o pci_sysfs.o \ > > > > > pci_event.o pci_debug.o pci_insn.o pci_mmio.o \ > > > > > pci_bus.o > > > > > obj-$(CONFIG_PCI_IOV) += pci_iov.o > > > > > + > > > > > +obj-y += pci_kvm_hook.o > > > > > > > > I thought we wanted to compile this only for CONFIG_PCI? > > > > > > Ah sorry, that is indeed what I understood with Matt but then I > > > misunderstood your own answer from yesterday. > > > I change to > > > obj-$(CONFIG_PCI) += pci_kvm_hook.o > > > > > > > > diff --git a/arch/s390/pci/pci_kvm_hook.c b/arch/s390/pci/pci_kvm_hook.c > > > > > new file mode 100644 > > > > > index 000000000000..ff34baf50a3e > > > > ---8<--- > > > > > > > > Ok with the two things above plus the comment by Matt incorporated: > > > > Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com> > > > > Just a little correction, it changes nothing if the pci_kvm_hook.c goes > on same lines as other CONFIG_PCI depending files. > So I put it on the same line. > > Thanks > > Pierre > Of course yes. Thanks for fixing this and I'm assuming this would either go through the KVM or vfio trees, correct?
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h index f39092e0ceaa..b1e98a9ed152 100644 --- a/arch/s390/include/asm/kvm_host.h +++ b/arch/s390/include/asm/kvm_host.h @@ -1038,16 +1038,11 @@ static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {} #define __KVM_HAVE_ARCH_VM_FREE void kvm_arch_free_vm(struct kvm *kvm); -#ifdef CONFIG_VFIO_PCI_ZDEV_KVM -int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm); -void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev); -#else -static inline int kvm_s390_pci_register_kvm(struct zpci_dev *dev, - struct kvm *kvm) -{ - return -EPERM; -} -static inline void kvm_s390_pci_unregister_kvm(struct zpci_dev *dev) {} -#endif +struct zpci_kvm_hook { + int (*kvm_register)(void *opaque, struct kvm *kvm); + void (*kvm_unregister)(void *opaque); +}; + +extern struct zpci_kvm_hook zpci_kvm_hook; #endif diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c index 4946fb7757d6..22c025538323 100644 --- a/arch/s390/kvm/pci.c +++ b/arch/s390/kvm/pci.c @@ -431,8 +431,9 @@ static void kvm_s390_pci_dev_release(struct zpci_dev *zdev) * available, enable them and let userspace indicate whether or not they will * be used (specify SHM bit to disable). */ -int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm) +static int kvm_s390_pci_register_kvm(void *opaque, struct kvm *kvm) { + struct zpci_dev *zdev = opaque; int rc; if (!zdev) @@ -510,10 +511,10 @@ int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm) kvm_put_kvm(kvm); return rc; } -EXPORT_SYMBOL_GPL(kvm_s390_pci_register_kvm); -void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev) +static void kvm_s390_pci_unregister_kvm(void *opaque) { + struct zpci_dev *zdev = opaque; struct kvm *kvm; if (!zdev) @@ -566,7 +567,6 @@ void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev) kvm_put_kvm(kvm); } -EXPORT_SYMBOL_GPL(kvm_s390_pci_unregister_kvm); void kvm_s390_pci_init_list(struct kvm *kvm) { @@ -678,6 +678,8 @@ int kvm_s390_pci_init(void) spin_lock_init(&aift->gait_lock); mutex_init(&aift->aift_lock); + zpci_kvm_hook.kvm_register = kvm_s390_pci_register_kvm; + zpci_kvm_hook.kvm_unregister = kvm_s390_pci_unregister_kvm; return 0; } diff --git a/arch/s390/pci/Makefile b/arch/s390/pci/Makefile index bf557a1b789c..c02dbfb415d9 100644 --- a/arch/s390/pci/Makefile +++ b/arch/s390/pci/Makefile @@ -7,3 +7,5 @@ obj-$(CONFIG_PCI) += pci.o pci_irq.o pci_dma.o pci_clp.o pci_sysfs.o \ pci_event.o pci_debug.o pci_insn.o pci_mmio.o \ pci_bus.o obj-$(CONFIG_PCI_IOV) += pci_iov.o + +obj-y += pci_kvm_hook.o diff --git a/arch/s390/pci/pci_kvm_hook.c b/arch/s390/pci/pci_kvm_hook.c new file mode 100644 index 000000000000..ff34baf50a3e --- /dev/null +++ b/arch/s390/pci/pci_kvm_hook.c @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * VFIO ZPCI devices support + * + * Copyright (C) IBM Corp. 2022. All rights reserved. + * Author(s): Pierre Morel <pmorel@linux.ibm.com> + */ +#include <linux/kvm_host.h> + +struct zpci_kvm_hook zpci_kvm_hook; +EXPORT_SYMBOL_GPL(zpci_kvm_hook); diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c index e163aa9f6144..0cbdcd14f1c8 100644 --- a/drivers/vfio/pci/vfio_pci_zdev.c +++ b/drivers/vfio/pci/vfio_pci_zdev.c @@ -151,7 +151,10 @@ int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev) if (!vdev->vdev.kvm) return 0; - return kvm_s390_pci_register_kvm(zdev, vdev->vdev.kvm); + if (zpci_kvm_hook.kvm_register) + return zpci_kvm_hook.kvm_register(zdev, vdev->vdev.kvm); + + return -ENOENT; } void vfio_pci_zdev_close_device(struct vfio_pci_core_device *vdev) @@ -161,5 +164,6 @@ void vfio_pci_zdev_close_device(struct vfio_pci_core_device *vdev) if (!zdev || !vdev->vdev.kvm) return; - kvm_s390_pci_unregister_kvm(zdev); + if (zpci_kvm_hook.kvm_unregister) + zpci_kvm_hook.kvm_unregister(zdev); }
We have a cross dependency between KVM and VFIO when using s390 vfio_pci_zdev extensions for PCI passthrough To be able to keep both subsystem modular we add a registering hook inside the S390 core code. This fixes a build problem when VFIO is built-in and KVM is built as a module. Reported-by: Randy Dunlap <rdunlap@infradead.org> Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> Fixes: 09340b2fca007 ("KVM: s390: pci: add routines to start/stop inter..") Cc: <stable@vger.kernel.org> --- arch/s390/include/asm/kvm_host.h | 17 ++++++----------- arch/s390/kvm/pci.c | 10 ++++++---- arch/s390/pci/Makefile | 2 ++ arch/s390/pci/pci_kvm_hook.c | 11 +++++++++++ drivers/vfio/pci/vfio_pci_zdev.c | 8 ++++++-- 5 files changed, 31 insertions(+), 17 deletions(-) create mode 100644 arch/s390/pci/pci_kvm_hook.c