Date:   Sun, 12 Mar 2023 10:12:29 +0000
Message-ID: <87h6uq5lde.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Sean Christopherson <seanjc@google.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Oliver Upton <oliver.upton@linux.dev>,
        James Morse <james.morse@arm.com>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Zenghui Yu <yuzenghui@huawei.com>, kvmarm@lists.linux.dev,
        Huacai Chen <chenhuacai@kernel.org>,
        Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>,
        Anup Patel <anup@brainfault.org>,
        Atish Patra <atishp@atishpatra.org>,
        kvm-riscv@lists.infradead.org
Subject: Re: [PATCH 1/2] KVM: Use syscore_ops instead of reboot_notifier to hook restart/shutdown
In-Reply-To: <20230310221414.811690-2-seanjc@google.com>
References: <20230310221414.811690-1-seanjc@google.com>
        <20230310221414.811690-2-seanjc@google.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Fri, 10 Mar 2023 22:14:13 +0000,
Sean Christopherson <seanjc@google.com> wrote:
> 
> Use syscore_ops.shutdown to disable hardware virtualization during a
> reboot instead of using the dedicated reboot_notifier so that KVM disables
> virtualization _after_ system_state has been updated.  This will allow
> fixing a race in KVM's handling of a forced reboot where KVM can end up
> enabling hardware virtualization between kernel_restart_prepare() and
> machine_restart().
> 
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: James Morse <james.morse@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Zenghui Yu <yuzenghui@huawei.com>
> Cc: kvmarm@lists.linux.dev
> Cc: Huacai Chen <chenhuacai@kernel.org>
> Cc: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
> Cc: Anup Patel <anup@brainfault.org>
> Cc: Atish Patra <atishp@atishpatra.org>
> Cc: kvm-riscv@lists.infradead.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  virt/kvm/kvm_main.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index d255964ec331..6cdfbb2c641b 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -5211,8 +5211,7 @@ static int hardware_enable_all(void)
>  	return r;
>  }
>  
> -static int kvm_reboot(struct notifier_block *notifier, unsigned long val,
> -		      void *v)
> +static void kvm_reboot(void)
>  {
>  	/*
>  	 * Some (well, at least mine) BIOSes hang on reboot if
> @@ -5223,14 +5222,8 @@ static int kvm_reboot(struct notifier_block *notifier, unsigned long val,
>  	pr_info("kvm: exiting hardware virtualization\n");
>  	kvm_rebooting = true;
>  	on_each_cpu(hardware_disable_nolock, NULL, 1);
> -	return NOTIFY_OK;
>  }
>  
> -static struct notifier_block kvm_reboot_notifier = {
> -	.notifier_call = kvm_reboot,
> -	.priority = 0,
> -};
> -
>  static int kvm_suspend(void)
>  {
>  	/*
> @@ -5261,6 +5254,8 @@ static void kvm_resume(void)
>  static struct syscore_ops kvm_syscore_ops = {
>  	.suspend = kvm_suspend,
>  	.resume = kvm_resume,
> +	.shutdown = kvm_reboot,
> +

nit: consider renaming the kvm_reboot to kvm_shutdown to match the
syscore structure, and drop the spurious blank line.

	M.

-- 
Without deviation from the norm, progress is not possible.