Re: [PATCH v3 07/27] KVM: arm64: Provide arm64 KVM API for non-native architectures

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The refactoring of `kvm_arm.h` violates the design convention explicitly stated in the commit message.
- [Low] `system_supported_vcpu_features()` is made globally visible without a prototype declaration.
- [Low] Newly extracted headers are not self-contained and lack necessary forward declarations and `#include` directives.
--

commit 106dc17001f0c5affae224d8814a51a51af0225e
Author: Steffen Eiden <seiden@linux.ibm.com>

KVM: arm64: Provide arm64 KVM API for non-native architectures

This patch moves KVM-related headers to include/kvm/arm64/ to decouple them
from the arm64 architecture directory. This allows non-native hosts to consume
the arm64 KVM infrastructure without duplicating code.

> The KVM-related headers are moved to include/kvm/arm64/, decoupling them from
> the arm64 architecture directory. The design convention is that
> architecture-specific headers under <arch>/include/asm/ include from this
> shared location, allowing non-native hosts to consume the arm64 KVM
> infrastructure without duplicating code.

[Severity: Medium]
The commit message states "architecture-specific headers under
<arch>/include/asm/ include from this shared location". Does moving kvm_arm.h
entirely and updating all the source files to include <kvm/arm64/kvm_arm.h>
directly bypass this convention?

Would it be more consistent to leave a wrapper header in
arch/arm64/include/asm/kvm_arm.h as was done for kvm_mmu.h and kvm_host.h?

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 9453321ef8c67..04a4ef1c7f062 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c

[ ... ]

> @@ -1541,7 +1543,7 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level,
>  	return -EINVAL;
>  }
>  
> -static unsigned long system_supported_vcpu_features(void)
> +unsigned long system_supported_vcpu_features(void)
>  {
>  	unsigned long features = KVM_VCPU_VALID_FEATURES;
>  

[Severity: Low]
The function system_supported_vcpu_features() is now globally visible. Should
a prototype be added to a header file to avoid missing prototype compiler
warnings?

> diff --git a/include/kvm/arm64/guest.h b/include/kvm/arm64/guest.h
> new file mode 100644
> index 0000000000000..fa67d992e8fd2
> --- /dev/null
> +++ b/include/kvm/arm64/guest.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __KVM_ARM64_GUEST_H__
> +#define __KVM_ARM64_GUEST_H__
> +
> +/* Implemented by virt/kvm/arm64/guest.c */
> +unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
> +int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
> +
> +#endif /* __KVM_ARM64_GUEST_H__ */

[Severity: Low]
This new header uses types like u64, __user, and struct kvm_vcpu. Should it
include <linux/types.h>, <linux/compiler.h>, and forward-declare
struct kvm_vcpu so it is fully self-contained?

> diff --git a/include/kvm/arm64/kvm_mmu.h b/include/kvm/arm64/kvm_mmu.h
> new file mode 100644
> index 0000000000000..91607105eaf67
> --- /dev/null
> +++ b/include/kvm/arm64/kvm_mmu.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef KVM_ARM64_KVM_MMU_H__
> +#define KVM_ARM64_KVM_MMU_H__
> +
> +/*
> + * We currently support using a VM-specified IPA size. For backward
> + * compatibility, the default IPA size is fixed to 40bits.
> + */
> +#define KVM_PHYS_SHIFT	(40)
> +
> +/*
> + * We are not in the kvm->srcu critical section most of the time, so we take
> + * the SRCU read lock here. Since we copy the data from the user page, we
> + * can immediately drop the lock again.
> + */
> +static inline int kvm_read_guest_lock(struct kvm *kvm,
> +				      gpa_t gpa, void *data, unsigned long len)
> +{

[Severity: Low]
This header uses gpa_t, phys_addr_t, size_t, bool, and srcu functions. Does it
need to include <linux/types.h>, <linux/srcu.h>, and other relevant headers to
ensure it compiles independently?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529155050.2902245-1-seiden@linux.ibm.com?part=7