Received-SPF: pass (google.com: domain of linux-kernel+bounces-159716-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Fri, 26 Apr 2024 16:32:37 +0800
From: Chao Gao <chao.gao@intel.com>
To: Sean Christopherson <seanjc@google.com>
CC: Paolo Bonzini <pbonzini@redhat.com>, <kvm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/4] KVM: Register cpuhp and syscore callbacks when
 enabling hardware
Message-ID: <ZitmpS0lt+wVjRwl@chao-email>
References: <20240425233951.3344485-1-seanjc@google.com>
 <20240425233951.3344485-4-seanjc@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240425233951.3344485-4-seanjc@google.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?k1ynHgLQKTth9aqt+Z2GLpeG0Uh9NlfcBuNmDYf126kQtZ07B6FaufaEDEWl?=
 =?us-ascii?Q?izziKFBZhaUkN6GmVtCCOD6L4QvalRe9gwjOQuCWNXwXDSqzc2m+JvSuX7JE?=
 =?us-ascii?Q?q6dKItRVN4oQFiDZ7+TB7nKlSn+AbM+MCJl7VgQGplSPgsICMAW7W7ga8xfh?=
 =?us-ascii?Q?AEXm2G1x0R+pXrOKqFOiapwVDA0oedZcGdOZe1rwW/hNdieCkkoFX4za6GD2?=
 =?us-ascii?Q?BOnmfZtkQjNCAzpAEPaLp6RCsZ+E4bwdjoWSnL3SvzaE4Vp6LEbdcGTzNL6A?=
 =?us-ascii?Q?mOxep7EYFVktsalLKsw/wkhkyW8eMbDYpvru+xDjz9UXiMrJTQcHyMeV0c3J?=
 =?us-ascii?Q?PABklIFVNM+NzmMul2c1h/aUA86KXuYZJq0L0t03mj71u2PKHnnZIBUPISEo?=
 =?us-ascii?Q?epiK8VvbaNRalJOyslhg1FwNiaZy3WT6d9DFl9kdvqDyB15jLUZM+4h4BI18?=
 =?us-ascii?Q?bCCM1sQ7tHMiP/vlHwvgT7l6WNIwzXbbYm0fJuYZMsQ94YdYfBQc5i/BUmH6?=
 =?us-ascii?Q?SNw/fyL0kB01FRJiNlh5pH6ntDY9vPsua26o+O1Txv24zTMsFeasJUrhtW2k?=
 =?us-ascii?Q?7LOMJBhQQ+S1Gp9GzZVhqRXPe+OzaexN/6BcxZZEgIk/yA9I1dsvidkNR5cP?=
 =?us-ascii?Q?xgXGTI1oPHgbz7TkjOqSxrW6tPsO+9I6F/G8vURHzDcxrfWwwc8oHjNuG0a9?=
 =?us-ascii?Q?rE6/vi3yzKxkp1Cb8tGsz1vEIF6XrGxe6QP8g9mmbOaKm4hJ6QHvVgtC3W24?=
 =?us-ascii?Q?qPHfm/+azbmiWD6XgnExacKs7UFCxmFYxWL+QRHHY8pXuwkaqwUAUrCnc3w7?=
 =?us-ascii?Q?NBqEY0zMHNLOjv+gIfdqkFusjZRYv/ipz1zgGeukivhygtUf1NPHAJLztN7C?=
 =?us-ascii?Q?gcu0euEOWUzZZiW/xMkPI2HUZXwrixcapPOhx11nCA6hhAI+LD/DB2M0xwzb?=
 =?us-ascii?Q?Eb0e3xicLAWWNLWALA/pgKQP1HIESCDsqTTxprr0ZDea27lh1NK7par0Does?=
 =?us-ascii?Q?/iKPpX6Ye36PjnRMggbXinKCPrwbVY32AW1ofICfUDPovzTRJd4HOlarb34Z?=
 =?us-ascii?Q?Dc0SrAC76SqWE3uFr0pUXUIU7zM83Vs90oAUw0L3cVWhYVrpEYkXy7vDYyCi?=
 =?us-ascii?Q?aE6z68esK7dggKzKgp+AEiKpWnIPzft7ZNWsRpyEHeNtOLN6WNfI6A/14CPD?=
 =?us-ascii?Q?e0bKtx/I+PNvcKHsCgwwY3phAMpufhmp3LJl4/idrgcbtEmcx3wEWewRQUGH?=
 =?us-ascii?Q?Hx94B1vXObi4+3iUKI9T0WdA09GKVOj72KTMbjw9EJirxyj2K8HivHRkpr7H?=
 =?us-ascii?Q?HGp63mFb5/VZ4CHf2TSzrbgg7lLUYULEVLlGcmAaOl3uMEFbUyZpoRMaVpw7?=
 =?us-ascii?Q?HoQ9ckeK8GE3Mj+HlrLpy/XKdQEE87fJqK4qQ8xEuW7+8ZVTerlaIxBniZTk?=
 =?us-ascii?Q?d9MfE/75Lwa9HhIyLkOi2RgYXgjAdt3KD1A+kp0WoDmCZPqTa3HliG6noaLk?=
 =?us-ascii?Q?Dt2akEDnQ2BcdN1NKC3ZLI5xDDnNi24/UDoXRN7zsEb3UYQ1GZFu+mfdRyYl?=
 =?us-ascii?Q?iw/d5m8O0B83iCUUiAPi3NFpdPUzcT9S9kT7yuxB?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 9608dbe8-a805-4c34-8448-08dc65cb71fa
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2024 08:32:44.5846
 (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: uyw5GTAyfwFDCBg2eX05Jq26FafLZOEsKnH1hKtpNIAe3cQoseVfUb3iBK7jxmvXwQWKNf6K+SilMlsJRBPVyg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB4977
X-OriginatorOrg: intel.com

>+static int hardware_enable_all(void)
>+{
>+	int r;
>+
>+	guard(mutex)(&kvm_lock);
>+
>+	if (kvm_usage_count++)
>+		return 0;
>+
>+	r = cpuhp_setup_state(CPUHP_AP_KVM_ONLINE, "kvm/cpu:online",
>+			      kvm_online_cpu, kvm_offline_cpu);

A subtle change is: cpuhp_setup_state() calls kvm_online_cpu() serially
on all CPUs. Previously, hardware enabling is done with on_each_cpu().
I assume performance isn't a concern here. Right?

>+	if (r)
>+		return r;

decrease kvm_usage_count on error?

>+
>+	register_syscore_ops(&kvm_syscore_ops);
>+
>+	/*
>+	 * Undo virtualization enabling and bail if the system is going down.
>+	 * If userspace initiated a forced reboot, e.g. reboot -f, then it's
>+	 * possible for an in-flight operation to enable virtualization after
>+	 * syscore_shutdown() is called, i.e. without kvm_shutdown() being
>+	 * invoked.  Note, this relies on system_state being set _before_
>+	 * kvm_shutdown(), e.g. to ensure either kvm_shutdown() is invoked
>+	 * or this CPU observes the impending shutdown.  Which is why KVM uses
>+	 * a syscore ops hook instead of registering a dedicated reboot
>+	 * notifier (the latter runs before system_state is updated).
>+	 */
>+	if (system_state == SYSTEM_HALT || system_state == SYSTEM_POWER_OFF ||
>+	    system_state == SYSTEM_RESTART) {
>+		unregister_syscore_ops(&kvm_syscore_ops);
>+		cpuhp_remove_state(CPUHP_AP_KVM_ONLINE);
>+		return -EBUSY;

ditto

>+	}
>+
>+	return 0;
>+}