From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 05BD23E4C7F; Sun, 13 Sep 2026 10:26:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789295220; cv=none; b=iFZKM5U1Um0ubd/wl6DxZLOazu/WepAfrwC4GCxdp1+VuKu72f9QZ+hsPE847dnY4oZNXGw11dukGZa/DoVC8N4VtxpDA+TaRjSzZEeAKBaQwH80Zve5eLlHKJ1bsKlp9vPRbOJ42u5HvO54mGe0GxDs49DJlK22othO7ujnVp4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789295220; c=relaxed/simple; bh=ZLX8NOH1OUjVKH2KjlT+1+vvXSxFp3Df20ZPxwvYFCw=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=jct6SrrCItim2rvPrw9xKSbh10gTtRhyODSPc9cdw0UV1q4Y5vYEGEmOxBCqnSe76MZPH7NQNSDHVLGYyeF/in9GK4ZHw3lcCOyzra3Z1+4bw+hITztJsmUYMFyi9HFApvYH2uBUDmL9meggoiNQu+Kl03wkaVBNtnMvfWhs3Ug= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=HxcoM8ZX; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="HxcoM8ZX" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 20E9A1F000FF; Sun, 13 Sep 2026 10:26:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1789295206; bh=CCyNmu1S4QSxghM5bhyr4PoubfLL8i9XAe0yigjLH6Y=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=HxcoM8ZXxZaNZrIj6+HVVWW1vHAG6K1S39AqGPxOTk0cJ2yrto7ofHs/WDrKv30n0 8qd9TV37KTpXM9g/QdqQlp/GQAbfxi3l3g2wvy/BZJLOQoGHNHjJmzdWMtb4bZFs7J N2tR268wkupaNarLFzulRr8hj1RAXL2Zk/WC+rc0+EoKYwYoPf4NtNLY7NRdX2MdZP tpyk8+nh8A+CQ2+mWVHf1IZog4NM7nEra4hS5sjJVE1cZjNPoBXMbVgaNujyFAgeu/ v5roWW+0X+HDJLoyS3Qo+5xqSw6PUbD6uR46WaLCcuak1FhJ0py0M5tWiJLVGQJc8I aZENK3W1L1Svw== 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 1x5hQ3-00000008GZK-37Ow; Sun, 13 Sep 2026 10:26:43 +0000 Date: Sun, 13 Sep 2026 11:26:43 +0100 Message-ID: <867bkp78gs.wl-maz@kernel.org> From: Marc Zyngier To: Suzuki K Poulose Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev, will@kernel.org, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, steven.price@arm.com, aneesh.kumar@kernel.org, oupton@kernel.org, gshan@redhat.com, joey.gouly@arm.com, tabba@google.com, yuzenghui@huawei.com, linux-coco@lists.linux.dev, gankulkarni@os.amperecomputing.com, sdonthineni@nvidia.com, alpergun@google.com, fj0570is@fujitsu.com, WeiLin.Chang@arm.com, lpieralisi@kernel.org, enju.kohei@fujitsu.com Subject: Re: [PATCH v17 05/20] KVM: arm64: Add vcpu load/put call backs for flavors In-Reply-To: <20260908162223.1683432-6-suzuki.poulose@arm.com> References: <20260908162223.1683432-1-suzuki.poulose@arm.com> <20260908162223.1683432-6-suzuki.poulose@arm.com> 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) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: 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: suzuki.poulose@arm.com, kvm@vger.kernel.org, kvmarm@lists.linux.dev, will@kernel.org, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, steven.price@arm.com, aneesh.kumar@kernel.org, oupton@kernel.org, gshan@redhat.com, joey.gouly@arm.com, tabba@google.com, yuzenghui@huawei.com, linux-coco@lists.linux.dev, gankulkarni@os.amperecomputing.com, sdonthineni@nvidia.com, alpergun@google.com, fj0570is@fujitsu.com, WeiLin.Chang@arm.com, lpieralisi@kernel.org, enju.kohei@fujitsu.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Tue, 08 Sep 2026 17:22:08 +0100, Suzuki K Poulose wrote: > > Add VM flavor specific handlers for VCPU load/put, in an effort to make it > easier to follow the code. > > Based on a patch by Marc Zyngier > > Suggested-by: Marc Zyngier > Signed-off-by: Suzuki K Poulose > --- > arch/arm64/include/asm/kvm_host.h | 6 ++ > arch/arm64/kvm/arm.c | 156 ++++++++++++++++++++++-------- > 2 files changed, 123 insertions(+), 39 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index d0dccc9ad6aa8..b2e99c5cb1cd3 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -150,6 +150,11 @@ struct kvm_vmid { > atomic64_t id; > }; > > +struct kvm_vcpu_ops { > + void (*vcpu_load)(struct kvm_vcpu *vcpu, int cpu); > + void (*vcpu_put)(struct kvm_vcpu *vcpu); > +}; > + > struct kvm_s2_mmu { > struct kvm_vmid vmid; > > @@ -854,6 +859,7 @@ struct vncr_tlb; > > struct kvm_vcpu_arch { > struct kvm_cpu_context ctxt; > + const struct kvm_vcpu_ops *vcpu_ops; > > /* > * Guest floating point state > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 51fc651267157..9af3bbb2f8c24 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -74,6 +74,8 @@ struct kvm_ioctl_cap_map { > long ext; > }; > > +static const struct kvm_vcpu_ops *arm64_vcpu_ops[VM_FLAVOR_MAX]; > + > /* Make KVM_CAP_NR_VCPUS the reference for features we always supported */ > #define KVM_CAP_ARM_BASIC KVM_CAP_NR_VCPUS > > @@ -569,6 +571,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) > mutex_unlock(&vcpu->mutex); > #endif > > + vcpu->arch.vcpu_ops = arm64_vcpu_ops[vcpu->kvm->arch.vm_flavor]; > + > /* Force users to call KVM_ARM_VCPU_INIT */ > vcpu_clear_flag(vcpu, VCPU_INITIALIZED); > > @@ -738,36 +742,72 @@ static void vcpu_load_pvtime(struct kvm_vcpu *vcpu) > kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu); > } > > +static void vhe_vcpu_load(struct kvm_vcpu *vcpu, int cpu) > +{ > + vcpu_prepare_mmu(vcpu); > + /* > + * The timer must be loaded before the vgic to correctly set up physical > + * interrupt deactivation in nested state (e.g. timer interrupt). > + */ > + kvm_timer_vcpu_load(vcpu); > + kvm_vgic_load(vcpu); > + kvm_vcpu_load_debug(vcpu); > + kvm_vcpu_load_fgt(vcpu); > + kvm_vcpu_load_vhe(vcpu); > + kvm_arch_vcpu_load_fp(vcpu); > + kvm_vcpu_pmu_restore_guest(vcpu); > + > + vcpu_load_pvtime(vcpu); > + vcpu_set_wfx_traps(vcpu); > + vcpu_set_pauth_traps(vcpu); > +} > + > +static void nvhe_vcpu_load(struct kvm_vcpu *vcpu, int cpu) > +{ > + vcpu_prepare_mmu(vcpu); > + /* > + * The timer must be loaded before the vgic to correctly set up physical > + * interrupt deactivation in nested state (e.g. timer interrupt). > + */ This comment makes no sense here -- it is strictly for VHE, which is the only mode to implement NV. Same thing for the pKVM vcpu_load(). > + kvm_timer_vcpu_load(vcpu); > + kvm_vgic_load(vcpu); > + kvm_vcpu_load_debug(vcpu); > + kvm_vcpu_load_fgt(vcpu); > + kvm_arch_vcpu_load_fp(vcpu); > + kvm_vcpu_pmu_restore_guest(vcpu); > + > + vcpu_load_pvtime(vcpu); > + vcpu_set_wfx_traps(vcpu); > + vcpu_set_pauth_traps(vcpu); > +} > + > +static void pkvm_vcpu_load(struct kvm_vcpu *vcpu, int cpu) > +{ > + /* > + * The timer must be loaded before the vgic to correctly set up physical > + * interrupt deactivation in nested state (e.g. timer interrupt). > + */ > + kvm_timer_vcpu_load(vcpu); > + kvm_vgic_load(vcpu); > + kvm_vcpu_load_debug(vcpu); > + kvm_vcpu_load_fgt(vcpu); > + kvm_arch_vcpu_load_fp(vcpu); > + kvm_vcpu_pmu_restore_guest(vcpu); > + > + vcpu_load_pvtime(vcpu); > + vcpu_set_wfx_traps(vcpu); > + > + kvm_call_hyp_nvhe(__pkvm_vcpu_load, > + vcpu->kvm->arch.pkvm.handle, > + vcpu->vcpu_idx, vcpu->arch.hcr_el2); > + kvm_call_hyp(__vgic_v3_restore_vmcr_aprs, > + &vcpu->arch.vgic_cpu.vgic_v3); This can also be turned into a kvm_call_hyp_nvhe(). > +} > + > void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) > { > - if (!is_protected_kvm_enabled()) > - vcpu_prepare_mmu(vcpu); > - > vcpu->cpu = cpu; > - /* > - * The timer must be loaded before the vgic to correctly set up physical > - * interrupt deactivation in nested state (e.g. timer interrupt). > - */ > - kvm_timer_vcpu_load(vcpu); > - kvm_vgic_load(vcpu); > - kvm_vcpu_load_debug(vcpu); > - kvm_vcpu_load_fgt(vcpu); > - if (has_vhe()) > - kvm_vcpu_load_vhe(vcpu); > - kvm_arch_vcpu_load_fp(vcpu); > - kvm_vcpu_pmu_restore_guest(vcpu); > - > - vcpu_load_pvtime(vcpu); > - vcpu_set_wfx_traps(vcpu); > - vcpu_set_pauth_traps(vcpu); > - > - if (is_protected_kvm_enabled()) { > - kvm_call_hyp_nvhe(__pkvm_vcpu_load, > - vcpu->kvm->arch.pkvm.handle, > - vcpu->vcpu_idx, vcpu->arch.hcr_el2); > - kvm_call_hyp(__vgic_v3_restore_vmcr_aprs, > - &vcpu->arch.vgic_cpu.vgic_v3); > - } > + vcpu->arch.vcpu_ops->vcpu_load(vcpu, cpu); > > if (!cpumask_test_cpu(cpu, vcpu->kvm->arch.supported_cpus)) > vcpu_set_on_unsupported_cpu(vcpu); > @@ -775,28 +815,44 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) > vcpu->arch.pid = pid_nr(vcpu->pid); > } > > -void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) > +static void vhe_vcpu_put(struct kvm_vcpu *vcpu) > { > - if (is_protected_kvm_enabled()) { > - kvm_call_hyp(__vgic_v3_save_aprs, &vcpu->arch.vgic_cpu.vgic_v3); > - kvm_call_hyp_nvhe(__pkvm_vcpu_put); > - > - /* __pkvm_vcpu_put implies a sync of the state */ > - if (!kvm_vm_is_protected(vcpu->kvm)) > - vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY); > - } > - > kvm_vcpu_put_debug(vcpu); > kvm_arch_vcpu_put_fp(vcpu); > - if (has_vhe()) > - kvm_vcpu_put_vhe(vcpu); > + kvm_vcpu_put_vhe(vcpu); > kvm_timer_vcpu_put(vcpu); > kvm_vgic_put(vcpu); > kvm_vcpu_pmu_restore_host(vcpu); > if (vcpu_has_nv(vcpu)) > kvm_vcpu_put_hw_mmu(vcpu); > kvm_arm_vmid_clear_active(); > +} > > +static void nvhe_vcpu_put(struct kvm_vcpu *vcpu) > +{ > + kvm_vcpu_put_debug(vcpu); > + kvm_arch_vcpu_put_fp(vcpu); > + kvm_timer_vcpu_put(vcpu); > + kvm_vgic_put(vcpu); > + kvm_vcpu_pmu_restore_host(vcpu); > + kvm_arm_vmid_clear_active(); > +} > + > +static void pkvm_vcpu_put(struct kvm_vcpu *vcpu) > +{ > + kvm_call_hyp(__vgic_v3_save_aprs, &vcpu->arch.vgic_cpu.vgic_v3); Same thing here about kvm_call_hyp_nvhe(). > + kvm_call_hyp_nvhe(__pkvm_vcpu_put); > + > + /* __pkvm_vcpu_put implies a sync of the state */ > + if (!kvm_vm_is_protected(vcpu->kvm)) > + vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY); > + > + nvhe_vcpu_put(vcpu); I'm not overly fond of this. Yes, that was in my original patch. But for example, we end-up calling kvm_arm_vmid_clear_active() for pKVM. This is harmless, but conceptually wrong. I'd rather you expand the whole thing. > +} > + > +void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) > +{ > + vcpu->arch.vcpu_ops->vcpu_put(vcpu); > vcpu_clear_on_unsupported_cpu(vcpu); > vcpu->cpu = -1; > } > @@ -2136,6 +2192,28 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) > } > } > > +static const struct kvm_vcpu_ops vhe_vcpu_ops = { > + .vcpu_load = vhe_vcpu_load, > + .vcpu_put = vhe_vcpu_put, > +}; > + > +static const struct kvm_vcpu_ops nvhe_vcpu_ops = { > + .vcpu_load = nvhe_vcpu_load, > + .vcpu_put = nvhe_vcpu_put, > +}; > + > +static const struct kvm_vcpu_ops pkvm_vcpu_ops = { > + .vcpu_load = pkvm_vcpu_load, > + .vcpu_put = pkvm_vcpu_put, > +}; > + > +static const struct kvm_vcpu_ops *arm64_vcpu_ops[] = { > + [VM_VHE] = &vhe_vcpu_ops, > + [VM_NVHE] = &nvhe_vcpu_ops, > + [VM_PKVM] = &pkvm_vcpu_ops, > + [VM_PROTECTED_PKVM] = &pkvm_vcpu_ops, nit: my OCD-self wants to align all the '=' signs vertically... Thanks, M. -- Without deviation from the norm, progress is not possible.