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();

Reply via email to