[PATCH 0/3] Tidy some parts of the VFIO kconfig world

[PATCH 0/3] Tidy some parts of the VFIO kconfig world

[Jason Gunthorpe]

The main goal here is to add enough COMPILE_TEST that an ARM cross
compiler is not needed to be able to get compile coverage on VFIO. A
normal x86 compiler with COMPILE_TEST will do the job.

s390 and ppc are still needed, and I'm looking to see if any of that is
reasonable to change too.

Thanks,
Jason

Jason Gunthorpe (3):
  vfio: IOMMU_API should be selected
  vfio-platform: Add COMPILE_TEST to VFIO_PLATFORM
  ARM: amba: Allow some ARM_AMBA users to compile with COMPILE_TEST

 drivers/vfio/Kconfig          |  2 +-
 drivers/vfio/platform/Kconfig |  4 ++--
 include/linux/amba/bus.h      | 11 +++++++++++
 3 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.30.0

[PATCH 1/3] vfio: IOMMU_API should be selected

[Jason Gunthorpe]

As IOMMU_API is a kconfig without a description (eg does not show in the
menu) the correct operator is select not 'depends on'. Using 'depends on'
for this kind of symbol means VFIO is not selectable unless some other
random kconfig has already enabled IOMMU_API for it.

Fixes: cba3345cc494 ("vfio: VFIO core")
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/vfio/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 5533df91b257d6..90c0525b1e0cf4 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -21,7 +21,7 @@ config VFIO_VIRQFD
 
 menuconfig VFIO
 	tristate "VFIO Non-Privileged userspace driver framework"
-	depends on IOMMU_API
+	select IOMMU_API
 	select VFIO_IOMMU_TYPE1 if (X86 || S390 || ARM || ARM64)
 	help
 	  VFIO provides a framework for secure userspace device drivers.
-- 
2.30.0

[PATCH 2/3] vfio-platform: Add COMPILE_TEST to VFIO_PLATFORM

[Jason Gunthorpe]

x86 can build platform bus code too, so vfio-platform and all the platform
reset implementations compile successfully on x86.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/vfio/platform/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
index dc1a3c44f2c62b..233efde219cc10 100644
--- a/drivers/vfio/platform/Kconfig
+++ b/drivers/vfio/platform/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config VFIO_PLATFORM
 	tristate "VFIO support for platform devices"
-	depends on VFIO && EVENTFD && (ARM || ARM64)
+	depends on VFIO && EVENTFD && (ARM || ARM64 || COMPILE_TEST)
 	select VFIO_VIRQFD
 	help
 	  Support for platform devices with VFIO. This is required to make
-- 
2.30.0

[PATCH 3/3] ARM: amba: Allow some ARM_AMBA users to compile with COMPILE_TEST

[Jason Gunthorpe]

CONFIG_VFIO_AMBA has a light use of AMBA, adding some inline fallbacks
when AMBA is disabled will allow it to be compiled under COMPILE_TEST and
make VFIO easier to maintain.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/vfio/platform/Kconfig |  2 +-
 include/linux/amba/bus.h      | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
index 233efde219cc10..ab341108a0be94 100644
--- a/drivers/vfio/platform/Kconfig
+++ b/drivers/vfio/platform/Kconfig
@@ -12,7 +12,7 @@ config VFIO_PLATFORM
 
 config VFIO_AMBA
 	tristate "VFIO support for AMBA devices"
-	depends on VFIO_PLATFORM && ARM_AMBA
+	depends on VFIO_PLATFORM && (ARM_AMBA || COMPILE_TEST)
 	help
 	  Support for ARM AMBA devices with VFIO. This is required to make
 	  use of ARM AMBA devices present on the system using the VFIO
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index 0bbfd647f5c6de..977edd6e541ddd 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -105,8 +105,19 @@ extern struct bus_type amba_bustype;
 #define amba_get_drvdata(d)	dev_get_drvdata(&d->dev)
 #define amba_set_drvdata(d,p)	dev_set_drvdata(&d->dev, p)
 
+#ifdef CONFIG_ARM_AMBA
 int amba_driver_register(struct amba_driver *);
 void amba_driver_unregister(struct amba_driver *);
+#else
+static inline int amba_driver_register(struct amba_driver *drv)
+{
+	return -EINVAL;
+}
+static inline void amba_driver_unregister(struct amba_driver *drv)
+{
+}
+#endif
+
 struct amba_device *amba_device_alloc(const char *, resource_size_t, size_t);
 void amba_device_put(struct amba_device *);
 int amba_device_add(struct amba_device *, struct resource *);
-- 
2.30.0

Re: [PATCH 2/3] vfio-platform: Add COMPILE_TEST to VFIO_PLATFORM

[Auger Eric]

Hi Jason,
On 2/23/21 8:17 PM, Jason Gunthorpe wrote:
> x86 can build platform bus code too, so vfio-platform and all the platform
> reset implementations compile successfully on x86.

A similar patch was sent recently, see
[PATCH v1] vfio: platform: enable compile test
https://www.spinics.net/lists/kvm/msg230677.html

Thanks

Eric


> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/vfio/platform/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
> index dc1a3c44f2c62b..233efde219cc10 100644
> --- a/drivers/vfio/platform/Kconfig
> +++ b/drivers/vfio/platform/Kconfig
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  config VFIO_PLATFORM
>  	tristate "VFIO support for platform devices"
> -	depends on VFIO && EVENTFD && (ARM || ARM64)
> +	depends on VFIO && EVENTFD && (ARM || ARM64 || COMPILE_TEST)
>  	select VFIO_VIRQFD
>  	help
>  	  Support for platform devices with VFIO. This is required to make
> 

Re: [PATCH 3/3] ARM: amba: Allow some ARM_AMBA users to compile with COMPILE_TEST

[Auger Eric]

Hi Jason,

On 2/23/21 8:17 PM, Jason Gunthorpe wrote:
> CONFIG_VFIO_AMBA has a light use of AMBA, adding some inline fallbacks
> when AMBA is disabled will allow it to be compiled under COMPILE_TEST and
> make VFIO easier to maintain.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric

> ---
>  drivers/vfio/platform/Kconfig |  2 +-
>  include/linux/amba/bus.h      | 11 +++++++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
> index 233efde219cc10..ab341108a0be94 100644
> --- a/drivers/vfio/platform/Kconfig
> +++ b/drivers/vfio/platform/Kconfig
> @@ -12,7 +12,7 @@ config VFIO_PLATFORM
>  
>  config VFIO_AMBA
>  	tristate "VFIO support for AMBA devices"
> -	depends on VFIO_PLATFORM && ARM_AMBA
> +	depends on VFIO_PLATFORM && (ARM_AMBA || COMPILE_TEST)
>  	help
>  	  Support for ARM AMBA devices with VFIO. This is required to make
>  	  use of ARM AMBA devices present on the system using the VFIO
> diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
> index 0bbfd647f5c6de..977edd6e541ddd 100644
> --- a/include/linux/amba/bus.h
> +++ b/include/linux/amba/bus.h
> @@ -105,8 +105,19 @@ extern struct bus_type amba_bustype;
>  #define amba_get_drvdata(d)	dev_get_drvdata(&d->dev)
>  #define amba_set_drvdata(d,p)	dev_set_drvdata(&d->dev, p)
>  
> +#ifdef CONFIG_ARM_AMBA
>  int amba_driver_register(struct amba_driver *);
>  void amba_driver_unregister(struct amba_driver *);
> +#else
> +static inline int amba_driver_register(struct amba_driver *drv)
> +{
> +	return -EINVAL;
> +}
> +static inline void amba_driver_unregister(struct amba_driver *drv)
> +{
> +}
> +#endif
> +
>  struct amba_device *amba_device_alloc(const char *, resource_size_t, size_t);
>  void amba_device_put(struct amba_device *);
>  int amba_device_add(struct amba_device *, struct resource *);
> 

Re: [PATCH 1/3] vfio: IOMMU_API should be selected

[Auger Eric]

Hi,
On 2/23/21 8:17 PM, Jason Gunthorpe wrote:
> As IOMMU_API is a kconfig without a description (eg does not show in the
> menu) the correct operator is select not 'depends on'. Using 'depends on'
> for this kind of symbol means VFIO is not selectable unless some other
> random kconfig has already enabled IOMMU_API for it.

looks sensible to me
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> 
> Fixes: cba3345cc494 ("vfio: VFIO core")
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/vfio/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 5533df91b257d6..90c0525b1e0cf4 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -21,7 +21,7 @@ config VFIO_VIRQFD
>  
>  menuconfig VFIO
>  	tristate "VFIO Non-Privileged userspace driver framework"
> -	depends on IOMMU_API
> +	select IOMMU_API
>  	select VFIO_IOMMU_TYPE1 if (X86 || S390 || ARM || ARM64)
>  	help
>  	  VFIO provides a framework for secure userspace device drivers.
> 

Re: [PATCH 2/3] vfio-platform: Add COMPILE_TEST to VFIO_PLATFORM

[Jason Gunthorpe]

On Wed, Feb 24, 2021 at 10:42:10AM +0100, Auger Eric wrote:
> Hi Jason,
> On 2/23/21 8:17 PM, Jason Gunthorpe wrote:
> > x86 can build platform bus code too, so vfio-platform and all the platform
> > reset implementations compile successfully on x86.
> 
> A similar patch was sent recently, see
> [PATCH v1] vfio: platform: enable compile test
> https://www.spinics.net/lists/kvm/msg230677.html

Well, it does look like it got lost/abandoned, so lets focus on this
series.

Thanks,
Jason

Re: [PATCH 1/3] vfio: IOMMU_API should be selected

[Cornelia Huck]

On Tue, 23 Feb 2021 15:17:46 -0400
Jason Gunthorpe <jgg@nvidia.com> wrote:

> As IOMMU_API is a kconfig without a description (eg does not show in the
> menu) the correct operator is select not 'depends on'. Using 'depends on'
> for this kind of symbol means VFIO is not selectable unless some other
> random kconfig has already enabled IOMMU_API for it.
> 
> Fixes: cba3345cc494 ("vfio: VFIO core")
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/vfio/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 5533df91b257d6..90c0525b1e0cf4 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -21,7 +21,7 @@ config VFIO_VIRQFD
>  
>  menuconfig VFIO
>  	tristate "VFIO Non-Privileged userspace driver framework"
> -	depends on IOMMU_API
> +	select IOMMU_API
>  	select VFIO_IOMMU_TYPE1 if (X86 || S390 || ARM || ARM64)
>  	help
>  	  VFIO provides a framework for secure userspace device drivers.

I'm wondering whether this should depend on MMU?

Re: [PATCH 1/3] vfio: IOMMU_API should be selected

[Jason Gunthorpe]

On Wed, Mar 03, 2021 at 01:36:26PM +0100, Cornelia Huck wrote:
> On Tue, 23 Feb 2021 15:17:46 -0400
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > As IOMMU_API is a kconfig without a description (eg does not show in the
> > menu) the correct operator is select not 'depends on'. Using 'depends on'
> > for this kind of symbol means VFIO is not selectable unless some other
> > random kconfig has already enabled IOMMU_API for it.
> > 
> > Fixes: cba3345cc494 ("vfio: VFIO core")
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> >  drivers/vfio/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> > index 5533df91b257d6..90c0525b1e0cf4 100644
> > +++ b/drivers/vfio/Kconfig
> > @@ -21,7 +21,7 @@ config VFIO_VIRQFD
> >  
> >  menuconfig VFIO
> >  	tristate "VFIO Non-Privileged userspace driver framework"
> > -	depends on IOMMU_API
> > +	select IOMMU_API
> >  	select VFIO_IOMMU_TYPE1 if (X86 || S390 || ARM || ARM64)
> >  	help
> >  	  VFIO provides a framework for secure userspace device drivers.
> 
> I'm wondering whether this should depend on MMU?

Hum. My ARM cross compiler says it won't compile:

../drivers/vfio/vfio_iommu_type1.c: In function 'follow_fault_pfn':
../drivers/vfio/vfio_iommu_type1.c:536:22: error: implicit declaration of function 'pte_write'; did you mean 'vfs_write'? [-Werror=implicit-function-declaration]
  if (write_fault && !pte_write(*ptep))
                      ^~~~~~~~~
                      vfs_write
../drivers/vfio/vfio_iommu_type1.c:539:10: error: implicit declaration of function 'pte_pfn'; did you mean 'put_pfn'? [-Werror=implicit-function-declaration]
   *pfn = pte_pfn(*ptep);
          ^~~~~~~
          put_pfn
In file included from ../include/linux/highmem.h:8,
                 from ../drivers/vfio/vfio_iommu_type1.c:27:
../include/linux/mm.h:2200:2: error: implicit declaration of function 'pte_unmap'; did you mean 'memunmap'? [-Werror=implicit-function-declaration]
  pte_unmap(pte);     \
  ^~~~~~~~~
../drivers/vfio/vfio_iommu_type1.c:541:2: note: in expansion of macro 'pte_unmap_unlock'
  pte_unmap_unlock(ptep, ptl);
  ^~~~~~~~~~~~~~~~

So, yes, it does.

Interesting that a compile bot hasn't reported this before.

-       select VFIO_IOMMU_TYPE1 if (X86 || S390 || ARM || ARM64)
+       select VFIO_IOMMU_TYPE1 if MMU && (X86 || S390 || ARM || ARM64)

Is enough to make !MMU ARM compile.

I'll send an additonal patch

Jason