Re: [PATCH 1/1] arch/x86: Add CONFIG_IOMMU_DEFAULT_PT for iommu=pt

[Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>]

On 29/09/16 20:01, Alex Williamson wrote:
> On Thu, 29 Sep 2016 11:41:53 -0700
> Glenn Serre <gserre-yk0uUPyXz1dBDgjK7y7TUQ@public.gmane.org> wrote:
> 
>> Good morning,
>>
>> On 09/29/2016 11:31 AM, Alex Williamson wrote:
>>> On Thu, 29 Sep 2016 09:58:55 -0700
>>> Glenn Serre <gserre-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org> wrote:
>>>  
>>>> Add CONFIG_IOMMU_DEFAULT_PT for iommu=pt
>>>>
>>>> Allow default to iommu=pt without adding to command line.
>>>> Add iommu=nopt to turn off default.
>>>>
>>>> Signed-off-by: Glenn Serre <gserre-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>
>>>> ---
>>>>   Documentation/kernel-parameters.txt | 3 ++-
>>>>   arch/x86/Kconfig                    | 7 +++++++
>>>>   arch/x86/kernel/pci-dma.c           | 6 ++++++
>>>>   3 files changed, 15 insertions(+), 1 deletion(-)
>>>>
>>>> --- a/Documentation/kernel-parameters.txt
>>>> +++ b/Documentation/kernel-parameters.txt
>>>> @@ -1607,7 +1607,8 @@
>>>>   		nomerge
>>>>   		forcesac
>>>>   		soft
>>>> -		pt		[x86, IA-64]
>>>> +		pt		[x86]
>>>> +		nopt		[x86]
>>>>   		nobypass	[PPC/POWERNV]
>>>>   			Disable IOMMU bypass, using IOMMU for PCI devices.
>>>>
>>>> --- a/arch/x86/Kconfig
>>>> +++ b/arch/x86/Kconfig
>>>> @@ -845,6 +845,15 @@
>>>>   	def_bool y
>>>>   	depends on CALGARY_IOMMU || GART_IOMMU || SWIOTLB || AMD_IOMMU
>>>>
>>>> +config IOMMU_DEFAULT_PT
>>>> +	int "Default iommu pt value (0-1)"
>>>> +	range 0 1
>>>> +	default "0"
>>>> +	prompt "Set to 1 to default to iommu=pt"  
>>>
>>> An integer that can be either zero or one... IOW a bool.  There's
>>> support for bools in Kconfig.  
>>
>> We had discussion about that during in-house code review.  My initial 
>> take was bool like INTEL_IOMMU_DEFAULT_ON, but reviewers thought the 
>> current "right" way was range 0-1 like MTRR_SANITIZER_ENABLE_DEFAULT to 
>> avoid an ifdef, else, endif:
>>
>> #ifdef CONFIG_IOMMU_DEFAULT_PT
>> int iommu_pass_through __read_mostly = 1;
>> #else
>> int iommu_pass_through __read_mostly;
>> #endif /* CONFIG_IOMMU_DEFAULT_PT */
>>
>> I'll go with whatever y'all prefer.
> 
> Ok, I didn't really think it all the way through I guess.  Still, my
> opinion is that it's strange to have a yes/no Kconfig option take a 0/1
> parameter and I'd probably rather have the slightly uglier code for the
> benefit of a more intuitive Kconfig.  Maybe others disagree.  Thanks,

Actually, there's no need for ugly code *or* an unintuitive Kconfig
type. See e.g. __pat_enabled in arch/x86/mm/pat.c ;)

Robin.

> 
> Alex
> 
>>>  
>>>> +	help
>>>> +	  Setting this option to 1 will set iommu passthrough (iommu=pt)
>>>> +	  by default.  Use iommu=nopt to override.
>>>> +
>>>>   config MAXSMP
>>>>   	bool "Enable Maximum number of SMP Processors and NUMA Nodes"
>>>>   	depends on X86_64 && SMP && DEBUG_KERNEL
>>>> --- a/arch/x86/kernel/pci-dma.c
>>>> +++ b/arch/x86/kernel/pci-dma.c
>>>> @@ -43,7 +43,7 @@
>>>>    * useful if a user wants to use an IOMMU only for KVM device
>>>> assignment to
>>>>    * guests and not for driver dma translation.
>>>>    */
>>>> -int iommu_pass_through __read_mostly;
>>>> +int iommu_pass_through __read_mostly = CONFIG_IOMMU_DEFAULT_PT;
>>>>
>>>>   extern struct iommu_table_entry __iommu_table[], __iommu_table_end[];
>>>>
>>>> @@ -196,6 +196,8 @@
>>>>   #endif
>>>>   		if (!strncmp(p, "pt", 2))
>>>>   			iommu_pass_through = 1;
>>>> +		if (!strncmp(p, "nopt", 4))
>>>> +			iommu_pass_through = 0;
>>>>
>>>>   		gart_parse_options(p);
>>>>
>>>> _______________________________________________
>>>> iommu mailing list
>>>> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.linuxfoundation.org_mailman_listinfo_iommu&d=DQICAg&c=IL_XqQWOjubgfqINi2jTzg&r=3biwyMBB1JD9rm0R5wqz7zcDVkNif9HJ5Pflrr18dkw&m=6weXbuobXVwdVUVGUJUzgHf_32Tr95Ey128v4XLFPjs&s=XTyH-JkxzIj-W47wN4KHobMOdUQjXNnrngKhCndmG9I&e=  
>>>  
> 
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>