Re: [PATCH v4 07/32] KVM: x86: Don't inhibit APICv/AVIC if xAPIC ID mismatch is due to 32-bit ID

[Maxim Levitsky]

On Sat, 2022-10-01 at 00:58 +0000, Sean Christopherson wrote:
> Truncate the vcpu_id, a.k.a. x2APIC ID, to an 8-bit value when comparing
> it against the xAPIC ID to avoid false positives (sort of) on systems
> with >255 CPUs, i.e. with IDs that don't fit into a u8.  The intent of
> APIC_ID_MODIFIED is to inhibit APICv/AVIC when the xAPIC is changed from
> it's original value,
> 
> The mismatch isn't technically a false positive, as architecturally the
> xAPIC IDs do end up being aliased in this scenario, and neither APICv
> nor AVIC correctly handles IPI virtualization when there is aliasing.
> However, KVM already deliberately does not honor the aliasing behavior
> that results when an x2APIC ID gets truncated to an xAPIC ID.  I.e. the
> resulting APICv/AVIC behavior is aligned with KVM's existing behavior
> when KVM's x2APIC hotplug hack is effectively enabled.
> 
> If/when KVM provides a way to disable the hotplug hack, APICv/AVIC can
> piggyback whatever logic disables the optimized APIC map (which is what
> provides the hotplug hack), i.e. so that KVM's optimized map and APIC
> virtualization yield the same behavior.
> 
> For now, fix the immediate problem of APIC virtualization being disabled
> for large VMs, which is a much more pressing issue than ensuring KVM
> honors architectural behavior for APIC ID aliasing.
> 
> Fixes: 3743c2f02517 ("KVM: x86: inhibit APICv/AVIC on changes to APIC ID or APIC base")
> Reported-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> Cc: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>  arch/x86/kvm/lapic.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 251856ba0750..2503f162eb95 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -2078,7 +2078,12 @@ static void kvm_lapic_xapic_id_updated(struct kvm_lapic *apic)
>  	if (KVM_BUG_ON(apic_x2apic_mode(apic), kvm))
>  		return;
>  
> -	if (kvm_xapic_id(apic) == apic->vcpu->vcpu_id)
> +	/*
> +	 * Deliberately truncate the vCPU ID when detecting a modified APIC ID
> +	 * to avoid false positives if the vCPU ID, i.e. x2APIC ID, is a 32-bit
> +	 * value.
> +	 */
> +	if (kvm_xapic_id(apic) == (u8)apic->vcpu->vcpu_id)
>  		return;
>  
>  	kvm_set_apicv_inhibit(apic->vcpu->kvm, APICV_INHIBIT_REASON_APIC_ID_MODIFIED);

Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>

Best regards,
	Maxim Levitsky