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

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

On 4/23/2013 1:26 AM, Jan Beulich wrote:
On 23.04.13 at 02:55, Suravee Suthikulanit <suravee.suthikulpanit@xxxxxxx>
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.

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);


Xen-devel mailing list



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