From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [RFC v1 08/15] Update IRTE according to guest interrupt config changes Date: Thu, 26 Mar 2015 19:59:03 +0000 Message-ID: <55146507.4000707@citrix.com> References: <1427286717-4093-1-git-send-email-feng.wu@intel.com> <1427286717-4093-9-git-send-email-feng.wu@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1427286717-4093-9-git-send-email-feng.wu@intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Feng Wu , xen-devel@lists.xen.org Cc: yang.z.zhang@intel.com, kevin.tian@intel.com, keir@xen.org, JBeulich@suse.com List-Id: xen-devel@lists.xenproject.org On 25/03/15 12:31, Feng Wu wrote: > When guest changes its interrupt configuration (such as, vector, etc.) > for direct-assigned devices, we need to update the associated IRTE > with the new guest vector, so external interrupts from the assigned > devices can be injected to guests without VM-Exit. > > For lowest-priority interrupts, we use vector-hashing mechamisn to find > the destination vCPU. This follows the hardware behavior, since modern > Intel CPUs use vector hashing to handle the lowest-priority interrupt. > > For multicase/broadcast vCPU, we cannot handle it via interrupt posting, > still use interrupt remapping. > > Signed-off-by: Feng Wu > --- > xen/drivers/passthrough/io.c | 77 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 76 insertions(+), 1 deletion(-) > > diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c > index ae050df..1d9a132 100644 > --- a/xen/drivers/passthrough/io.c > +++ b/xen/drivers/passthrough/io.c > @@ -26,6 +26,7 @@ > #include > #include > #include > +#include > > static DEFINE_PER_CPU(struct list_head, dpci_list); > > @@ -199,6 +200,61 @@ void free_hvm_irq_dpci(struct hvm_irq_dpci *dpci) > xfree(dpci); > } > > +/* > + * Here we handle the following cases: > + * - For lowest-priority interrupts, we find the destination vCPU from the > + * guest vector using vector-hashing mechamisn and return true. This follows > + * the hardware behavior, since modern Intel CPUs use vector hashing to > + * handle the lowest-priority interrupt. What is the hashing algorithm, or can I have some hint as to where to find it in a manual? > + * - Otherwise, for single destination interrupt, it is straightforward to > + * find the destination vCPU and return true. > + * - For multicase/broadcast vCPU, we cannot handle it via interrupt posting, > + * so return false. > + */ > +static bool_t pi_find_dest_vcpu(struct domain *d, uint8_t dest_id, > + uint8_t dest_mode, uint8_t deliver_mode, > + uint32_t gvec, struct vcpu **dest_vcpu) > +{ > + struct vcpu *v, **dest_vcpu_array; > + unsigned int dest_vcpu_num = 0; > + int ret; > + > + if ( deliver_mode == dest_LowestPrio ) > + dest_vcpu_array = xzalloc_array(struct vcpu *, d->max_vcpus); This allocation can fail, but you really should see about avoiding it entirely, if possible. > + > + for_each_vcpu ( d, v ) > + { > + if ( !vlapic_match_dest(vcpu_vlapic(v), NULL, 0, > + dest_id, dest_mode) ) > + continue; > + > + dest_vcpu_num++; > + > + if ( deliver_mode == dest_LowestPrio ) > + dest_vcpu_array[dest_vcpu_num] = v; > + else > + *dest_vcpu = v; > + } > + > + if ( deliver_mode == dest_LowestPrio ) > + { > + if ( dest_vcpu_num != 0 ) > + { > + *dest_vcpu = dest_vcpu_array[gvec % dest_vcpu_num]; > + ret = 1; > + } > + else > + ret = 0; > + > + xfree(dest_vcpu_array); > + return ret; > + } > + else if ( dest_vcpu_num == 1 ) > + return 1; > + else > + return 0; > +} > + > int pt_irq_create_bind( > struct domain *d, xen_domctl_bind_pt_irq_t *pt_irq_bind) > { > @@ -257,7 +313,7 @@ int pt_irq_create_bind( > { > case PT_IRQ_TYPE_MSI: > { > - uint8_t dest, dest_mode; > + uint8_t dest, dest_mode, deliver_mode; > int dest_vcpu_id; > > if ( !(pirq_dpci->flags & HVM_IRQ_DPCI_MAPPED) ) > @@ -330,11 +386,30 @@ int pt_irq_create_bind( > /* Calculate dest_vcpu_id for MSI-type pirq migration. */ > dest = pirq_dpci->gmsi.gflags & VMSI_DEST_ID_MASK; > dest_mode = !!(pirq_dpci->gmsi.gflags & VMSI_DM_MASK); > + deliver_mode = (pirq_dpci->gmsi.gflags >> GFLAGS_SHIFT_DELIV_MODE) & > + VMSI_DELIV_MASK; s/deliver/delivery/ Also, you should be able to use MASK_EXTR() rather than manual shifts and masks. > dest_vcpu_id = hvm_girq_dest_2_vcpu_id(d, dest, dest_mode); > pirq_dpci->gmsi.dest_vcpu_id = dest_vcpu_id; > spin_unlock(&d->event_lock); > if ( dest_vcpu_id >= 0 ) > hvm_migrate_pirqs(d->vcpu[dest_vcpu_id]); > + > + /* Use interrupt posting if it is supported */ > + if ( iommu_intpost ) > + { > + struct vcpu *vcpu = NULL; > + > + if ( !pi_find_dest_vcpu(d, dest, dest_mode, deliver_mode, > + pirq_dpci->gmsi.gvec, &vcpu) ) > + break; > + > + if ( pi_update_irte( vcpu, info, pirq_dpci->gmsi.gvec ) != 0 ) > + { > + dprintk(XENLOG_G_INFO, "failed to update PI IRTE\n"); Please put far more information this error message. > + return -EBUSY; Under what circumstances can this happen. I don't think it is valid to fail the userspace bind hypercall in this case. ~Andrew > + } > + } > + > break; > } >