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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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?

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