Re: [PATCH v2 1/2] KVM: selftests: Add a helper to read a vCPU's APIC ID

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] vcpu_get_apic_id() incorrectly computes the APIC ID for x2APIC vCPUs, particularly when KVM_X2APIC_API_USE_32BIT_IDS is enabled, returning 0 for typical APIC IDs and truncating IDs > 255.
--

commit dd16ed0d3dc6ed699be3a1e4ae7b88bd360cc286
Author: Hemanth Selam <hemanth.selam@gmail.com>

KVM: selftests: Add a helper to read a vCPU's APIC ID

This commit adds a helper function to read a vCPU's APIC ID directly from
the host. This prevents tests from having to open code the KVM_GET_LAPIC
ioctl when targeting a vCPU with an IPI.

> diff --git a/tools/testing/selftests/kvm/include/x86/apic.h b/tools/testing/selftests/kvm/include/x86/apic.h
> index 31887bdc3d6c4..293044c81228d 100644
> --- a/tools/testing/selftests/kvm/include/x86/apic.h
> +++ b/tools/testing/selftests/kvm/include/x86/apic.h
> @@ -79,6 +79,15 @@ void apic_disable(void);
>  void xapic_enable(void);
>  void x2apic_enable(void);
>  
> +/* Reads the APIC ID of a vCPU from the host, e.g. to target an IPI at it. */
> +static inline u32 vcpu_get_apic_id(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_lapic_state lapic;
> +
> +	vcpu_ioctl(vcpu, KVM_GET_LAPIC, &lapic);
> +	return GET_APIC_ID_FIELD(*(u32 *)&lapic.regs[APIC_ID]);

[Severity: Medium]
Does this unconditionally apply xAPIC bitwise extraction, failing to account
for the x2APIC format where the ID may be stored unshifted or exceed 8 bits?

If a test invokes vcpu_get_apic_id() on an x2APIC vCPU and the VM has
KVM_X2APIC_API_USE_32BIT_IDS enabled, KVM stores the unshifted 32-bit ID in
the APIC_ID register, but GET_APIC_ID_FIELD shifts it by 24.

If not enabled, KVM shifts the ID by 24, but GET_APIC_ID_FIELD applies an
8-bit mask (0xFF), silently truncating any x2APIC ID > 255.

This incorrect APIC ID computation could cause tests that rely on this
helper for cross-vCPU IPIs to target the wrong vCPU (often ID 0), leading
to test failures or false passes.

> +}
> +
>  static inline u32 get_bsp_flag(void)
>  {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826115915.2882221-1-hemanth.selam@gmail.com?part=1