Re: [PATCH resend 1/3] VT-d: enable for multi-vector MSI

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 16/07/13 12:32, Jan Beulich wrote:
>>>> On 16.07.13 at 13:15, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 16/07/13 11:13, Jan Beulich wrote:
>>> The main change being to make alloc_remap_entry() capable of allocating
>>> a block of entries.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> --- a/xen/drivers/passthrough/vtd/intremap.c
>>> +++ b/xen/drivers/passthrough/vtd/intremap.c
>>> @@ -194,18 +194,18 @@ static void free_remap_entry(struct iomm
>>>  }
>>>  
>>>  /*
>>> - * Look for a free intr remap entry.
>>> + * Look for a free intr remap entry (or a contiguous set thereof).
>>>   * Need hold iremap_lock, and setup returned entry before releasing lock.
>>>   */
>>> -static int alloc_remap_entry(struct iommu *iommu)
>>> +static unsigned int alloc_remap_entry(struct iommu *iommu, unsigned int nr)
>> alloc_remap_entries() now that it unconditionally takes a count (and you
>> already have to patch all callsites)
> Actually I checked with Linux, and the use singular in the function
> name too (albeit the name isn't identical).
>
>>> @@ -555,31 +556,29 @@ static int msi_msg_to_remap_entry(
>>>      struct iremap_entry *iremap_entry = NULL, *iremap_entries;
>>>      struct iremap_entry new_ire;
>>>      struct msi_msg_remap_entry *remap_rte;
>>> -    int index;
>>> +    unsigned int index, i, nr = 1;
>> Does this hardcoding of nr=1 defeat the purpose of the following logic?
> In what way?
>
>>>      unsigned long flags;
>>>      struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
>>>  
>>> -    remap_rte = (struct msi_msg_remap_entry *) msg;
>>> +    if ( msi_desc->msi_attrib.type == PCI_CAP_ID_MSI )
>>> +        nr = msi_desc->msi.nvec;
> The logic here makes the vector count 1 for MSI-X and msi.nvec
> for MSI.
>
> Jan
>

Ah yes - I see now.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>