* [PATCH] vfio-pci/zdev: require KVM to be built-in @ 2022-08-14 21:51 Randy Dunlap 2022-08-15 9:43 ` Pierre Morel 0 siblings, 1 reply; 17+ messages in thread From: Randy Dunlap @ 2022-08-14 21:51 UTC (permalink / raw) To: linux-kernel Cc: Randy Dunlap, kernel test robot, Matthew Rosato, Christian Borntraeger, Pierre Morel, Eric Farman, linux-s390, kvm Fix build errors when CONFIG_KVM=m: s390-linux-ld: drivers/vfio/pci/vfio_pci_zdev.o: in function `vfio_pci_zdev_open_device': vfio_pci_zdev.c:(.text+0x242): undefined reference to `kvm_s390_pci_register_kvm' s390-linux-ld: drivers/vfio/pci/vfio_pci_zdev.o: in function `vfio_pci_zdev_close_device': vfio_pci_zdev.c:(.text+0x296): undefined reference to `kvm_s390_pci_unregister_kvm' Having a bool Kconfig symbol depend on a tristate symbol can often lead to problems like this. Fixes: 8061d1c31f1a ("vfio-pci/zdev: add open/close device hooks") Signed-off-by: Randy Dunlap <rdunlap@infradead.org> Reported-by: kernel test robot <lkp@intel.com> Cc: Matthew Rosato <mjrosato@linux.ibm.com> Cc: Christian Borntraeger <borntraeger@linux.ibm.com> Cc: Pierre Morel <pmorel@linux.ibm.com> Cc: Eric Farman <farman@linux.ibm.com> Cc: linux-s390@vger.kernel.org Cc: kvm@vger.kernel.org --- drivers/vfio/pci/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/vfio/pci/Kconfig +++ b/drivers/vfio/pci/Kconfig @@ -46,7 +46,7 @@ endif config VFIO_PCI_ZDEV_KVM bool "VFIO PCI extensions for s390x KVM passthrough" - depends on S390 && KVM + depends on S390 && KVM=y default y help Support s390x-specific extensions to enable support for enhancements ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] vfio-pci/zdev: require KVM to be built-in 2022-08-14 21:51 [PATCH] vfio-pci/zdev: require KVM to be built-in Randy Dunlap @ 2022-08-15 9:43 ` Pierre Morel 2022-08-16 6:04 ` Randy Dunlap 0 siblings, 1 reply; 17+ messages in thread From: Pierre Morel @ 2022-08-15 9:43 UTC (permalink / raw) To: Randy Dunlap, linux-kernel Cc: kernel test robot, Matthew Rosato, Christian Borntraeger, Eric Farman, linux-s390, kvm Thank you Randy for this good catch. However forcing KVM to be include statically in the kernel when using VFIO_PCI extensions is not a good solution for us I think. I suggest we better do something like: ---- diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h index 6287a843e8bc..1733339cc4eb 100644 --- a/arch/s390/include/asm/kvm_host.h +++ b/arch/s390/include/asm/kvm_host.h @@ -1038,7 +1038,7 @@ 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 +#if defined(CONFIG_VFIO_PCI_ZDEV_KVM) || defined(CONFIG_VFIO_PCI_ZDEV_KVM_MODULE) int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm); void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev); #else diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig index f9d0c908e738..bbc375b028ef 100644 --- a/drivers/vfio/pci/Kconfig +++ b/drivers/vfio/pci/Kconfig @@ -45,9 +45,9 @@ config VFIO_PCI_IGD endif config VFIO_PCI_ZDEV_KVM - bool "VFIO PCI extensions for s390x KVM passthrough" + def_tristate y + prompt "VFIO PCI extensions for s390x KVM passthrough" depends on S390 && KVM - default y help Support s390x-specific extensions to enable support for enhancements to KVM passthrough capabilities, such as interpretive execution of ---- What do you think? It seems to me it solves the problem, what do you think? Regards, Pierre On 8/14/22 23:51, Randy Dunlap wrote: > Fix build errors when CONFIG_KVM=m: > > s390-linux-ld: drivers/vfio/pci/vfio_pci_zdev.o: in function `vfio_pci_zdev_open_device': > vfio_pci_zdev.c:(.text+0x242): undefined reference to `kvm_s390_pci_register_kvm' > s390-linux-ld: drivers/vfio/pci/vfio_pci_zdev.o: in function `vfio_pci_zdev_close_device': > vfio_pci_zdev.c:(.text+0x296): undefined reference to `kvm_s390_pci_unregister_kvm' > > Having a bool Kconfig symbol depend on a tristate symbol can often > lead to problems like this. > > Fixes: 8061d1c31f1a ("vfio-pci/zdev: add open/close device hooks") > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > Reported-by: kernel test robot <lkp@intel.com> > Cc: Matthew Rosato <mjrosato@linux.ibm.com> > Cc: Christian Borntraeger <borntraeger@linux.ibm.com> > Cc: Pierre Morel <pmorel@linux.ibm.com> > Cc: Eric Farman <farman@linux.ibm.com> > Cc: linux-s390@vger.kernel.org > Cc: kvm@vger.kernel.org > --- > drivers/vfio/pci/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > --- a/drivers/vfio/pci/Kconfig > +++ b/drivers/vfio/pci/Kconfig > @@ -46,7 +46,7 @@ endif > > config VFIO_PCI_ZDEV_KVM > bool "VFIO PCI extensions for s390x KVM passthrough" > - depends on S390 && KVM > + depends on S390 && KVM=y > default y > help > Support s390x-specific extensions to enable support for enhancements > -- Pierre Morel IBM Lab Boeblingen ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] vfio-pci/zdev: require KVM to be built-in 2022-08-15 9:43 ` Pierre Morel @ 2022-08-16 6:04 ` Randy Dunlap 2022-08-16 7:55 ` Pierre Morel 0 siblings, 1 reply; 17+ messages in thread From: Randy Dunlap @ 2022-08-16 6:04 UTC (permalink / raw) To: Pierre Morel, linux-kernel Cc: kernel test robot, Matthew Rosato, Christian Borntraeger, Eric Farman, linux-s390, kvm Hi-- On 8/15/22 02:43, Pierre Morel wrote: > Thank you Randy for this good catch. > However forcing KVM to be include statically in the kernel when using VFIO_PCI extensions is not a good solution for us I think. > > I suggest we better do something like: > > ---- > > diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h > index 6287a843e8bc..1733339cc4eb 100644 > --- a/arch/s390/include/asm/kvm_host.h > +++ b/arch/s390/include/asm/kvm_host.h > @@ -1038,7 +1038,7 @@ 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 > +#if defined(CONFIG_VFIO_PCI_ZDEV_KVM) || defined(CONFIG_VFIO_PCI_ZDEV_KVM_MODULE) This all looks good except for the line above. It should be: #if IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) Thanks. > int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm); > void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev); > #else > diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig > index f9d0c908e738..bbc375b028ef 100644 > --- a/drivers/vfio/pci/Kconfig > +++ b/drivers/vfio/pci/Kconfig > @@ -45,9 +45,9 @@ config VFIO_PCI_IGD > endif > > config VFIO_PCI_ZDEV_KVM > - bool "VFIO PCI extensions for s390x KVM passthrough" > + def_tristate y > + prompt "VFIO PCI extensions for s390x KVM passthrough" > depends on S390 && KVM > - default y > help > Support s390x-specific extensions to enable support for enhancements > to KVM passthrough capabilities, such as interpretive execution of > > ---- > > What do you think? It seems to me it solves the problem, what do you think? > > Regards, > Pierre -- ~Randy ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] vfio-pci/zdev: require KVM to be built-in 2022-08-16 6:04 ` Randy Dunlap @ 2022-08-16 7:55 ` Pierre Morel 2022-08-16 13:47 ` Pierre Morel 2022-08-16 19:46 ` Matthew Rosato 0 siblings, 2 replies; 17+ messages in thread From: Pierre Morel @ 2022-08-16 7:55 UTC (permalink / raw) To: Randy Dunlap, linux-kernel Cc: kernel test robot, Matthew Rosato, Christian Borntraeger, Eric Farman, linux-s390, kvm On 8/16/22 08:04, Randy Dunlap wrote: > Hi-- > > On 8/15/22 02:43, Pierre Morel wrote: >> Thank you Randy for this good catch. >> However forcing KVM to be include statically in the kernel when using VFIO_PCI extensions is not a good solution for us I think. >> >> I suggest we better do something like: >> >> ---- >> >> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h >> index 6287a843e8bc..1733339cc4eb 100644 >> --- a/arch/s390/include/asm/kvm_host.h >> +++ b/arch/s390/include/asm/kvm_host.h >> @@ -1038,7 +1038,7 @@ 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 >> +#if defined(CONFIG_VFIO_PCI_ZDEV_KVM) || defined(CONFIG_VFIO_PCI_ZDEV_KVM_MODULE) > > This all looks good except for the line above. > It should be: > > #if IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) > > Thanks. Yes, better, thanks. How do we do? Should I repost it with reported-by you or do you want to post it? Pierre > > >> int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm); >> void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev); >> #else >> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig >> index f9d0c908e738..bbc375b028ef 100644 >> --- a/drivers/vfio/pci/Kconfig >> +++ b/drivers/vfio/pci/Kconfig >> @@ -45,9 +45,9 @@ config VFIO_PCI_IGD >> endif >> >> config VFIO_PCI_ZDEV_KVM >> - bool "VFIO PCI extensions for s390x KVM passthrough" >> + def_tristate y >> + prompt "VFIO PCI extensions for s390x KVM passthrough" >> depends on S390 && KVM >> - default y >> help >> Support s390x-specific extensions to enable support for enhancements >> to KVM passthrough capabilities, such as interpretive execution of >> >> ---- >> >> What do you think? It seems to me it solves the problem, what do you think? >> >> Regards, >> Pierre > > -- Pierre Morel IBM Lab Boeblingen ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] vfio-pci/zdev: require KVM to be built-in 2022-08-16 7:55 ` Pierre Morel @ 2022-08-16 13:47 ` Pierre Morel 2022-08-16 18:06 ` Randy Dunlap 2022-08-16 19:46 ` Matthew Rosato 1 sibling, 1 reply; 17+ messages in thread From: Pierre Morel @ 2022-08-16 13:47 UTC (permalink / raw) To: Randy Dunlap, linux-kernel Cc: kernel test robot, Matthew Rosato, Christian Borntraeger, Eric Farman, linux-s390, kvm Randy, I need to provide the correction patch rapidly. Without answer I will propose the patch. Regards, Pierre On 8/16/22 09:55, Pierre Morel wrote: > > > On 8/16/22 08:04, Randy Dunlap wrote: >> Hi-- >> >> On 8/15/22 02:43, Pierre Morel wrote: >>> Thank you Randy for this good catch. >>> However forcing KVM to be include statically in the kernel when using >>> VFIO_PCI extensions is not a good solution for us I think. >>> >>> I suggest we better do something like: >>> >>> ---- >>> >>> diff --git a/arch/s390/include/asm/kvm_host.h >>> b/arch/s390/include/asm/kvm_host.h >>> index 6287a843e8bc..1733339cc4eb 100644 >>> --- a/arch/s390/include/asm/kvm_host.h >>> +++ b/arch/s390/include/asm/kvm_host.h >>> @@ -1038,7 +1038,7 @@ 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 >>> +#if defined(CONFIG_VFIO_PCI_ZDEV_KVM) || >>> defined(CONFIG_VFIO_PCI_ZDEV_KVM_MODULE) >> >> This all looks good except for the line above. >> It should be: >> >> #if IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) >> >> Thanks. > > Yes, better, thanks. > How do we do? Should I repost it with reported-by you or do you want to > post it? > > Pierre > > >> >> >>> int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm); >>> void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev); >>> #else >>> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig >>> index f9d0c908e738..bbc375b028ef 100644 >>> --- a/drivers/vfio/pci/Kconfig >>> +++ b/drivers/vfio/pci/Kconfig >>> @@ -45,9 +45,9 @@ config VFIO_PCI_IGD >>> endif >>> >>> config VFIO_PCI_ZDEV_KVM >>> - bool "VFIO PCI extensions for s390x KVM passthrough" >>> + def_tristate y >>> + prompt "VFIO PCI extensions for s390x KVM passthrough" >>> depends on S390 && KVM >>> - default y >>> help >>> Support s390x-specific extensions to enable support for >>> enhancements >>> to KVM passthrough capabilities, such as interpretive >>> execution of >>> >>> ---- >>> >>> What do you think? It seems to me it solves the problem, what do you >>> think? >>> >>> Regards, >>> Pierre >> >> > -- Pierre Morel IBM Lab Boeblingen ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] vfio-pci/zdev: require KVM to be built-in 2022-08-16 13:47 ` Pierre Morel @ 2022-08-16 18:06 ` Randy Dunlap 0 siblings, 0 replies; 17+ messages in thread From: Randy Dunlap @ 2022-08-16 18:06 UTC (permalink / raw) To: Pierre Morel, linux-kernel Cc: kernel test robot, Matthew Rosato, Christian Borntraeger, Eric Farman, linux-s390, kvm On 8/16/22 06:47, Pierre Morel wrote: > Randy, > > I need to provide the correction patch rapidly. > Without answer I will propose the patch. > > Regards, > Pierre Please go ahead with it. Thanks. > > On 8/16/22 09:55, Pierre Morel wrote: >> >> >> On 8/16/22 08:04, Randy Dunlap wrote: >>> Hi-- >>> >>> On 8/15/22 02:43, Pierre Morel wrote: >>>> Thank you Randy for this good catch. >>>> However forcing KVM to be include statically in the kernel when using VFIO_PCI extensions is not a good solution for us I think. >>>> >>>> I suggest we better do something like: >>>> >>>> ---- >>>> >>>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h >>>> index 6287a843e8bc..1733339cc4eb 100644 >>>> --- a/arch/s390/include/asm/kvm_host.h >>>> +++ b/arch/s390/include/asm/kvm_host.h >>>> @@ -1038,7 +1038,7 @@ 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 >>>> +#if defined(CONFIG_VFIO_PCI_ZDEV_KVM) || defined(CONFIG_VFIO_PCI_ZDEV_KVM_MODULE) >>> >>> This all looks good except for the line above. >>> It should be: >>> >>> #if IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) >>> >>> Thanks. >> >> Yes, better, thanks. >> How do we do? Should I repost it with reported-by you or do you want to post it? >> >> Pierre >> >> >>> >>> >>>> int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm); >>>> void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev); >>>> #else >>>> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig >>>> index f9d0c908e738..bbc375b028ef 100644 >>>> --- a/drivers/vfio/pci/Kconfig >>>> +++ b/drivers/vfio/pci/Kconfig >>>> @@ -45,9 +45,9 @@ config VFIO_PCI_IGD >>>> endif >>>> >>>> config VFIO_PCI_ZDEV_KVM >>>> - bool "VFIO PCI extensions for s390x KVM passthrough" >>>> + def_tristate y >>>> + prompt "VFIO PCI extensions for s390x KVM passthrough" >>>> depends on S390 && KVM >>>> - default y >>>> help >>>> Support s390x-specific extensions to enable support for enhancements >>>> to KVM passthrough capabilities, such as interpretive execution of >>>> >>>> ---- >>>> >>>> What do you think? It seems to me it solves the problem, what do you think? >>>> >>>> Regards, >>>> Pierre >>> >>> >> > -- ~Randy ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] vfio-pci/zdev: require KVM to be built-in 2022-08-16 7:55 ` Pierre Morel 2022-08-16 13:47 ` Pierre Morel @ 2022-08-16 19:46 ` Matthew Rosato 2022-08-16 20:22 ` Pierre Morel 1 sibling, 1 reply; 17+ messages in thread From: Matthew Rosato @ 2022-08-16 19:46 UTC (permalink / raw) To: Pierre Morel, Randy Dunlap, linux-kernel Cc: kernel test robot, Christian Borntraeger, Eric Farman, linux-s390, kvm On 8/16/22 3:55 AM, Pierre Morel wrote: > > > On 8/16/22 08:04, Randy Dunlap wrote: >> Hi-- >> >> On 8/15/22 02:43, Pierre Morel wrote: >>> Thank you Randy for this good catch. >>> However forcing KVM to be include statically in the kernel when using VFIO_PCI extensions is not a good solution for us I think. >>> >>> I suggest we better do something like: >>> >>> ---- >>> >>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h >>> index 6287a843e8bc..1733339cc4eb 100644 >>> --- a/arch/s390/include/asm/kvm_host.h >>> +++ b/arch/s390/include/asm/kvm_host.h >>> @@ -1038,7 +1038,7 @@ 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 >>> +#if defined(CONFIG_VFIO_PCI_ZDEV_KVM) || defined(CONFIG_VFIO_PCI_ZDEV_KVM_MODULE) >> >> This all looks good except for the line above. >> It should be: >> >> #if IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) >> >> Thanks. > > Yes, better, thanks. > How do we do? Should I repost it with reported-by you or do you want to post it? > > Pierre Thanks for looking into this while I was away. I think the issue is not just CONFIG_KVM=m && CONFIG_VFIO_PCI_ZDEV_KVM=y -- it also requires CONFIG_VFIO_PCI=y && CONFIG_VFIO_PCI_CORE=y. This combination results in building in vfio_pci (which tries to link the calls to kvm_s390_pci_register_kvm and kvm_s390_pci_unregister_kvm which is not built in). However... this tristate + IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) check in kvm_host.h will not solve the issue. Rather, due to the #ifdef CONFIG_VFIO_PCI_ZDEV_KVM in include/linux/vfio_pci_core.h, this change will just cause us to never call kvm_s390_pci_register_kvm or kvm_s390_pci_unregister_kvm when CONFIG_VFIO_PCI_ZDEV_KVM=m, effectively treating CONFIG_VFIO_PCI_ZDEV_KVM=m as a 'n' and we don't get the zdev kvm extensions, which I don't think was the intent. I'm still thinking & am open to other ideas, but one solution to avoiding building in KVM would be to go back to using symbol_get for these 2 interfaces (kvm_s390_pci_register_kvm and kvm_s390_pci_unregister_kvm) as was done in a prior version of this series like virt/kvm/vfio.c does and otherwise leave CONFIG_VFIO_PCI_ZDEV_KVM as-is. diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c index e163aa9f6144..09c2758134c7 100644 --- a/drivers/vfio/pci/vfio_pci_zdev.c +++ b/drivers/vfio/pci/vfio_pci_zdev.c @@ -144,6 +144,8 @@ int vfio_pci_info_zdev_add_caps(struct vfio_pci_core_device *vdev, int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev) { struct zpci_dev *zdev = to_zpci(vdev->pdev); + int (*fn)(struct zpci_dev *zdev, struct kvm *kvm); + int rc; if (!zdev) return -ENODEV; @@ -151,15 +153,30 @@ 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); + fn = symbol_get(kvm_s390_pci_register_kvm); + if (!fn) + return -EPERM; + + rc = fn(zdev, vdev->vdev.kvm); + + symbol_put(kvm_s390_pci_register_kvm); + + return rc; } void vfio_pci_zdev_close_device(struct vfio_pci_core_device *vdev) { struct zpci_dev *zdev = to_zpci(vdev->pdev); + void (*fn)(struct zpci_dev *zdev); if (!zdev || !vdev->vdev.kvm) return; - kvm_s390_pci_unregister_kvm(zdev); + fn = symbol_get(kvm_s390_pci_unregister_kvm); + if (!fn) + return; + + fn(zdev); + + symbol_put(kvm_s390_pci_unregister_kvm); } ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] vfio-pci/zdev: require KVM to be built-in 2022-08-16 19:46 ` Matthew Rosato @ 2022-08-16 20:22 ` Pierre Morel 2022-08-16 20:28 ` [PATCH] KVM: s390: pci: VFIO_PCI ZDEV configuration fix Pierre Morel 2022-08-18 10:23 ` [PATCH] KVM: s390: pci: Hook to access KVM lowlevel from VFIO Pierre Morel 0 siblings, 2 replies; 17+ messages in thread From: Pierre Morel @ 2022-08-16 20:22 UTC (permalink / raw) To: Matthew Rosato, Randy Dunlap, linux-kernel Cc: kernel test robot, Christian Borntraeger, Eric Farman, linux-s390, kvm On 8/16/22 21:46, Matthew Rosato wrote: > On 8/16/22 3:55 AM, Pierre Morel wrote: >> >> >> On 8/16/22 08:04, Randy Dunlap wrote: >>> Hi-- >>> >>> On 8/15/22 02:43, Pierre Morel wrote: >>>> Thank you Randy for this good catch. >>>> However forcing KVM to be include statically in the kernel when using VFIO_PCI extensions is not a good solution for us I think. >>>> >>>> I suggest we better do something like: >>>> >>>> ---- >>>> >>>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h >>>> index 6287a843e8bc..1733339cc4eb 100644 >>>> --- a/arch/s390/include/asm/kvm_host.h >>>> +++ b/arch/s390/include/asm/kvm_host.h >>>> @@ -1038,7 +1038,7 @@ 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 >>>> +#if defined(CONFIG_VFIO_PCI_ZDEV_KVM) || defined(CONFIG_VFIO_PCI_ZDEV_KVM_MODULE) >>> >>> This all looks good except for the line above. >>> It should be: >>> >>> #if IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) >>> >>> Thanks. >> >> Yes, better, thanks. >> How do we do? Should I repost it with reported-by you or do you want to post it? >> >> Pierre > > Thanks for looking into this while I was away. > > I think the issue is not just CONFIG_KVM=m && CONFIG_VFIO_PCI_ZDEV_KVM=y -- it also requires CONFIG_VFIO_PCI=y && CONFIG_VFIO_PCI_CORE=y. This combination results in building in vfio_pci (which tries to link the calls to kvm_s390_pci_register_kvm and kvm_s390_pci_unregister_kvm which is not built in). > > However... this tristate + IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) check in kvm_host.h will not solve the issue. Rather, due to the #ifdef CONFIG_VFIO_PCI_ZDEV_KVM in include/linux/vfio_pci_core.h, this change will just cause us to never call kvm_s390_pci_register_kvm or kvm_s390_pci_unregister_kvm when CONFIG_VFIO_PCI_ZDEV_KVM=m, effectively treating CONFIG_VFIO_PCI_ZDEV_KVM=m as a 'n' and we don't get the zdev kvm extensions, which I don't think was the intent. > > I'm still thinking & am open to other ideas, but one solution to avoiding building in KVM would be to go back to using symbol_get for these 2 interfaces (kvm_s390_pci_register_kvm and kvm_s390_pci_unregister_kvm) as was done in a prior version of this series like virt/kvm/vfio.c does and otherwise leave CONFIG_VFIO_PCI_ZDEV_KVM as-is. > > diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c > index e163aa9f6144..09c2758134c7 100644 > --- a/drivers/vfio/pci/vfio_pci_zdev.c > +++ b/drivers/vfio/pci/vfio_pci_zdev.c > @@ -144,6 +144,8 @@ int vfio_pci_info_zdev_add_caps(struct vfio_pci_core_device *vdev, > int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev) > { > struct zpci_dev *zdev = to_zpci(vdev->pdev); > + int (*fn)(struct zpci_dev *zdev, struct kvm *kvm); > + int rc; > > if (!zdev) > return -ENODEV; > @@ -151,15 +153,30 @@ 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); > + fn = symbol_get(kvm_s390_pci_register_kvm); > + if (!fn) > + return -EPERM; > + > + rc = fn(zdev, vdev->vdev.kvm); > + > + symbol_put(kvm_s390_pci_register_kvm); > + > + return rc; > } > > void vfio_pci_zdev_close_device(struct vfio_pci_core_device *vdev) > { > struct zpci_dev *zdev = to_zpci(vdev->pdev); > + void (*fn)(struct zpci_dev *zdev); > > if (!zdev || !vdev->vdev.kvm) > return; > > - kvm_s390_pci_unregister_kvm(zdev); > + fn = symbol_get(kvm_s390_pci_unregister_kvm); > + if (!fn) > + return; > + > + fn(zdev); > + > + symbol_put(kvm_s390_pci_unregister_kvm); > } > > Hello Matt, In between I came to another solution that seems to satisfy the dependencies. Still need to check that the functionality is still intact :) I send you the proposition as a reply. Regards, Pierre -- Pierre Morel IBM Lab Boeblingen ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] KVM: s390: pci: VFIO_PCI ZDEV configuration fix 2022-08-16 20:22 ` Pierre Morel @ 2022-08-16 20:28 ` Pierre Morel 2022-08-16 22:15 ` Matthew Rosato 2022-08-18 10:23 ` [PATCH] KVM: s390: pci: Hook to access KVM lowlevel from VFIO Pierre Morel 1 sibling, 1 reply; 17+ messages in thread From: Pierre Morel @ 2022-08-16 20:28 UTC (permalink / raw) To: mjrosato; +Cc: rdunlap, linux-kernel, lkp, borntraeger, farman, linux-s390, kvm Fixing configuration for VFIO PCI interpretation. 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..") Fixes: c435c54639aa5 ("vfio/pci: introduce CONFIG_VFIO_PCI_ZDEV_KVM..") Cc: <stable@vger.kernel.org> --- arch/s390/include/asm/kvm_host.h | 9 --------- arch/s390/kvm/Makefile | 2 +- arch/s390/kvm/pci.c | 4 ++-- drivers/vfio/pci/Kconfig | 4 ++-- include/linux/vfio_pci_core.h | 2 +- 5 files changed, 6 insertions(+), 15 deletions(-) diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h index f39092e0ceaa..f6cf961731af 100644 --- a/arch/s390/include/asm/kvm_host.h +++ b/arch/s390/include/asm/kvm_host.h @@ -1038,16 +1038,7 @@ 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 #endif diff --git a/arch/s390/kvm/Makefile b/arch/s390/kvm/Makefile index 02217fb4ae10..be36afcfd6ff 100644 --- a/arch/s390/kvm/Makefile +++ b/arch/s390/kvm/Makefile @@ -9,6 +9,6 @@ ccflags-y := -Ivirt/kvm -Iarch/s390/kvm kvm-y += kvm-s390.o intercept.o interrupt.o priv.o sigp.o kvm-y += diag.o gaccess.o guestdbg.o vsie.o pv.o +kvm-y += pci.o -kvm-$(CONFIG_VFIO_PCI_ZDEV_KVM) += pci.o obj-$(CONFIG_KVM) += kvm.o diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c index 4946fb7757d6..cf8ab72a2109 100644 --- a/arch/s390/kvm/pci.c +++ b/arch/s390/kvm/pci.c @@ -435,7 +435,7 @@ int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm) { int rc; - if (!zdev) + if (!IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) || !zdev) return -EINVAL; mutex_lock(&zdev->kzdev_lock); @@ -516,7 +516,7 @@ void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev) { struct kvm *kvm; - if (!zdev) + if (!IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) || !zdev) return; mutex_lock(&zdev->kzdev_lock); diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig index f9d0c908e738..bbc375b028ef 100644 --- a/drivers/vfio/pci/Kconfig +++ b/drivers/vfio/pci/Kconfig @@ -45,9 +45,9 @@ config VFIO_PCI_IGD endif config VFIO_PCI_ZDEV_KVM - bool "VFIO PCI extensions for s390x KVM passthrough" + def_tristate y + prompt "VFIO PCI extensions for s390x KVM passthrough" depends on S390 && KVM - default y help Support s390x-specific extensions to enable support for enhancements to KVM passthrough capabilities, such as interpretive execution of diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h index 5579ece4347b..7db3bb8129b1 100644 --- a/include/linux/vfio_pci_core.h +++ b/include/linux/vfio_pci_core.h @@ -205,7 +205,7 @@ static inline int vfio_pci_igd_init(struct vfio_pci_core_device *vdev) } #endif -#ifdef CONFIG_VFIO_PCI_ZDEV_KVM +#if IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) int vfio_pci_info_zdev_add_caps(struct vfio_pci_core_device *vdev, struct vfio_info_cap *caps); int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev); -- 2.31.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: s390: pci: VFIO_PCI ZDEV configuration fix 2022-08-16 20:28 ` [PATCH] KVM: s390: pci: VFIO_PCI ZDEV configuration fix Pierre Morel @ 2022-08-16 22:15 ` Matthew Rosato 2022-08-17 7:10 ` Pierre Morel 0 siblings, 1 reply; 17+ messages in thread From: Matthew Rosato @ 2022-08-16 22:15 UTC (permalink / raw) To: Pierre Morel Cc: rdunlap, linux-kernel, lkp, borntraeger, farman, linux-s390, kvm On 8/16/22 4:28 PM, Pierre Morel wrote: > Fixing configuration for VFIO PCI interpretation. > > 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..") > Fixes: c435c54639aa5 ("vfio/pci: introduce CONFIG_VFIO_PCI_ZDEV_KVM..") > Cc: <stable@vger.kernel.org> > --- > arch/s390/include/asm/kvm_host.h | 9 --------- > arch/s390/kvm/Makefile | 2 +- > arch/s390/kvm/pci.c | 4 ++-- > drivers/vfio/pci/Kconfig | 4 ++-- > include/linux/vfio_pci_core.h | 2 +- > 5 files changed, 6 insertions(+), 15 deletions(-) > > diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h > index f39092e0ceaa..f6cf961731af 100644 > --- a/arch/s390/include/asm/kvm_host.h > +++ b/arch/s390/include/asm/kvm_host.h > @@ -1038,16 +1038,7 @@ 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 > > #endif > diff --git a/arch/s390/kvm/Makefile b/arch/s390/kvm/Makefile > index 02217fb4ae10..be36afcfd6ff 100644 > --- a/arch/s390/kvm/Makefile > +++ b/arch/s390/kvm/Makefile > @@ -9,6 +9,6 @@ ccflags-y := -Ivirt/kvm -Iarch/s390/kvm > > kvm-y += kvm-s390.o intercept.o interrupt.o priv.o sigp.o > kvm-y += diag.o gaccess.o guestdbg.o vsie.o pv.o > +kvm-y += pci.o > > -kvm-$(CONFIG_VFIO_PCI_ZDEV_KVM) += pci.o You would need to switch this to CONFIG_PCI at least, else we get build errors with CONFIG_PCI=n. > obj-$(CONFIG_KVM) += kvm.o > diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c > index 4946fb7757d6..cf8ab72a2109 100644 > --- a/arch/s390/kvm/pci.c > +++ b/arch/s390/kvm/pci.c > @@ -435,7 +435,7 @@ int kvm_s390_pci_register_kvm(struct zpci_dev *zdev, struct kvm *kvm) > { > int rc; > > - if (!zdev) > + if (!IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) || !zdev) > return -EINVAL; > > mutex_lock(&zdev->kzdev_lock); > @@ -516,7 +516,7 @@ void kvm_s390_pci_unregister_kvm(struct zpci_dev *zdev) > { > struct kvm *kvm; > > - if (!zdev) > + if (!IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) || !zdev) > return; > > mutex_lock(&zdev->kzdev_lock); > diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig > index f9d0c908e738..bbc375b028ef 100644 > --- a/drivers/vfio/pci/Kconfig > +++ b/drivers/vfio/pci/Kconfig > @@ -45,9 +45,9 @@ config VFIO_PCI_IGD > endif > > config VFIO_PCI_ZDEV_KVM > - bool "VFIO PCI extensions for s390x KVM passthrough" > + def_tristate y > + prompt "VFIO PCI extensions for s390x KVM passthrough" > depends on S390 && KVM > - default y > help > Support s390x-specific extensions to enable support for enhancements > to KVM passthrough capabilities, such as interpretive execution of > diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h > index 5579ece4347b..7db3bb8129b1 100644 > --- a/include/linux/vfio_pci_core.h > +++ b/include/linux/vfio_pci_core.h > @@ -205,7 +205,7 @@ static inline int vfio_pci_igd_init(struct vfio_pci_core_device *vdev) > } > #endif > > -#ifdef CONFIG_VFIO_PCI_ZDEV_KVM > +#if IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) > int vfio_pci_info_zdev_add_caps(struct vfio_pci_core_device *vdev, > struct vfio_info_cap *caps); > int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev); This still doesn't seem quite right... I tried some variations: 1) CONFIG_KVM=m CONFIG_VFIO_PCI_CORE=m CONFIG_VFIO_PCI=m CONFIG_VFIO_PCI_ZDEV_KVM=m compiles, works with a small change: diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h index f054d0360a75..99734e135420 100644 --- a/include/linux/sched/user.h +++ b/include/linux/sched/user.h @@ -25,7 +25,7 @@ struct user_struct { #if defined(CONFIG_PERF_EVENTS) || defined(CONFIG_BPF_SYSCALL) || \ defined(CONFIG_NET) || defined(CONFIG_IO_URING) || \ - defined(CONFIG_VFIO_PCI_ZDEV_KVM) + IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) 2) CONFIG_KVM=y CONFIG_VFIO_PCI_CORE=m CONFIG_VFIO_PCI=m CONFIG_VFIO_PCI_ZDEV_KVM=m compiles, works with above user.h change 3) CONFIG_KVM=y CONFIG_VFIO_PCI_CORE=y CONFIG_VFIO_PCI=y CONFIG_VFIO_PCI_ZDEV_KVM=y compiles, works with above user.h change 4) CONFIG_KVM=m CONFIG_VFIO_PCI_CORE=y CONFIG_VFIO_PCI=y CONFIG_VFIO_PCI_ZDEV_KVM=m fails with: ld: drivers/vfio/pci/vfio_pci_core.o: in function `vfio_pci_core_enable': /usr/src/linux/drivers/vfio/pci/vfio_pci_core.c:320: undefined reference to `vfio_pci_zdev_open_device' ld: /usr/src/linux/drivers/vfio/pci/vfio_pci_core.c:349: undefined reference to `vfio_pci_zdev_close_device' ld: drivers/vfio/pci/vfio_pci_core.o: in function `vfio_pci_core_disable': /usr/src/linux/drivers/vfio/pci/vfio_pci_core.c:428: undefined reference to `vfio_pci_zdev_close_device' ld: drivers/vfio/pci/vfio_pci_core.o: in function `vfio_pci_core_ioctl': /usr/src/linux/drivers/vfio/pci/vfio_pci_core.c:712: undefined reference to `vfio_pci_info_zdev_add_caps' 5) CONFIG_KVM=m CONFIG_VFIO_PCI_CORE=y CONFIG_VFIO_PCI=y CONFIG_VFIO_PCI_ZDEV_KVM=y This forces CONFIG_VFIO_PCI_ZDEV_KVM to 'm' and fails as above. ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: s390: pci: VFIO_PCI ZDEV configuration fix 2022-08-16 22:15 ` Matthew Rosato @ 2022-08-17 7:10 ` Pierre Morel 0 siblings, 0 replies; 17+ messages in thread From: Pierre Morel @ 2022-08-17 7:10 UTC (permalink / raw) To: Matthew Rosato Cc: rdunlap, linux-kernel, lkp, borntraeger, farman, linux-s390, kvm On 8/17/22 00:15, Matthew Rosato wrote: > On 8/16/22 4:28 PM, Pierre Morel wrote: >> Fixing configuration for VFIO PCI interpretation. >> > 4) > CONFIG_KVM=m > CONFIG_VFIO_PCI_CORE=y > CONFIG_VFIO_PCI=y > CONFIG_VFIO_PCI_ZDEV_KVM=m > > fails with: > > ld: drivers/vfio/pci/vfio_pci_core.o: in function `vfio_pci_core_enable': > /usr/src/linux/drivers/vfio/pci/vfio_pci_core.c:320: undefined reference to `vfio_pci_zdev_open_device' > ld: /usr/src/linux/drivers/vfio/pci/vfio_pci_core.c:349: undefined reference to `vfio_pci_zdev_close_device' > ld: drivers/vfio/pci/vfio_pci_core.o: in function `vfio_pci_core_disable': > /usr/src/linux/drivers/vfio/pci/vfio_pci_core.c:428: undefined reference to `vfio_pci_zdev_close_device' > ld: drivers/vfio/pci/vfio_pci_core.o: in function `vfio_pci_core_ioctl': > /usr/src/linux/drivers/vfio/pci/vfio_pci_core.c:712: undefined reference to `vfio_pci_info_zdev_add_caps' > > > 5) > CONFIG_KVM=m > CONFIG_VFIO_PCI_CORE=y > CONFIG_VFIO_PCI=y > CONFIG_VFIO_PCI_ZDEV_KVM=y > > This forces CONFIG_VFIO_PCI_ZDEV_KVM to 'm' and fails as above. > > > yes right I was really too stupid yesterday I think you are right and you should go as you proposed or use some pointer hook inside S390 to register with KVM. -- Pierre Morel IBM Lab Boeblingen ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] KVM: s390: pci: Hook to access KVM lowlevel from VFIO 2022-08-16 20:22 ` Pierre Morel 2022-08-16 20:28 ` [PATCH] KVM: s390: pci: VFIO_PCI ZDEV configuration fix Pierre Morel @ 2022-08-18 10:23 ` Pierre Morel 2022-08-18 13:33 ` Matthew Rosato 1 sibling, 1 reply; 17+ messages in thread From: Pierre Morel @ 2022-08-18 10:23 UTC (permalink / raw) To: mjrosato Cc: rdunlap, linux-kernel, lkp, borntraeger, farman, linux-s390, kvm, gor, hca, schnelle We have a cross dependency between KVM and VFIO. 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 or excluded. 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..8312ed9d1937 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 kvm_register_hook { + int (*kvm_register)(void *opaque, struct kvm *kvm); + void (*kvm_unregister)(void *opaque); +}; + +extern struct kvm_register_hook kvm_pci_hook; #endif diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c index 4946fb7757d6..e173fce64c4f 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); + kvm_pci_hook.kvm_register = kvm_s390_pci_register_kvm; + kvm_pci_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..9d8799b72dbf --- /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 kvm_register_hook kvm_pci_hook; +EXPORT_SYMBOL_GPL(kvm_pci_hook); diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c index e163aa9f6144..3b7a707e2fe5 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 (kvm_pci_hook.kvm_register) + return kvm_pci_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 (kvm_pci_hook.kvm_unregister) + return kvm_pci_hook.kvm_unregister(zdev); } -- 2.31.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: s390: pci: Hook to access KVM lowlevel from VFIO 2022-08-18 10:23 ` [PATCH] KVM: s390: pci: Hook to access KVM lowlevel from VFIO Pierre Morel @ 2022-08-18 13:33 ` Matthew Rosato 2022-08-18 14:06 ` Pierre Morel 2022-08-18 14:20 ` Niklas Schnelle 0 siblings, 2 replies; 17+ messages in thread From: Matthew Rosato @ 2022-08-18 13:33 UTC (permalink / raw) To: Pierre Morel Cc: rdunlap, linux-kernel, lkp, borntraeger, farman, linux-s390, kvm, gor, hca, schnelle On 8/18/22 6:23 AM, Pierre Morel wrote: > We have a cross dependency between KVM and VFIO. maybe add something like '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 or excluded. s/or excluded// There's no problem when KVM is excluded, that forces CONFIG_VFIO_PCI_ZDEV_KVM=n because of the 'depends on S390 && KVM'. > > 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..8312ed9d1937 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 kvm_register_hook { Nit: zpci_kvm_register_hook ? Just to make it clear it's for zpci. > + int (*kvm_register)(void *opaque, struct kvm *kvm); > + void (*kvm_unregister)(void *opaque); > +}; > + > +extern struct kvm_register_hook kvm_pci_hook; Nit: kvm_zpci_hook ? > > #endif > diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c > index 4946fb7757d6..e173fce64c4f 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); > + kvm_pci_hook.kvm_register = kvm_s390_pci_register_kvm; > + kvm_pci_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 guess it doesn't harm anything to add this unconditionally, but I think it would also be OK to just include this in the CONFIG_PCI list - vfio_pci_zdev and arch/s390/kvm/pci all rely on CONFIG_PCI via CONFIG_VFIO_PCI_ZDEV_KVM which implies PCI via VFIO_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..9d8799b72dbf > --- /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 kvm_register_hook kvm_pci_hook; > +EXPORT_SYMBOL_GPL(kvm_pci_hook); Following the comments above, zpci_kvm_register_hook, kvm_zpci_hook ? I'm not sure if this really needs to be in a separate file or if it could just go into arch/s390/pci.c with the zpci_aipb -- If going the route of a separate file, up to Niklas whether he wants this under the S390 PCI maintainership or added to the list for s390 vfio-pci like arch/kvm/pci* and vfio_pci_zdev. > diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c > index e163aa9f6144..3b7a707e2fe5 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 (kvm_pci_hook.kvm_register) > + return kvm_pci_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 (kvm_pci_hook.kvm_unregister) > + return kvm_pci_hook.kvm_unregister(zdev); No need for the return here, this is a void function calling a void function. Overall, this looks good to me and survives a series of compile and device passthrough tests on my end, just a matter of a few of these minor comments above. Thanks for tackling this Pierre! ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: s390: pci: Hook to access KVM lowlevel from VFIO 2022-08-18 13:33 ` Matthew Rosato @ 2022-08-18 14:06 ` Pierre Morel 2022-08-18 14:20 ` Niklas Schnelle 1 sibling, 0 replies; 17+ messages in thread From: Pierre Morel @ 2022-08-18 14:06 UTC (permalink / raw) To: Matthew Rosato Cc: rdunlap, linux-kernel, lkp, borntraeger, farman, linux-s390, kvm, gor, hca, schnelle On 8/18/22 15:33, Matthew Rosato wrote: > On 8/18/22 6:23 AM, Pierre Morel wrote: >> We have a cross dependency between KVM and VFIO. > > maybe add something like '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 or excluded. > > s/or excluded// > > There's no problem when KVM is excluded, that forces CONFIG_VFIO_PCI_ZDEV_KVM=n because of the 'depends on S390 && KVM'. OK > >> >> 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..8312ed9d1937 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 kvm_register_hook { > > Nit: zpci_kvm_register_hook ? Just to make it clear it's for zpci. OK > >> + int (*kvm_register)(void *opaque, struct kvm *kvm); >> + void (*kvm_unregister)(void *opaque); >> +}; >> + >> +extern struct kvm_register_hook kvm_pci_hook; > > Nit: kvm_zpci_hook ? OK too, > >> >> #endif >> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c >> index 4946fb7757d6..e173fce64c4f 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); >> + kvm_pci_hook.kvm_register = kvm_s390_pci_register_kvm; >> + kvm_pci_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 guess it doesn't harm anything to add this unconditionally, but I think it would also be OK to just include this in the CONFIG_PCI list - vfio_pci_zdev and arch/s390/kvm/pci all rely on CONFIG_PCI via CONFIG_VFIO_PCI_ZDEV_KVM which implies PCI via VFIO_PCI. Right,CONFIG_PCI is a bool so we can put the hook in arch/s390/pci/pci.c and use a defined(CONFIG_PCI) to protect the initialization inside KVM. > >> diff --git a/arch/s390/pci/pci_kvm_hook.c b/arch/s390/pci/pci_kvm_hook.c >> new file mode 100644 >> index 000000000000..9d8799b72dbf >> --- /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 kvm_register_hook kvm_pci_hook; >> +EXPORT_SYMBOL_GPL(kvm_pci_hook); > > Following the comments above, zpci_kvm_register_hook, kvm_zpci_hook ? OK > > I'm not sure if this really needs to be in a separate file or if it could just go into arch/s390/pci.c with the zpci_aipb -- If going the route of a separate file, up to Niklas whether he wants this under the S390 PCI maintainership or added to the list for s390 vfio-pci like arch/kvm/pci* and vfio_pci_zdev. agreed no need for a separate file, it is much better. > >> diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c >> index e163aa9f6144..3b7a707e2fe5 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 (kvm_pci_hook.kvm_register) >> + return kvm_pci_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 (kvm_pci_hook.kvm_unregister) >> + return kvm_pci_hook.kvm_unregister(zdev); > > No need for the return here, this is a void function calling a void function. right. > > > Overall, this looks good to me and survives a series of compile and device passthrough tests on my end, just a matter of a few of these minor comments above. Thanks for tackling this Pierre! > Thanks, Pierre -- Pierre Morel IBM Lab Boeblingen ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: s390: pci: Hook to access KVM lowlevel from VFIO 2022-08-18 13:33 ` Matthew Rosato 2022-08-18 14:06 ` Pierre Morel @ 2022-08-18 14:20 ` Niklas Schnelle 2022-08-18 15:13 ` Matthew Rosato 1 sibling, 1 reply; 17+ messages in thread From: Niklas Schnelle @ 2022-08-18 14:20 UTC (permalink / raw) To: Matthew Rosato, Pierre Morel Cc: rdunlap, linux-kernel, lkp, farman, linux-s390, kvm, gor, hca, Janosch Frank On Thu, 2022-08-18 at 09:33 -0400, Matthew Rosato wrote: > On 8/18/22 6:23 AM, Pierre Morel wrote: > > We have a cross dependency between KVM and VFIO. > > maybe add something like '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 or excluded. > > s/or excluded// > > There's no problem when KVM is excluded, that forces CONFIG_VFIO_PCI_ZDEV_KVM=n because of the 'depends on S390 && KVM'. > > > 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..8312ed9d1937 100644 > > --- a/arch/s390/include/asm/kvm_host.h > > +++ b/arch/s390/include/asm/kvm_host.h I added Janosch as second S390 KVM maintainer in case he wants to chime in. > > @@ -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 kvm_register_hook { > > Nit: zpci_kvm_register_hook ? Just to make it clear it's for zpci. Hmm, I guess one could re-use the same struct for another such KVM dependency but I lean towards the same thinking as Matt, for now this is for zpci so stay specific we can always generalize later. Nit: For me hook and register together sound a bit redudant, maybe "zpci_kvm_register"? Also question for Matt as a native speaker, should it rather be "registration" when used as a noun here? > > > + int (*kvm_register)(void *opaque, struct kvm *kvm); > > + void (*kvm_unregister)(void *opaque); I do wonder if this needs to be opague "struct zpci_dev" should be defined even if CONFIG_PCI is unset. > > +}; > > + > > +extern struct kvm_register_hook kvm_pci_hook; > > Nit: kvm_zpci_hook ? Analogous to zpci_kvm_regist(er|ration) I would call the variable simply zpci_kvm i.e. the type is a registration and the variable is the instance of it that links zpci and kvm. > > > > > #endif > > diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c > > index 4946fb7757d6..e173fce64c4f 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); > > + kvm_pci_hook.kvm_register = kvm_s390_pci_register_kvm; > > + kvm_pci_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 guess it doesn't harm anything to add this unconditionally, but I think it would also be OK to just include this in the CONFIG_PCI list - vfio_pci_zdev and arch/s390/kvm/pci all rely on CONFIG_PCI via CONFIG_VFIO_PCI_ZDEV_KVM which implies PCI via VFIO_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..9d8799b72dbf > > --- /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 kvm_register_hook kvm_pci_hook; > > +EXPORT_SYMBOL_GPL(kvm_pci_hook); > > Following the comments above, zpci_kvm_register_hook, kvm_zpci_hook ? > > I'm not sure if this really needs to be in a separate file or if it could just go into arch/s390/pci.c with the zpci_aipb -- If going the route of a separate file, up to Niklas whether he wants this under the S390 PCI maintainership or added to the list for s390 vfio-pci like arch/kvm/pci* and vfio_pci_zdev. I'm fine with a separate file, pci.c is long enough as it is. I also don't have a problem with having it maintained as part of S390 PCI but logically I think it does fall more under arch/kvm/pci* so one could argue it should be added in the MAINTAINERS file in that section. If you change the struct name as I proposed above I would probably go with "pci_kvm_register.c" > > > diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c > > index e163aa9f6144..3b7a707e2fe5 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 (kvm_pci_hook.kvm_register) > > + return kvm_pci_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 (kvm_pci_hook.kvm_unregister) > > + return kvm_pci_hook.kvm_unregister(zdev); > > No need for the return here, this is a void function calling a void function. > > > Overall, this looks good to me and survives a series of compile and device passthrough tests on my end, just a matter of a few of these minor comments above. Thanks for tackling this Pierre! Yes I agree, overall this looks good to me though I'm admittedly not very knowledgable about how to best handle module dependencies like this. It does look cleaner than the symbol_get() alternative we discussed. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: s390: pci: Hook to access KVM lowlevel from VFIO 2022-08-18 14:20 ` Niklas Schnelle @ 2022-08-18 15:13 ` Matthew Rosato 2022-08-18 15:22 ` Niklas Schnelle 0 siblings, 1 reply; 17+ messages in thread From: Matthew Rosato @ 2022-08-18 15:13 UTC (permalink / raw) To: Niklas Schnelle, Pierre Morel Cc: rdunlap, linux-kernel, lkp, farman, linux-s390, kvm, gor, hca, Janosch Frank On 8/18/22 10:20 AM, Niklas Schnelle wrote: > On Thu, 2022-08-18 at 09:33 -0400, Matthew Rosato wrote: >> On 8/18/22 6:23 AM, Pierre Morel wrote: >>> We have a cross dependency between KVM and VFIO. >> >> maybe add something like '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 or excluded. >> >> s/or excluded// >> >> There's no problem when KVM is excluded, that forces CONFIG_VFIO_PCI_ZDEV_KVM=n because of the 'depends on S390 && KVM'. >> >>> 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..8312ed9d1937 100644 >>> --- a/arch/s390/include/asm/kvm_host.h >>> +++ b/arch/s390/include/asm/kvm_host.h > > I added Janosch as second S390 KVM maintainer in case he wants to chime > in. > >>> @@ -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 kvm_register_hook { >> >> Nit: zpci_kvm_register_hook ? Just to make it clear it's for zpci. > > Hmm, I guess one could re-use the same struct for another such KVM > dependency but I lean towards the same thinking as Matt, for now this > is for zpci so stay specific we can always generalize later. Yes, let's keep this zpci-specific. > > Nit: For me hook and register together sound a bit redudant, maybe > "zpci_kvm_register"? Also question for Matt as a native speaker, should > it rather be "registration" when used as a noun here? > Maybe just drop the 'register'. If there is a need for a 3rd function later, for example, it might not be related to registration. e.g. struct kvm_zpci_hook { ... }; extern struct kvm_zpci_hook zpci_kvm; > >> >>> + int (*kvm_register)(void *opaque, struct kvm *kvm); >>> + void (*kvm_unregister)(void *opaque); > > I do wonder if this needs to be opague "struct zpci_dev" should be > defined even if CONFIG_PCI is unset. > > >>> +}; >>> + >>> +extern struct kvm_register_hook kvm_pci_hook; >> >> Nit: kvm_zpci_hook ? > > Analogous to zpci_kvm_regist(er|ration) I would call the variable > simply zpci_kvm i.e. the type is a registration and the variable is the > instance of it that links zpci and kvm. > Yeah, see above. >> >>> >>> #endif >>> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c >>> index 4946fb7757d6..e173fce64c4f 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); >>> + kvm_pci_hook.kvm_register = kvm_s390_pci_register_kvm; >>> + kvm_pci_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 guess it doesn't harm anything to add this unconditionally, but I think it would also be OK to just include this in the CONFIG_PCI list - vfio_pci_zdev and arch/s390/kvm/pci all rely on CONFIG_PCI via CONFIG_VFIO_PCI_ZDEV_KVM which implies PCI via VFIO_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..9d8799b72dbf >>> --- /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 kvm_register_hook kvm_pci_hook; >>> +EXPORT_SYMBOL_GPL(kvm_pci_hook); >> >> Following the comments above, zpci_kvm_register_hook, kvm_zpci_hook ? >> >> I'm not sure if this really needs to be in a separate file or if it could just go into arch/s390/pci.c with the zpci_aipb -- If going the route of a separate file, up to Niklas whether he wants this under the S390 PCI maintainership or added to the list for s390 vfio-pci like arch/kvm/pci* and vfio_pci_zdev. > > I'm fine with a separate file, pci.c is long enough as it is. I also > don't have a problem with having it maintained as part of S390 PCI but > logically I think it does fall more under arch/kvm/pci* so one could > argue it should be added in the MAINTAINERS file in that section. > If you change the struct name as I proposed above I would probably go > with "pci_kvm_register.c" OK, no problem with me for a separate file then, or maintaining said file. But I guess not pci_kvm_register.c per my comments above > >> >>> diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c >>> index e163aa9f6144..3b7a707e2fe5 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 (kvm_pci_hook.kvm_register) >>> + return kvm_pci_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 (kvm_pci_hook.kvm_unregister) >>> + return kvm_pci_hook.kvm_unregister(zdev); >> >> No need for the return here, this is a void function calling a void function. >> >> >> Overall, this looks good to me and survives a series of compile and device passthrough tests on my end, just a matter of a few of these minor comments above. Thanks for tackling this Pierre! > > Yes I agree, overall this looks good to me though I'm admittedly not > very knowledgable about how to best handle module dependencies like > this. It does look cleaner than the symbol_get() alternative we > discussed. > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: s390: pci: Hook to access KVM lowlevel from VFIO 2022-08-18 15:13 ` Matthew Rosato @ 2022-08-18 15:22 ` Niklas Schnelle 0 siblings, 0 replies; 17+ messages in thread From: Niklas Schnelle @ 2022-08-18 15:22 UTC (permalink / raw) To: Matthew Rosato, Pierre Morel Cc: rdunlap, linux-kernel, lkp, farman, linux-s390, kvm, gor, hca, Janosch Frank On Thu, 2022-08-18 at 11:13 -0400, Matthew Rosato wrote: > On 8/18/22 10:20 AM, Niklas Schnelle wrote: > > On Thu, 2022-08-18 at 09:33 -0400, Matthew Rosato wrote: > > > On 8/18/22 6:23 AM, Pierre Morel wrote: > > > > We have a cross dependency between KVM and VFIO. > > > > > > maybe add something like '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 or excluded. > > > > > > s/or excluded// > > > > > > There's no problem when KVM is excluded, that forces CONFIG_VFIO_PCI_ZDEV_KVM=n because of the 'depends on S390 && KVM'. > > > > > > > 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..8312ed9d1937 100644 > > > > --- a/arch/s390/include/asm/kvm_host.h > > > > +++ b/arch/s390/include/asm/kvm_host.h > > > > I added Janosch as second S390 KVM maintainer in case he wants to chime > > in. > > > > > > @@ -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 kvm_register_hook { > > > > > > Nit: zpci_kvm_register_hook ? Just to make it clear it's for zpci. > > > > Hmm, I guess one could re-use the same struct for another such KVM > > dependency but I lean towards the same thinking as Matt, for now this > > is for zpci so stay specific we can always generalize later. > > Yes, let's keep this zpci-specific. > > > Nit: For me hook and register together sound a bit redudant, maybe > > "zpci_kvm_register"? Also question for Matt as a native speaker, should > > it rather be "registration" when used as a noun here? > > > > Maybe just drop the 'register'. If there is a need for a 3rd function later, for example, it might not be related to registration. Yes, that sounds good and makes sense so "zpci_kvm_hook". > > e.g. struct kvm_zpci_hook { > ... > }; > > extern struct kvm_zpci_hook zpci_kvm; > ---8<--- > > > > > > > diff --git a/arch/s390/pci/pci_kvm_hook.c b/arch/s390/pci/pci_kvm_hook.c > > > > new file mode 100644 > > > > index 000000000000..9d8799b72dbf > > > > --- /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 kvm_register_hook kvm_pci_hook; > > > > +EXPORT_SYMBOL_GPL(kvm_pci_hook); > > > > > > Following the comments above, zpci_kvm_register_hook, kvm_zpci_hook ? > > > > > > I'm not sure if this really needs to be in a separate file or if it could just go into arch/s390/pci.c with the zpci_aipb -- If going the route of a separate file, up to Niklas whether he wants this under the S390 PCI maintainership or added to the list for s390 vfio-pci like arch/kvm/pci* and vfio_pci_zdev. > > > > I'm fine with a separate file, pci.c is long enough as it is. I also > > don't have a problem with having it maintained as part of S390 PCI but > > logically I think it does fall more under arch/kvm/pci* so one could > > argue it should be added in the MAINTAINERS file in that section. > > If you change the struct name as I proposed above I would probably go > > with "pci_kvm_register.c" > > OK, no problem with me for a separate file then, or maintaining said file. But I guess not pci_kvm_register.c per my comments above Yes, let's go with pci_kvm_hook.c then > ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-08-18 15:23 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-14 21:51 [PATCH] vfio-pci/zdev: require KVM to be built-in Randy Dunlap 2022-08-15 9:43 ` Pierre Morel 2022-08-16 6:04 ` Randy Dunlap 2022-08-16 7:55 ` Pierre Morel 2022-08-16 13:47 ` Pierre Morel 2022-08-16 18:06 ` Randy Dunlap 2022-08-16 19:46 ` Matthew Rosato 2022-08-16 20:22 ` Pierre Morel 2022-08-16 20:28 ` [PATCH] KVM: s390: pci: VFIO_PCI ZDEV configuration fix Pierre Morel 2022-08-16 22:15 ` Matthew Rosato 2022-08-17 7:10 ` Pierre Morel 2022-08-18 10:23 ` [PATCH] KVM: s390: pci: Hook to access KVM lowlevel from VFIO Pierre Morel 2022-08-18 13:33 ` Matthew Rosato 2022-08-18 14:06 ` Pierre Morel 2022-08-18 14:20 ` Niklas Schnelle 2022-08-18 15:13 ` Matthew Rosato 2022-08-18 15:22 ` Niklas Schnelle
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).