From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PART1 RFC 5/9] svm: Add VMEXIT handlers for AVIC Date: Fri, 12 Feb 2016 16:38:26 +0100 Message-ID: <56BDFC72.7030905@redhat.com> References: <1455285574-27892-1-git-send-email-suravee.suthikulpanit@amd.com> <1455285574-27892-6-git-send-email-suravee.suthikulpanit@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, wei@redhat.com, sherry.hurwitz@amd.com To: Suravee Suthikulpanit , joro@8bytes.org, alex.williamson@redhat.com, gleb@kernel.org Return-path: In-Reply-To: <1455285574-27892-6-git-send-email-suravee.suthikulpanit@amd.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org 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();