Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC

[Paolo Bonzini]

On 12/02/2016 14:59, Suravee Suthikulpanit wrote:
> +		 "icrh:icrl=%#010x:%08x, id=%u, index=%u\n",
> +		 __func__, svm->vcpu.cpu, svm->vcpu.vcpu_id,
> +		 icrh, icrl, id, index);
> +
> +	switch (id) {
> +	case AVIC_INCMP_IPI_ERR_INVALID_INT_TYPE:
> +		/*
> +		 * AVIC hardware handles the generation of
> +		 * IPIs when the specified Message Type is Fixed
> +		 * (also known as fixed delivery mode) and
> +		 * the Trigger Mode is edge-triggered. The hardware
> +		 * also supports self and broadcast delivery modes
> +		 * specified via the Destination Shorthand(DSH)
> +		 * field of the ICRL. Logical and physical APIC ID
> +		 * formats are supported. All other IPI types cause
> +		 * a #VMEXIT, which needs to emulated.
> +		 */
> +		kvm_lapic_reg_write(apic, APIC_ICR2, icrh);
> +		kvm_lapic_reg_write(apic, APIC_ICR, icrl);
> +		break;
> +	case AVIC_INCMP_IPI_ERR_TARGET_NOT_RUN:
> +		kvm_lapic_reg_write(apic, APIC_ICR2, icrh);
> +		kvm_lapic_reg_write(apic, APIC_ICR, icrl);

Wouldn't this cause a double injection of the IPI if the following happens:

1) destination 1 is running, so the processor sets IRR and sends a
doorbell message

2) destination 2 is not running, so the processor sets IRR and exits

3) destination 1 processes the interrupt, moving it from IRR to ISR

4) destination 1 sends an EOI

5) the source exits and reinjects the interrupt

6) destination 1 then receives the interrupt again.


Or alternatively:

1) destination 1 is not running, so the processor sets IRR and exits

2) another CPU executes VMRUN for destination 1, so the processor
injects the interrupt

3) destination 1 sends an EOI

4) the source exits and reinjects the interrupt

5) destination 1 then receives the interrupt again.


The handling of races for IsRunning and incomplete IPIs has always been
very confusing to me whenever I read the AVIC specification.  It would
be great if you could clarify this.

Paolo

> +		break;
> +	case AVIC_INCMP_IPI_ERR_INV_TARGET:
> +		pr_err("SVM: %s: Invalid IPI target (icr=%#08x:%08x, idx=%u)\n",
> +		       __func__, icrh, icrl, index);
> +		BUG();
> +		break;
> +	case AVIC_INCMP_IPI_ERR_INV_BK_PAGE:
> +		pr_err("SVM: %s: Invalid bk page (icr=%#08x:%08x, idx=%u)\n",
> +		       __func__, icrh, icrl, index);
> +		BUG();