Re: [PATCH] KVM: arm64: Don't miss pending interrupts for suspended vCPU

From: Marc Zyngier <maz@kernel.org>
To: Oliver Upton <oupton@google.com>
Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	Peter Shier <pshier@google.com>,
	Ricardo Koller <ricarkol@google.com>,
	Reiji Watanabe <reijiw@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>
Subject: Re: [PATCH] KVM: arm64: Don't miss pending interrupts for suspended vCPU
Date: Thu, 17 Feb 2022 13:40:53 +0000	[thread overview]
Message-ID: <aa6851d90aeb0dfade28527687253219@kernel.org> (raw)
In-Reply-To: <20220217101242.3013716-1-oupton@google.com>

On 2022-02-17 10:12, Oliver Upton wrote:
> In order to properly emulate the WFI instruction, KVM reads back
> ICH_VMCR_EL2 and enables doorbells for GICv4. These preparations are
> necessary in order to recognize pending interrupts in
> kvm_arch_vcpu_runnable() and return to the guest. Until recently, this
> work was done by kvm_arch_vcpu_{blocking,unblocking}(). Since commit
> 6109c5a6ab7f ("KVM: arm64: Move vGIC v4 handling for WFI out arch
> callback hook"), these callbacks were gutted and superseded by
> kvm_vcpu_wfi().
> 
> It is important to note that KVM implements PSCI CPU_SUSPEND calls as
> a WFI within the guest. However, the implementation calls directly into
> kvm_vcpu_halt(), which skips the needed work done in kvm_vcpu_wfi()
> to detect pending interrupts. Fix the issue by calling the WFI helper.
> 
> Fixes: 6109c5a6ab7f ("KVM: arm64: Move vGIC v4 handling for WFI out
> arch callback hook")
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
>  arch/arm64/kvm/psci.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c
> index 3eae32876897..2ce60fecd861 100644
> --- a/arch/arm64/kvm/psci.c
> +++ b/arch/arm64/kvm/psci.c
> @@ -46,8 +46,7 @@ static unsigned long kvm_psci_vcpu_suspend(struct
> kvm_vcpu *vcpu)
>  	 * specification (ARM DEN 0022A). This means all suspend states
>  	 * for KVM will preserve the register state.
>  	 */
> -	kvm_vcpu_halt(vcpu);
> -	kvm_clear_request(KVM_REQ_UNHALT, vcpu);
> +	kvm_vcpu_wfi(vcpu);
> 
>  	return PSCI_RET_SUCCESS;
>  }

Thanks for picking this up, I kept forgetting about fixing it.
I'll merge it once I'm back home.

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel