Re: [PATCH 1/1] iommu/vt-d: Add Kconfig option to enable/disable scalable mode

[Lu Baolu]

Hi Christoph,

On 11/7/19 5:34 PM, Christoph Hellwig wrote:
> On Wed, Nov 06, 2019 at 01:11:30PM +0800, Lu Baolu wrote:
> > This adds a Kconfig option INTEL_IOMMU_SCALABLE_MODE_ON to make
> > it easier for distributions to enable or disable the Intel IOMMU
> > scalable mode during kernel build.
>
>
>
> > Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/iommu/Kconfig       | 10 ++++++++++
> >   drivers/iommu/intel-iommu.c |  5 +++++
> >   2 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index e3842eabcfdd..32f30e27791c 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -242,6 +242,16 @@ config INTEL_IOMMU_FLOPPY_WA
> >   	  workaround will setup a 1:1 mapping for the first
> >   	  16MiB to make floppy (an ISA device) work.
> >   
> > +config INTEL_IOMMU_SCALABLE_MODE_ON
>
> That should have a DEFAULT in the name as it is a default.

Agreed.

> > +	def_bool n
>
> n is the default default, so this can just be bool.

Agreed.

> > +#ifdef CONFIG_INTEL_IOMMU_SCALABLE_MODE_ON
> > +int intel_iommu_sm = 1;
> > +#else
> >   int intel_iommu_sm;
> > +#endif /* CONFIG_INTEL_IOMMU_SCALABLE_MODE_ON */
>
> This can use IS_ENABLED().

We already have below in the code

#ifdef CONFIG_INTEL_IOMMU_DEFAULT_ON
int dmar_disabled = 0;
#else
int dmar_disabled = 1;
#endif /*CONFIG_INTEL_IOMMU_DEFAULT_ON*/

I prefer to make the code style consistent if these two doesn't make
much difference.

> But then again the distro can just add iommu=sm_on to CONFIG_CMDLINE
> and have the same effect, so I don't really get the point of the whole
> patch.
>
> Or why we can't just enable it by default for that matter.

Currently Intel IOMMU scalable mode is by default off since some related
features are still under active development. We will make it default on
later once all features are ready.

No matter scalable mode default on or off, we provide two ways to switch
it between on and off: kernel command and Kconfig option. The former is
mainly used for debugging and testing purpose and the later is liked by
the distributions.

Best regards,
baolu