[Qemu-devel] [RFC] mmap of BAR0 fails for ivshmem device

[Qemu-devel] [RFC] mmap of BAR0 fails for ivshmem device

[Cam Macdonell]

[-- Attachment #1: Type: text/plain, Size: 2141 bytes --]

Hello,

A bug was reported to me regarding mmaping of BAR0 in ivshmem.  Indeed the
mmap fails.  This bug will affect those using the ivshmem server as BAR0
contains the registers for sending and receiving interrupts.  It does not
affect those mapping just the shared memory region.

I have bisected to a patch from 3.12

commit 7314e613d5ff9f0934f7a0f74ed7973b903315d1
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Tue Oct 29 10:21:34 2013 -0700

    Fix a few incorrectly checked [io_]remap_pfn_range() calls

    Nico Golde reports a few straggling uses of [io_]remap_pfn_range() that
    really should use the vm_iomap_memory() helper.  This trivially converts
    two of them to the helper, and comments about why the third one really
    needs to continue to use remap_pfn_range(), and adds the missing size
    check.

    Reported-by: Nico Golde <nico@ngolde.de>
    Cc: stable@kernel.org
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org.

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index ba47563..0e808cf 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -642,16 +642,29 @@ static int uio_mmap_physical(struct vm_area_struct
*vma)
 {
        struct uio_device *idev = vma->vm_private_data;
        int mi = uio_find_mem_index(vma);
+       struct uio_mem *mem;
        if (mi < 0)
                return -EINVAL;
+       mem = idev->info->mem + mi;

-       vma->vm_ops = &uio_physical_vm_ops;
+       if (vma->vm_end - vma->vm_start > mem->size)
+               return -EINVAL;

<snip>

The last two lines shown above that check the length of the vm area cause
the mmap to fail because ivshmem's BAR0 is only 256 bytes.

One possible fix is to increase the size of BAR0 to the size of a page.  Of
course, I'd prefer to be able to fix this from my uio driver, but I'm not
sure that is possible given the patch above changes the generic uio code.
 Advice is welcome.

Finally, I apologize for not catching this bug earlier.  It's an effect of
not having the uio driver in the kernel.  To avoid this in future, I will
work to get the UIO ivshmem driver into the kernel.

Sincerely,
Cam

[-- Attachment #2: Type: text/html, Size: 3007 bytes --]

Re: [Qemu-devel] [RFC] mmap of BAR0 fails for ivshmem device

[Chrysostomos Nanakos]

Hello,

On Fri, Aug 29, 2014 at 04:58:20PM -0600, Cam Macdonell wrote:
> Hello,
> 
> A bug was reported to me regarding mmaping of BAR0 in ivshmem.  Indeed the
> mmap fails.  This bug will affect those using the ivshmem server as BAR0
> contains the registers for sending and receiving interrupts.  It does not
> affect those mapping just the shared memory region.
> 
> I have bisected to a patch from 3.12
> 
> commit 7314e613d5ff9f0934f7a0f74ed7973b903315d1
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date:   Tue Oct 29 10:21:34 2013 -0700
> 
>     Fix a few incorrectly checked [io_]remap_pfn_range() calls
> 
>     Nico Golde reports a few straggling uses of [io_]remap_pfn_range() that
>     really should use the vm_iomap_memory() helper.  This trivially converts
>     two of them to the helper, and comments about why the third one really
>     needs to continue to use remap_pfn_range(), and adds the missing size
>     check.
> 
>     Reported-by: Nico Golde <nico@ngolde.de>
>     Cc: stable@kernel.org
>     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org.
> 
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index ba47563..0e808cf 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c
> @@ -642,16 +642,29 @@ static int uio_mmap_physical(struct vm_area_struct
> *vma)
>  {
>         struct uio_device *idev = vma->vm_private_data;
>         int mi = uio_find_mem_index(vma);
> +       struct uio_mem *mem;
>         if (mi < 0)
>                 return -EINVAL;
> +       mem = idev->info->mem + mi;
> 
> -       vma->vm_ops = &uio_physical_vm_ops;
> +       if (vma->vm_end - vma->vm_start > mem->size)
> +               return -EINVAL;
> 
> <snip>
> 
> The last two lines shown above that check the length of the vm area cause
> the mmap to fail because ivshmem's BAR0 is only 256 bytes.
> 
> One possible fix is to increase the size of BAR0 to the size of a page.  Of
> course, I'd prefer to be able to fix this from my uio driver, but I'm not
> sure that is possible given the patch above changes the generic uio code.
>  Advice is welcome.

Another possible solution is to page align the size of BAR0 in your uio driver
without changing the size of BAR0 in hw/misc/ivshmem.c.

diff --git a/kernel_module/uio/uio_ivshmem.c b/kernel_module/uio/uio_ivshmem.c
index 63504ce..25e6eb7 100644
--- a/kernel_module/uio/uio_ivshmem.c
+++ b/kernel_module/uio/uio_ivshmem.c
@@ -149,7 +149,7 @@ static int ivshmem_pci_probe(struct pci_dev *dev,
        if (!info->mem[0].addr)
                goto out_release;

-       info->mem[0].size = pci_resource_len(dev, 0);
+       info->mem[0].size = PAGE_ALIGN(pci_resource_len(dev, 0));
        info->mem[0].internal_addr = pci_ioremap_bar(dev, 0);
        if (!info->mem[0].internal_addr) {
                goto out_release;

but I don't know if this is the best solution.


Regards,
Chrysostomos.
> 
> Finally, I apologize for not catching this bug earlier.  It's an effect of
> not having the uio driver in the kernel.  To avoid this in future, I will
> work to get the UIO ivshmem driver into the kernel.
> 
> Sincerely,
> Cam