Re: [PATCH v17 05/20] KVM: arm64: Add vcpu load/put call backs for flavors

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 13/09/2026 11:26, Marc Zyngier wrote:
> On Tue, 08 Sep 2026 17:22:08 +0100,
> Suzuki K Poulose <suzuki.poulose@arm.com> 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 <maz@kernel.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   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;

...

>> 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 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().

Ack, removed everywhere except for VHE.

> 
>> +	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().

Ack

> 
>> +}
>> +

...

>> +
>> +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().

Ack

> 
>> +	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.

Done

> 
>> +}
>> +
>> +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 *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...

I have switched to a macro initialiser, consistent with vm_s2_mmu_ops. 
So it looks much cleaner

Cheers
Suzuki