Re: [PATCH 2/2] KVM: arm64: Skip unreset vCPUs in MPIDR lookup table

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `kvm_vcpu_mpidr_is_reset()` check unconditionally evaluates to false due to bitmasking and sign-extension bugs, causing the MPIDR lookup table to remain unpopulated and forcing a slow linear-search fallback for almost all vCPUs.
--

commit 492eed93a0927af5d722a777b1e148b77bf39d93
Author: fuqiang wang <fuqiang.wng@gmail.com>

KVM: arm64: Skip unreset vCPUs in MPIDR lookup table

This commit attempts to fix an issue where unreset vCPUs overwrite the MPIDR
lookup table index for vCPU0 by skipping vCPUs that do not have the RES1 bit
set in their MPIDR_EL1 value.

> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
[ ... ]
> @@ -506,6 +506,15 @@ static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
>  	return __vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;
>  }
>  
> +/*
> + * Check if MPIDR matches hardware reset value. RES1 bits are set after reset,
> + * while a zero default indicates a vCPU that has not been reset.
> + */
> +static inline bool kvm_vcpu_mpidr_is_reset(u64 val)
> +{
> +	return (val & MIDR_RES1_MASK) == MIDR_RES1_MASK;
> +}

[Severity: High]
Does this check always evaluate to false due to sign-extension? MIDR_RES1_MASK
is defined as (1 << 31), which evaluates to a signed 32-bit int. When compared
against the unsigned 64-bit integer val, the mask is sign-extended to
0xffffffff80000000ULL.

Since val only contains the lower 32 bits, the bitwise AND will never match
the sign-extended mask.

[ ... ]
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
[ ... ]
> @@ -884,6 +884,16 @@ static void kvm_init_mpidr_data(struct kvm *kvm)
>  		u64 aff = kvm_vcpu_get_mpidr_aff(vcpu);
>  		u16 index = kvm_mpidr_index(data, aff);
>  
> +		/*
> +		 * In the CPU hot-plug scenario, the kvm_for_each_vcpu() macro
> +		 * iterates over all possible vcpus. For vcpus that are not
> +		 * reset, kvm_vcpu_get_mpidr_aff() returns 0, then the index
> +		 * computed by kvm_mpidr_index() will also be 0, thereby
> +		 * overwriting the original value of vcpu0.
> +		 */
> +		if (!kvm_vcpu_mpidr_is_reset(aff))
> +			continue;

[Severity: High]
Will this unconditionally skip all vCPUs?

The aff argument passed here is obtained from kvm_vcpu_get_mpidr_aff(), which
masks the raw MPIDR value with MPIDR_HWID_BITMASK (0xff00ffffff). This
explicitly clears bits 24-31, stripping the RES1 bit (bit 31) from the
returned value before the check even occurs.

Because the check will always fail, the continue statement executes for
every vCPU, leaving the cmpidr_to_idx table completely zero-initialized.
This causes all MPIDR lookups to fail the cache verification check and
permanently fall back to the slow linear search.

>  		data->cmpidr_to_idx[index] = c;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611090934.84469-1-fuqiang.wng@gmail.com?part=2