Received-SPF: pass (google.com: domain of linux-kernel+bounces-186970-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 23 May 2024 12:23:23 +0800
From: Chao Gao <chao.gao@intel.com>
To: "Huang, Kai" <kai.huang@intel.com>
CC: Sean Christopherson <seanjc@google.com>, Paolo Bonzini
	<pbonzini@redhat.com>, <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 3/6] KVM: Add a module param to allow enabling
 virtualization when KVM is loaded
Message-ID: <Zk7Eu0WS0j6/mmZT@chao-email>
References: <20240522022827.1690416-1-seanjc@google.com>
 <20240522022827.1690416-4-seanjc@google.com>
 <8b344a16-b28a-4f75-9c1a-a4edf2aa4a11@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <8b344a16-b28a-4f75-9c1a-a4edf2aa4a11@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?GeLVfI+wFTYPHqCj5wLyxATf+IZ4bMIP1L0P1n5/vQSAa7o87RPU0FaYWUkn?=
 =?us-ascii?Q?bdU2KrcGU7aeS7O/KLyFxmTn7/bslk9LnAlnasw/zYu6oMvoz40RzT0ygBtU?=
 =?us-ascii?Q?v8tRp/1oo3U84aJL11WIxlCjqfdFgR6rRoN9DhK2jf2co5awp1+T2maPi69Y?=
 =?us-ascii?Q?qSv4HZ4E9ANjR88xD3ONDrHxhweTpyxmof9Xl6VIa/5dIb99ZH5FsBUaMtH6?=
 =?us-ascii?Q?VoaCK3xNQp7sxFtSGcnIuoZl38ZVD4vwTt6lH1YJDPYEG+oA8FzxOHl4aFAp?=
 =?us-ascii?Q?+mIOmF4u0QrEI0uJamSkDH4XNQML9OkaJZn+Dpi6WS/Fdcxb6OMWERyrVW5A?=
 =?us-ascii?Q?Fea88sjs3ptEDK6jWT6bV1XCNHTReW4O0eELUysqkyVBJE9guTTiCxP3Voit?=
 =?us-ascii?Q?NAA02Ezw36GAblEOlO2xDc9/NCF3Bi88GMx2SwdyVAgfAKgvAHqair0a51PV?=
 =?us-ascii?Q?3Woo/in+551g4xCBZZxAIjNYZWZrWhe40PYCsCB6xm3SB9Rm++PZ1b4RQw5B?=
 =?us-ascii?Q?7L1mO9hcIoQUiiMbPYDfn7DbS0VlVg33c7BE4zHhp8Iw+YtXs0POUqDJyhcP?=
 =?us-ascii?Q?i2Z5MoKHZhuls4/NnCA8ElGIXiOu1akBxwLADLrnvaj8hSlycvin9WblIutA?=
 =?us-ascii?Q?slUBPR0VHTy+2k2ybQ5xW4u6C/KbCHSvyES+kJvt+23AYC3gmlbqmBdInnam?=
 =?us-ascii?Q?qxoKdEBMvyXIpSq7RPTjtgF4sY3FLrD9el9WxbKonUPUGjXDQ6gNmAd3mdw5?=
 =?us-ascii?Q?4qwikOp2pnjM7pUqqzkJ/6JwQNCpONos3whkVBlfN5biB8svY5LiikMbbyfR?=
 =?us-ascii?Q?hkAipFdKY6/lAURYUaYJf9vEW62YXWQn4CxOegXHRGMtA8EC2vxwzzdIYK12?=
 =?us-ascii?Q?Z7FpQ2Sb5yBIIl6onJWOy3FeMYAThYwQurOPJOQBxb2BlifQnrqHtrs9n628?=
 =?us-ascii?Q?57bU/2LtCgU5QIWKWY4VrB2ofGPIqSJpjk54/4ssqo5FvddLRuS1VhR+WUvx?=
 =?us-ascii?Q?UZ5odXxaDVlovHk4yw5jBhyf6Orykjpnj3Y6WflQhCO4RLQZ2w2qzxUTTQJb?=
 =?us-ascii?Q?Gx2QcVJF3vlsOxVt+nsVx+WlIlznJ2bhVyqtefPHIRwLW3Wc+yB2Itr8TnX4?=
 =?us-ascii?Q?1DDua8AqwTXIw7WsvnI6RU+TvMP1UB6LRqqfUaTKzyYpllNyI82TK34ED7KZ?=
 =?us-ascii?Q?vWDNBiBp7GaqB6q/Ba9/w/vtCSmYbYtubb6Atrn/LOM87CojfmyflGqwgPv9?=
 =?us-ascii?Q?qJKY8Sx63WTntQwP7jEDb41Z1/VP4MLpfmn8iSYt+COWWbptXMO7f4i3cFCq?=
 =?us-ascii?Q?hgyzVTMHiGP+PRsfFRT2ZLpVdGwyAArvoYzLBs34oQYQ6oE0Pa5MzsV9xwaa?=
 =?us-ascii?Q?ogcfAZ/OwvGPzc3ZEoVgU8GHR7Up/IFqQDKAmGwnIXBrnw0fJR42SLeOXNSW?=
 =?us-ascii?Q?OG1ppbxgKGOTD8elekEwECjTTjoqZK7qRFxeDa4lMbp7A9zgOXVKqFF8UWBp?=
 =?us-ascii?Q?fz3NP3mrkN/bblVeO/OjsQObM4d+wdcu6c6fwN0OJJBpI31cr0maQFit7Bdv?=
 =?us-ascii?Q?LMhbuHeoxLKUZlHIgzqG871rebq+8fl7Yhtl2M5a?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ef4a9d4a-d0a8-4f51-93a1-08dc7ae01a2e
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2024 04:23:31.1438
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: UKrg1ovKBE0p1P98W1q5Wp87foHcSA/+Qv9YUXNB7GqA8ucG296dfQnlfYsRpQHMcQfSuyNNHiVd7BwTG66ntw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6044
X-OriginatorOrg: intel.com

On Thu, May 23, 2024 at 10:27:53AM +1200, Huang, Kai wrote:
>
>
>On 22/05/2024 2:28 pm, Sean Christopherson wrote:
>> Add an off-by-default module param, enable_virt_at_load, to let userspace
>> force virtualization to be enabled in hardware when KVM is initialized,
>> i.e. just before /dev/kvm is exposed to userspace.  Enabling virtualization
>> during KVM initialization allows userspace to avoid the additional latency
>> when creating/destroying the first/last VM.  Now that KVM uses the cpuhp
>> framework to do per-CPU enabling, the latency could be non-trivial as the
>> cpuhup bringup/teardown is serialized across CPUs, e.g. the latency could
>> be problematic for use case that need to spin up VMs quickly.
>
>How about we defer this until there's a real complain that this isn't
>acceptable?  To me it doesn't sound "latency of creating the first VM"
>matters a lot in the real CSP deployments.

I suspect kselftest and kvm-unit-tests will be impacted a lot because
hundreds of tests are run serially. And it looks clumsy to reload KVM
module to set enable_virt_at_load to make tests run faster. I think the
test slowdown is a more realistic problem than running an off-tree
hypervisor, so I vote to make enabling virtualization at load time the
default behavior and if we really want to support an off-tree hypervisor,
we can add a new module param to opt in enabling virtualization at runtime.

>
>The concern of adding a new module param is once we add it, we need to
>maintain it even it is no longer needed in the future for backward
>compatibility.  Especially this param is in kvm.ko, and for all ARCHs.
>
>E.g., I think _IF_ the core cpuhp code is enhanced to call those callbacks in
>parallel in cpuhp_setup_state(), then this issue could be mitigated to an
>unnoticeable level.
>
>Or we just still do:
>
>	cpus_read_lock();
>	on_each_cpu(hardware_enable_nolock, ...);
>	cpuhp_setup_state_nocalls_cpuslocked(...);
>	cpus_read_unlock();
>
>I think the main benefit of series is to put all virtualization enabling
>related things into one single function.  Whether using cpuhp_setup_state()
>or using on_each_cpu() shouldn't be the main point.
>