Re: [Xen-devel] [RFC v1 08/15] Update IRTE according to guest interrupt config changes

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@xxxxxxxxxx]
> Sent: Friday, March 27, 2015 3:47 AM
> To: Wu, Feng
> Cc: xen-devel@xxxxxxxxxxxxx; Zhang, Yang Z; Tian, Kevin; keir@xxxxxxx;
> JBeulich@xxxxxxxx
> Subject: Re: [Xen-devel] [RFC v1 08/15] Update IRTE according to guest
> interrupt config changes
> 
> On Wed, Mar 25, 2015 at 08:31:50PM +0800, Feng Wu wrote:
> > When guest changes its interrupt configuration (such as, vector, etc.)
> 
> s/such as,/such as/
> > 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,
> 
> multicase? Or multicast? or multicascade??

multicast. Sorry for the typo.

> > still use interrupt remapping.
> >
> > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
> > ---
> >  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 <asm/hvm/iommu.h>
> >  #include <asm/hvm/support.h>
> >  #include <xen/hvm/irq.h>
> > +#include <asm/io_apic.h>
> >
> >  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
> 
> s/mechamism/mechanism/
> 
> > + *   the hardware behavior, since modern Intel CPUs use vector hashing to
> > + *   handle the lowest-priority interrupt.
> > + * - 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,
> 
> s/multicase/??/
> > + *   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);
> > +
> Please check that dest_vcpu_array was allocated.

Oh, yes, forgot it.

> 
> > +    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;
> 
> Should there be an break here?

We need get 'dest_vcpu_num' after the whole loop, so we can check
whether it is a multicast/broadcast interrupt, so we cannot add break
here.

Thanks,
Feng

> > +    }
> > +
> > +    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)
> >  {
> > @@ -256,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;
> >          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 )
> 
> s/ != 0//
> 
> 
> > +            {
> > +                dprintk(XENLOG_G_INFO, "failed to update PI IRTE\n");
> 
> Perhaps with some data on which domain it is for? And what vector?
> 
> > +                return -EBUSY;
> 
> Hmm.. Under what conditions can this actually happen? What should the
> recepient do?

pi_update_irte() returns false in some cases, such as:
- Cannot find 'struct irq_desc' for the pirq.
- Cannot get 'struct msi_desc' of the 'struct irq_desc'.
- Cannot get 'struct pci_dev' of the 'struct msi_desc'.
- Cannot get drhd structure for the PCI device.
- Cannot get 'struct ir_ctrl' of the iommu engine.

All these are normal checking, I think it seldom returns false. But if it 
happens,
may need more thinking about how to loop back the previous state. Do you have
any ideas about this? Thanks!

Thanks,
Feng


> > +            }
> > +        }
> > +
> >          break;
> >      }
> >
> > --
> > 2.1.0
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel