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 16A95CD8C8E for ; Sat, 6 Jun 2026 10:49:19 +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=s2Xy2or30i+9Y/BGQbdomuCCP79IAxaruc93U4v6Yno=; b=jAq1FTY0h5w8BDjhAHWeVxemEf 27p7AktizmjLOgk/PlDhRFy9T6Nn8oKwdGkTDh3KBaDk3HZ1ltFNb7CFWKLir+glvvnzi2nMl0bj0 HLAhnWW0dDtMTj145eOWUdsXlnAufmIJEjuk0huuOOB25gfVJ3rPNwpcFVNNtX5KNh2b2EviBkuBB W/3lEbpIldTinNEX0lOQn3eYnwEcaxiLgs76yKZLZrmTRsgouXsCjKVOVz5dZc2afdbX7D5Qy1/fR ReFz+XiqkjTXWzIpBd7DdIYvYdbnFE4xT/kfp5vUOKCilaKumTeazOvMgTlK0rmA0oWuAX5FXL8/o +sK6OTmA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1wVoaV-00000001Z5x-1FCJ; Sat, 06 Jun 2026 10:49:11 +0000 Received: from sea.source.kernel.org ([2600:3c0a:e001:78e:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1wVoaU-00000001Z5q-2ve6 for linux-arm-kernel@lists.infradead.org; Sat, 06 Jun 2026 10:49:10 +0000 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by sea.source.kernel.org (Postfix) with ESMTP id BD95042ADB; Sat, 6 Jun 2026 10:49:08 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9EAFC1F00893; Sat, 6 Jun 2026 10:49:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1780742948; bh=s2Xy2or30i+9Y/BGQbdomuCCP79IAxaruc93U4v6Yno=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=oavCZS4BNSkzNMynpY1daQBDgMwItyQE2wXObLBg0M4z84CIM8p9I525OUjVN2s9I bv//xerVlZ0leGUIV+L1xFXjnS1lJTQ+TUcPeQTA7QrPLFWz2mhji9EUHu/mXAtMWF +P6EULSnSYNEUQceg5j5fkT/Gjp5DmsdFPY01TSzO99cPE9zJKtvFMPp1rkZElOD1y LIH6Ttzr6IjJ1+Jf9wrRfDjOSDyKBAf1CXFSyNfFSZHRjtg6ofV7OWJpS3MN8eHojM CeKioACIdQv0yZbwdPDFnwzXpWD+eAcmgfa8rngl4iejpDQMQxS56vzMVKB6Fa0/dM nZ6zgxgS96HRQ== Received: from sofa.misterjones.org ([185.219.108.64] helo=lobster-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 1wVoaQ-0000000A0yQ-1bVr; Sat, 06 Jun 2026 10:49:06 +0000 Date: Sat, 06 Jun 2026 11:52:23 +0100 Message-ID: <877boc9c0o.wl-maz@kernel.org> From: Marc Zyngier To: Hyunwoo Kim Cc: tabba@google.com, 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, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev Subject: Re: [PATCH v2 2/2] KVM: arm64: Bound used_lrs when flushing the pKVM hyp vCPU In-Reply-To: References: <20260604151210.1304051-1-imv4bel@gmail.com> <20260604151210.1304051-3-imv4bel@gmail.com> <86zf19tlcu.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/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: imv4bel@gmail.com, tabba@google.com, 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, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev 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 Sat, 06 Jun 2026 09:52:08 +0100, Hyunwoo Kim wrote: > > On Fri, Jun 05, 2026 at 09:58:09AM +0100, Marc Zyngier wrote: > > On Thu, 04 Jun 2026 16:12:03 +0100, > > Hyunwoo Kim wrote: > > > > > > flush_hyp_vcpu() copies the host vGIC state into the hyp's private vCPU > > > on every run. The vGIC list register save and restore use used_lrs as > > > their loop bound and expect it to stay within the number of implemented > > > list registers. While this is generally the case, flush_hyp_vcpu() > > > copies vgic_v3 verbatim and does not enforce this, so a value provided > > > by the host is used at EL2 to index vgic_lr[] and access ICH_LR_EL2 > > > (host -> EL2). > > > > > > Fix by clamping used_lrs to the number of implemented list registers > > > after the copy, as the trusted path already does in > > > vgic_flush_lr_state(). > > > > > > Fixes: be66e67f1750 ("KVM: arm64: Use the pKVM hyp vCPU structure in handle___kvm_vcpu_run()") > > > Signed-off-by: Hyunwoo Kim > > > --- > > > arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > > > index 02c5d6e5abcbf..cd807fdb11ba8 100644 > > > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c > > > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > > > @@ -7,6 +7,7 @@ > > > #include > > > #include > > > > > > +#include > > > #include > > > #include > > > #include > > > @@ -142,6 +143,13 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu) > > > > > > hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3 = host_vcpu->arch.vgic_cpu.vgic_v3; > > > > > > + /* Bound used_lrs by the number of implemented list registers. */ > > > + if (static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif)) > > > > There is no pKVM support without a GICv3 CPU interface, and absolutely > > everything already assumes it. Why do we need this extra check? > > > > > + hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3.used_lrs = > > > + min_t(unsigned int, > > > + hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3.used_lrs, > > > + (read_gicreg(ICH_VTR_EL2) & 0xf) + 1); > > > + > > > > Reading ICH_VTR_EL2 on each entry is going to cause some really heavy > > trapping under NV, and we should avoid this. > > > > kvm_vgic_global_state.nr_lr contains this information, and it should > > only be a matter of replicating it (or compute it once) at init time. > > Does this approach look reasonable to you? > > --- > > diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h > index 8d06b62e7188..25199769a1d6 100644 > --- a/arch/arm64/include/asm/kvm_hyp.h > +++ b/arch/arm64/include/asm/kvm_hyp.h > @@ -157,5 +157,6 @@ extern unsigned long kvm_nvhe_sym(__icache_flags); > extern unsigned int kvm_nvhe_sym(kvm_arm_vmid_bits); > extern unsigned int kvm_nvhe_sym(kvm_host_sve_max_vl); > extern unsigned long kvm_nvhe_sym(hyp_nr_cpus); > +extern unsigned int kvm_nvhe_sym(hyp_vgic_nr_lr); > > #endif /* __ARM64_KVM_HYP_H__ */ > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 9453321ef8c6..891fe2c7b854 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -2426,6 +2426,7 @@ static int __init init_subsystems(void) > switch (err) { > case 0: > vgic_present = true; > + kvm_nvhe_sym(hyp_vgic_nr_lr) = kvm_vgic_global_state.nr_lr; You probably want to guard this on kvm_vgic_global_state.gicv3_cpuif to be sure this stays to 0 when running with a GICv2 implementation. And maybe rename it to hyp_gicv3_nr_lr to avoid any confusion. > break; > case -ENODEV: > case -ENXIO: > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > index 06db299c37a8..8bb9362bc284 100644 > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > @@ -128,6 +128,9 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu) > > hyp_vcpu->vcpu.arch.ctxt = host_vcpu->arch.ctxt; > > + /* A guest context must keep a NULL __hyp_running_vcpu. */ > + hyp_vcpu->vcpu.arch.ctxt.__hyp_running_vcpu = NULL; > + > hyp_vcpu->vcpu.arch.mdcr_el2 = host_vcpu->arch.mdcr_el2; > hyp_vcpu->vcpu.arch.hcr_el2 &= ~(HCR_TWI | HCR_TWE); > hyp_vcpu->vcpu.arch.hcr_el2 |= READ_ONCE(host_vcpu->arch.hcr_el2) & > @@ -139,6 +142,12 @@ static void flush_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu) > > hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3 = host_vcpu->arch.vgic_cpu.vgic_v3; > > + /* Bound used_lrs by the number of implemented list registers. */ > + hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3.used_lrs = > + min_t(unsigned int, > + hyp_vcpu->vcpu.arch.vgic_cpu.vgic_v3.used_lrs, > + hyp_vgic_nr_lr); > + > hyp_vcpu->vcpu.arch.pid = host_vcpu->arch.pid; > } > > diff --git a/arch/arm64/kvm/hyp/nvhe/setup.c b/arch/arm64/kvm/hyp/nvhe/setup.c > index d461981616d9..ebc6b4afc336 100644 > --- a/arch/arm64/kvm/hyp/nvhe/setup.c > +++ b/arch/arm64/kvm/hyp/nvhe/setup.c > @@ -20,6 +20,7 @@ > #include > > unsigned long hyp_nr_cpus; > +unsigned int hyp_vgic_nr_lr; You could probably move this close to the place where it is used instead of an unrelated file. Other than that, and with the vcpu-related hunk in its own patch, this looks reasonable. Thanks, M. -- Jazz isn't dead. It just smells funny.