Re: [PATCH v18 05/23] KVM: arm64: Track the type of VM in kvm_arch

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

On 17/09/2026 12:24, Fuad Tabba wrote:
> Hi Suzuki,
> 
> On Tue, 15 Sep 2026 17:01:23 +0100, Suzuki K Poulose
> <suzuki.poulose@arm.com> wrote:
> [...]
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> [...]
>> +#define kvm_vm_hyp_is_pkvm(kvm)                (is_protected_kvm_enabled())
> 
> I'd keep is_protected_kvm_enabled() at the three call sites: this
> takes a VM and ignores it, so it reads as per-VM when it's the mode,
> and patch 20's comment about not being able to use the kvm_vm_ helpers
> with a NULL kvm goes away too.

Actually, that comment is invalid. kvm_arch_vm_ioctl_allowed, is only
called from the kvm_arch_vm_ioctl() with a valid kvm instance. I have 
changed that hunk to:


+/*
+ * Check whether the KVM VM IOCTL is allowed. For pKVM and Realm VMs, 
certain
+ * ioctls are not allowed. Further, certain features are allowed only for
+ * non-protected VMs in pKVM.
+ */
+static inline bool kvm_arch_vm_ioctl_allowed(struct kvm *kvm, unsigned 
int ioctl)
+{
+       long ext;
+       int r;
+
+       /*
+        * We are guaranteed to be called with a valid kvm instance, as the
+        * only caller is kvm_arch_vm_ioctl(). Catch any deviations, as we
+        * rely on the kvm instance below.
+        */
+       if (WARN_ON_ONCE(!kvm))
+               return false;
+
+       /* Cover both pKVM host and Realm VMs */
+       if (!kvm_vm_hyp_is_distrusting(kvm))
+               return true;
+

I resorted to is_protected_kvm_enabled() to make it faster than using
the vm_flavor checks. I could rever those anyways.

> 
>> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> [...]
>> @@ -432,7 +432,7 @@ static void init_pkvm_hyp_vm(struct kvm *host_kvm, struct pkvm_hyp_vm *hyp_vm,
>>
>>           hyp_vm->host_kvm = host_kvm;
>>           hyp_vm->kvm.created_vcpus = nr_vcpus;
>> -        hyp_vm->kvm.arch.pkvm.is_protected = READ_ONCE(host_kvm->arch.pkvm.is_protected);
>> +        hyp_vm->kvm.arch.vm_flavor = READ_ONCE(host_kvm->arch.vm_flavor);
> 
> Could EL2 reduce this to VM_PKVM or VM_PROTECTED_PKVM here instead of
> storing what it reads? The host supplies the value, and it's now an
> enum rather than a bool. Patch 12 relies on EL2 only ever seeing those
> two, and with VM_REALM above the marker a host-written value passes
> kvm_vm_is_protected() but not the == form, if EL2 ever adds one.

Do you mean something like :

	if (kvm_vm_is_protected_pkvm(host_kvm))
		hyp_vm->kvm.arch.vm_flavor = VM_PROTECTED_PKVM;
	else
		hyp_vm->kvm.arch.vm_flavor = VM_PKVM;

Do we additionally need to WARN_ON() if we encounter a VM_REALM ?

> 
> Both hunks are new since v17, which is where I gave the Reviewed-by.
> Happy to keep it if you can settle these please.

Apologies, I thought this one didn't change, but later patches changes 
them, which is why I kept this.

Cheers
Suzuki