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

Re: [Xen-devel] pt_msix_init: Error: Can't map physical MSI-X table: Invalid argument


  • To: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
  • From: Bruce Edge <bruce.edge@xxxxxxxxx>
  • Date: Thu, 15 Oct 2009 11:13:40 -0700
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 15 Oct 2009 11:14:04 -0700
  • Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=sb9BsicruQ4eUAtNFsvHB+I8YrJ7xC0GpDmNTUnsKC5nVp6aQjM0h4fQTeChtwkSX0 dazcy+A1xoPUJN6zh6mAXkUYCDu8Xe2qOHw+ECSE6dykVmAvybP3rtVCTlU5XzfnOJS2 wPfbHk51c1P1s9tIqVDPM0A1UsPHZbaf0bmDw=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>

On Thu, Oct 15, 2009 at 3:08 AM, Stefano Stabellini
<stefano.stabellini@xxxxxxxxxxxxx> wrote:
> On Wed, 14 Oct 2009, Bruce Edge wrote:
>> On Wed, Oct 14, 2009 at 5:47 AM, Stefano Stabellini
>> <stefano.stabellini@xxxxxxxxxxxxx> wrote:
>> > On Tue, 13 Oct 2009, Cinco, Dante wrote:
>> >> I get the above error message when I boot the domU (Linux 2.6.30.1). I'm 
>> >> using Xen 3.5-unstable (changeset 20303) with
>> >> pv_ops kernel 2.6.31.1. I use pci-stub to hide the device so that I can 
>> >> assign/pass it through to domU. The error is
>> >> coming from ioemu-xxx/hw/pt-msi.c. I think it is complaining about the 
>> >> table_off that is part of the mmap() call:
>> >>
>> >>     dev->msix->phys_iomem_base = mmap(0, total_entries * 16,
>> >>                           PROT_WRITE | PROT_READ, MAP_SHARED | MAP_LOCKED,
>> >>                           fd, dev->msix->table_base + table_off);
>> >>
>> >> When I printed table_off, its value is 0x4100 (not page-aligned) which is 
>> >> consistent with the BAR4 offset of the PCI
>> >> device (see below). I think mmap() requires the last argument to be a 
>> >> multiple of the page size and results is a map
>> >> error if it is not. Here's the partial output of "lspci -vv -s 0:07:0.1" 
>> >> from dom0:
>> >>
>> >> 07:00.0 Fibre Channel: PMC-Sierra Inc. Device 8032 (rev 05)
>> >>         Subsystem: Atto Technology Device 003c
>> >>         Latency: 0, Cache Line Size: 64 bytes
>> >>         Interrupt: pin A routed to IRQ 11
>> >>         Capabilities: [60] Message Signalled Interrupts: Mask- 64bit+ 
>> >> Queue=0/1 Enable+
>> >>                 Address: 00000000fee11000  Data: 405e
>> >>         Capabilities: [70] Express (v2) Endpoint, MSI 01
>> >>                 DevCap: MaxPayload 4096 bytes, PhantFunc 0, Latency L0s 
>> >> <4us, L1 unlimited
>> >>                         ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
>> >>         Capabilities: [b0] MSI-X: Enable- Mask- TabSize=9
>> >>                 Vector table: BAR=4 offset=00004100
>> >>                 PBA: BAR=4 offset=00004000
>> >>
>> >> I'm able to workaround this problem by modifying pt-msi.c: force offset 
>> >> to zero in mmap() and on the next line, adjust
>> >> the pointer by table_off:
>> >>
>> >>     table_off_adjust = table_off & 0x0fff;
>> >>     dev->msix->phys_iomem_base = mmap(0, total_entries * 16 + 
>> >> table_off_adjust,
>> >>                           PROT_WRITE | PROT_READ, MAP_SHARED | MAP_LOCKED,
>> >>                           fd, dev->msix->table_base + table_off - 
>> >> table_off_adjust);
>> >>     dev->msix->phys_iomem_base = (void *)((char 
>> >> *)dev->msix->phys_iomem_base + table_off_adjust);
>> >>
>> >> Ideally, I should only be applying this to my device but since I'm not 
>> >> passing any other device that use MSI, I don't see
>> >> any negative effect. In addition, my device only uses MSI and not MSI-X.
>> >>
>> >> Obviously, this is a custom hack but I was wondering if there is a 
>> >> cleaner solution for this. Is a non-page aligned BAR
>> >> offset unusual? When I boot Linux 2.6.30.1 on bare metal (no Xen), I 
>> >> don't see this error.
>> >
>> > I think this workaround is not so bad, you just need to keep
>> > table_off_adjust around because it has to be used in munmap too.
>> >
>>
>>
>> Here's a patch that propagates the offset to the munmap as well. Any
>> chance this could be applied to the hg repository?
>>
>> -Bruce
>>
>>  diff -Naur ./tools/ioemu-remote/hw/pt-msi.c.orig
>> ./tools/ioemu-remote/hw/pt-msi.c
>>
>> --- ./tools/ioemu-remote/hw/pt-msi.c.orig       2009-10-13
>> 11:54:11.000000000 -0700
>> +++ ./tools/ioemu-remote/hw/pt-msi.c    2009-10-14 15:16:31.000000000 -0700
>> @@ -535,6 +535,10 @@
>>                  DPCI_REMOVE_MAPPING);
>>  }
>>
>> +/* Calculated offset to page-align MSI-X mmap
>> + * Needed for munmap as well */
>> +static int table_off_adjust = 0;
>> +
>>  int pt_msix_init(struct pt_dev *dev, int pos)
>>  {
>>      uint8_t id;
>> @@ -542,6 +546,7 @@
>>      int i, total_entries, table_off, bar_index;
>>      struct pci_dev *pd = dev->pci_dev;
>>      int fd;
>> +    int err;
>>
>>      id = pci_read_byte(pd, pos + PCI_CAP_LIST_ID);
>>
>> @@ -584,9 +589,14 @@
>>          PT_LOG("Error: Can't open /dev/mem: %s\n", strerror(errno));
>>          goto error_out;
>>      }
>> -    dev->msix->phys_iomem_base = mmap(0, total_entries * 16,
>> +    PT_LOG("table_off = %llx, total_entries = 
>> %d\n",table_off,total_entries);
>> +    table_off_adjust = table_off & 0x0fff;
>> +    dev->msix->phys_iomem_base = mmap(0, total_entries * 16 + 
>> table_off_adjust,
>>                            PROT_WRITE | PROT_READ, MAP_SHARED | MAP_LOCKED,
>> -                          fd, dev->msix->table_base + table_off);
>> +                          fd, dev->msix->table_base + table_off -
>> table_off_adjust);
>> +    dev->msix->phys_iomem_base = (void *)((char
>> *)dev->msix->phys_iomem_base + table_off_adjust);
>> +    err = errno;
>> +    PT_LOG("errno = %d\n",err);
>>      if ( dev->msix->phys_iomem_base == MAP_FAILED )
>>      {
>>          PT_LOG("Error: Can't map physical MSI-X table: %s\n", 
>> strerror(errno));
>> @@ -612,7 +622,7 @@
>>      {
>>          PT_LOG("unmapping physical MSI-X table from %lx\n",
>>             (unsigned long)dev->msix->phys_iomem_base);
>> -        munmap(dev->msix->phys_iomem_base, dev->msix->total_entries * 16);
>> +        munmap(dev->msix->phys_iomem_base, dev->msix->total_entries *
>> 16 + table_off_adjust);
>>      }
>>
>>      free(dev->msix);
>>
>
> I think this is fine apart from the fact that table_off_adjust should be
> stored as another field in dev->msix, that is "struct pt_msix_info" in
> hw/pass-through.h.
>
> Any other comments, Keir, Ian?
>

Here it is again with the suggested changes.

-Bruce


--- ./tools/ioemu-remote/hw/pass-through.h.orig 2009-10-15
10:22:17.000000000 -0700
+++ ./tools/ioemu-remote/hw/pass-through.h      2009-10-15
11:08:58.000000000 -0700
@@ -193,6 +193,7 @@
     int mmio_index;
     void *phys_iomem_base;
     struct msix_entry_info msix_entry[0];
+       uint32_t table_offset_adjust;   /* page align mmap */
 };

 struct pt_pm_info {


--- ./tools/ioemu-remote/hw/pt-msi.c.orig       2009-10-13
11:54:11.000000000 -0700
+++ ./tools/ioemu-remote/hw/pt-msi.c    2009-10-15 10:29:50.000000000 -0700
@@ -542,6 +542,7 @@
     int i, total_entries, table_off, bar_index;
     struct pci_dev *pd = dev->pci_dev;
     int fd;
+    int err;

     id = pci_read_byte(pd, pos + PCI_CAP_LIST_ID);

@@ -584,9 +585,15 @@
         PT_LOG("Error: Can't open /dev/mem: %s\n", strerror(errno));
         goto error_out;
     }
-    dev->msix->phys_iomem_base = mmap(0, total_entries * 16,
+    PT_LOG("table_off = %llx, total_entries = %d\n",table_off,total_entries);
+    dev->msix->table_offset_adjust = table_off & 0x0fff;
+    dev->msix->phys_iomem_base = mmap(0, total_entries * 16 +
dev->msix->table_offset_adjust,
                           PROT_WRITE | PROT_READ, MAP_SHARED | MAP_LOCKED,
-                          fd, dev->msix->table_base + table_off);
+                          fd, dev->msix->table_base + table_off -
dev->msix->table_offset_adjust);
+    dev->msix->phys_iomem_base = (void *)((char *)dev->msix->phys_iomem_base +
+                          dev->msix->table_offset_adjust);
+    err = errno;
+    PT_LOG("errno = %d\n",err);
     if ( dev->msix->phys_iomem_base == MAP_FAILED )
     {
         PT_LOG("Error: Can't map physical MSI-X table: %s\n", strerror(errno));
@@ -612,7 +619,8 @@
     {
         PT_LOG("unmapping physical MSI-X table from %lx\n",
            (unsigned long)dev->msix->phys_iomem_base);
-        munmap(dev->msix->phys_iomem_base, dev->msix->total_entries * 16);
+        munmap(dev->msix->phys_iomem_base, dev->msix->total_entries * 16 +
+           dev->msix->table_offset_adjust);
     }

     free(dev->msix);

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

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