Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected

From: Jason Gunthorpe <jgg@nvidia.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Alexander Gordeev <agordeev@linux.ibm.com>,
	Christian Borntraeger <borntraeger@linux.ibm.com>,
	Borislav Petkov <bp@alien8.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	David Hildenbrand <david@redhat.com>,
	Janosch Frank <frankja@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Heiko Carstens <hca@linux.ibm.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>,
	James Morse <james.morse@arm.com>,
	kvm@vger.kernel.org, kvmarm@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, Marc Zyngier <maz@kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Nicholas Piggin <npiggin@gmail.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Sven Schnelle <svens@linux.ibm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Will Deacon <will@kernel.org>,
	x86@kernel.org, Zenghui Yu <yuzenghui@huawei.com>
Subject: Re: Ping? Re: [PATCH rc] kvm: Prevent compiling virt/kvm/vfio.c unless VFIO is selected
Date: Wed, 29 Nov 2023 21:16:50 -0400	[thread overview]
Message-ID: <20231130011650.GD1389974@nvidia.com> (raw)
In-Reply-To: <ZWfgYdSoJAZqL2Gx@google.com>

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