diff mbox series

[v8,1/7] iommu: enhance IOMMU default DMA mode build options

Message ID 20190530034831.4184-2-thunder.leizhen@huawei.com
State Superseded
Headers show
Series iommu: enhance IOMMU default DMA mode build options | expand

Commit Message

Leizhen (ThunderTown) May 30, 2019, 3:48 a.m. UTC
First, add build option IOMMU_DEFAULT_{LAZY|STRICT}, so that we have the
opportunity to set {lazy|strict} mode as default at build time. Then put
the three config options in an choice, make people can only choose one of
the three at a time.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

---
 drivers/iommu/Kconfig | 42 +++++++++++++++++++++++++++++++++++-------
 drivers/iommu/iommu.c |  3 ++-
 2 files changed, 37 insertions(+), 8 deletions(-)

-- 
1.8.3

Comments

John Garry May 30, 2019, 12:20 p.m. UTC | #1
On 30/05/2019 04:48, Zhen Lei wrote:
> First, add build option IOMMU_DEFAULT_{LAZY|STRICT}, so that we have the

> opportunity to set {lazy|strict} mode as default at build time. Then put

> the three config options in an choice, make people can only choose one of

> the three at a time.

>


Since this was not picked up, but modulo (somtimes same) comments below:

Reviewed-by: John Garry <john.garry@huawei.com>


> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

> ---

>  drivers/iommu/Kconfig | 42 +++++++++++++++++++++++++++++++++++-------

>  drivers/iommu/iommu.c |  3 ++-

>  2 files changed, 37 insertions(+), 8 deletions(-)

>

> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig

> index 83664db5221df02..d6a1a45f80ffbf5 100644

> --- a/drivers/iommu/Kconfig

> +++ b/drivers/iommu/Kconfig

> @@ -75,17 +75,45 @@ config IOMMU_DEBUGFS

>  	  debug/iommu directory, and then populate a subdirectory with

>  	  entries as required.

>

> -config IOMMU_DEFAULT_PASSTHROUGH

> -	bool "IOMMU passthrough by default"

> +choice

> +	prompt "IOMMU default DMA mode"

>  	depends on IOMMU_API

> -        help

> -	  Enable passthrough by default, removing the need to pass in

> -	  iommu.passthrough=on or iommu=pt through command line. If this

> -	  is enabled, you can still disable with iommu.passthrough=off

> -	  or iommu=nopt depending on the architecture.

> +	default IOMMU_DEFAULT_STRICT

> +	help

> +	  This option allows IOMMU DMA mode to be chose at build time, to


As before:
/s/chose/chosen/, /s/allows IOMMU/allows an IOMMU/

> +	  override the default DMA mode of each ARCHs, removing the need to


Again, as before:
ARCHs should be singular

> +	  pass in kernel parameters through command line. You can still use

> +	  ARCHs specific boot options to override this option again.

> +

> +config IOMMU_DEFAULT_PASSTHROUGH

> +	bool "passthrough"

> +	help

> +	  In this mode, the DMA access through IOMMU without any addresses

> +	  translation. That means, the wrong or illegal DMA access can not

> +	  be caught, no error information will be reported.

>

>  	  If unsure, say N here.

>

> +config IOMMU_DEFAULT_LAZY

> +	bool "lazy"

> +	help

> +	  Support lazy mode, where for every IOMMU DMA unmap operation, the

> +	  flush operation of IOTLB and the free operation of IOVA are deferred.

> +	  They are only guaranteed to be done before the related IOVA will be

> +	  reused.


why no advisory on how to set if unsure?

> +

> +config IOMMU_DEFAULT_STRICT

> +	bool "strict"

> +	help

> +	  For every IOMMU DMA unmap operation, the flush operation of IOTLB and

> +	  the free operation of IOVA are guaranteed to be done in the unmap

> +	  function.

> +

> +	  This mode is safer than the two above, but it maybe slower in some

> +	  high performace scenarios.


and here?

> +

> +endchoice

> +

>  config OF_IOMMU

>         def_bool y

>         depends on OF && IOMMU_API

> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c

> index 67ee6623f9b2a4d..56bce221285b15f 100644

> --- a/drivers/iommu/iommu.c

> +++ b/drivers/iommu/iommu.c

> @@ -43,7 +43,8 @@

>  #else

>  static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;

>  #endif

> -static bool iommu_dma_strict __read_mostly = true;

> +static bool iommu_dma_strict __read_mostly =

> +			IS_ENABLED(CONFIG_IOMMU_DEFAULT_STRICT);

>

>  struct iommu_group {

>  	struct kobject kobj;

>
Leizhen (ThunderTown) May 31, 2019, 10:03 a.m. UTC | #2
On 2019/5/30 20:20, John Garry wrote:
> On 30/05/2019 04:48, Zhen Lei wrote:

>> First, add build option IOMMU_DEFAULT_{LAZY|STRICT}, so that we have the

>> opportunity to set {lazy|strict} mode as default at build time. Then put

>> the three config options in an choice, make people can only choose one of

>> the three at a time.

>>

> 

> Since this was not picked up, but modulo (somtimes same) comments below:

> 

> Reviewed-by: John Garry <john.garry@huawei.com>

> 

>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

>> ---

>>  drivers/iommu/Kconfig | 42 +++++++++++++++++++++++++++++++++++-------

>>  drivers/iommu/iommu.c |  3 ++-

>>  2 files changed, 37 insertions(+), 8 deletions(-)

>>

>> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig

>> index 83664db5221df02..d6a1a45f80ffbf5 100644

>> --- a/drivers/iommu/Kconfig

>> +++ b/drivers/iommu/Kconfig

>> @@ -75,17 +75,45 @@ config IOMMU_DEBUGFS

>>        debug/iommu directory, and then populate a subdirectory with

>>        entries as required.

>>

>> -config IOMMU_DEFAULT_PASSTHROUGH

>> -    bool "IOMMU passthrough by default"

>> +choice

>> +    prompt "IOMMU default DMA mode"

>>      depends on IOMMU_API

>> -        help

>> -      Enable passthrough by default, removing the need to pass in

>> -      iommu.passthrough=on or iommu=pt through command line. If this

>> -      is enabled, you can still disable with iommu.passthrough=off

>> -      or iommu=nopt depending on the architecture.

>> +    default IOMMU_DEFAULT_STRICT

>> +    help

>> +      This option allows IOMMU DMA mode to be chose at build time, to

> 

> As before:

> /s/chose/chosen/, /s/allows IOMMU/allows an IOMMU/

I'm sorry that the previous version was not modified.

> 

>> +      override the default DMA mode of each ARCHs, removing the need to

> 

> Again, as before:

> ARCHs should be singular

OK

> 

>> +      pass in kernel parameters through command line. You can still use

>> +      ARCHs specific boot options to override this option again.

>> +

>> +config IOMMU_DEFAULT_PASSTHROUGH

>> +    bool "passthrough"

>> +    help

>> +      In this mode, the DMA access through IOMMU without any addresses

>> +      translation. That means, the wrong or illegal DMA access can not

>> +      be caught, no error information will be reported.

>>

>>        If unsure, say N here.

>>

>> +config IOMMU_DEFAULT_LAZY

>> +    bool "lazy"

>> +    help

>> +      Support lazy mode, where for every IOMMU DMA unmap operation, the

>> +      flush operation of IOTLB and the free operation of IOVA are deferred.

>> +      They are only guaranteed to be done before the related IOVA will be

>> +      reused.

> 

> why no advisory on how to set if unsure?

Because the LAZY and STRICT have their own advantages and disadvantages.

Should I say: If unsure, keep the default。

> 

>> +

>> +config IOMMU_DEFAULT_STRICT

>> +    bool "strict"

>> +    help

>> +      For every IOMMU DMA unmap operation, the flush operation of IOTLB and

>> +      the free operation of IOVA are guaranteed to be done in the unmap

>> +      function.

>> +

>> +      This mode is safer than the two above, but it maybe slower in some

>> +      high performace scenarios.

> 

> and here?

> 

>> +

>> +endchoice

>> +

>>  config OF_IOMMU

>>         def_bool y

>>         depends on OF && IOMMU_API

>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c

>> index 67ee6623f9b2a4d..56bce221285b15f 100644

>> --- a/drivers/iommu/iommu.c

>> +++ b/drivers/iommu/iommu.c

>> @@ -43,7 +43,8 @@

>>  #else

>>  static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;

>>  #endif

>> -static bool iommu_dma_strict __read_mostly = true;

>> +static bool iommu_dma_strict __read_mostly =

>> +            IS_ENABLED(CONFIG_IOMMU_DEFAULT_STRICT);

>>

>>  struct iommu_group {

>>      struct kobject kobj;

>>

> 

> 

> 

> .

>
John Garry May 31, 2019, 10:42 a.m. UTC | #3
>>> -config IOMMU_DEFAULT_PASSTHROUGH

>>> -    bool "IOMMU passthrough by default"

>>> +choice

>>> +    prompt "IOMMU default DMA mode"

>>>      depends on IOMMU_API

>>> -        help

>>> -      Enable passthrough by default, removing the need to pass in

>>> -      iommu.passthrough=on or iommu=pt through command line. If this

>>> -      is enabled, you can still disable with iommu.passthrough=off

>>> -      or iommu=nopt depending on the architecture.

>>> +    default IOMMU_DEFAULT_STRICT

>>> +    help

>>> +      This option allows IOMMU DMA mode to be chose at build time, to

>>

>> As before:

>> /s/chose/chosen/, /s/allows IOMMU/allows an IOMMU/

> I'm sorry that the previous version was not modified.

>

>>

>>> +      override the default DMA mode of each ARCHs, removing the need to

>>

>> Again, as before:

>> ARCHs should be singular

> OK

>

>>

>>> +      pass in kernel parameters through command line. You can still use

>>> +      ARCHs specific boot options to override this option again.


*

>>> +

>>> +config IOMMU_DEFAULT_PASSTHROUGH

>>> +    bool "passthrough"

>>> +    help

>>> +      In this mode, the DMA access through IOMMU without any addresses

>>> +      translation. That means, the wrong or illegal DMA access can not

>>> +      be caught, no error information will be reported.

>>>

>>>        If unsure, say N here.

>>>

>>> +config IOMMU_DEFAULT_LAZY

>>> +    bool "lazy"

>>> +    help

>>> +      Support lazy mode, where for every IOMMU DMA unmap operation, the

>>> +      flush operation of IOTLB and the free operation of IOVA are deferred.

>>> +      They are only guaranteed to be done before the related IOVA will be

>>> +      reused.

>>

>> why no advisory on how to set if unsure?

> Because the LAZY and STRICT have their own advantages and disadvantages.

>

> Should I say: If unsure, keep the default。


Maybe. So you could put this in the help for the choice, * above, and 
remove the advisory on IOMMU_DEFAULT_PASSTHROUGH.

However the maintainer may have a different view.

Thanks,
John

>

>>

>>> +

>>> +config IOMMU_DEFAULT_STRICT

>>> +    bool "strict"

>>> +    help

>>> +      For every IOMMU DMA unmap operation, the flush operation of IOTLB and

>>> +      the free operation of IOVA are guaranteed to be done in the unmap

>>> +      function.

>>> +

>>> +      This mode is safer than the two above, but it maybe slower in some

>>> +      high performace scenarios.

>>

>> and here?
Leizhen (ThunderTown) June 13, 2019, 8:30 a.m. UTC | #4
On 2019/5/31 18:42, John Garry wrote:
> 

>>>> -config IOMMU_DEFAULT_PASSTHROUGH

>>>> -    bool "IOMMU passthrough by default"

>>>> +choice

>>>> +    prompt "IOMMU default DMA mode"

>>>>      depends on IOMMU_API

>>>> -        help

>>>> -      Enable passthrough by default, removing the need to pass in

>>>> -      iommu.passthrough=on or iommu=pt through command line. If this

>>>> -      is enabled, you can still disable with iommu.passthrough=off

>>>> -      or iommu=nopt depending on the architecture.

>>>> +    default IOMMU_DEFAULT_STRICT

>>>> +    help

>>>> +      This option allows IOMMU DMA mode to be chose at build time, to

>>>

>>> As before:

>>> /s/chose/chosen/, /s/allows IOMMU/allows an IOMMU/

>> I'm sorry that the previous version was not modified.

>>

>>>

>>>> +      override the default DMA mode of each ARCHs, removing the need to

>>>

>>> Again, as before:

>>> ARCHs should be singular

>> OK

>>

>>>

>>>> +      pass in kernel parameters through command line. You can still use

>>>> +      ARCHs specific boot options to override this option again.

> 

> *

> 

>>>> +

>>>> +config IOMMU_DEFAULT_PASSTHROUGH

>>>> +    bool "passthrough"

>>>> +    help

>>>> +      In this mode, the DMA access through IOMMU without any addresses

>>>> +      translation. That means, the wrong or illegal DMA access can not

>>>> +      be caught, no error information will be reported.

>>>>

>>>>        If unsure, say N here.

>>>>

>>>> +config IOMMU_DEFAULT_LAZY

>>>> +    bool "lazy"

>>>> +    help

>>>> +      Support lazy mode, where for every IOMMU DMA unmap operation, the

>>>> +      flush operation of IOTLB and the free operation of IOVA are deferred.

>>>> +      They are only guaranteed to be done before the related IOVA will be

>>>> +      reused.

>>>

>>> why no advisory on how to set if unsure?

>> Because the LAZY and STRICT have their own advantages and disadvantages.

>>

>> Should I say: If unsure, keep the default。

> 

> Maybe. So you could put this in the help for the choice, * above, and remove the advisory on IOMMU_DEFAULT_PASSTHROUGH.


OK, I'll revise it according to this idea in v9.

> 

> However the maintainer may have a different view.

> 

> Thanks,

> John

> 

>>

>>>

>>>> +

>>>> +config IOMMU_DEFAULT_STRICT

>>>> +    bool "strict"

>>>> +    help

>>>> +      For every IOMMU DMA unmap operation, the flush operation of IOTLB and

>>>> +      the free operation of IOVA are guaranteed to be done in the unmap

>>>> +      function.

>>>> +

>>>> +      This mode is safer than the two above, but it maybe slower in some

>>>> +      high performace scenarios.

>>>

>>> and here?

> 

> 

> .

>
diff mbox series

Patch

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 83664db5221df02..d6a1a45f80ffbf5 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -75,17 +75,45 @@  config IOMMU_DEBUGFS
 	  debug/iommu directory, and then populate a subdirectory with
 	  entries as required.
 
-config IOMMU_DEFAULT_PASSTHROUGH
-	bool "IOMMU passthrough by default"
+choice
+	prompt "IOMMU default DMA mode"
 	depends on IOMMU_API
-        help
-	  Enable passthrough by default, removing the need to pass in
-	  iommu.passthrough=on or iommu=pt through command line. If this
-	  is enabled, you can still disable with iommu.passthrough=off
-	  or iommu=nopt depending on the architecture.
+	default IOMMU_DEFAULT_STRICT
+	help
+	  This option allows IOMMU DMA mode to be chose at build time, to
+	  override the default DMA mode of each ARCHs, removing the need to
+	  pass in kernel parameters through command line. You can still use
+	  ARCHs specific boot options to override this option again.
+
+config IOMMU_DEFAULT_PASSTHROUGH
+	bool "passthrough"
+	help
+	  In this mode, the DMA access through IOMMU without any addresses
+	  translation. That means, the wrong or illegal DMA access can not
+	  be caught, no error information will be reported.
 
 	  If unsure, say N here.
 
+config IOMMU_DEFAULT_LAZY
+	bool "lazy"
+	help
+	  Support lazy mode, where for every IOMMU DMA unmap operation, the
+	  flush operation of IOTLB and the free operation of IOVA are deferred.
+	  They are only guaranteed to be done before the related IOVA will be
+	  reused.
+
+config IOMMU_DEFAULT_STRICT
+	bool "strict"
+	help
+	  For every IOMMU DMA unmap operation, the flush operation of IOTLB and
+	  the free operation of IOVA are guaranteed to be done in the unmap
+	  function.
+
+	  This mode is safer than the two above, but it maybe slower in some
+	  high performace scenarios.
+
+endchoice
+
 config OF_IOMMU
        def_bool y
        depends on OF && IOMMU_API
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 67ee6623f9b2a4d..56bce221285b15f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -43,7 +43,8 @@ 
 #else
 static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
 #endif
-static bool iommu_dma_strict __read_mostly = true;
+static bool iommu_dma_strict __read_mostly =
+			IS_ENABLED(CONFIG_IOMMU_DEFAULT_STRICT);
 
 struct iommu_group {
 	struct kobject kobj;