* [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected @ 2023-09-20 18:18 Jason Gunthorpe 2023-09-22 0:00 ` Michael Ellerman 2023-11-10 6:08 ` Ping? " Michael Ellerman 0 siblings, 2 replies; 13+ messages in thread From: Jason Gunthorpe @ 2023-09-20 18:18 UTC (permalink / raw) To: Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Michael Ellerman, Nicholas Piggin, Oliver Upton, Paolo Bonzini, Sean Christopherson, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu There are a bunch of reported randconfig failures now because of this, something like: >> arch/powerpc/kvm/../../../virt/kvm/vfio.c:89:7: warning: attribute declaration must precede definition [-Wignored-attributes] fn = symbol_get(vfio_file_iommu_group); ^ include/linux/module.h:805:60: note: expanded from macro 'symbol_get' #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak,visibility("hidden"))); &(x); }) It happens because the arch forces KVM_VFIO without knowing if VFIO is even enabled. Split the kconfig so the arch selects the usual HAVE_KVM_ARCH_VFIO and then KVM_VFIO is only enabled if the arch wants it and VFIO is turned on. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202308251949.5IiaV0sz-lkp@intel.com/ Closes: https://lore.kernel.org/oe-kbuild-all/202309030741.82aLACDG-lkp@intel.com/ Closes: https://lore.kernel.org/oe-kbuild-all/202309110914.QLH0LU6L-lkp@intel.com/ Cc: Nick Desaulniers <ndesaulniers@google.com> Fixes: c1cce6d079b8 ("vfio: Compile vfio_group infrastructure optionally") Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- arch/arm64/kvm/Kconfig | 2 +- arch/powerpc/kvm/Kconfig | 2 +- arch/s390/kvm/Kconfig | 2 +- arch/x86/kvm/Kconfig | 2 +- virt/kvm/Kconfig | 7 ++++++- 5 files changed, 10 insertions(+), 5 deletions(-) Sean's large series will also address this: https://lore.kernel.org/kvm/20230916003118.2540661-7-seanjc@google.com/ I don't know if it is sever enough to fix in the rc cycle, but here is the patch. diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig index 83c1e09be42e5b..7c43eaea51ce05 100644 --- a/arch/arm64/kvm/Kconfig +++ b/arch/arm64/kvm/Kconfig @@ -28,7 +28,7 @@ menuconfig KVM select KVM_MMIO select KVM_GENERIC_DIRTYLOG_READ_PROTECT select KVM_XFER_TO_GUEST_WORK - select KVM_VFIO + select HAVE_KVM_ARCH_VFIO select HAVE_KVM_EVENTFD select HAVE_KVM_IRQFD select HAVE_KVM_DIRTY_RING_ACQ_REL diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig index 902611954200df..b64824e4cbc1eb 100644 --- a/arch/powerpc/kvm/Kconfig +++ b/arch/powerpc/kvm/Kconfig @@ -22,7 +22,7 @@ config KVM select PREEMPT_NOTIFIERS select HAVE_KVM_EVENTFD select HAVE_KVM_VCPU_ASYNC_IOCTL - select KVM_VFIO + select HAVE_KVM_ARCH_VFIO select IRQ_BYPASS_MANAGER select HAVE_KVM_IRQ_BYPASS select INTERVAL_TREE diff --git a/arch/s390/kvm/Kconfig b/arch/s390/kvm/Kconfig index 45fdf2a9b2e326..d206ad3a777d5d 100644 --- a/arch/s390/kvm/Kconfig +++ b/arch/s390/kvm/Kconfig @@ -31,7 +31,7 @@ config KVM select HAVE_KVM_IRQ_ROUTING select HAVE_KVM_INVALID_WAKEUPS select HAVE_KVM_NO_POLL - select KVM_VFIO + select HAVE_KVM_ARCH_VFIO select INTERVAL_TREE select MMU_NOTIFIER help diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig index ed90f148140dfe..8e70e693f90e30 100644 --- a/arch/x86/kvm/Kconfig +++ b/arch/x86/kvm/Kconfig @@ -45,7 +45,7 @@ config KVM select HAVE_KVM_NO_POLL select KVM_XFER_TO_GUEST_WORK select KVM_GENERIC_DIRTYLOG_READ_PROTECT - select KVM_VFIO + select HAVE_KVM_ARCH_VFIO select INTERVAL_TREE select HAVE_KVM_PM_NOTIFIER if PM select KVM_GENERIC_HARDWARE_ENABLING diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig index 484d0873061ca5..0bf34809e1bbfe 100644 --- a/virt/kvm/Kconfig +++ b/virt/kvm/Kconfig @@ -59,9 +59,14 @@ config HAVE_KVM_MSI config HAVE_KVM_CPU_RELAX_INTERCEPT bool -config KVM_VFIO +config HAVE_KVM_ARCH_VFIO bool +config KVM_VFIO + def_bool y + depends on HAVE_KVM_ARCH_VFIO + depends on VFIO + config HAVE_KVM_INVALID_WAKEUPS bool base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d -- 2.42.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-09-20 18:18 [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected Jason Gunthorpe @ 2023-09-22 0:00 ` Michael Ellerman 2023-11-10 6:08 ` Ping? " Michael Ellerman 1 sibling, 0 replies; 13+ messages in thread From: Michael Ellerman @ 2023-09-22 0:00 UTC (permalink / raw) To: Jason Gunthorpe, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Paolo Bonzini, Sean Christopherson, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu Jason Gunthorpe <jgg@nvidia.com> writes: > There are a bunch of reported randconfig failures now because of this, > something like: > >>> arch/powerpc/kvm/../../../virt/kvm/vfio.c:89:7: warning: attribute declaration must precede definition [-Wignored-attributes] > fn = symbol_get(vfio_file_iommu_group); > ^ > include/linux/module.h:805:60: note: expanded from macro 'symbol_get' > #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak,visibility("hidden"))); &(x); }) > > It happens because the arch forces KVM_VFIO without knowing if VFIO is > even enabled. > > Split the kconfig so the arch selects the usual HAVE_KVM_ARCH_VFIO and > then KVM_VFIO is only enabled if the arch wants it and VFIO is turned on. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202308251949.5IiaV0sz-lkp@intel.com/ > Closes: https://lore.kernel.org/oe-kbuild-all/202309030741.82aLACDG-lkp@intel.com/ > Closes: https://lore.kernel.org/oe-kbuild-all/202309110914.QLH0LU6L-lkp@intel.com/ > Cc: Nick Desaulniers <ndesaulniers@google.com> > Fixes: c1cce6d079b8 ("vfio: Compile vfio_group infrastructure optionally") > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > arch/arm64/kvm/Kconfig | 2 +- > arch/powerpc/kvm/Kconfig | 2 +- > arch/s390/kvm/Kconfig | 2 +- > arch/x86/kvm/Kconfig | 2 +- > virt/kvm/Kconfig | 7 ++++++- > 5 files changed, 10 insertions(+), 5 deletions(-) > > Sean's large series will also address this: > > https://lore.kernel.org/kvm/20230916003118.2540661-7-seanjc@google.com/ > > I don't know if it is sever enough to fix in the rc cycle, but here is the > patch. Thanks for debugging this, I had seen it but hadn't got around to it. I think it's definitely worth fixing now. It's a pretty simple patch and it's still early in the rc cycle. Tested-by: Michael Ellerman <mpe@ellerman.id.au> cheers > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig > index 83c1e09be42e5b..7c43eaea51ce05 100644 > --- a/arch/arm64/kvm/Kconfig > +++ b/arch/arm64/kvm/Kconfig > @@ -28,7 +28,7 @@ menuconfig KVM > select KVM_MMIO > select KVM_GENERIC_DIRTYLOG_READ_PROTECT > select KVM_XFER_TO_GUEST_WORK > - select KVM_VFIO > + select HAVE_KVM_ARCH_VFIO > select HAVE_KVM_EVENTFD > select HAVE_KVM_IRQFD > select HAVE_KVM_DIRTY_RING_ACQ_REL > diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig > index 902611954200df..b64824e4cbc1eb 100644 > --- a/arch/powerpc/kvm/Kconfig > +++ b/arch/powerpc/kvm/Kconfig > @@ -22,7 +22,7 @@ config KVM > select PREEMPT_NOTIFIERS > select HAVE_KVM_EVENTFD > select HAVE_KVM_VCPU_ASYNC_IOCTL > - select KVM_VFIO > + select HAVE_KVM_ARCH_VFIO > select IRQ_BYPASS_MANAGER > select HAVE_KVM_IRQ_BYPASS > select INTERVAL_TREE > diff --git a/arch/s390/kvm/Kconfig b/arch/s390/kvm/Kconfig > index 45fdf2a9b2e326..d206ad3a777d5d 100644 > --- a/arch/s390/kvm/Kconfig > +++ b/arch/s390/kvm/Kconfig > @@ -31,7 +31,7 @@ config KVM > select HAVE_KVM_IRQ_ROUTING > select HAVE_KVM_INVALID_WAKEUPS > select HAVE_KVM_NO_POLL > - select KVM_VFIO > + select HAVE_KVM_ARCH_VFIO > select INTERVAL_TREE > select MMU_NOTIFIER > help > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > index ed90f148140dfe..8e70e693f90e30 100644 > --- a/arch/x86/kvm/Kconfig > +++ b/arch/x86/kvm/Kconfig > @@ -45,7 +45,7 @@ config KVM > select HAVE_KVM_NO_POLL > select KVM_XFER_TO_GUEST_WORK > select KVM_GENERIC_DIRTYLOG_READ_PROTECT > - select KVM_VFIO > + select HAVE_KVM_ARCH_VFIO > select INTERVAL_TREE > select HAVE_KVM_PM_NOTIFIER if PM > select KVM_GENERIC_HARDWARE_ENABLING > diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig > index 484d0873061ca5..0bf34809e1bbfe 100644 > --- a/virt/kvm/Kconfig > +++ b/virt/kvm/Kconfig > @@ -59,9 +59,14 @@ config HAVE_KVM_MSI > config HAVE_KVM_CPU_RELAX_INTERCEPT > bool > > -config KVM_VFIO > +config HAVE_KVM_ARCH_VFIO > bool > > +config KVM_VFIO > + def_bool y > + depends on HAVE_KVM_ARCH_VFIO > + depends on VFIO > + > config HAVE_KVM_INVALID_WAKEUPS > bool > > > base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d > -- > 2.42.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-09-20 18:18 [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected Jason Gunthorpe 2023-09-22 0:00 ` Michael Ellerman @ 2023-11-10 6:08 ` Michael Ellerman 2023-11-29 2:21 ` Sean Christopherson 1 sibling, 1 reply; 13+ messages in thread From: Michael Ellerman @ 2023-11-10 6:08 UTC (permalink / raw) To: Paolo Bonzini, Sean Christopherson Cc: Jason Gunthorpe, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu Jason Gunthorpe <jgg@nvidia.com> writes: > There are a bunch of reported randconfig failures now because of this, > something like: > >>> arch/powerpc/kvm/../../../virt/kvm/vfio.c:89:7: warning: attribute declaration must precede definition [-Wignored-attributes] > fn = symbol_get(vfio_file_iommu_group); > ^ > include/linux/module.h:805:60: note: expanded from macro 'symbol_get' > #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak,visibility("hidden"))); &(x); }) > > It happens because the arch forces KVM_VFIO without knowing if VFIO is > even enabled. This is still breaking some builds. Can we get this fix in please? cheers > Split the kconfig so the arch selects the usual HAVE_KVM_ARCH_VFIO and > then KVM_VFIO is only enabled if the arch wants it and VFIO is turned on. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202308251949.5IiaV0sz-lkp@intel.com/ > Closes: https://lore.kernel.org/oe-kbuild-all/202309030741.82aLACDG-lkp@intel.com/ > Closes: https://lore.kernel.org/oe-kbuild-all/202309110914.QLH0LU6L-lkp@intel.com/ > Cc: Nick Desaulniers <ndesaulniers@google.com> > Fixes: c1cce6d079b8 ("vfio: Compile vfio_group infrastructure optionally") > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > arch/arm64/kvm/Kconfig | 2 +- > arch/powerpc/kvm/Kconfig | 2 +- > arch/s390/kvm/Kconfig | 2 +- > arch/x86/kvm/Kconfig | 2 +- > virt/kvm/Kconfig | 7 ++++++- > 5 files changed, 10 insertions(+), 5 deletions(-) > > Sean's large series will also address this: > > https://lore.kernel.org/kvm/20230916003118.2540661-7-seanjc@google.com/ > > I don't know if it is sever enough to fix in the rc cycle, but here is the > patch. > > diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig > index 83c1e09be42e5b..7c43eaea51ce05 100644 > --- a/arch/arm64/kvm/Kconfig > +++ b/arch/arm64/kvm/Kconfig > @@ -28,7 +28,7 @@ menuconfig KVM > select KVM_MMIO > select KVM_GENERIC_DIRTYLOG_READ_PROTECT > select KVM_XFER_TO_GUEST_WORK > - select KVM_VFIO > + select HAVE_KVM_ARCH_VFIO > select HAVE_KVM_EVENTFD > select HAVE_KVM_IRQFD > select HAVE_KVM_DIRTY_RING_ACQ_REL > diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig > index 902611954200df..b64824e4cbc1eb 100644 > --- a/arch/powerpc/kvm/Kconfig > +++ b/arch/powerpc/kvm/Kconfig > @@ -22,7 +22,7 @@ config KVM > select PREEMPT_NOTIFIERS > select HAVE_KVM_EVENTFD > select HAVE_KVM_VCPU_ASYNC_IOCTL > - select KVM_VFIO > + select HAVE_KVM_ARCH_VFIO > select IRQ_BYPASS_MANAGER > select HAVE_KVM_IRQ_BYPASS > select INTERVAL_TREE > diff --git a/arch/s390/kvm/Kconfig b/arch/s390/kvm/Kconfig > index 45fdf2a9b2e326..d206ad3a777d5d 100644 > --- a/arch/s390/kvm/Kconfig > +++ b/arch/s390/kvm/Kconfig > @@ -31,7 +31,7 @@ config KVM > select HAVE_KVM_IRQ_ROUTING > select HAVE_KVM_INVALID_WAKEUPS > select HAVE_KVM_NO_POLL > - select KVM_VFIO > + select HAVE_KVM_ARCH_VFIO > select INTERVAL_TREE > select MMU_NOTIFIER > help > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > index ed90f148140dfe..8e70e693f90e30 100644 > --- a/arch/x86/kvm/Kconfig > +++ b/arch/x86/kvm/Kconfig > @@ -45,7 +45,7 @@ config KVM > select HAVE_KVM_NO_POLL > select KVM_XFER_TO_GUEST_WORK > select KVM_GENERIC_DIRTYLOG_READ_PROTECT > - select KVM_VFIO > + select HAVE_KVM_ARCH_VFIO > select INTERVAL_TREE > select HAVE_KVM_PM_NOTIFIER if PM > select KVM_GENERIC_HARDWARE_ENABLING > diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig > index 484d0873061ca5..0bf34809e1bbfe 100644 > --- a/virt/kvm/Kconfig > +++ b/virt/kvm/Kconfig > @@ -59,9 +59,14 @@ config HAVE_KVM_MSI > config HAVE_KVM_CPU_RELAX_INTERCEPT > bool > > -config KVM_VFIO > +config HAVE_KVM_ARCH_VFIO > bool > > +config KVM_VFIO > + def_bool y > + depends on HAVE_KVM_ARCH_VFIO > + depends on VFIO > + > config HAVE_KVM_INVALID_WAKEUPS > bool > > > base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d > -- > 2.42.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-11-10 6:08 ` Ping? " Michael Ellerman @ 2023-11-29 2:21 ` Sean Christopherson 2023-11-29 9:38 ` Michael Ellerman 2023-11-29 12:48 ` Jason Gunthorpe 0 siblings, 2 replies; 13+ messages in thread From: Sean Christopherson @ 2023-11-29 2:21 UTC (permalink / raw) To: Michael Ellerman Cc: Paolo Bonzini, Jason Gunthorpe, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu On Fri, Nov 10, 2023, Michael Ellerman wrote: > Jason Gunthorpe <jgg@nvidia.com> writes: > > There are a bunch of reported randconfig failures now because of this, > > something like: > > > >>> arch/powerpc/kvm/../../../virt/kvm/vfio.c:89:7: warning: attribute declaration must precede definition [-Wignored-attributes] > > fn = symbol_get(vfio_file_iommu_group); > > ^ > > include/linux/module.h:805:60: note: expanded from macro 'symbol_get' > > #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak,visibility("hidden"))); &(x); }) > > > > It happens because the arch forces KVM_VFIO without knowing if VFIO is > > even enabled. > > This is still breaking some builds. Can we get this fix in please? > > cheers > > > Split the kconfig so the arch selects the usual HAVE_KVM_ARCH_VFIO and > > then KVM_VFIO is only enabled if the arch wants it and VFIO is turned on. Heh, so I was trying to figure out why things like vfio_file_set_kvm() aren't problematic, i.e. why the existing mess didn't cause failures. I can't repro the warning (requires clang-16?), but IIUC the reason only the group code is problematic is that vfio.h creates a stub for vfio_file_iommu_group() and thus there's no symbol, whereas vfio.h declares vfio_file_set_kvm() unconditionally. Because KVM is doing symbol_get() and not taking a direct dependency, the lack of an exported symbol doesn't cause problems, i.e. simply declaring the symbol makes the compiler happy. Given that the vfio_file_iommu_group() stub shouldn't exist (KVM is the only user, and so if I'm correct the stub is worthless), what about this as a temporary "fix"? I'm 100% on-board with fixing KVM properly, my motivation is purely to minimize the total amount of churn. E.g. if this works, then the only extra churn is to move the declaration of vfio_file_iommu_group() back under the #if, versus having to churn all of the KVM Kconfigs twice (once now, and again for the full cleanup). diff --git a/include/linux/vfio.h b/include/linux/vfio.h index 454e9295970c..a65b2513f8cd 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -289,16 +289,12 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes, /* * External user API */ -#if IS_ENABLED(CONFIG_VFIO_GROUP) struct iommu_group *vfio_file_iommu_group(struct file *file); + +#if IS_ENABLED(CONFIG_VFIO_GROUP) bool vfio_file_is_group(struct file *file); bool vfio_file_has_dev(struct file *file, struct vfio_device *device); #else -static inline struct iommu_group *vfio_file_iommu_group(struct file *file) -{ - return NULL; -} - static inline bool vfio_file_is_group(struct file *file) { return false; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-11-29 2:21 ` Sean Christopherson @ 2023-11-29 9:38 ` Michael Ellerman 2023-11-30 1:07 ` Sean Christopherson 2023-11-29 12:48 ` Jason Gunthorpe 1 sibling, 1 reply; 13+ messages in thread From: Michael Ellerman @ 2023-11-29 9:38 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, Jason Gunthorpe, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu Sean Christopherson <seanjc@google.com> writes: > On Fri, Nov 10, 2023, Michael Ellerman wrote: >> Jason Gunthorpe <jgg@nvidia.com> writes: >> > There are a bunch of reported randconfig failures now because of this, >> > something like: >> > >> >>> arch/powerpc/kvm/../../../virt/kvm/vfio.c:89:7: warning: attribute declaration must precede definition [-Wignored-attributes] >> > fn = symbol_get(vfio_file_iommu_group); >> > ^ >> > include/linux/module.h:805:60: note: expanded from macro 'symbol_get' >> > #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak,visibility("hidden"))); &(x); }) >> > >> > It happens because the arch forces KVM_VFIO without knowing if VFIO is >> > even enabled. >> >> This is still breaking some builds. Can we get this fix in please? >> >> cheers >> >> > Split the kconfig so the arch selects the usual HAVE_KVM_ARCH_VFIO and >> > then KVM_VFIO is only enabled if the arch wants it and VFIO is turned on. > > Heh, so I was trying to figure out why things like vfio_file_set_kvm() aren't > problematic, i.e. why the existing mess didn't cause failures. I can't repro the > warning (requires clang-16?), but IIUC the reason only the group code is problematic > is that vfio.h creates a stub for vfio_file_iommu_group() and thus there's no symbol, > whereas vfio.h declares vfio_file_set_kvm() unconditionally. That warning I'm unsure about. But the final report linked in Jason's mail shows a different one: In file included from arch/powerpc/kvm/../../../virt/kvm/vfio.c:17: include/linux/vfio.h: In function 'kvm_vfio_file_iommu_group': include/linux/vfio.h:294:35: error: weak declaration of 'vfio_file_iommu_group' being applied to a already existing, static definition 294 | static inline struct iommu_group *vfio_file_iommu_group(struct file *file) | ^~~~~~~~~~~~~~~~~~~~~ Which is simple to reproduce, just build ppc64le_defconfig and then turn off CONFIG_MODULES (I'm using GCC 13, the report is for GCC 12). > Because KVM is doing symbol_get() and not taking a direct dependency, the lack of > an exported symbol doesn't cause problems, i.e. simply declaring the symbol makes > the compiler happy. > > Given that the vfio_file_iommu_group() stub shouldn't exist (KVM is the only user, > and so if I'm correct the stub is worthless), what about this as a temporary "fix"? > > I'm 100% on-board with fixing KVM properly, my motivation is purely to minimize > the total amount of churn. E.g. if this works, then the only extra churn is to > move the declaration of vfio_file_iommu_group() back under the #if, versus having > to churn all of the KVM Kconfigs twice (once now, and again for the full cleanup). Fine by me. > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > index 454e9295970c..a65b2513f8cd 100644 > --- a/include/linux/vfio.h > +++ b/include/linux/vfio.h > @@ -289,16 +289,12 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes, > /* > * External user API > */ > -#if IS_ENABLED(CONFIG_VFIO_GROUP) > struct iommu_group *vfio_file_iommu_group(struct file *file); > + > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > bool vfio_file_is_group(struct file *file); > bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > #else > -static inline struct iommu_group *vfio_file_iommu_group(struct file *file) > -{ > - return NULL; > -} > - > static inline bool vfio_file_is_group(struct file *file) > { > return false; That fixes the build for me. Tested-by: Michael Ellerman <mpe@ellerman.id.au> cheers _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-11-29 9:38 ` Michael Ellerman @ 2023-11-30 1:07 ` Sean Christopherson 2023-11-30 1:16 ` Jason Gunthorpe 0 siblings, 1 reply; 13+ messages in thread From: Sean Christopherson @ 2023-11-30 1:07 UTC (permalink / raw) To: Michael Ellerman Cc: Paolo Bonzini, Jason Gunthorpe, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu On Wed, Nov 29, 2023, Michael Ellerman wrote: > Sean Christopherson <seanjc@google.com> writes: > > On Fri, Nov 10, 2023, Michael Ellerman wrote: > >> Jason Gunthorpe <jgg@nvidia.com> writes: > >> > There are a bunch of reported randconfig failures now because of this, > >> > something like: > >> > > >> >>> arch/powerpc/kvm/../../../virt/kvm/vfio.c:89:7: warning: attribute declaration must precede definition [-Wignored-attributes] > >> > fn = symbol_get(vfio_file_iommu_group); > >> > ^ > >> > include/linux/module.h:805:60: note: expanded from macro 'symbol_get' > >> > #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak,visibility("hidden"))); &(x); }) > >> > > >> > It happens because the arch forces KVM_VFIO without knowing if VFIO is > >> > even enabled. > >> > >> This is still breaking some builds. Can we get this fix in please? > >> > >> cheers > >> > >> > Split the kconfig so the arch selects the usual HAVE_KVM_ARCH_VFIO and > >> > then KVM_VFIO is only enabled if the arch wants it and VFIO is turned on. > > > > Heh, so I was trying to figure out why things like vfio_file_set_kvm() aren't > > problematic, i.e. why the existing mess didn't cause failures. I can't repro the > > warning (requires clang-16?), but IIUC the reason only the group code is problematic > > is that vfio.h creates a stub for vfio_file_iommu_group() and thus there's no symbol, > > whereas vfio.h declares vfio_file_set_kvm() unconditionally. > > That warning I'm unsure about. Ah, it's the same warning, I just missed the CONFIG_MODULES=n requirement. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-11-30 1:07 ` Sean Christopherson @ 2023-11-30 1:16 ` Jason Gunthorpe 2023-11-30 2:02 ` Sean Christopherson 0 siblings, 1 reply; 13+ messages in thread From: Jason Gunthorpe @ 2023-11-30 1:16 UTC (permalink / raw) To: Sean Christopherson Cc: Michael Ellerman, Paolo Bonzini, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu On Wed, Nov 29, 2023 at 05:07:45PM -0800, Sean Christopherson wrote: > On Wed, Nov 29, 2023, Michael Ellerman wrote: > > Sean Christopherson <seanjc@google.com> writes: > > > On Fri, Nov 10, 2023, Michael Ellerman wrote: > > >> Jason Gunthorpe <jgg@nvidia.com> writes: > > >> > There are a bunch of reported randconfig failures now because of this, > > >> > something like: > > >> > > > >> >>> arch/powerpc/kvm/../../../virt/kvm/vfio.c:89:7: warning: attribute declaration must precede definition [-Wignored-attributes] > > >> > fn = symbol_get(vfio_file_iommu_group); > > >> > ^ > > >> > include/linux/module.h:805:60: note: expanded from macro 'symbol_get' > > >> > #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak,visibility("hidden"))); &(x); }) > > >> > > > >> > It happens because the arch forces KVM_VFIO without knowing if VFIO is > > >> > even enabled. > > >> > > >> This is still breaking some builds. Can we get this fix in please? > > >> > > >> cheers > > >> > > >> > Split the kconfig so the arch selects the usual HAVE_KVM_ARCH_VFIO and > > >> > then KVM_VFIO is only enabled if the arch wants it and VFIO is turned on. > > > > > > Heh, so I was trying to figure out why things like vfio_file_set_kvm() aren't > > > problematic, i.e. why the existing mess didn't cause failures. I can't repro the > > > warning (requires clang-16?), but IIUC the reason only the group code is problematic > > > is that vfio.h creates a stub for vfio_file_iommu_group() and thus there's no symbol, > > > whereas vfio.h declares vfio_file_set_kvm() unconditionally. > > > > That warning I'm unsure about. > > Ah, it's the same warning, I just missed the CONFIG_MODULES=n requirement. Oh, wait, doesn't that mean the approach won't work? IIRC doesn't symbol_get turn into just &fn when non-modular turning this into a link failure without the kconfig part? Jason _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-11-30 1:16 ` Jason Gunthorpe @ 2023-11-30 2:02 ` Sean Christopherson 2023-11-30 6:38 ` Michael Ellerman 2023-11-30 12:07 ` Jason Gunthorpe 0 siblings, 2 replies; 13+ messages in thread From: Sean Christopherson @ 2023-11-30 2:02 UTC (permalink / raw) To: Jason Gunthorpe Cc: Michael Ellerman, Paolo Bonzini, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu On Wed, Nov 29, 2023, Jason Gunthorpe wrote: > On Wed, Nov 29, 2023 at 05:07:45PM -0800, Sean Christopherson wrote: > > On Wed, Nov 29, 2023, Michael Ellerman wrote: > > > Sean Christopherson <seanjc@google.com> writes: > > > > On Fri, Nov 10, 2023, Michael Ellerman wrote: > > > >> Jason Gunthorpe <jgg@nvidia.com> writes: > > > >> > There are a bunch of reported randconfig failures now because of this, > > > >> > something like: > > > >> > > > > >> >>> arch/powerpc/kvm/../../../virt/kvm/vfio.c:89:7: warning: attribute declaration must precede definition [-Wignored-attributes] > > > >> > fn = symbol_get(vfio_file_iommu_group); > > > >> > ^ > > > >> > include/linux/module.h:805:60: note: expanded from macro 'symbol_get' > > > >> > #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak,visibility("hidden"))); &(x); }) > > > >> > > > > >> > It happens because the arch forces KVM_VFIO without knowing if VFIO is > > > >> > even enabled. > > > >> > > > >> This is still breaking some builds. Can we get this fix in please? > > > >> > > > >> cheers > > > >> > > > >> > Split the kconfig so the arch selects the usual HAVE_KVM_ARCH_VFIO and > > > >> > then KVM_VFIO is only enabled if the arch wants it and VFIO is turned on. > > > > > > > > Heh, so I was trying to figure out why things like vfio_file_set_kvm() aren't > > > > problematic, i.e. why the existing mess didn't cause failures. I can't repro the > > > > warning (requires clang-16?), but IIUC the reason only the group code is problematic > > > > is that vfio.h creates a stub for vfio_file_iommu_group() and thus there's no symbol, > > > > whereas vfio.h declares vfio_file_set_kvm() unconditionally. > > > > > > That warning I'm unsure about. > > > > Ah, it's the same warning, I just missed the CONFIG_MODULES=n requirement. > > Oh, wait, doesn't that mean the approach won't work? IIRC doesn't > symbol_get turn into just &fn when non-modular turning this into a > link failure without the kconfig part? Yes, but it doesn't cause linker errors. IIUC, because the extern declaration is tagged "weak", a dummy default is used. E.g. on x86, this is what is generated with VFIO=y fn = symbol_get(vfio_file_is_valid); if (!fn) 0xffffffff810396c5 <+5>: mov $0xffffffff81829230,%rax 0xffffffff810396cc <+12>: test %rax,%rax whereas VFIO=n gets fn = symbol_get(vfio_file_is_valid); if (!fn) 0xffffffff810396c5 <+5>: mov $0x0,%rax 0xffffffff810396cc <+12>: test %rax,%rax I have no idea if the fact that symbol_get() generates '0', i.e. the !NULL checks work as expected, is intentional or if KVM works by sheer dumb luck. I'm not entirely sure I want to find out, though it's definitely extra motiviation to get the KVM mess fixed sooner than later. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-11-30 2:02 ` Sean Christopherson @ 2023-11-30 6:38 ` Michael Ellerman 2023-11-30 12:07 ` Jason Gunthorpe 1 sibling, 0 replies; 13+ messages in thread From: Michael Ellerman @ 2023-11-30 6:38 UTC (permalink / raw) To: Sean Christopherson, Jason Gunthorpe Cc: Paolo Bonzini, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu Sean Christopherson <seanjc@google.com> writes: > On Wed, Nov 29, 2023, Jason Gunthorpe wrote: >> On Wed, Nov 29, 2023 at 05:07:45PM -0800, Sean Christopherson wrote: >> > On Wed, Nov 29, 2023, Michael Ellerman wrote: >> > > Sean Christopherson <seanjc@google.com> writes: >> > > > On Fri, Nov 10, 2023, Michael Ellerman wrote: >> > > >> Jason Gunthorpe <jgg@nvidia.com> writes: >> > > >> > There are a bunch of reported randconfig failures now because of this, >> > > >> > something like: >> > > >> > >> > > >> >>> arch/powerpc/kvm/../../../virt/kvm/vfio.c:89:7: warning: attribute declaration must precede definition [-Wignored-attributes] >> > > >> > fn = symbol_get(vfio_file_iommu_group); >> > > >> > ^ >> > > >> > include/linux/module.h:805:60: note: expanded from macro 'symbol_get' >> > > >> > #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak,visibility("hidden"))); &(x); }) >> > > >> > >> > > >> > It happens because the arch forces KVM_VFIO without knowing if VFIO is >> > > >> > even enabled. >> > > >> >> > > >> This is still breaking some builds. Can we get this fix in please? >> > > >> >> > > >> cheers >> > > >> >> > > >> > Split the kconfig so the arch selects the usual HAVE_KVM_ARCH_VFIO and >> > > >> > then KVM_VFIO is only enabled if the arch wants it and VFIO is turned on. >> > > > >> > > > Heh, so I was trying to figure out why things like vfio_file_set_kvm() aren't >> > > > problematic, i.e. why the existing mess didn't cause failures. I can't repro the >> > > > warning (requires clang-16?), but IIUC the reason only the group code is problematic >> > > > is that vfio.h creates a stub for vfio_file_iommu_group() and thus there's no symbol, >> > > > whereas vfio.h declares vfio_file_set_kvm() unconditionally. >> > > >> > > That warning I'm unsure about. >> > >> > Ah, it's the same warning, I just missed the CONFIG_MODULES=n requirement. >> >> Oh, wait, doesn't that mean the approach won't work? IIRC doesn't >> symbol_get turn into just &fn when non-modular turning this into a >> link failure without the kconfig part? It does build. I haven't boot tested it, but TBH I don't really care as long as the build is green, I don't think anyone's actually using this weird combination of config options. > Yes, but it doesn't cause linker errors. IIUC, because the extern declaration > is tagged "weak", a dummy default is used. E.g. on x86, this is what is generated > with VFIO=y > > fn = symbol_get(vfio_file_is_valid); > if (!fn) > 0xffffffff810396c5 <+5>: mov $0xffffffff81829230,%rax > 0xffffffff810396cc <+12>: test %rax,%rax > > whereas VFIO=n gets > > fn = symbol_get(vfio_file_is_valid); > if (!fn) > 0xffffffff810396c5 <+5>: mov $0x0,%rax > 0xffffffff810396cc <+12>: test %rax,%rax > > I have no idea if the fact that symbol_get() generates '0', i.e. the !NULL checks > work as expected, is intentional or if KVM works by sheer dumb luck. I think it's intentional: https://lore.kernel.org/all/20030117045054.9A2F72C073@lists.samba.org/ cheers _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-11-30 2:02 ` Sean Christopherson 2023-11-30 6:38 ` Michael Ellerman @ 2023-11-30 12:07 ` Jason Gunthorpe 1 sibling, 0 replies; 13+ messages in thread From: Jason Gunthorpe @ 2023-11-30 12:07 UTC (permalink / raw) To: Sean Christopherson Cc: Michael Ellerman, Paolo Bonzini, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu On Wed, Nov 29, 2023 at 06:02:08PM -0800, Sean Christopherson wrote: > > > Ah, it's the same warning, I just missed the CONFIG_MODULES=n requirement. > > > > Oh, wait, doesn't that mean the approach won't work? IIRC doesn't > > symbol_get turn into just &fn when non-modular turning this into a > > link failure without the kconfig part? > > Yes, but it doesn't cause linker errors. IIUC, because the extern declaration > is tagged "weak", a dummy default is used. E.g. on x86, this is what is generated > with VFIO=y Oh wow that is some pretty dark magic there :| Jason _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-11-29 2:21 ` Sean Christopherson 2023-11-29 9:38 ` Michael Ellerman @ 2023-11-29 12:48 ` Jason Gunthorpe 2023-11-29 18:31 ` Sean Christopherson 1 sibling, 1 reply; 13+ messages in thread From: Jason Gunthorpe @ 2023-11-29 12:48 UTC (permalink / raw) To: Sean Christopherson Cc: Michael Ellerman, Paolo Bonzini, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu On Tue, Nov 28, 2023 at 06:21:42PM -0800, Sean Christopherson wrote: > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > index 454e9295970c..a65b2513f8cd 100644 > --- a/include/linux/vfio.h > +++ b/include/linux/vfio.h > @@ -289,16 +289,12 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes, > /* > * External user API > */ > -#if IS_ENABLED(CONFIG_VFIO_GROUP) > struct iommu_group *vfio_file_iommu_group(struct file *file); > + > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > bool vfio_file_is_group(struct file *file); > bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > #else > -static inline struct iommu_group *vfio_file_iommu_group(struct file *file) > -{ > - return NULL; > -} > - > static inline bool vfio_file_is_group(struct file *file) > { > return false; > So you symbol get on a symbol that can never be defined? Still says to me the kconfig needs fixing :| But sure, as a small patch it looks fine Thanks, Jason _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-11-29 12:48 ` Jason Gunthorpe @ 2023-11-29 18:31 ` Sean Christopherson 2023-11-29 21:45 ` Alex Williamson 0 siblings, 1 reply; 13+ messages in thread From: Sean Christopherson @ 2023-11-29 18:31 UTC (permalink / raw) To: Jason Gunthorpe Cc: Michael Ellerman, Paolo Bonzini, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu On Wed, Nov 29, 2023, Jason Gunthorpe wrote: > On Tue, Nov 28, 2023 at 06:21:42PM -0800, Sean Christopherson wrote: > > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > > index 454e9295970c..a65b2513f8cd 100644 > > --- a/include/linux/vfio.h > > +++ b/include/linux/vfio.h > > @@ -289,16 +289,12 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes, > > /* > > * External user API > > */ > > -#if IS_ENABLED(CONFIG_VFIO_GROUP) > > struct iommu_group *vfio_file_iommu_group(struct file *file); > > + > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > > bool vfio_file_is_group(struct file *file); > > bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > > #else > > -static inline struct iommu_group *vfio_file_iommu_group(struct file *file) > > -{ > > - return NULL; > > -} > > - > > static inline bool vfio_file_is_group(struct file *file) > > { > > return false; > > > > So you symbol get on a symbol that can never be defined? Still says to > me the kconfig needs fixing :| Yeah, I completely agree, and if KVM didn't already rely on this horrific behavior and there wasn't a more complete overhaul in-flight, I wouldn't suggest this. I'll send the KVM Kconfig/Makefile cleanups from my "Hide KVM internals from others" series separately (which is still the bulk of the series) so as to prioritize getting the cleanups landed. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected 2023-11-29 18:31 ` Sean Christopherson @ 2023-11-29 21:45 ` Alex Williamson 0 siblings, 0 replies; 13+ messages in thread From: Alex Williamson @ 2023-11-29 21:45 UTC (permalink / raw) To: Sean Christopherson Cc: Jason Gunthorpe, Michael Ellerman, Paolo Bonzini, Alexander Gordeev, Christian Borntraeger, Borislav Petkov, Catalin Marinas, Christophe Leroy, Dave Hansen, David Hildenbrand, Janosch Frank, Vasily Gorbik, Heiko Carstens, H. Peter Anvin, Claudio Imbrenda, James Morse, kvm, kvmarm, linux-arm-kernel, linux-s390, linuxppc-dev, Marc Zyngier, Ingo Molnar, Nicholas Piggin, Oliver Upton, Suzuki K Poulose, Sven Schnelle, Thomas Gleixner, Will Deacon, x86, Zenghui Yu On Wed, 29 Nov 2023 10:31:03 -0800 Sean Christopherson <seanjc@google.com> wrote: > On Wed, Nov 29, 2023, Jason Gunthorpe wrote: > > On Tue, Nov 28, 2023 at 06:21:42PM -0800, Sean Christopherson wrote: > > > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > > > index 454e9295970c..a65b2513f8cd 100644 > > > --- a/include/linux/vfio.h > > > +++ b/include/linux/vfio.h > > > @@ -289,16 +289,12 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes, > > > /* > > > * External user API > > > */ > > > -#if IS_ENABLED(CONFIG_VFIO_GROUP) > > > struct iommu_group *vfio_file_iommu_group(struct file *file); > > > + > > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > > > bool vfio_file_is_group(struct file *file); > > > bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > > > #else > > > -static inline struct iommu_group *vfio_file_iommu_group(struct file *file) > > > -{ > > > - return NULL; > > > -} > > > - > > > static inline bool vfio_file_is_group(struct file *file) > > > { > > > return false; > > > > > > > So you symbol get on a symbol that can never be defined? Still says to > > me the kconfig needs fixing :| > > Yeah, I completely agree, and if KVM didn't already rely on this horrific > behavior and there wasn't a more complete overhaul in-flight, I wouldn't suggest > this. > > I'll send the KVM Kconfig/Makefile cleanups from my "Hide KVM internals from others" > series separately (which is still the bulk of the series) so as to prioritize > getting the cleanups landed. > Seems we have agreement and confirmation of the fix above as an interim, do you want to post it formally and I can pick it up for v6.7-rc? Thanks, Alex _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-11-30 12:08 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-20 18:18 [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected Jason Gunthorpe 2023-09-22 0:00 ` Michael Ellerman 2023-11-10 6:08 ` Ping? " Michael Ellerman 2023-11-29 2:21 ` Sean Christopherson 2023-11-29 9:38 ` Michael Ellerman 2023-11-30 1:07 ` Sean Christopherson 2023-11-30 1:16 ` Jason Gunthorpe 2023-11-30 2:02 ` Sean Christopherson 2023-11-30 6:38 ` Michael Ellerman 2023-11-30 12:07 ` Jason Gunthorpe 2023-11-29 12:48 ` Jason Gunthorpe 2023-11-29 18:31 ` Sean Christopherson 2023-11-29 21:45 ` Alex Williamson
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).