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

[Will Deacon]

On Tue, May 07, 2024 at 11:09:46AM -0300, Jason Gunthorpe wrote:
> 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 at leemhuis.info>
> > > Link: https://lore.kernel.org/all/aeea8546-5bce-4c51-b506-5d2008e52fef@leemhuis.info
> > > Signed-off-by: Jason Gunthorpe <jgg at 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.

Which symbols were hidden in which cases? Having ARM_SMMU_V3_KUNIT_TEST
disappear from menuconfig when the dependency isn't satisifed sounds
fine to me, but you make it sound like it was something else.

> > 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.

Well, only if they select CONFIG_ARM_SMMU_V3_KUNIT_TEST, right? There
also seems to be precedence for that if I grep around:

drivers/base/Kconfig:	depends on KUNIT=y
drivers/fpga/tests/Kconfig:	depends on FPGA=y && FPGA_REGION=y && FPGA_BRIDGE=y && KUNIT=y && MODULES=n
drivers/gpu/drm/xe/Kconfig:	depends on DRM && PCI && MMU && (m || (y && KUNIT=y))
drivers/mmc/host/Kconfig:	depends on (MMC_SDHCI_OF_ASPEED=m || KUNIT=y)
drivers/net/ethernet/microchip/vcap/Kconfig:	depends on KUNIT=y && VCAP=y && y
drivers/thunderbolt/Kconfig:	depends on USB4 && KUNIT=y
drivers/virt/nitro_enclaves/Kconfig:	depends on NITRO_ENCLAVES && KUNIT=y
fs/Kconfig.binfmt:	depends on KUNIT=y && BINFMT_ELF=y
lib/Kconfig.debug:	depends on KUNIT=y
lib/Kconfig.debug:	depends on KUNIT=y
lib/Kconfig.debug:	depends on RUST && KUNIT=y
mm/damon/Kconfig:	depends on DAMON && KUNIT=y
mm/damon/Kconfig:	depends on DAMON_VADDR && KUNIT=y
mm/damon/Kconfig:	depends on DAMON_SYSFS && KUNIT=y
mm/damon/Kconfig:	depends on DAMON_DBGFS && KUNIT=y
net/mctp/Kconfig:        depends on MCTP=y && KUNIT=y
security/landlock/Kconfig:	depends on KUNIT=y

Will