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 47BDECC6B1D for ; Wed, 14 Oct 2026 13:34:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=xlbe4IDve5sVa3bicBDrKckx0LdiPpTCwmNH0wErMrk=; b=eqeROrbW+SSR4S1MgRxjTrkMxy S1QMXklUdAQJ4niOVa8zZX9F+4+LGgTpaFB/uXQuYkutLbO8Qf3pZJ4McfaTjNOp3f/Y31EqgGTvF dmENWCzG3TQivmitOz9f4aTUSmYxSENFDePk0R+A0fxvOzR+xk2uztAgujPV6VWVP83RtIDBCUTAe eZuE0m2JpJ/EBDE+TIbk2c7404PLsG6f2dJ96OIMfD/hWllj7xIlqAkpEvAyuXQMDpM2ywoMtqqE9 FJw2bjptY4MOvEF8AGyNiBUthKfrj/4mTjntXnbIqhwpMu1qBZJXwb+oOiE65ovkWeIP3jNNf6OLq Ta/ik24Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1xGz7W-0000000FbpO-3Cab; Wed, 14 Oct 2026 13:34:14 +0000 Received: from tor.source.kernel.org ([2600:3c04:e001:324:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1xGz7V-0000000Fboy-2Ctq for linux-arm-kernel@lists.infradead.org; Wed, 14 Oct 2026 13:34:13 +0000 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by tor.source.kernel.org (Postfix) with ESMTP id DB758600DA; Wed, 14 Oct 2026 13:34:12 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 73CCC1F00893; Wed, 14 Oct 2026 13:34:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1791984852; bh=xlbe4IDve5sVa3bicBDrKckx0LdiPpTCwmNH0wErMrk=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=euj6GWmHlaz7C/KEd7JCVSEhRJsOzvJDpdVYhnxnG+iJZKo6DPq5lnGN0OZs32nlm KWrnCmPNVBakogEapCVV7u8jH3HXd3iYSX4Extyjed3dbHCo0UnPFon6iiCEz15UWE gN5B+2iOa3o8Sh3ksxwvpFEkzQRiRcglwdg9uQw42kgbUubYWVZ732cOBBlPFxHi6t m5bFEP7JlgicNJ6pDlMUMf67OJ134IlNZFOf55XgfzMs5DXY7jBgCdSVFFzY8F4I2q LX7ZOjzU0ox34ZDPpucyLxxDkHoWckTA9vl8SFOn4dTWIv/XsRvFtneiDkZPF3ANNC c21G3FKmH1Bfw== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1xGz7R-00000002tLm-48r5; Wed, 14 Oct 2026 13:34:10 +0000 Date: Wed, 14 Oct 2026 14:34:09 +0100 Message-ID: <86se281k4u.wl-maz@kernel.org> From: Marc Zyngier To: Fuad Tabba Cc: Oliver Upton , Joey Gouly , Steffen Eiden , Suzuki K Poulose , Zenghui Yu , Catalin Marinas , Will Deacon , Mark Rutland , Andre Przywara , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, Fuad Tabba Subject: Re: [PATCH v2 0/2] KVM: arm64: vgic-its: Fix LPI double mapping and DISCARD In-Reply-To: <20261013143338.815377-1-fuad.tabba@linux.dev> References: <86tsmp20ne.wl-maz@kernel.org> <20261013143338.815377-1-fuad.tabba@linux.dev> 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/30.1 (aarch64-unknown-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: fuad.tabba@linux.dev, oupton@kernel.org, joey.gouly@arm.com, seiden@linux.ibm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, catalin.marinas@arm.com, will@kernel.org, mark.rutland@arm.com, andre.przywara@arm.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, tabba@google.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, 13 Oct 2026 15:33:38 +0100, Fuad Tabba wrote: > > > [...] > > Is the hack below what you are asking for? I can't tell. If it isn't, > > please post your own version. > > Almost: it's missing a request check between the IN_GUEST_MODE store and > the flush. Without it, a halt that reads OUTSIDE_GUEST_MODE just before > the store doesn't wait for the vCPU, which then loads the LPI into an > LR, takes the early exit on KVM_REQ_SLEEP and folds the LPI back after > DISCARD has cleared it. That's the outcome in the first litmus test's > exists clause. With the debug hooks forcing it on QEMU, the guest takes > the discarded LPI in every run with your diff, and in none with this on > top: > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index f1449b0..896bc76 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -1329,11 +1329,29 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > */ > smp_store_mb(vcpu->mode, IN_GUEST_MODE); > > + /* > + * Don't load the LRs with a request pending: > + * kvm_arm_halt_guest() doesn't wait for a vCPU it reads > + * as OUTSIDE_GUEST_MODE. > + */ > + if (ret <= 0 || kvm_vcpu_exit_request(vcpu, &ret)) { > + isb(); /* Ensure work in x_flush_hwstate is committed */ > + if (kvm_vcpu_has_pmu(vcpu)) > + kvm_pmu_sync_hwstate(vcpu); > + if (unlikely(!irqchip_in_kernel(vcpu->kvm))) > + kvm_timer_sync_user(vcpu); > + smp_store_release(&vcpu->mode, OUTSIDE_GUEST_MODE); > + local_irq_enable(); > + preempt_enable(); > + continue; > + } > + > kvm_vgic_flush_hwstate(vcpu); > > kvm_pmu_update_vcpu_events(vcpu); > > - if (ret <= 0 || kvm_vcpu_exit_request(vcpu, &ret)) { > + /* kvm_vgic_flush_hwstate() can make a request when nested */ > + if (kvm_vcpu_exit_request(vcpu, &ret)) { > isb(); /* Ensure work in x_flush_hwstate is committed */ > if (kvm_vcpu_has_pmu(vcpu)) > kvm_pmu_sync_hwstate(vcpu); > I don't think this is right. Yes, it probably does enough request checks that we end-up with not getting it completely wrong, but the issues are that: - we have state change before we evaluate the requests - we mix request generation and state change in a single helper and we should be able to fix that so that there is only a single request checking point in the whole entry sequence. I ended-up with a much bigger change, which really needs to be split in two: - untangle state generation and requests: this means that the maintenance interrupt generation and the request. The request generating stuff must happen before being IN_GUEST_MODE and request checking. - the flushing must happen later on, once we don't look back anymore and have committed to entering the guest. This, in turn, simplifies the "shit, I must rollback all the previous changes as I can't enter the guest and must handle a request instead" horror. Another bug fix is that we now generate the maintenance interrupt before deciding whether to generate a request switching from L2 to L1 as we have a pending interrupt. Thoughts? M. diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 0576c2022ef57..3fa9e158f0be1 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -1321,10 +1321,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) local_irq_disable(); - kvm_vgic_flush_hwstate(vcpu); - - kvm_pmu_update_vcpu_events(vcpu); - /* * Ensure we set mode to IN_GUEST_MODE after we disable * interrupts and before the final VCPU requests check. @@ -1334,18 +1330,19 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) smp_store_mb(vcpu->mode, IN_GUEST_MODE); if (ret <= 0 || kvm_vcpu_exit_request(vcpu, &ret)) { - vcpu->mode = OUTSIDE_GUEST_MODE; isb(); /* Ensure work in x_flush_hwstate is committed */ - if (kvm_vcpu_has_pmu(vcpu)) - kvm_pmu_sync_hwstate(vcpu); if (unlikely(!irqchip_in_kernel(vcpu->kvm))) kvm_timer_sync_user(vcpu); - kvm_vgic_sync_hwstate(vcpu); + smp_store_release(&vcpu->mode, OUTSIDE_GUEST_MODE); local_irq_enable(); preempt_enable(); continue; } + kvm_vgic_flush_hwstate(vcpu); + + kvm_pmu_update_vcpu_events(vcpu); + kvm_arch_vcpu_ctxflush_fp(vcpu); /************************************************************** @@ -1356,7 +1353,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) ret = kvm_arm_vcpu_enter_exit(vcpu); - vcpu->mode = OUTSIDE_GUEST_MODE; vcpu->stat.exits++; /* * Back from guest @@ -1390,6 +1386,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) kvm_arch_vcpu_ctxsync_fp(vcpu); + /* + * All the state has been synchronised, let advertise + * we're outside of the guest. + */ + smp_store_release(&vcpu->mode, OUTSIDE_GUEST_MODE); + /* * We must ensure that any pending interrupts are taken before * we exit guest timing so that timer ticks are accounted as diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c index b191365d97cc2..41f3def0eecae 100644 --- a/arch/arm64/kvm/nested.c +++ b/arch/arm64/kvm/nested.c @@ -2046,6 +2046,9 @@ void check_nested_vcpu_requests(struct kvm_vcpu *vcpu) * * What follows is some slop to (1) react to exception routing / masking and (2) * preserve the pending SError state across translation regimes. + * + * Also look generate the maintenance interrupt state, and finally check + * that if we need to take an interrupt targetting the guest hypervisor. */ void kvm_nested_flush_hwstate(struct kvm_vcpu *vcpu) { @@ -2054,6 +2057,11 @@ void kvm_nested_flush_hwstate(struct kvm_vcpu *vcpu) if (unlikely(vcpu_test_and_clear_flag(vcpu, NESTED_SERROR_PENDING))) kvm_inject_serror_esr(vcpu, vcpu_get_vsesr(vcpu)); + + vgic_v3_nested_update_mi(vcpu); + + if (vgic_state_is_nested(vcpu) && kvm_vgic_vcpu_pending_irq(vcpu)) + kvm_make_request(KVM_REQ_GUEST_HYP_IRQ_PENDING, vcpu); } void kvm_nested_sync_hwstate(struct kvm_vcpu *vcpu) diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c index 5cf5a1ef86cdd..7061b4ab206c9 100644 --- a/arch/arm64/kvm/vgic/vgic.c +++ b/arch/arm64/kvm/vgic/vgic.c @@ -1152,7 +1152,7 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu) * * We do that by placing a request to ourselves which will * abort the entry procedure and inject the exception at the - * beginning of the run loop. + * beginning of the run loop (see kvm_nested_flush_hwstate()). * * - Otherwise, do exactly *NOTHING* apart from enabling the virtual * CPU interface. The guest state is already loaded, and we can @@ -1162,16 +1162,10 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu) * maintenance interrupt state, as it may fire. */ if (vgic_state_is_nested(vcpu)) { - if (kvm_vgic_vcpu_pending_irq(vcpu)) - kvm_make_request(KVM_REQ_GUEST_HYP_IRQ_PENDING, vcpu); - vgic_v3_flush_nested(vcpu); return; } - if (vcpu_has_nv(vcpu)) - vgic_v3_nested_update_mi(vcpu); - DEBUG_SPINLOCK_BUG_ON(!irqs_disabled()); vgic_flush_state(vcpu); -- 2.47.3 -- Without deviation from the norm, progress is not possible.