Date:   Mon, 13 Mar 2023 16:26:54 +0800
From:   Chao Gao <chao.gao@intel.com>
To:     Sean Christopherson <seanjc@google.com>
CC:     Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, <x86@kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        <linux-kernel@vger.kernel.org>, <kvm@vger.kernel.org>,
        Andrew Cooper <Andrew.Cooper3@citrix.com>,
        Kai Huang <kai.huang@intel.com>
Subject: Re: [PATCH v2 03/18] x86/reboot: Harden virtualization hooks for
 emergency reboot
Message-ID: <ZA7eTpG5tpo5yIo3@gao-cwp>
References: <20230310214232.806108-1-seanjc@google.com>
 <20230310214232.806108-4-seanjc@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230310214232.806108-4-seanjc@google.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?PvDoX0lB8s1sH8LX5Pb3jR4wBMm0Q6JfQlxmnJF7BfLNAd9+80nSt2REJoMU?=
 =?us-ascii?Q?ltN/9GfhwiSRhRQWmpX7O55Mk2uCYAcSt9SF4IsilAW3Gef1ETRnL+lEDH9i?=
 =?us-ascii?Q?ike61s4mnGkM+y8rJ4uPT0+riwOzTHE/2/E/DneNqd1pm3sS5JG8ED/vYFNV?=
 =?us-ascii?Q?n6wui8A9kWP4D516vzMc6QJZD3iGmpaJ1KGjm/E0euHyj+TGMp6NSLpn79JI?=
 =?us-ascii?Q?4aB89izlEOXV9B7wsrhz1i2WMx6ajTgICHJAjVPetyzTHM+67+36GUX2Y61d?=
 =?us-ascii?Q?P+KefeWP+aHwqWBIbNH1Y9QhNXG9IGuALZPHRl3qBAquTozjn2Qf/G71j4Qq?=
 =?us-ascii?Q?Z4Ja+WvXbHOszveo9H1w6MrDjk2w89SS+c0Jm+1WRAWAE7VBj5bqRnde6OQN?=
 =?us-ascii?Q?DH7BEebuSrOBdNwuCQytl7kTD/uhu7XdX7UnOVldYTJv3DT9tG3AiPvouMx/?=
 =?us-ascii?Q?a1mYIqn37qRO02xzNX7MkLqb/LwbcNaUObnT+nJcxIU+hsE5quvuZ0OratFa?=
 =?us-ascii?Q?EZnW+EOLvdLal34NtUePLsdDxOfFVQhYdZFtxA8EvH3xxVggqZQqFJPQfGzw?=
 =?us-ascii?Q?KR5uAmPRm6RUoZvtSrVXSqA0feb+J2+T1MYQTgAxlKzfFIlsJjCvMlfn9zUX?=
 =?us-ascii?Q?SF/QHnJ8hXENmMRBwl69Fm6i4uiXgzJUOyLBx7hQpnCfDGSKEwEGo4LFUOTp?=
 =?us-ascii?Q?O7cmgUcHIVjwwjtSxnyCCeE9239czjcVTI8oIe5G3wdAOm25B7y5ezxBK53V?=
 =?us-ascii?Q?SGrSZFfJkIRquOpuFLFxr1ERi6RBUpc+0KnTba3vVrrWW36uSi3PB6W5ZoM8?=
 =?us-ascii?Q?99CS4WdT8Bd25r/ZDHRZ/rJLNKD+TLOphYWWoknxmw93MKg1/QLz2yWToJ8S?=
 =?us-ascii?Q?6AD0Geg28X+dcA14tlWHTE42DLQ5rgKyGRJObHLJxlqd8OFnjvHETlakwWgC?=
 =?us-ascii?Q?M9ETjF2c/Mlvk3f0dwqjdvgHRtZl9zhfxCFPgER44lUG68EL9GaCCE3xHrXh?=
 =?us-ascii?Q?xqNYOvAydargILAuYDg5z7nME1dS+kxOKApRI7P8aClll9ApY6zqGqVEp6UQ?=
 =?us-ascii?Q?s14OM73HFJfViaO4NpcyZFmuK0mhyAaxOwT0rEl6jP0wxuD5zJJtNIcNVXTX?=
 =?us-ascii?Q?0lAZ8lMEk5n2LueLpsIXvPhuG/BekngyWRzaEbP9C/49WBF20mxzZbYv2BlZ?=
 =?us-ascii?Q?/52am+w+RO1Eja6pBYSIu8ynu5m6zl9vGUnsQpVpwGGmwVIdhVWaWBocpc4c?=
 =?us-ascii?Q?YO5cl4khXwhVD7XNJupMjOOQDttG1osga43QcZkYi4jFgf8v/WvViP32W3j8?=
 =?us-ascii?Q?65jsHqSTJyNXKJSpju4gWY+kxAHPe/GfKQx+BQ8MRw4DKc8Q5QL0cYLbDcdu?=
 =?us-ascii?Q?RrpAiMo3abVnSomVYvQaQiCT3geAaW/o6ipHJlrGckjnqR22XDS4L9Jt9AAb?=
 =?us-ascii?Q?eoEeFElLUB3MGwV9sS2bHYFCtaTaffhC6lmEHe5Zmx23BcsWlrHkIBaGD0LM?=
 =?us-ascii?Q?+itinhSk86D9BpQ8bf14WuPiRw0/ileD+pJxXMczd7cNv1nxv+kc/DAJrT2v?=
 =?us-ascii?Q?f63mdU/vWb5M7WMee8zoKMenGEQvOHxvbOvJLoIc?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e1c89a0c-f774-4f9a-5168-08db239ca58a
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB6780.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Mar 2023 08:26:30.3794
 (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: ps40XnSN1BQNdxzsUeYyQ1CBxjmupJj8HtIGhusthHjtvTGM+GXVGN+McP4a8Mxd94B/Jd9Ml+yEBiH3DqJstg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7420
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Mar 10, 2023 at 01:42:17PM -0800, Sean Christopherson wrote:
>+void cpu_emergency_register_virt_callback(cpu_emergency_virt_cb *callback)
>+{
>+	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback)))
>+		return;
>+
>+	rcu_assign_pointer(cpu_emergency_virt_callback, callback);

Was it intentional to not call synchronize_rcu() (in the original
code), different from the un-registration path?

>+}
>+EXPORT_SYMBOL_GPL(cpu_emergency_register_virt_callback);
>+
>+void cpu_emergency_unregister_virt_callback(cpu_emergency_virt_cb *callback)
>+{
>+	if (WARN_ON_ONCE(rcu_access_pointer(cpu_emergency_virt_callback) != callback))
>+		return;
>+
>+	rcu_assign_pointer(cpu_emergency_virt_callback, NULL);
>+	synchronize_rcu();
>+}
>+EXPORT_SYMBOL_GPL(cpu_emergency_unregister_virt_callback);
> 
> static inline void cpu_crash_vmclear_loaded_vmcss(void)
> {
>-	crash_vmclear_fn *do_vmclear_operation = NULL;
>+	cpu_emergency_virt_cb *callback;
> 
> 	rcu_read_lock();
>-	do_vmclear_operation = rcu_dereference(crash_vmclear_loaded_vmcss);
>-	if (do_vmclear_operation)
>-		do_vmclear_operation();
>+	callback = rcu_dereference(cpu_emergency_virt_callback);
>+	if (callback)
>+		callback();
> 	rcu_read_unlock();
> }
> #endif
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index 302086255be6..41095fc864f3 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -8551,8 +8551,7 @@ static void __vmx_exit(void)
> {
> 	allow_smaller_maxphyaddr = false;
> 
>-	RCU_INIT_POINTER(crash_vmclear_loaded_vmcss, NULL);
>-	synchronize_rcu();
>+	cpu_emergency_unregister_virt_callback(crash_vmclear_local_loaded_vmcss);
> 
> 	vmx_cleanup_l1d_flush();
> }
>@@ -8602,8 +8601,7 @@ static int __init vmx_init(void)
> 		pi_init_cpu(cpu);
> 	}
> 
>-	rcu_assign_pointer(crash_vmclear_loaded_vmcss,
>-			   crash_vmclear_local_loaded_vmcss);
>+	cpu_emergency_register_virt_callback(crash_vmclear_local_loaded_vmcss);
> 
> 	vmx_check_vmcs12_offsets();
> 
>-- 
>2.40.0.rc1.284.g88254d51c5-goog
>