Re: [PATCH v7 9/9] iommu/arm-smmu-v3: Add unit tests for arm_smmu_write_entry

From: Nicolin Chen <nicolinc@nvidia.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Michael Shavit <mshavit@google.com>, <iommu@lists.linux.dev>,
	Joerg Roedel <joro@8bytes.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Will Deacon <will@kernel.org>, Eric Auger <eric.auger@redhat.com>,
	Moritz Fischer <mdf@kernel.org>,
	Moritz Fischer <moritzf@google.com>, <patches@lists.linux.dev>,
	Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>,
	Mostafa Saleh <smostafa@google.com>
Subject: Re: [PATCH v7 9/9] iommu/arm-smmu-v3: Add unit tests for arm_smmu_write_entry
Date: Wed, 17 Apr 2024 09:13:11 -0700	[thread overview]
Message-ID: <Zh/1F6nqvDOEbSfi@Asurada-Nvidia> (raw)
In-Reply-To: <20240417141618.GS3637727@nvidia.com>

On Wed, Apr 17, 2024 at 11:16:18AM -0300, Jason Gunthorpe wrote:
> On Wed, Apr 17, 2024 at 01:09:40AM -0700, Nicolin Chen wrote:
> > On Tue, Apr 16, 2024 at 04:28:20PM -0300, Jason Gunthorpe wrote:
> > > Add tests for some of the more common STE update operations that we expect
> > > to see, as well as some artificial STE updates to test the edges of
> > > arm_smmu_write_entry. These also serve as a record of which common
> > > operation is expected to be hitless, and how many syncs they require.
> > > 
> > > arm_smmu_write_entry implements a generic algorithm that updates an STE/CD
> > > to any other abritrary STE/CD configuration. The update requires a
> > > sequence of write+sync operations with some invariants that must be held
> > > true after each sync. arm_smmu_write_entry lends itself well to
> > > unit-testing since the function's interaction with the STE/CD is already
> > > abstracted by input callbacks that we can hook to introspect into the
> > > sequence of operations. We can use these hooks to guarantee that
> > > invariants are held throughout the entire update operation.
> > > 
> > > Link: https://lore.kernel.org/r/20240106083617.1173871-3-mshavit@google.com
> > > Signed-off-by: Michael Shavit <mshavit@google.com>
> > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > > ---
> > >  drivers/iommu/Kconfig                         |  12 +-
> > >  drivers/iommu/arm/arm-smmu-v3/Makefile        |   2 +
> > >  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |   6 +-
> > >  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c  | 467 ++++++++++++++++++
> > >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  36 +-
> > >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  30 ++
> >  
> > > +config ARM_SMMU_V3_KUNIT_TEST
> > > +	tristate "KUnit tests for arm-smmu-v3 driver"  if !KUNIT_ALL_TESTS
> > > +	depends on KUNIT
> > > +	default KUNIT_ALL_TESTS
> > > +	help
> > > +	  Enable this option to unit-test arm-smmu-v3 driver functions.
> > > +
> > > +	  If unsure, say N.
> > 
> > Forgot that my SVA sanity doesn't cover this patch. And it looks
> > like some problems here when building it with "=m":
> > 
> > ERROR: modpost: missing MODULE_LICENSE() in drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.o
> > ERROR: modpost: "arm_smmu_make_cdtable_ste" [drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.ko] undefined!
> > ERROR: modpost: "arm_smmu_make_bypass_ste" [drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.ko] undefined!
> > ERROR: modpost: "arm_smmu_make_abort_ste" [drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.ko] undefined!
> > ERROR: modpost: "arm_smmu_make_s2_domain_ste" [drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.ko] undefined!
> > ERROR: modpost: "arm_smmu_get_ste_used" [drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.ko] undefined!
> > ERROR: modpost: "arm_smmu_write_entry" [drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.ko] undefined!
> > 
> > Likely needs MODULE_LICENSE and some EXPORT_SYMBOLs.
> 
> Oh! The kbuild never tested this kconfig combination...
> 
> I think just this? Michael?

Verified with the following change.

Thanks
Nicolin

> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index d03c729c4142dc..7b6a4e244e99cf 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -411,7 +411,7 @@ config ARM_SMMU_V3_SVA
>  	  and PRI.
>  
>  config ARM_SMMU_V3_KUNIT_TEST
> -	tristate "KUnit tests for arm-smmu-v3 driver"  if !KUNIT_ALL_TESTS
> +	bool "KUnit tests for arm-smmu-v3 driver"  if !KUNIT_ALL_TESTS
>  	depends on KUNIT
>  	default KUNIT_ALL_TESTS
>  	help
> diff --git a/drivers/iommu/arm/arm-smmu-v3/Makefile b/drivers/iommu/arm/arm-smmu-v3/Makefile
> index 014a997753a8a2..0b97054b3929b7 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/Makefile
> +++ b/drivers/iommu/arm/arm-smmu-v3/Makefile
> @@ -2,6 +2,5 @@
>  obj-$(CONFIG_ARM_SMMU_V3) += arm_smmu_v3.o
>  arm_smmu_v3-objs-y += arm-smmu-v3.o
>  arm_smmu_v3-objs-$(CONFIG_ARM_SMMU_V3_SVA) += arm-smmu-v3-sva.o
> +arm_smmu_v3-objs-$(CONFIG_ARM_SMMU_V3_KUNIT_TEST) += arm-smmu-v3-test.o
>  arm_smmu_v3-objs := $(arm_smmu_v3-objs-y)
> -
> -obj-$(CONFIG_ARM_SMMU_V3_KUNIT_TEST) += arm-smmu-v3-test.o
> 
> 
> Jason

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel