Re: [Xen-devel] [PATCH resend 2/3] x86: enable multi-vector MSI

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.07.13 at 13:36, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 16/07/13 11:14, Jan Beulich wrote:
>> +int get_free_pirqs(struct domain *d, unsigned int nr)
>> +{
>> +    unsigned int i, found = 0;
>> +
>> +    ASSERT(spin_is_locked(&d->event_lock));
>> +
>> +    for ( i = d->nr_pirqs - 1; i >= nr_irqs_gsi; --i )
>> +        if ( is_free_pirq(d, pirq_info(d, i)) )
>> +        {
>> +            pirq_get_info(d, i);
>> +            if ( ++found == nr )
>> +                return i;
>> +        }
>> +        else
>> +            found = 0;
>> +
>> +    return -ENOSPC;
>> +}
>> +
> 
> Is there any reason why this loop is backwards? Unless I am mistaken,
> guests can choose their own pirqs when binding them, reducing the
> likelyhood that the top of the available space will be free.

This just follows the behavior of get_free_pirq(). I'm not up to
having the two behave differently.

>> @@ -210,8 +237,15 @@ int physdev_map_pirq(domid_t domid, int 
>>   done:
>>      spin_unlock(&d->event_lock);
>>      spin_unlock(&pcidevs_lock);
>> -    if ( (ret != 0) && (type == MAP_PIRQ_TYPE_MSI) && (*index == -1) )
>> -        destroy_irq(irq);
>> +    if ( ret != 0 )
>> +        switch ( type )
>> +        {
>> +        case MAP_PIRQ_TYPE_MSI:
>> +            if ( *index == -1 )
>> +        case MAP_PIRQ_TYPE_MULTI_MSI:
>> +                destroy_irq(irq);
>> +            break;
>> +        }
> 
> Do we not need to create and destroy entry_nr irqs in this function, or
> is a multi-vector-msi now considered as just as single irq ?
> 
> I ask because this appears to lack the "repeating certain operations for
> all involved IRQs" described in the comment.

No, there's a single create_irq() in the function, and having a single
destroy_irq() here matches this. The remaining ones (both!) are in
map_domain_pirq().

Also, as a general remark, asking for changes in a series that was
posted 2.5 months ago (and deferred just because of the 4.3
release process) seems a little strange to me. I had to repost
merely to collect ack-s, and didn't really expect further requests
for adjustments as there was ample time before to do so.

Jan


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