Re: [PATCH 1/2] arm64: Add MIDR_EL1 RES1 definitions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The macro `MIDR_RES1_MASK` is defined as `(1 << 31)`, which causes undefined behavior via signed integer overflow and dangerous sign-extension when applied to 64-bit values.
- [High] The commit incorrectly asserts that bit 31 of `MIDR_EL1` is `RES1`. Bit 31 is actually part of the `Implementor` field.
--

commit 8b397f704d83868070b0e6c30f4a5f7c1b4f12e3
Author: fuqiang wang <fuqiang.wng@gmail.com>

arm64: Add MIDR_EL1 RES1 definitions

This patch adds definitions for bit 31 of MIDR_EL1, identifying it as a
RES1 bit.

> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -39,6 +39,8 @@
>  #define MIDR_IMPLEMENTOR_MASK	(0xffU << MIDR_IMPLEMENTOR_SHIFT)
>  #define MIDR_IMPLEMENTOR(midr)	\
>  	(((midr) & MIDR_IMPLEMENTOR_MASK) >> MIDR_IMPLEMENTOR_SHIFT)
> +#define MIDR_RES1_SHIFT		31
[Severity: High]
Does this correctly identify the register?

According to the ARM Architecture Reference Manual, bits [31:24] of MIDR_EL1
represent the 8-bit Implementor code.

Could this be confusing MIDR_EL1 with MPIDR_EL1, where bit 31 is indeed
RES1?

> +#define MIDR_RES1_MASK		(1 << MIDR_RES1_SHIFT)
[Severity: Critical]
Can this cause undefined behavior via signed integer overflow?

The literal 1 is a 32-bit signed integer, so shifting it by 31 overflows.

When this mask is used in kvm_vcpu_mpidr_is_reset(u64 val), it is
sign-extended to a 64-bit value (0xffffffff80000000). The condition
(val & MIDR_RES1_MASK) == MIDR_RES1_MASK would then expect all upper 33 bits
of val to be 1.

Since val is obtained from kvm_vcpu_get_mpidr_aff() which explicitly masks
the register value with MPIDR_HWID_BITMASK and clears bit 31,
kvm_vcpu_mpidr_is_reset() will always evaluate to false.

Will this cause kvm_init_mpidr_data() to unconditionally skip all vCPUs,
leaving the KVM MPIDR-to-vCPU lookup table empty and guests unbootable?

>  
>  #define MIDR_CPU_MODEL(imp, partnum) \
>  	((_AT(u32, imp)		<< MIDR_IMPLEMENTOR_SHIFT) | \

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