Re: [PATCH 5/6] x86: enable multi-vector MSI

[Suravee Suthikulanit <suravee.suthikulpanit@amd.com>]

On 4/23/2013 1:26 AM, Jan Beulich wrote:
>>>> On 23.04.13 at 02:55, Suravee Suthikulanit <suravee.suthikulpanit@amd.com>
> wrote:
>> On 4/19/2013 5:59 AM, Jan Beulich wrote:
>>> --- a/xen/arch/x86/msi.c
>>> +++ b/xen/arch/x86/msi.c
>>> @@ -238,6 +238,11 @@ static int write_msi_msg(struct msi_desc
>>>            u8 bus = dev->bus;
>>>            u8 slot = PCI_SLOT(dev->devfn);
>>>            u8 func = PCI_FUNC(dev->devfn);
>>> +        int nr = entry->msi_attrib.entry_nr;
>>> +
>>> +        ASSERT((msg->data & (entry[-nr].msi.nvec - 1)) == nr);
>>> +        if ( nr )
>>> +            return 0;
>> This logic seems incorrect.  Do you meant to write --nr?
> No, this indeed has to be -nr (i.e. the "masterkj" entry, which is the
> first on in the array.
>
>> This causes assertion here.  Also, investigation showing the
>> value of nr is 0 here.
> nr being 0 here is perfectly fine, meaning this is the first ("master")
> entry of a multi-vector device (it can't be a single-vector one, as in
> that case entry[0].msi.nvec == 1, i.e. the & yields zero regardless
> of msg->data).
>
> And the assertion should hold, due to
>
>      *data = (msg->data & ~(INTREMAP_ENTRIES - 1)) | offset;
>
> in update_intremap_entry_from_msi_msg(), and
> alloc_intremap_entry() returning only aligned blocks.
>
> So the question isn't just what value nr there has, but also what
> the other involved values are.
>
> Jan
Ok, thanks for explanation.  Do you think you could add comment in the 
code?kkk  It was not quite clear at the beginning why we need this 
assertion.

The problem occurs when the function 
"xen/driver/passthrough/amd/iommu_init.c: enable_iommu()" trying to 
initialize IOMMU and calling the "set_msi_affinity", which in turn 
calling "write_msi_msg".  At this point, "nvec" is still zero.  So, the 
following code should fix it.

     unsigned int nvec = entry[-nr].msi.nvec;
     if ( nvec > 0 )
             ASSERT((msg->data & (nvec - 1)) == nr);

Suravee