Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 2 May 2023 10:12:34 -0700
In-Reply-To: <20230502112502.14859-1-yanjiewtw@gmail.com>
Mime-Version: 1.0
References: <20230502112502.14859-1-yanjiewtw@gmail.com>
Message-ID: <ZFFEgptZg1P367F7@google.com>
Subject: Re: [PATCH] docs: clarify KVM related kernel parameters' descriptions
From:   Sean Christopherson <seanjc@google.com>
To:     Yan-Jie Wang <yanjiewtw@gmail.com>
Cc:     linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        Jonathan Corbet <corbet@lwn.net>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        Andre Przywara <andre.przywara@arm.com>,
        Avi Kivity <avi@redhat.com>,
        Ching-Chun Huang <jserv@ccns.ncku.edu.tw>, trivial@kernel.org,
        kvm@vger.kernel.org
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Tue, May 02, 2023, Yan-Jie Wang wrote:
> The descriptions of certain KVM related kernel parameters can be
> ambiguous and confusing. They state 'Disable ...,' which implies that
> setting them to 1 would disable the associated features or options,
> when in fact the opposite is true.
> 
> This commit addresses this issue by revising the descriptions of these
> parameters to make their intended behavior clear.

Less wrong perhaps, but IMO the actual behavior is still not captured, and from
a certain perspective the existing "Disable" verbiage better reflects how/when
most users would actually want to explicitly set a param.

Rather than commit one way or the other, what about reworking the descriptions
using this as a template?  E.g. state that the param controls something, state
the default and use that to communicate that 1==enabled, and then, when appropriate,
clarify that KVM disables (or in some cases ignores) params if hardware doesn't
support the related feature(s).

	kvm-intel.ept=	[KVM,Intel] Control KVM's use of Extended Page Tables,
			a.k.a. Two-Dimensional Page Tables.  Default is 1
			(enabled).  Disabled by KVM if hardware lacks support
			for EPT.

> 
> Signed-off-by: Yan-Jie Wang <yanjiewtw@gmail.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9e5bab29685f..cc5abb3d54b9 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2552,10 +2552,10 @@
>  			on the ratio, such that a page is zapped after 1 hour on average.
>  
>  	kvm-amd.nested=	[KVM,AMD] Allow nested virtualization in KVM/SVM.

Eh, I don't see any reason to have this one say "allow" instead of "enable/disable".

> -			Default is 1 (enabled)
> +			Default is 1 (allow)
>  
> -	kvm-amd.npt=	[KVM,AMD] Disable nested paging (virtualized MMU)
> -			for all guests.
> +	kvm-amd.npt=	[KVM,AMD] Enable nested paging (virtualized MMU)
> +			for all guests on capable AMD chips.
>  			Default is 1 (enabled) if in 64-bit or 32-bit PAE mode.
>  
>  	kvm-arm.mode=
> @@ -2602,12 +2602,12 @@
>  			Format: <integer>
>  			Default: 5
>  
> -	kvm-intel.ept=	[KVM,Intel] Disable extended page tables
> +	kvm-intel.ept=	[KVM,Intel] Enable extended page tables
>  			(virtualized MMU) support on capable Intel chips.
>  			Default is 1 (enabled)
>  
>  	kvm-intel.emulate_invalid_guest_state=
> -			[KVM,Intel] Disable emulation of invalid guest state.
> +			[KVM,Intel] Enable emulation of invalid guest state.
>  			Ignored if kvm-intel.enable_unrestricted_guest=1, as
>  			guest state is never invalid for unrestricted guests.
>  			This param doesn't apply to nested guests (L2), as KVM
> @@ -2615,7 +2615,7 @@
>  			Default is 1 (enabled)
>  
>  	kvm-intel.flexpriority=
> -			[KVM,Intel] Disable FlexPriority feature (TPR shadow).
> +			[KVM,Intel] Enable FlexPriority feature (TPR shadow).
>  			Default is 1 (enabled)
>  
>  	kvm-intel.nested=
> @@ -2623,7 +2623,7 @@
>  			Default is 0 (disabled)

Heh, kvm-intel.nested has been enabled by default for quite some time.  Can you
fix that up too?

>  
>  	kvm-intel.unrestricted_guest=
> -			[KVM,Intel] Disable unrestricted guest feature
> +			[KVM,Intel] Enable unrestricted guest feature
>  			(virtualized real and unpaged mode) on capable
>  			Intel chips. Default is 1 (enabled)
>  
> @@ -2639,7 +2639,7 @@
>  
>  			Default is cond (do L1 cache flush in specific instances)
>  
> -	kvm-intel.vpid=	[KVM,Intel] Disable Virtual Processor Identification
> +	kvm-intel.vpid=	[KVM,Intel] Enable Virtual Processor Identification
>  			feature (tagged TLBs) on capable Intel chips.
>  			Default is 1 (enabled)
>  
> 
> base-commit: 865fdb08197e657c59e74a35fa32362b12397f58
> -- 
> 2.34.1
>