[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v5 24/30] ARM: vITS: handle MOVI command



On Thu, 6 Apr 2017, Andre Przywara wrote:
> On 06/04/17 01:47, Stefano Stabellini wrote:
> > On Thu, 6 Apr 2017, Andre Przywara wrote:
> >> The MOVI command moves the interrupt affinity from one redistributor
> >> (read: VCPU) to another.
> >> For now migration of "live" LPIs is not yet implemented, but we store
> >> the changed affinity in the host LPI structure and in our virtual ITTE.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> >> ---
> >>  xen/arch/arm/gic-v3-its.c        | 24 ++++++++++++++++++++
> >>  xen/arch/arm/gic-v3-lpi.c        | 15 +++++++++++++
> >>  xen/arch/arm/vgic-v3-its.c       | 47 
> >> ++++++++++++++++++++++++++++++++++++++++
> >>  xen/include/asm-arm/gic_v3_its.h |  4 ++++
> >>  4 files changed, 90 insertions(+)
> >>
> >> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> >> index d970119..a57e63a 100644
> >> --- a/xen/arch/arm/gic-v3-its.c
> >> +++ b/xen/arch/arm/gic-v3-its.c
> >> @@ -851,6 +851,30 @@ struct pending_irq *gicv3_assign_guest_event(struct 
> >> domain *d,
> >>      return pirq;
> >>  }
> >>  
> >> +/* Changes the target VCPU for a given host LPI assigned to a domain. */
> >> +int gicv3_lpi_change_vcpu(struct domain *d, paddr_t vdoorbell,
> >> +                          uint32_t vdevid, uint32_t veventid,
> >> +                          unsigned int vcpu_id)
> >> +{
> >> +    uint32_t host_lpi;
> >> +    struct its_devices *dev;
> >> +
> >> +    spin_lock(&d->arch.vgic.its_devices_lock);
> >> +    dev = get_its_device(d, vdoorbell, vdevid);
> >> +    if ( dev )
> >> +        host_lpi = get_host_lpi(dev, veventid);
> >> +    else
> >> +        host_lpi = 0;
> >> +    spin_unlock(&d->arch.vgic.its_devices_lock);
> >> +
> >> +    if ( !host_lpi )
> >> +        return -ENOENT;
> >> +
> >> +    gicv3_lpi_update_host_vcpuid(host_lpi, vcpu_id);
> > 
> > we need to call vgic_migrate_irq
> 
> Mmmh, are you sure? The very first statement there reads:
> 
>     /* nothing to do for virtual interrupts */
>     if ( p->desc == NULL )
>         return;
> 
> Also my understanding of this command is that it only affects future
> MSIs, so ITS translations. If an MSI has already been translated to a
> certain redistributor and the ITS signaled it already, this command has
> no effect on this particular one.
> Also we don't care about benign races, so if this command comes in a tad
> to late for a just happening MSI, this is the same problem on real hardware.

It is more complicated than I thought. I would add a in-code comment
here like this:

  TODO: we do not change physical irq affinity, in response to a virtual
  movi command. In other words, the physical LPI will still be delivered
  to the same pcpu.

In addition to the comment, I wouldn't mind if also a warning was
printed (with gdprintk).


> >> +    return 0;
> >> +}
> >> +
> >>  /* Scan the DT for any ITS nodes and create a list of host ITSes out of 
> >> it. */
> >>  void gicv3_its_dt_init(const struct dt_device_node *node)
> >>  {
> >> diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
> >> index c997ed5..b9960aa 100644
> >> --- a/xen/arch/arm/gic-v3-lpi.c
> >> +++ b/xen/arch/arm/gic-v3-lpi.c
> >> @@ -234,6 +234,21 @@ void gicv3_lpi_update_host_entry(uint32_t host_lpi, 
> >> int domain_id,
> >>      write_u64_atomic(&hlpip->data, hlpi.data);
> >>  }
> >>  
> >> +int gicv3_lpi_update_host_vcpuid(uint32_t host_lpi, unsigned int vcpu_id)
> >> +{
> >> +    union host_lpi *hlpip;
> >> +
> >> +    ASSERT(host_lpi >= LPI_OFFSET);
> >> +
> >> +    host_lpi -= LPI_OFFSET;
> >> +
> >> +    hlpip = &lpi_data.host_lpis[host_lpi / HOST_LPIS_PER_PAGE][host_lpi % 
> >> HOST_LPIS_PER_PAGE];
> >> +
> >> +    write_u16_atomic(&hlpip->vcpu_id, vcpu_id);
> >> +
> >> +    return 0;
> >> +}
> >> +
> >>  static int gicv3_lpi_allocate_pendtable(uint64_t *reg)
> >>  {
> >>      uint64_t val;
> >> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> >> index 079dd44..6afb915 100644
> >> --- a/xen/arch/arm/vgic-v3-its.c
> >> +++ b/xen/arch/arm/vgic-v3-its.c
> >> @@ -508,6 +508,47 @@ static int its_handle_mapti(struct virt_its *its, 
> >> uint64_t *cmdptr)
> >>      return 0;
> >>  }
> >>  
> >> +static int its_handle_movi(struct virt_its *its, uint64_t *cmdptr)
> >> +{
> >> +    uint32_t devid = its_cmd_get_deviceid(cmdptr);
> >> +    uint32_t eventid = its_cmd_get_id(cmdptr);
> >> +    int collid = its_cmd_get_collection(cmdptr);
> >> +    struct pending_irq *p;
> >> +    struct vcpu *vcpu;
> >> +    uint32_t vlpi;
> >> +    int ret = -1;
> >> +
> >> +    spin_lock(&its->its_lock);
> >> +    /* Check for a mapped LPI and get the LPI number. */
> >> +    if ( !read_itte_locked(its, devid, eventid, &vcpu, &vlpi) )
> >> +        goto out_unlock;
> >> +
> >> +    /* Check the new collection ID and get the new VCPU pointer */
> >> +    vcpu = get_vcpu_from_collection(its, collid);
> >> +    if ( !vcpu )
> >> +        goto out_unlock;
> >> +
> >> +    /* Update our cached vcpu_id in the pending_irq. */
> >> +    p = its->d->arch.vgic.handler->lpi_to_pending(its->d, vlpi);
> >> +    p->vcpu_id = vcpu->vcpu_id;
> >> +
> >> +    /* Now store the new collection in the translation table. */
> >> +    if ( !write_itte_locked(its, devid, eventid, collid, vlpi, &vcpu) )
> >> +        goto out_unlock;
> >> +
> >> +    spin_unlock(&its->its_lock);
> >> +
> >> +    /* TODO: lookup currently-in-guest virtual IRQs and migrate them? */
> >> +
> >> +    return gicv3_lpi_change_vcpu(its->d, its->doorbell_address,
> >> +                                 devid, eventid, vcpu->vcpu_id);
> >> +
> >> +out_unlock:
> >> +    spin_unlock(&its->its_lock);
> >> +
> >> +    return ret;
> >> +}
> >> +
> >>  #define ITS_CMD_BUFFER_SIZE(baser)      ((((baser) & 0xff) + 1) << 12)
> >>  
> >>  /*
> >> @@ -552,6 +593,12 @@ static int vgic_its_handle_cmds(struct domain *d, 
> >> struct virt_its *its)
> >>          case GITS_CMD_MAPTI:
> >>              ret = its_handle_mapti(its, command);
> >>              break;
> >> +        case GITS_CMD_MOVALL:
> >> +            gdprintk(XENLOG_G_INFO, "ITS: ignoring MOVALL command\n");
> >> +            break;
> >> +        case GITS_CMD_MOVI:
> >> +            ret = its_handle_movi(its, command);
> >> +            break;
> >>          case GITS_CMD_SYNC:
> >>              /* We handle ITS commands synchronously, so we ignore SYNC. */
> >>              break;
> >> diff --git a/xen/include/asm-arm/gic_v3_its.h 
> >> b/xen/include/asm-arm/gic_v3_its.h
> >> index 30aa1ef..daae143 100644
> >> --- a/xen/include/asm-arm/gic_v3_its.h
> >> +++ b/xen/include/asm-arm/gic_v3_its.h
> >> @@ -177,8 +177,12 @@ void gicv3_free_host_lpi_block(uint32_t first_lpi);
> >>  struct pending_irq *gicv3_assign_guest_event(struct domain *d, paddr_t 
> >> doorbell,
> >>                                               uint32_t devid, uint32_t 
> >> eventid,
> >>                                               struct vcpu *v, uint32_t 
> >> virt_lpi);
> >> +int gicv3_lpi_change_vcpu(struct domain *d, paddr_t doorbell,
> >> +                          uint32_t devid, uint32_t eventid,
> >> +                          unsigned int vcpu_id);
> >>  void gicv3_lpi_update_host_entry(uint32_t host_lpi, int domain_id,
> >>                                   unsigned int vcpu_id, uint32_t virt_lpi);
> >> +int gicv3_lpi_update_host_vcpuid(uint32_t host_lpi, unsigned int vcpu_id);
> >>  
> >>  #else
> >>  
> >> -- 
> >> 2.8.2
> >>
> 

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.