From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8406DC433EF for ; Fri, 1 Oct 2021 13:57:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 58D996138F for ; Fri, 1 Oct 2021 13:57:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231778AbhJAN7H (ORCPT ); Fri, 1 Oct 2021 09:59:07 -0400 Received: from mail.kernel.org ([198.145.29.99]:47986 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230408AbhJAN7G (ORCPT ); Fri, 1 Oct 2021 09:59:06 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 8A6DE611C8; Fri, 1 Oct 2021 13:57:22 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mWJ2N-00ECvK-Uk; Fri, 01 Oct 2021 14:57:20 +0100 Date: Fri, 01 Oct 2021 14:57:19 +0100 Message-ID: <87h7e0sv9c.wl-maz@kernel.org> From: Marc Zyngier To: Sean Christopherson Cc: Oliver Upton , kvm@vger.kernel.org, Peter Shier , kvmarm@lists.cs.columbia.edu Subject: Re: [PATCH v2 05/11] KVM: arm64: Defer WFI emulation as a requested event In-Reply-To: References: <20210923191610.3814698-1-oupton@google.com> <20210923191610.3814698-6-oupton@google.com> <878rzetk0o.wl-maz@kernel.org> 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 X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: seanjc@google.com, oupton@google.com, kvm@vger.kernel.org, pshier@google.com, kvmarm@lists.cs.columbia.edu X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Thu, 30 Sep 2021 18:09:07 +0100, Sean Christopherson wrote: > > On Thu, Sep 30, 2021, Marc Zyngier wrote: > > On Thu, 23 Sep 2021 20:16:04 +0100, Oliver Upton wrote: > > > @@ -681,6 +687,9 @@ static void check_vcpu_requests(struct kvm_vcpu *vcpu) > > > if (kvm_check_request(KVM_REQ_SLEEP, vcpu)) > > > kvm_vcpu_sleep(vcpu); > > > > > > + if (kvm_check_request(KVM_REQ_SUSPEND, vcpu)) > > > + kvm_vcpu_suspend(vcpu); > > > + > > ... > > > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > > > index 275a27368a04..5e5ef9ff4fba 100644 > > > --- a/arch/arm64/kvm/handle_exit.c > > > +++ b/arch/arm64/kvm/handle_exit.c > > > @@ -95,8 +95,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu) > > > } else { > > > trace_kvm_wfx_arm64(*vcpu_pc(vcpu), false); > > > vcpu->stat.wfi_exit_stat++; > > > - kvm_vcpu_block(vcpu); > > > - kvm_clear_request(KVM_REQ_UNHALT, vcpu); > > > + kvm_make_request(KVM_REQ_SUSPEND, vcpu); > > > } > > > > > > kvm_incr_pc(vcpu); > > > > This is a change in behaviour. At the point where the blocking > > happens, PC will have already been incremented. I'd rather you don't > > do that. Instead, make the helper available and call into it directly, > > preserving the current semantics. > > Is there architectural behavior that KVM can emulate? E.g. if you > were to probe a physical CPU while it's waiting, would you observe > the pre-WFI PC, or the post-WFI PC? Following arch behavior would > be ideal because it eliminates subjectivity. The architecture doesn't really say (that's one of the many IMPDEF behaviours). However, there are at least some extensions (such as statistical profiling) that do require the PC to be accurately recorded together with the effects of the instructions at that PC. If an implementation was to pre-increment the PC, the corresponding trace would be inaccurate. > Regardless of the architectural behavior, changing KVM's behavior > should be done explicitly in a separate patch. > > Irrespective of PC behavior, I would caution against using a request > for handling WFI. Deferring the WFI opens up the possibility for > all sorts of ordering oddities, e.g. if KVM exits to userspace > between here and check_vcpu_requests(), then KVM can end up with a > "spurious" pending KVM_REQ_SUSPEND if maniupaltes vCPU state. I > highly doubt that userspace VMMs would actually do that, as it would > basically require a signal from userspace, but it's not impossible, > and at the very least the pending request is yet another thing to > worry about in the future. +1. It really isn't worth the complexity. Thanks, M. -- Without deviation from the norm, progress is not possible.