diff mbox series

[1/3] vfio: IOMMU_API should be selected

Message ID 1-v1-df057e0f92c3+91-vfio_arm_compile_test_jgg@nvidia.com (mailing list archive)
State New, archived
Headers show
Series Tidy some parts of the VFIO kconfig world | expand

Commit Message

Jason Gunthorpe Feb. 23, 2021, 7:17 p.m. UTC
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(-)

Comments

Auger Eric Feb. 24, 2021, 9:50 a.m. UTC | #1
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.
>
Cornelia Huck March 3, 2021, 12:36 p.m. UTC | #2
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?
Jason Gunthorpe March 3, 2021, 12:52 p.m. UTC | #3
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
diff mbox series

Patch

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.