linux-kernel - Re: [PATCH] vfio: Fix build break when SPAPR_TCE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 4 Nov 2016 11:01:10 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Michael Ellerman <mpe@...erman.id.au>, alex.williamson@...hat.com
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linuxppc-dev@...abs.org, aik@...abs.ru, gwshan@...ux.vnet.ibm.com
Subject: Re: [PATCH] vfio: Fix build break when SPAPR_TCE_IOMMU=n



On 04/11/2016 06:48, Michael Ellerman wrote:
> Currently the kconfig logic for VFIO_IOMMU_SPAPR_TCE and VFIO_SPAPR_EEH
> is broken when SPAPR_TCE_IOMMU=n. Leading to:
> 
>     warning: (VFIO) selects VFIO_IOMMU_SPAPR_TCE which has unmet direct dependencies (VFIO && SPAPR_TCE_IOMMU)
>     warning: (VFIO) selects VFIO_IOMMU_SPAPR_TCE which has unmet direct dependencies (VFIO && SPAPR_TCE_IOMMU)
>     drivers/vfio/vfio_iommu_spapr_tce.c:113:8: error: implicit declaration of function 'mm_iommu_find'
> 
> This stems from the fact that VFIO selects VFIO_IOMMU_SPAPR_TCE, and
> although it has an if clause, the condition is not correct.
> 
> We could fix it by doing select VFIO_IOMMU_SPAPR_TCE if SPAPR_TCE_IOMMU,
> but the cleaner fix is to drop the selects and tie VFIO_IOMMU_SPAPR_TCE
> to the value of VFIO, and express the dependencies in only once place.
> 
> Do the same for VFIO_SPAPR_EEH.
> 
> The end result is that the values of VFIO_IOMMU_SPAPR_TCE and
> VFIO_SPAPR_EEH follow the value of VFIO, except when SPAPR_TCE_IOMMU=n
> and/or EEH=n. Which is exactly what we want to happen.
> 
> Signed-off-by: Michael Ellerman <mpe@...erman.id.au>
> ---
>  drivers/vfio/Kconfig | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index da6e2ce77495..6b51a4ebed8a 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -6,12 +6,12 @@ config VFIO_IOMMU_TYPE1
>  config VFIO_IOMMU_SPAPR_TCE
>  	tristate
>  	depends on VFIO && SPAPR_TCE_IOMMU
> -	default n
> +	default VFIO

No need to depend on VFIO since you already have it in "default".  (I
assume you cannot use "default y" because "depends on" doesn't downgrade
"y" to "m" when VFIO is a module.

A shorthand is

	def_tristate VFIO && SPAPR_TCE_IOMMU

>  config VFIO_SPAPR_EEH
>  	tristate
>  	depends on EEH && VFIO_IOMMU_SPAPR_TCE
> -	default n
> +	default VFIO

Likewise:

	def_tristate VFIO_IOMMU_SPAPR_TCE && EEH

Thanks,

Paolo

>  config VFIO_VIRQFD
>  	tristate
> @@ -22,8 +22,6 @@ menuconfig VFIO
>  	tristate "VFIO Non-Privileged userspace driver framework"
>  	depends on IOMMU_API
>  	select VFIO_IOMMU_TYPE1 if (X86 || S390 || ARM_SMMU || ARM_SMMU_V3)
> -	select VFIO_IOMMU_SPAPR_TCE if (PPC_POWERNV || PPC_PSERIES)
> -	select VFIO_SPAPR_EEH if (PPC_POWERNV || PPC_PSERIES)
>  	select ANON_INODES
>  	help
>  	  VFIO provides a framework for secure userspace device drivers.
>