| Message ID | 20170517151516.52421-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 17.05.17 at 17:15, <roger.pau@citrix.com> wrote: > --- a/xen/arch/x86/irq.c > +++ b/xen/arch/x86/irq.c > @@ -2537,3 +2537,162 @@ bool_t hvm_domain_use_pirq(const struct domain *d, const struct pirq *pirq) > return is_hvm_domain(d) && pirq && > pirq->arch.hvm.emuirq != IRQ_UNBOUND; > } > + > +static int allocate_pirq(struct domain *d, int pirq, int irq, int type, > + int *nr) > +{ > + int current_pirq; > + > + ASSERT(spin_is_locked(&d->event_lock)); > + current_pirq = domain_irq_to_pirq(d, irq); > + if ( pirq < 0 ) > + { > + if ( current_pirq ) > + { > + dprintk(XENLOG_G_ERR, "dom%d: %d:%d already mapped to %d\n", > + d->domain_id, irq, pirq, current_pirq); Instead of "irq" the old code did pass "*index", i.e. a Dom0 kernel specified value (which is going to be more useful, as any problem here will need to be diagnosed in the Dom0 kernel). The two values are identical only if, in the GSI case, domain_pirq_to_irq(current->domain, *index) in allocate_and_map_gsi_pirq() returned a negative value. > + if ( current_pirq < 0 ) > + return -EBUSY; > + } > + else if ( type == MAP_PIRQ_TYPE_MULTI_MSI ) > + { > + if ( *nr <= 0 || *nr > 32 ) > + return -EDOM; > + else if ( *nr != 1 && !iommu_intremap ) > + return -EOPNOTSUPP; > + else Pointless "else" (twice). > + { > + while ( *nr & (*nr - 1) ) > + *nr += *nr & -*nr; > + pirq = get_free_pirqs(d, *nr); > + if ( pirq < 0 ) > + { > + while ( (*nr >>= 1) > 1 ) > + if ( get_free_pirqs(d, *nr) > 0 ) > + break; > + dprintk(XENLOG_G_ERR, "dom%d: no block of %d free pirqs\n", > + d->domain_id, *nr << 1); > + } > + } > + } > + else > + { > + pirq = get_free_pirq(d, type); > + if ( pirq < 0 ) > + dprintk(XENLOG_G_ERR, "dom%d: no free pirq\n", d->domain_id); > + } > + } > + else if ( current_pirq && pirq != current_pirq ) > + { > + dprintk(XENLOG_G_ERR, "dom%d: irq %d already mapped to pirq %d\n", > + d->domain_id, irq, current_pirq); > + pirq = -EEXIST; "return -EEXIST" please, to match the direct returns you use further up. > + } > + > + return pirq; > +} > + > +int allocate_and_map_gsi_pirq(struct domain *d, int *index, int *pirq_p) Neither here nor in the MSI sibling you ever write to *index, so there's no need for the parameter to have pointer type. > +{ > + int irq, pirq, ret; > + > + if ( *index < 0 || *index >= nr_irqs_gsi ) > + { > + dprintk(XENLOG_G_ERR, "dom%d: map invalid irq %d\n", d->domain_id, > + *index); > + return -EINVAL; > + } > + > + irq = domain_pirq_to_irq(current->domain, *index); > + if ( irq <= 0 ) > + { > + if ( is_hardware_domain(current->domain) ) > + irq = *index; > + else > + { > + dprintk(XENLOG_G_ERR, "dom%d: map pirq with incorrect irq!\n", > + d->domain_id); > + return -EINVAL; > + } > + } > + > + /* Verify or get pirq. */ > + spin_lock(&d->event_lock); > + pirq = allocate_pirq(d, *pirq_p, irq, MAP_PIRQ_TYPE_GSI, 0); The last parameter of allocate_pirq() is a pointer, so you really mean NULL here. > + if ( pirq < 0 ) > + { > + ret = pirq; > + goto done; > + } > + > + ret = map_domain_pirq(d, pirq, irq, MAP_PIRQ_TYPE_GSI, NULL); > + if ( ret == 0 ) > + *pirq_p = pirq; > + > + done: > + spin_unlock(&d->event_lock); > + return ret; > +} Blank line before final return statement please. > +int allocate_and_map_msi_pirq(struct domain *d, int *index, int *pirq_p, > + int type, struct msi_info *msi) > +{ > + int irq, pirq, ret; > + > + switch ( type ) > + { > + case MAP_PIRQ_TYPE_MSI: > + if ( !msi->table_base ) > + msi->entry_nr = 1; > + irq = *index; > + if ( irq == -1 ) > + case MAP_PIRQ_TYPE_MULTI_MSI: > + irq = create_irq(NUMA_NO_NODE); > + > + if ( irq < nr_irqs_gsi || irq >= nr_irqs ) > + { > + dprintk(XENLOG_G_ERR, "dom%d: can't create irq for msi!\n", > + d->domain_id); > + return -EINVAL; > + } > + > + msi->irq = irq; > + break; > + > + default: > + dprintk(XENLOG_G_ERR, "dom%d: wrong pirq type %x\n", > + d->domain_id, type); > + return -EINVAL; This really should be an ASSERT_UNREACHABLE() (with the return statement kept). > + } > + > + msi->irq = irq; > + > + pcidevs_lock(); > + /* Verify or get pirq. */ > + spin_lock(&d->event_lock); > + pirq = allocate_pirq(d, *pirq_p, irq, type, &msi->entry_nr); > + if ( pirq < 0 ) > + { > + ret = pirq; > + goto done; > + } > + > + ret = map_domain_pirq(d, pirq, irq, type, msi); > + if ( ret == 0 ) > + *pirq_p = pirq; > + > + done: > + spin_unlock(&d->event_lock); > + pcidevs_unlock(); > + if ( ret != 0 ) > + switch ( type ) > + { > + case MAP_PIRQ_TYPE_MSI: > + if ( *index == -1 ) > + case MAP_PIRQ_TYPE_MULTI_MSI: > + destroy_irq(irq); > + break; > + } > + return ret; > +} > + Please don't end a file with a blank line. > diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c > index eec4a41231..e99fd9a35f 100644 > --- a/xen/arch/x86/physdev.c > +++ b/xen/arch/x86/physdev.c > @@ -92,8 +92,7 @@ int physdev_map_pirq(domid_t domid, int type, int *index, > int *pirq_p, > struct msi_info *msi) > { > struct domain *d = current->domain; > - int pirq, irq, ret = 0; > - void *map_data = NULL; > + int ret; > > if ( domid == DOMID_SELF && is_hvm_domain(d) && has_pirq(d) ) > { > @@ -119,135 +118,22 @@ int physdev_map_pirq(domid_t domid, int type, int > *index, int *pirq_p, > switch ( type ) > { > case MAP_PIRQ_TYPE_GSI: > - if ( *index < 0 || *index >= nr_irqs_gsi ) > - { > - dprintk(XENLOG_G_ERR, "dom%d: map invalid irq %d\n", > - d->domain_id, *index); > - ret = -EINVAL; > - goto free_domain; > - } > - > - irq = domain_pirq_to_irq(current->domain, *index); > - if ( irq <= 0 ) > - { > - if ( is_hardware_domain(current->domain) ) > - irq = *index; > - else { > - dprintk(XENLOG_G_ERR, "dom%d: map pirq with incorrect irq!\n", > - d->domain_id); > - ret = -EINVAL; > - goto free_domain; > - } > - } > + ret = allocate_and_map_gsi_pirq(d, index, pirq_p); > break; > - > case MAP_PIRQ_TYPE_MSI: Please don't delete the blank lines between case blocks, even if the blocks are much smaller now. Jan
On Tue, May 30, 2017 at 03:16:51AM -0600, Jan Beulich wrote: > >>> On 17.05.17 at 17:15, <roger.pau@citrix.com> wrote: > > --- a/xen/arch/x86/irq.c > > +++ b/xen/arch/x86/irq.c > > @@ -2537,3 +2537,162 @@ bool_t hvm_domain_use_pirq(const struct domain *d, const struct pirq *pirq) > > return is_hvm_domain(d) && pirq && > > pirq->arch.hvm.emuirq != IRQ_UNBOUND; > > } > > + > > +static int allocate_pirq(struct domain *d, int pirq, int irq, int type, > > + int *nr) > > +{ > > + int current_pirq; > > + > > + ASSERT(spin_is_locked(&d->event_lock)); > > + current_pirq = domain_irq_to_pirq(d, irq); > > + if ( pirq < 0 ) > > + { > > + if ( current_pirq ) > > + { > > + dprintk(XENLOG_G_ERR, "dom%d: %d:%d already mapped to %d\n", > > + d->domain_id, irq, pirq, current_pirq); > > Instead of "irq" the old code did pass "*index", i.e. a Dom0 kernel > specified value (which is going to be more useful, as any problem > here will need to be diagnosed in the Dom0 kernel). The two > values are identical only if, in the GSI case, > domain_pirq_to_irq(current->domain, *index) in > allocate_and_map_gsi_pirq() returned a negative value. I guess I can propagate index into this function, I haven't done that because it's only used by the error message, but not the code itself. When I used the physdev hypoercalls in the past I think I was always setting index == GSI IIRC. > > + if ( current_pirq < 0 ) > > + return -EBUSY; > > + } > > + else if ( type == MAP_PIRQ_TYPE_MULTI_MSI ) > > + { > > + if ( *nr <= 0 || *nr > 32 ) > > + return -EDOM; > > + else if ( *nr != 1 && !iommu_intremap ) > > + return -EOPNOTSUPP; > > + else > > Pointless "else" (twice). Done (and re-indented the section below). > > + { > > + while ( *nr & (*nr - 1) ) > > + *nr += *nr & -*nr; > > + pirq = get_free_pirqs(d, *nr); > > + if ( pirq < 0 ) > > + { > > + while ( (*nr >>= 1) > 1 ) > > + if ( get_free_pirqs(d, *nr) > 0 ) > > + break; > > + dprintk(XENLOG_G_ERR, "dom%d: no block of %d free pirqs\n", > > + d->domain_id, *nr << 1); > > + } > > + } > > + } > > + else > > + { > > + pirq = get_free_pirq(d, type); > > + if ( pirq < 0 ) > > + dprintk(XENLOG_G_ERR, "dom%d: no free pirq\n", d->domain_id); > > + } > > + } > > + else if ( current_pirq && pirq != current_pirq ) > > + { > > + dprintk(XENLOG_G_ERR, "dom%d: irq %d already mapped to pirq %d\n", > > + d->domain_id, irq, current_pirq); > > + pirq = -EEXIST; > > "return -EEXIST" please, to match the direct returns you use further > up. Ack. > > + } > > + > > + return pirq; > > +} > > + > > +int allocate_and_map_gsi_pirq(struct domain *d, int *index, int *pirq_p) > > Neither here nor in the MSI sibling you ever write to *index, so > there's no need for the parameter to have pointer type. Done > > +{ > > + int irq, pirq, ret; > > + > > + if ( *index < 0 || *index >= nr_irqs_gsi ) > > + { > > + dprintk(XENLOG_G_ERR, "dom%d: map invalid irq %d\n", d->domain_id, > > + *index); > > + return -EINVAL; > > + } > > + > > + irq = domain_pirq_to_irq(current->domain, *index); > > + if ( irq <= 0 ) > > + { > > + if ( is_hardware_domain(current->domain) ) > > + irq = *index; > > + else > > + { > > + dprintk(XENLOG_G_ERR, "dom%d: map pirq with incorrect irq!\n", > > + d->domain_id); > > + return -EINVAL; > > + } > > + } > > + > > + /* Verify or get pirq. */ > > + spin_lock(&d->event_lock); > > + pirq = allocate_pirq(d, *pirq_p, irq, MAP_PIRQ_TYPE_GSI, 0); > > The last parameter of allocate_pirq() is a pointer, so you really > mean NULL here. > > > + if ( pirq < 0 ) > > + { > > + ret = pirq; > > + goto done; > > + } > > + > > + ret = map_domain_pirq(d, pirq, irq, MAP_PIRQ_TYPE_GSI, NULL); > > + if ( ret == 0 ) > > + *pirq_p = pirq; > > + > > + done: > > + spin_unlock(&d->event_lock); > > + return ret; > > +} > > Blank line before final return statement please. Fixed both of the above. > > +int allocate_and_map_msi_pirq(struct domain *d, int *index, int *pirq_p, > > + int type, struct msi_info *msi) > > +{ > > + int irq, pirq, ret; > > + > > + switch ( type ) > > + { > > + case MAP_PIRQ_TYPE_MSI: > > + if ( !msi->table_base ) > > + msi->entry_nr = 1; > > + irq = *index; > > + if ( irq == -1 ) > > + case MAP_PIRQ_TYPE_MULTI_MSI: > > + irq = create_irq(NUMA_NO_NODE); > > + > > + if ( irq < nr_irqs_gsi || irq >= nr_irqs ) > > + { > > + dprintk(XENLOG_G_ERR, "dom%d: can't create irq for msi!\n", > > + d->domain_id); > > + return -EINVAL; > > + } > > + > > + msi->irq = irq; > > + break; > > + > > + default: > > + dprintk(XENLOG_G_ERR, "dom%d: wrong pirq type %x\n", > > + d->domain_id, type); > > + return -EINVAL; > > This really should be an ASSERT_UNREACHABLE() (with the return > statement kept). Fixed. > > + } > > + > > + msi->irq = irq; > > + > > + pcidevs_lock(); > > + /* Verify or get pirq. */ > > + spin_lock(&d->event_lock); > > + pirq = allocate_pirq(d, *pirq_p, irq, type, &msi->entry_nr); > > + if ( pirq < 0 ) > > + { > > + ret = pirq; > > + goto done; > > + } > > + > > + ret = map_domain_pirq(d, pirq, irq, type, msi); > > + if ( ret == 0 ) > > + *pirq_p = pirq; > > + > > + done: > > + spin_unlock(&d->event_lock); > > + pcidevs_unlock(); > > + if ( ret != 0 ) > > + switch ( type ) > > + { > > + case MAP_PIRQ_TYPE_MSI: > > + if ( *index == -1 ) > > + case MAP_PIRQ_TYPE_MULTI_MSI: > > + destroy_irq(irq); > > + break; > > + } > > + return ret; > > +} > > + > > Please don't end a file with a blank line. Fixed, and I've also added a new line before the return statement. > > diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c > > index eec4a41231..e99fd9a35f 100644 > > --- a/xen/arch/x86/physdev.c > > +++ b/xen/arch/x86/physdev.c > > @@ -92,8 +92,7 @@ int physdev_map_pirq(domid_t domid, int type, int *index, > > int *pirq_p, > > struct msi_info *msi) > > { > > struct domain *d = current->domain; > > - int pirq, irq, ret = 0; > > - void *map_data = NULL; > > + int ret; > > > > if ( domid == DOMID_SELF && is_hvm_domain(d) && has_pirq(d) ) > > { > > @@ -119,135 +118,22 @@ int physdev_map_pirq(domid_t domid, int type, int > > *index, int *pirq_p, > > switch ( type ) > > { > > case MAP_PIRQ_TYPE_GSI: > > - if ( *index < 0 || *index >= nr_irqs_gsi ) > > - { > > - dprintk(XENLOG_G_ERR, "dom%d: map invalid irq %d\n", > > - d->domain_id, *index); > > - ret = -EINVAL; > > - goto free_domain; > > - } > > - > > - irq = domain_pirq_to_irq(current->domain, *index); > > - if ( irq <= 0 ) > > - { > > - if ( is_hardware_domain(current->domain) ) > > - irq = *index; > > - else { > > - dprintk(XENLOG_G_ERR, "dom%d: map pirq with incorrect irq!\n", > > - d->domain_id); > > - ret = -EINVAL; > > - goto free_domain; > > - } > > - } > > + ret = allocate_and_map_gsi_pirq(d, index, pirq_p); > > break; > > - > > case MAP_PIRQ_TYPE_MSI: > > Please don't delete the blank lines between case blocks, even if > the blocks are much smaller now. Fixed (and the one below). Roger.
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c index 676ba5216f..4590e85303 100644 --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -2537,3 +2537,162 @@ bool_t hvm_domain_use_pirq(const struct domain *d, const struct pirq *pirq) return is_hvm_domain(d) && pirq && pirq->arch.hvm.emuirq != IRQ_UNBOUND; } + +static int allocate_pirq(struct domain *d, int pirq, int irq, int type, + int *nr) +{ + int current_pirq; + + ASSERT(spin_is_locked(&d->event_lock)); + current_pirq = domain_irq_to_pirq(d, irq); + if ( pirq < 0 ) + { + if ( current_pirq ) + { + dprintk(XENLOG_G_ERR, "dom%d: %d:%d already mapped to %d\n", + d->domain_id, irq, pirq, current_pirq); + if ( current_pirq < 0 ) + return -EBUSY; + } + else if ( type == MAP_PIRQ_TYPE_MULTI_MSI ) + { + if ( *nr <= 0 || *nr > 32 ) + return -EDOM; + else if ( *nr != 1 && !iommu_intremap ) + return -EOPNOTSUPP; + else + { + while ( *nr & (*nr - 1) ) + *nr += *nr & -*nr; + pirq = get_free_pirqs(d, *nr); + if ( pirq < 0 ) + { + while ( (*nr >>= 1) > 1 ) + if ( get_free_pirqs(d, *nr) > 0 ) + break; + dprintk(XENLOG_G_ERR, "dom%d: no block of %d free pirqs\n", + d->domain_id, *nr << 1); + } + } + } + else + { + pirq = get_free_pirq(d, type); + if ( pirq < 0 ) + dprintk(XENLOG_G_ERR, "dom%d: no free pirq\n", d->domain_id); + } + } + else if ( current_pirq && pirq != current_pirq ) + { + dprintk(XENLOG_G_ERR, "dom%d: irq %d already mapped to pirq %d\n", + d->domain_id, irq, current_pirq); + pirq = -EEXIST; + } + + return pirq; +} + +int allocate_and_map_gsi_pirq(struct domain *d, int *index, int *pirq_p) +{ + int irq, pirq, ret; + + if ( *index < 0 || *index >= nr_irqs_gsi ) + { + dprintk(XENLOG_G_ERR, "dom%d: map invalid irq %d\n", d->domain_id, + *index); + return -EINVAL; + } + + irq = domain_pirq_to_irq(current->domain, *index); + if ( irq <= 0 ) + { + if ( is_hardware_domain(current->domain) ) + irq = *index; + else + { + dprintk(XENLOG_G_ERR, "dom%d: map pirq with incorrect irq!\n", + d->domain_id); + return -EINVAL; + } + } + + /* Verify or get pirq. */ + spin_lock(&d->event_lock); + pirq = allocate_pirq(d, *pirq_p, irq, MAP_PIRQ_TYPE_GSI, 0); + if ( pirq < 0 ) + { + ret = pirq; + goto done; + } + + ret = map_domain_pirq(d, pirq, irq, MAP_PIRQ_TYPE_GSI, NULL); + if ( ret == 0 ) + *pirq_p = pirq; + + done: + spin_unlock(&d->event_lock); + return ret; +} + +int allocate_and_map_msi_pirq(struct domain *d, int *index, int *pirq_p, + int type, struct msi_info *msi) +{ + int irq, pirq, ret; + + switch ( type ) + { + case MAP_PIRQ_TYPE_MSI: + if ( !msi->table_base ) + msi->entry_nr = 1; + irq = *index; + if ( irq == -1 ) + case MAP_PIRQ_TYPE_MULTI_MSI: + irq = create_irq(NUMA_NO_NODE); + + if ( irq < nr_irqs_gsi || irq >= nr_irqs ) + { + dprintk(XENLOG_G_ERR, "dom%d: can't create irq for msi!\n", + d->domain_id); + return -EINVAL; + } + + msi->irq = irq; + break; + + default: + dprintk(XENLOG_G_ERR, "dom%d: wrong pirq type %x\n", + d->domain_id, type); + return -EINVAL; + } + + msi->irq = irq; + + pcidevs_lock(); + /* Verify or get pirq. */ + spin_lock(&d->event_lock); + pirq = allocate_pirq(d, *pirq_p, irq, type, &msi->entry_nr); + if ( pirq < 0 ) + { + ret = pirq; + goto done; + } + + ret = map_domain_pirq(d, pirq, irq, type, msi); + if ( ret == 0 ) + *pirq_p = pirq; + + done: + spin_unlock(&d->event_lock); + pcidevs_unlock(); + if ( ret != 0 ) + switch ( type ) + { + case MAP_PIRQ_TYPE_MSI: + if ( *index == -1 ) + case MAP_PIRQ_TYPE_MULTI_MSI: + destroy_irq(irq); + break; + } + return ret; +} + diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c index eec4a41231..e99fd9a35f 100644 --- a/xen/arch/x86/physdev.c +++ b/xen/arch/x86/physdev.c @@ -92,8 +92,7 @@ int physdev_map_pirq(domid_t domid, int type, int *index, int *pirq_p, struct msi_info *msi) { struct domain *d = current->domain; - int pirq, irq, ret = 0; - void *map_data = NULL; + int ret; if ( domid == DOMID_SELF && is_hvm_domain(d) && has_pirq(d) ) { @@ -119,135 +118,22 @@ int physdev_map_pirq(domid_t domid, int type, int *index, int *pirq_p, switch ( type ) { case MAP_PIRQ_TYPE_GSI: - if ( *index < 0 || *index >= nr_irqs_gsi ) - { - dprintk(XENLOG_G_ERR, "dom%d: map invalid irq %d\n", - d->domain_id, *index); - ret = -EINVAL; - goto free_domain; - } - - irq = domain_pirq_to_irq(current->domain, *index); - if ( irq <= 0 ) - { - if ( is_hardware_domain(current->domain) ) - irq = *index; - else { - dprintk(XENLOG_G_ERR, "dom%d: map pirq with incorrect irq!\n", - d->domain_id); - ret = -EINVAL; - goto free_domain; - } - } + ret = allocate_and_map_gsi_pirq(d, index, pirq_p); break; - case MAP_PIRQ_TYPE_MSI: if ( !msi->table_base ) msi->entry_nr = 1; - irq = *index; - if ( irq == -1 ) + /* fallthrough */ case MAP_PIRQ_TYPE_MULTI_MSI: - irq = create_irq(NUMA_NO_NODE); - - if ( irq < nr_irqs_gsi || irq >= nr_irqs ) - { - dprintk(XENLOG_G_ERR, "dom%d: can't create irq for msi!\n", - d->domain_id); - ret = -EINVAL; - goto free_domain; - } - - msi->irq = irq; - map_data = msi; + ret = allocate_and_map_msi_pirq(d, index, pirq_p, type, msi); break; - default: dprintk(XENLOG_G_ERR, "dom%d: wrong map_pirq type %x\n", d->domain_id, type); ret = -EINVAL; - goto free_domain; - } - - pcidevs_lock(); - /* Verify or get pirq. */ - spin_lock(&d->event_lock); - pirq = domain_irq_to_pirq(d, irq); - if ( *pirq_p < 0 ) - { - if ( pirq ) - { - dprintk(XENLOG_G_ERR, "dom%d: %d:%d already mapped to %d\n", - d->domain_id, *index, *pirq_p, pirq); - if ( pirq < 0 ) - { - ret = -EBUSY; - goto done; - } - } - else if ( type == MAP_PIRQ_TYPE_MULTI_MSI ) - { - if ( msi->entry_nr <= 0 || msi->entry_nr > 32 ) - ret = -EDOM; - else if ( msi->entry_nr != 1 && !iommu_intremap ) - ret = -EOPNOTSUPP; - else - { - while ( msi->entry_nr & (msi->entry_nr - 1) ) - msi->entry_nr += msi->entry_nr & -msi->entry_nr; - pirq = get_free_pirqs(d, msi->entry_nr); - if ( pirq < 0 ) - { - while ( (msi->entry_nr >>= 1) > 1 ) - if ( get_free_pirqs(d, msi->entry_nr) > 0 ) - break; - dprintk(XENLOG_G_ERR, "dom%d: no block of %d free pirqs\n", - d->domain_id, msi->entry_nr << 1); - ret = pirq; - } - } - if ( ret < 0 ) - goto done; - } - else - { - pirq = get_free_pirq(d, type); - if ( pirq < 0 ) - { - dprintk(XENLOG_G_ERR, "dom%d: no free pirq\n", d->domain_id); - ret = pirq; - goto done; - } - } - } - else - { - if ( pirq && pirq != *pirq_p ) - { - dprintk(XENLOG_G_ERR, "dom%d: pirq %d conflicts with irq %d\n", - d->domain_id, *index, *pirq_p); - ret = -EEXIST; - goto done; - } - else - pirq = *pirq_p; + break; } - ret = map_domain_pirq(d, pirq, irq, type, map_data); - if ( ret == 0 ) - *pirq_p = pirq; - - done: - spin_unlock(&d->event_lock); - pcidevs_unlock(); - if ( ret != 0 ) - switch ( type ) - { - case MAP_PIRQ_TYPE_MSI: - if ( *index == -1 ) - case MAP_PIRQ_TYPE_MULTI_MSI: - destroy_irq(irq); - break; - } free_domain: rcu_unlock_domain(d); return ret; diff --git a/xen/include/asm-x86/irq.h b/xen/include/asm-x86/irq.h index ef625ebb13..d8d965b642 100644 --- a/xen/include/asm-x86/irq.h +++ b/xen/include/asm-x86/irq.h @@ -200,4 +200,9 @@ bool_t cpu_has_pending_apic_eoi(void); static inline void arch_move_irqs(struct vcpu *v) { } +struct msi_info; +int allocate_and_map_gsi_pirq(struct domain *d, int *index, int *pirq_p); +int allocate_and_map_msi_pirq(struct domain *d, int *index, int *pirq_p, + int type, struct msi_info *msi); + #endif /* _ASM_HW_IRQ_H */
Move the code to allocate and map a domain pirq (either GSI or MSI) into the x86 irq code base, so that it can be used outside of the physdev ops. This change shouldn't affect the functionality of the already existing physdev ops. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- Changes since v2: - Factor out the code to allocate the pirq. - Fix coding style issues. - Do not take the pci lock to bind a GSI. - Pass a type parameter to the MSI bind function. Changes since v1: - New in this version. --- xen/arch/x86/irq.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++ xen/arch/x86/physdev.c | 124 ++---------------------------------- xen/include/asm-x86/irq.h | 5 ++ 3 files changed, 169 insertions(+), 119 deletions(-)