Re: [PATCH] Revert "svm: Fix AVIC incomplete IPI emulation"

[Suthikulpanit, Suravee]

Ping

On 3/20/19 3:12 PM, Suthikulpanit, Suravee wrote:
> This reverts commit bb218fbcfaaa3b115d4cd7a43c0ca164f3a96e57.
> 
> As Oren Twaig pointed out the old discussion:
> 
>    https://patchwork.kernel.org/patch/8292231/
> 
> that the change coud potentially cause an extra IPI to be sent to
> the destination vcpu because the AVIC hardware already set the IRR bit
> before the incomplete IPI #VMEXIT with id=1 (target vcpu is not running).
> Since writting to ICR and ICR2 will also set the IRR. If something triggers
> the destination vcpu to get scheduled before the emulation finishes, then
> this could result in an additional IPI.
> 
> Also, the issue mentioned in the commit bb218fbcfaaa was misdiagnosed.
> 
> Cc: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Reported-by: Oren Twaig <oren@xxxxxxxxxxx>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> ---
>   arch/x86/kvm/svm.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index f13a3a24d360..47c4993448c7 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -4512,14 +4512,25 @@ static int avic_incomplete_ipi_interception(struct vcpu_svm *svm)
>   		kvm_lapic_reg_write(apic, APIC_ICR, icrl);
>   		break;
>   	case AVIC_IPI_FAILURE_TARGET_NOT_RUNNING: {
> +		int i;
> +		struct kvm_vcpu *vcpu;
> +		struct kvm *kvm = svm->vcpu.kvm;
>   		struct kvm_lapic *apic = svm->vcpu.arch.apic;
>   
>   		/*
> -		 * Update ICR high and low, then emulate sending IPI,
> -		 * which is handled when writing APIC_ICR.
> +		 * At this point, we expect that the AVIC HW has already
> +		 * set the appropriate IRR bits on the valid target
> +		 * vcpus. So, we just need to kick the appropriate vcpu.
>   		 */
> -		kvm_lapic_reg_write(apic, APIC_ICR2, icrh);
> -		kvm_lapic_reg_write(apic, APIC_ICR, icrl);
> +		kvm_for_each_vcpu(i, vcpu, kvm) {
> +			bool m = kvm_apic_match_dest(vcpu, apic,
> +						     icrl & KVM_APIC_SHORT_MASK,
> +						     GET_APIC_DEST_FIELD(icrh),
> +						     icrl & KVM_APIC_DEST_MASK);
> +
> +			if (m && !avic_vcpu_is_running(vcpu))
> +				kvm_vcpu_wake_up(vcpu);
> +		}
>   		break;
>   	}
>   	case AVIC_IPI_FAILURE_INVALID_TARGET:
>