Re: [PATCH 02/11] KVM: arm64: CCA: Introduce Realms

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] KVM: arm64: premature kvm_arch_has_private_mem() breaks gmem checks and caps
- [Low] KVM: arm64: duplicate kvm_arm_vcpu_finalize() declarations in kvm_host.h
--

commit d7e08ee0dbb02a4cf114f13c026edc843145fe6e
Author: Suzuki K Poulose <suzuki.poulose@arm.com>

KVM: arm64: CCA: Introduce Realms

This commit adds foundational infrastructure for Arm CCA Realm VMs,
including the VM_REALM flavor, a static key for RMI availability,
per-VM Realm state tracking in struct kvm_arch, and initial feature and
memslot checks.

> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index ccd2dc8f73c3a..7d0222047ca1e 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
[ ... ]
> @@ -1572,30 +1584,43 @@ struct kvm *kvm_arch_alloc_vm(void);
[ ... ]
> +#define kvm_vm_is_realm(kvm)		((kvm)->arch.vm_flavor == VM_REALM)
> +#define vcpu_is_rec(vcpu)		kvm_vm_is_realm((vcpu)->kvm)
>  
>  #define kvm_vm_hyp_is_distrusting(kvm)	((kvm)->arch.vm_flavor >= __VM_DISTRUSTING_HYP)
>  
> -#endif	/* __KVM_NVHE_HYPERVISOR__ */
> +#define kvm_arch_has_private_mem(kvm) kvm_vm_is_realm(kvm)

[Severity: High]
Does defining kvm_arch_has_private_mem(kvm) here cause a regression in
guest_memfd memslot validation and capability reporting across arm64?

Because CONFIG_KVM_PRIVATE_MEMATTR_VM is not selected on arm64, defining
kvm_arch_has_private_mem activates the preprocessor branch in
include/linux/kvm_host.h that sets gmem_in_place_conversion to true at
compile time:

include/linux/kvm_host.h:
    #elif defined kvm_arch_has_private_mem
    #define gmem_in_place_conversion true
    #define kvm_is_private_gfn kvm_gmem_is_private_gfn

With gmem_in_place_conversion unconditionally true,
kvm_gmem_prepare_memory_region() in virt/kvm/guest_memfd.c sets
KVM_MEMSLOT_GMEM_ONLY on every guest_memfd memslot even when
GUEST_MEMFD_FLAG_MMAP is not set:

virt/kvm/guest_memfd.c:kvm_gmem_prepare_memory_region() {
    ...
    if (gmem_in_place_conversion || kvm_gmem_supports_mmap(inode))
        slot->flags |= KVM_MEMSLOT_GMEM_ONLY;
}

As a result, the check in kvm_arch_prepare_memory_region() in
arch/arm64/kvm/mmu.c that rejects non-mappable guest_memfd memslots:

arch/arm64/kvm/mmu.c:kvm_arch_prepare_memory_region() {
    ...
    if (kvm_slot_has_gmem(new) && !kvm_memslot_is_gmem_only(new))
        return -EINVAL;
}

is bypassed for non-Realm VMs, allowing userspace to bind a private
guest_memfd created with flags == 0 via KVM_SET_USER_MEMORY_REGION2 and
fault those pages into non-Realm stage-2 page tables through gmem_abort().

In addition, when userspace queries
KVM_CHECK_EXTENSION(KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES) on the /dev/kvm
fd (where kvm is NULL), kvm_vm_ioctl_check_extension_generic() in
virt/kvm/kvm_main.c calls kvm_supports_private_mem(NULL):

virt/kvm/kvm_main.c:kvm_supports_private_mem() {
    return !kvm || kvm_arch_has_private_mem(kvm);
}

virt/kvm/kvm_main.c:kvm_vm_ioctl_check_extension_generic() {
    ...
    case KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES:
        if (!gmem_in_place_conversion || !kvm_supports_private_mem(kvm))
            return 0;

        return KVM_MEMORY_ATTRIBUTE_PRIVATE;
}

This advertises KVM_MEMORY_ATTRIBUTE_PRIVATE on all arm64 hosts even when
KVM_MODE_RMM is not active and while kvm_gmem_set_attributes() in
virt/kvm/guest_memfd.c rejects KVM_SET_MEMORY_ATTRIBUTES2 with -EINVAL on
every creatable VM.

> +
> +int kvm_arm_vcpu_finalize(struct kvm_vcpu *vcpu, int feature);
> +bool kvm_arm_vcpu_is_finalized(struct kvm_vcpu *vcpu);

[Severity: Low]
This isn't a bug, but commit df6a70516adec8 already moved the declarations
of kvm_arm_vcpu_finalize() and kvm_arm_vcpu_is_finalized() earlier in
arch/arm64/include/asm/kvm_host.h right below VCPU_PKVM_FINALIZED:

arch/arm64/include/asm/kvm_host.h:
    #define VCPU_PKVM_FINALIZED	__vcpu_single_flag(cflags, BIT(2))

    int kvm_arm_vcpu_finalize(struct kvm_vcpu *vcpu, int feature);
    bool kvm_arm_vcpu_is_finalized(struct kvm_vcpu *vcpu);

Can these duplicate declarations be dropped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102141304.126883-1-steven.price@arm.com?part=2