| Message ID | 20190530034831.4184-2-thunder.leizhen@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iommu: enhance IOMMU default DMA mode build options | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (8150a153c013aa2dd1ffae43370b89ac1347a7fb) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 61 lines checked |
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; >
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; >> > > > > . >
>>> -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?
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 --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;
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(-)