Re: [PATCH v5 08/31] KVM: arm64: Use accessor functions for gprs during reset

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The macro `kvm_vcpu_get_vregs` incorrectly applies the address-of operator (`&`) to an array, altering its C semantics.
- [Low] The macro `compat_lr_abt` expands to an undeclared identifier `__compat_lr_ab` instead of `__compat_lr_abt`.
--

--- Patch [8]: [PATCH v5 08/31] KVM: arm64: Use accessor functions for gprs during reset ---
commit 2e02b3a81c0140da07e44b13884114278108bf3e
Author: Steffen Eiden <[email protected]>

KVM: arm64: Use accessor functions for gprs during reset

Instead of accessing the value directly use assessor function that
abstract the actual location of the register values. Enable other
KVM implementations to reuse this code.

> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 1094a5997fc24..4a23d3662d6b3 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -1183,6 +1183,12 @@ static inline u64 *___ctxt_sys_reg(const struct kvm_cpu_context *ctxt, int r)
>  
>  #define ctxt_sys_reg(c,r)	(*__ctxt_sys_reg(c,r))
>  
> +#define kvm_vcpu_get_sp_el1(__vcpu) (__ctxt_sys_reg(&(__vcpu)->arch.ctxt, SP_EL1))
> +#define kvm_vcpu_get_vreg(__vcpu, _n) (&(__vcpu)->arch.ctxt.fp_regs.vregs[_n])
> +#define kvm_vcpu_get_vregs(__vcpu) (&(__vcpu)->arch.ctxt.fp_regs.vregs)

[Severity: Medium]
Does this macro incorrectly apply the address-of operator (&) to the vregs
array, altering its C semantics?

If a developer uses this macro with sizeof(), for example:
memset(kvm_vcpu_get_vregs(vcpu), 0, sizeof(kvm_vcpu_get_vregs(vcpu)))

sizeof() will evaluate to 8 bytes (the size of a pointer) instead of the
actual array size (512 bytes). This can lead to a silent partial memory
operation.

Could this accessor provide unexpected pointer-to-array semantics rather
than decaying to a normal pointer?

> +#define kvm_vcpu_get_fpsr(__vcpu) (&(__vcpu)->arch.ctxt.fp_regs.fpsr)
> +#define kvm_vcpu_get_fpcr(__vcpu) (&(__vcpu)->arch.ctxt.fp_regs.fpcr)
> +
>  u64 kvm_vcpu_apply_reg_masks(const struct kvm_vcpu *, enum vcpu_sysreg, u64);

I also noticed an issue in arch/arm64/include/asm/ptrace.h introduced in the
patch "arm64: ptrace: Use constants for compat register numbers":

arch/arm64/include/asm/ptrace.h:
> #define compat_lr_abt	regs[__compat_lr_ab]

[Severity: Low]
Can the compat_lr_abt macro fail to compile? It expands to an undeclared
identifier __compat_lr_ab instead of __compat_lr_abt.

Any code attempting to use the compat_lr_abt macro will fail to compile because
__compat_lr_ab is undefined.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8