Re: [PATCH] iommu/arm-smmu-v3: Make the kunit into a module

From: Jason Gunthorpe <jgg@nvidia.com>
To: Will Deacon <will@kernel.org>
Cc: iommu@lists.linux.dev, Joerg Roedel <joro@8bytes.org>,
	linux-arm-kernel@lists.infradead.org,
	Robin Murphy <robin.murphy@arm.com>,
	Thorsten Leemhuis <linux@leemhuis.info>,
	Michael Shavit <mshavit@google.com>,
	Nicolin Chen <nicolinc@nvidia.com>,
	patches@lists.linux.dev
Subject: Re: [PATCH] iommu/arm-smmu-v3: Make the kunit into a module
Date: Tue, 7 May 2024 11:09:46 -0300	[thread overview]
Message-ID: <20240507140946.GO3341011@nvidia.com> (raw)
In-Reply-To: <20240507135817.GA22408@willie-the-truck>

On Tue, May 07, 2024 at 02:58:17PM +0100, Will Deacon wrote:
> On Tue, May 07, 2024 at 10:21:10AM -0300, Jason Gunthorpe wrote:
> > It turns out kconfig has problems ensuring the SMMU module and the KUNIT
> > module are consistently y/m to allow linking. It will permit KUNIT to be a
> > module while SMMU is built in.
> > 
> > Also, Fedora apparently enables kunit on production kernels.
> > 
> > So, put the entire kunit in its own module using the
> > VISIBLE_IF_KUNIT/EXPORT_SYMBOL_IF_KUNIT machinery. This keeps it out of
> > vmlinus on Fedora and makes the kconfig work in the normal way. There is
> > no cost if kunit is disabled.
> > 
> > Fixes: 56e1a4cc2588 ("iommu/arm-smmu-v3: Add unit tests for arm_smmu_write_entry")
> > Reported-by: Thorsten Leemhuis <linux@leemhuis.info>
> > Link: https://lore.kernel.org/all/aeea8546-5bce-4c51-b506-5d2008e52fef@leemhuis.info
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > ---
> >  drivers/iommu/Kconfig                            | 2 +-
> >  drivers/iommu/arm/arm-smmu-v3/Makefile           | 3 ++-
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c  | 1 +
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c | 3 +++
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c      | 8 ++++++++
> >  5 files changed, 15 insertions(+), 2 deletions(-)
> > 
> > Joerg, can you pick this to solve the linux-next issue for Thorsten?
> > 
> > Thanks,
> > Jason
> > 
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index 66325210c8c986..c04584be30893f 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -415,7 +415,7 @@ config ARM_SMMU_V3_SVA
> >  	  and PRI.
> >  
> >  config ARM_SMMU_V3_KUNIT_TEST
> > -	bool "KUnit tests for arm-smmu-v3 driver"  if !KUNIT_ALL_TESTS
> > +	tristate "KUnit tests for arm-smmu-v3 driver"  if !KUNIT_ALL_TESTS
> >  	depends on KUNIT
> >  	depends on ARM_SMMU_V3_SVA
> >  	default KUNIT_ALL_TESTS
> 
> Would it work to leave this as 'bool' and have something like:
> 
> 	depends on KUNIT=y

Yes, there is a version like this (depends on KUNIT = ARM_SMMU_V3),
but it made kconfig act a little weird and hide the symbols.

> instead? That would be a lot simpler and avoids all the conditional
> symbol exports.

But then Fedora is linking this code into their production kernel
which doesn't seem right.

I avoided doing this from the start because I didn't know about
EXPORT_SYMBOL_IF_KUNIT and didn't want to make it costly.

Jason