Re: [PATCH v4 24/32] KVM: x86: Inhibit APICv/AVIC if the optimized physical map is disabled

[Maxim Levitsky]

On Sat, 2022-10-01 at 00:59 +0000, Sean Christopherson wrote:
> Inhibit APICv/AVIC if the optimized physical map is disabled so that KVM
> KVM provides consistent APIC behavior if xAPIC IDs are aliased due to
> vcpu_id being truncated and the x2APIC hotplug hack isn't enabled.  If
> the hotplug hack is disabled, events that are emulated by KVM will follow
> architectural behavior (all matching vCPUs receive events, even if the
> "match" is due to truncation), whereas APICv and AVIC will deliver events
> only to the first matching vCPU, i.e. the vCPU that matches without
> truncation.
> 
> Note, the "extra" inhibit is needed because  KVM deliberately ignores
> mismatches due to truncation when applying the APIC_ID_MODIFIED inhibit
> so that large VMs (>255 vCPUs) can run with APICv/AVIC.
> 
> Fixes: TDB
Do you mean Trade & Development Bank of Mongolia ? ;-)


> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>  arch/x86/include/asm/kvm_host.h |  6 ++++++
>  arch/x86/kvm/lapic.c            | 13 ++++++++++++-
>  arch/x86/kvm/svm/avic.c         |  1 +
>  arch/x86/kvm/vmx/vmx.c          |  1 +
>  4 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index ac28bbfbf0e3..171e38b94c89 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1104,6 +1104,12 @@ enum kvm_apicv_inhibit {
>  	 */
>  	APICV_INHIBIT_REASON_BLOCKIRQ,
>  
> +	/*
> +	 * APICv is disabled because not all vCPUs have a 1:1 mapping between
> +	 * APIC ID and vCPU, _and_ KVM is not applying its x2APIC hotplug hack.
> +	 */
> +	APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED,
> +
>  	/*
>  	 * For simplicity, the APIC acceleration is inhibited
>  	 * first time either APIC ID or APIC base are changed by the guest
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 340c2d3e781b..f6f328d36ae2 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -381,6 +381,16 @@ void kvm_recalculate_apic_map(struct kvm *kvm)
>  		cluster[ldr] = apic;
>  	}
>  out:
> +	/*
> +	 * The optimized map is effectively KVM's internal version of APICv,
Nitpick: APICv/AVIC?
> +	 * and all unwanted aliasing that results in disabling the optimized
> +	 * map also applies to APICv.
> +	 */
> +	if (!new)
> +		kvm_set_apicv_inhibit(kvm, APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED);
> +	else
> +		kvm_clear_apicv_inhibit(kvm, APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED);
> +
>  	old = rcu_dereference_protected(kvm->arch.apic_map,
>  			lockdep_is_held(&kvm->arch.apic_map_lock));
>  	rcu_assign_pointer(kvm->arch.apic_map, new);
> @@ -2153,7 +2163,8 @@ static void kvm_lapic_xapic_id_updated(struct kvm_lapic *apic)
>  	/*
>  	 * 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.
> +	 * value.  If the wrap/truncation results in unwatned aliasing, APICv
                                                       ^^ typo
> +	 * will be inhibited as part of updating KVM's optimized APIC maps.
>  	 */
>  	if (kvm_xapic_id(apic) == (u8)apic->vcpu->vcpu_id)
>  		return;
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index dd0e41d454a7..2908adc79ea6 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -965,6 +965,7 @@ bool avic_check_apicv_inhibit_reasons(enum kvm_apicv_inhibit reason)
>  			  BIT(APICV_INHIBIT_REASON_PIT_REINJ) |
>  			  BIT(APICV_INHIBIT_REASON_BLOCKIRQ) |
>  			  BIT(APICV_INHIBIT_REASON_SEV)      |
> +			  BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED) |
>  			  BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED) |
>  			  BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED) |
>  			  BIT(APICV_INHIBIT_REASON_X2APIC);
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 974d9a366d5d..5920166d7260 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7955,6 +7955,7 @@ static bool vmx_check_apicv_inhibit_reasons(enum kvm_apicv_inhibit reason)
>  			  BIT(APICV_INHIBIT_REASON_ABSENT) |
>  			  BIT(APICV_INHIBIT_REASON_HYPERV) |
>  			  BIT(APICV_INHIBIT_REASON_BLOCKIRQ) |
> +			  BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED) |
>  			  BIT(APICV_INHIBIT_REASON_APIC_ID_MODIFIED) |
>  			  BIT(APICV_INHIBIT_REASON_APIC_BASE_MODIFIED);
>  


Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>


Best regards,
	Maxim Levitsky