All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] KVM: arm64: Fix MPIDR lookup for unreset vCPUs
@ 2026-06-11  9:09 fuqiang wang
  2026-06-11  9:09 ` [PATCH 1/2] arm64: Add MIDR_EL1 RES1 definitions fuqiang wang
  2026-06-11  9:09 ` [PATCH 2/2] KVM: arm64: Skip unreset vCPUs in MPIDR lookup table fuqiang wang
  0 siblings, 2 replies; 6+ messages in thread
From: fuqiang wang @ 2026-06-11  9:09 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Zenghui Yu, linux-kernel, kvmarm
  Cc: dongxu zhang, wangfuqiang49

From: wangfuqiang49 <wangfuqiang49@jd.com>

Hi,

This series fixes an MPIDR lookup issue when a VM is created with CPU
hotplug support.

kvm_init_mpidr_data() builds a compressed MPIDR-to-vCPU lookup table by
walking all possible vCPUs. However, vCPUs that have not been reset yet
still have a zero MPIDR_EL1 state, which aliases vCPU0. This can cause
cmpidr_to_idx[0] to be overwritten with the index of an unreset vCPU.

As a result, MPIDR 0 lookups can return the wrong vCPU, preventing
interrupts targeting vCPU0 from being delivered correctly and making
guest boot extremely slow in configurations using CPU hotplug.

Patch 1 adds named definitions for the MIDR_EL1 RES1 bit used to detect
whether a vCPU's MPIDR_EL1 state has been reset.
Patch 2 skips unreset vCPUs when populating the compressed MPIDR lookup
table.

Thanks,
Fuqiang

fuqiang wang (2):
  arm64: Add MIDR_EL1 RES1 definitions
  KVM: arm64: Skip unreset vCPUs in MPIDR lookup table

 arch/arm64/include/asm/cputype.h     |  2 ++
 arch/arm64/include/asm/kvm_emulate.h |  9 +++++++++
 arch/arm64/kvm/arm.c                 | 10 ++++++++++
 3 files changed, 21 insertions(+)

-- 
2.47.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] arm64: Add MIDR_EL1 RES1 definitions
  2026-06-11  9:09 [PATCH 0/2] KVM: arm64: Fix MPIDR lookup for unreset vCPUs fuqiang wang
@ 2026-06-11  9:09 ` fuqiang wang
  2026-06-11  9:18   ` sashiko-bot
  2026-06-11  9:09 ` [PATCH 2/2] KVM: arm64: Skip unreset vCPUs in MPIDR lookup table fuqiang wang
  1 sibling, 1 reply; 6+ messages in thread
From: fuqiang wang @ 2026-06-11  9:09 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Zenghui Yu, linux-kernel, kvmarm
  Cc: dongxu zhang, wangfuqiang49, fuqiang wang

MIDR_EL1 bit 31 is RES1. Add named definitions for the bit.

Signed-off-by: fuqiang wang <fuqiang.wng@gmail.com>
---
 arch/arm64/include/asm/cputype.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 7b518e81dd15..4322b219f79a 100644
--- 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
+#define MIDR_RES1_MASK		(1 << MIDR_RES1_SHIFT)
 
 #define MIDR_CPU_MODEL(imp, partnum) \
 	((_AT(u32, imp)		<< MIDR_IMPLEMENTOR_SHIFT) | \
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] KVM: arm64: Skip unreset vCPUs in MPIDR lookup table
  2026-06-11  9:09 [PATCH 0/2] KVM: arm64: Fix MPIDR lookup for unreset vCPUs fuqiang wang
  2026-06-11  9:09 ` [PATCH 1/2] arm64: Add MIDR_EL1 RES1 definitions fuqiang wang
@ 2026-06-11  9:09 ` fuqiang wang
  2026-06-11  9:28   ` sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: fuqiang wang @ 2026-06-11  9:09 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Zenghui Yu, linux-kernel, kvmarm
  Cc: dongxu zhang, wangfuqiang49, fuqiang wang

When a VM is created with CPU hotplug support, kvm_for_each_vcpu() can
walk vCPUs that have not been reset yet. Their MPIDR_EL1 state is still
zero, which aliases vCPU0 when populating the compressed MPIDR lookup
table.

As a result, cmpidr_to_idx[0] can be overwritten with the index of an
unreset vCPU. A lookup for MPIDR 0 then returns the wrong vCPU, which
can prevent interrupts targeting vCPU0 from being delivered correctly
and make guest boot extremely slow.

Skip vCPUs whose MPIDR_EL1 value does not have the RES1 bit set.

Fixes: 5544750efd51 ("KVM: arm64: Build MPIDR to vcpu index cache at runtime")
Signed-off-by: fuqiang wang <fuqiang.wng@gmail.com>
---
 arch/arm64/include/asm/kvm_emulate.h |  9 +++++++++
 arch/arm64/kvm/arm.c                 | 10 ++++++++++
 2 files changed, 19 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 5bf3d7e1d92c..8158e88aaa4e 100644
--- 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;
+}
+
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
 {
 	if (vcpu_mode_is_32bit(vcpu)) {
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 3732ee9eb0d4..b47088201683 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -890,6 +890,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;
+
 		data->cmpidr_to_idx[index] = c;
 	}
 
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] arm64: Add MIDR_EL1 RES1 definitions
  2026-06-11  9:09 ` [PATCH 1/2] arm64: Add MIDR_EL1 RES1 definitions fuqiang wang
@ 2026-06-11  9:18   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-06-11  9:18 UTC (permalink / raw)
  To: fuqiang wang; +Cc: Oliver Upton, kvmarm, Marc Zyngier

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] KVM: arm64: Skip unreset vCPUs in MPIDR lookup table
  2026-06-11  9:09 ` [PATCH 2/2] KVM: arm64: Skip unreset vCPUs in MPIDR lookup table fuqiang wang
@ 2026-06-11  9:28   ` sashiko-bot
  2026-06-11 10:23     ` fuqiang wang
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-06-11  9:28 UTC (permalink / raw)
  To: fuqiang wang; +Cc: Oliver Upton, Marc Zyngier, kvmarm

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] KVM: arm64: Skip unreset vCPUs in MPIDR lookup table
  2026-06-11  9:28   ` sashiko-bot
@ 2026-06-11 10:23     ` fuqiang wang
  0 siblings, 0 replies; 6+ messages in thread
From: fuqiang wang @ 2026-06-11 10:23 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Oliver Upton, Marc Zyngier, kvmarm



On 6/11/26 5:28 PM, sashiko-bot@kernel.org wrote:
> 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;
>>   	}
> 

I'm very sorry. I made a very low-level mistake and will fix it in the next
patch and also do sufficient checks and tests before sending out the patch...

fuqiang


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-11 10:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-11  9:09 [PATCH 0/2] KVM: arm64: Fix MPIDR lookup for unreset vCPUs fuqiang wang
2026-06-11  9:09 ` [PATCH 1/2] arm64: Add MIDR_EL1 RES1 definitions fuqiang wang
2026-06-11  9:18   ` sashiko-bot
2026-06-11  9:09 ` [PATCH 2/2] KVM: arm64: Skip unreset vCPUs in MPIDR lookup table fuqiang wang
2026-06-11  9:28   ` sashiko-bot
2026-06-11 10:23     ` fuqiang wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.