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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 55378C433F5 for ; Fri, 17 Dec 2021 14:39:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=b4VxMHDcCr16WvsoO8YJ+T6vk7xWMiOCRAX37ceC3SI=; b=i7zpC9fFxtUuwu YpKKir86QHJWFSeRph3vXQYqO3sV18yd1f+8sPs8LyWDPyamEyl5i7GDjKKjRZhMyh4YMyWULtrZ4 rVosCMeg36xC2RHMkE7qDYFiS3iaJ1guvImwxDtra2I+MnP8zD1E/W0l5KeFsvxvj6/r+1nLfoazj cKDH4gNyNu4HSZe6h2lDznijnO7EeQE/bf3bJCDHRp+ikxSrUps3xXn6p24Yuj0o+alS+8YaMk/mI ug5rCEi4JBLjxuRhuV2P/ZsMfUqH4DN6MtSxiOnxAF1YXW355n9MLI2STXbtS5qfRrv7ThNcsLb2m jj7yJ5mw0Nn90Zg9FZfw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1myENQ-00AflG-R4; Fri, 17 Dec 2021 14:38:28 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1myENK-00Afiq-96 for linux-arm-kernel@lists.infradead.org; Fri, 17 Dec 2021 14:38:26 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CACEE12FC; Fri, 17 Dec 2021 06:38:18 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.67.184]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2C3CA3F5A1; Fri, 17 Dec 2021 06:38:17 -0800 (PST) Date: Fri, 17 Dec 2021 14:38:14 +0000 From: Mark Rutland To: Nicolas Saenz Julienne Cc: maz , Will Deacon , paulmck , linux-arm-kernel , rcu , Thomas Gleixner , frederic , kvmarm@lists.cs.columbia.edu, linux-kernel Subject: Re: Possible nohz-full/RCU issue in arm64 KVM Message-ID: References: <70f112072d9496d21901946ea82832d3ed3a8cb2.camel@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <70f112072d9496d21901946ea82832d3ed3a8cb2.camel@redhat.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211217_063822_438927_17059C3A X-CRM114-Status: GOOD ( 38.13 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Dec 17, 2021 at 03:15:29PM +0100, Nicolas Saenz Julienne wrote: > On Fri, 2021-12-17 at 13:21 +0000, Mark Rutland wrote: > > On Fri, Dec 17, 2021 at 12:51:57PM +0100, Nicolas Saenz Julienne wrote: > > > Hi All, > > > > Hi, > > > > > arm64's guest entry code does the following: > > > > > > int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > > > { > > > [...] > > > > > > guest_enter_irqoff(); > > > > > > ret = kvm_call_hyp_ret(__kvm_vcpu_run, vcpu); > > > > > > [...] > > > > > > local_irq_enable(); > > > > > > /* > > > * We do local_irq_enable() before calling guest_exit() so > > > * that if a timer interrupt hits while running the guest we > > > * account that tick as being spent in the guest. We enable > > > * preemption after calling guest_exit() so that if we get > > > * preempted we make sure ticks after that is not counted as > > > * guest time. > > > */ > > > guest_exit(); > > > [...] > > > } > > > > > > > > > On a nohz-full CPU, guest_{enter,exit}() delimit an RCU extended quiescent > > > state (EQS). Any interrupt happening between local_irq_enable() and > > > guest_exit() should disable that EQS. Now, AFAICT all el0 interrupt handlers > > > do the right thing if trggered in this context, but el1's won't. Is it > > > possible to hit an el1 handler (for example __el1_irq()) there? > > > > I think you're right that the EL1 handlers can trigger here and won't exit the > > EQS. > > > > I'm not immediately sure what we *should* do here. What does x86 do for an IRQ > > taken from a guest mode? I couldn't spot any handling of that case, but I'm not > > familiar enough with the x86 exception model to know if I'm looking in the > > right place. > > Well x86 has its own private KVM guest context exit function > 'kvm_guest_exit_irqoff()', which allows it to do the right thing (simplifying > things): > > local_irq_disable(); > kvm_guest_enter_irqoff() // Inform CT, enter EQS > __vmx_kvm_run() > kvm_guest_exit_irqoff() // Inform CT, exit EQS, task still marked with PF_VCPU > > /* > * Consume any pending interrupts, including the possible source of > * VM-Exit on SVM and any ticks that occur between VM-Exit and now. > * An instruction is required after local_irq_enable() to fully unblock > * interrupts on processors that implement an interrupt shadow, the > * stat.exits increment will do nicely. > */ > local_irq_enable(); > ++vcpu->stat.exits; > local_irq_disable(); > > /* > * Wait until after servicing IRQs to account guest time so that any > * ticks that occurred while running the guest are properly accounted > * to the guest. Waiting until IRQs are enabled degrades the accuracy > * of accounting via context tracking, but the loss of accuracy is > * acceptable for all known use cases. > */ > vtime_account_guest_exit(); // current->flags &= ~PF_VCPU I see. The abstraction's really messy here on x86, and the enter/exit sides aren't clearly balanced. For example kvm_guest_enter_irqoff() calls guest_enter_irq_off() which calls vtime_account_guest_enter(), but kvm_guest_exit_irqoff() doesn't call guest_exit_irq_off() and the call to vtime_account_guest_exit() is open-coded elsewhere. Also, guest_enter_irq_off() conditionally calls rcu_virt_note_context_switch(), but I can't immediately spot anything on the exit side that corresponded with that, which looks suspicious. > So I guess we should convert to x86's scheme, and maybe create another generic > guest_{enter,exit}() flavor for virtualization schemes that run with interrupts > disabled. I think we might need to do some preparatory refactoring here so that this is all clearly balanced even on x86, e.g. splitting the enter/exit steps into multiple phases. > > Note that the EL0 handlers *cannot* trigger for an exception taken from a > > guest. We use separate vectors while running a guest (for both VHE and nVHE > > modes), and from the main kernel's PoV we return from kvm_call_hyp_ret(). We > > can ony take IRQ from EL1 *after* that returns. > > > > We *might* need to audit the KVM vector handlers to make sure they're not > > dependent on RCU protection (I assume they're not, but it's possible something > > has leaked into the VHE code). > > IIUC in the window between local_irq_enable() and guest_exit() any driver > interrupt might trigger, isn't it? Yes, via the EL1 interrupt vectors, which I assume we'll fix in one go above. Here I was trying to point out that there's another potential issue here if we do anything in the context of the KVM exception vectors, as those can run C code in a shallow exeption context, and can either return back into the guest OR return to the caller of kvm_call_hyp_ret(__kvm_vcpu_run, vcpu). So even if we fix kvm_arch_vcpu_ioctl_run() we might need to also rework handlers that run in that shallow exception context, if they rely on RCU for something. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel