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 <address@hidden>
> 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 <address@hidden>
>     Cc: address@hidden
>     Signed-off-by: Linus Torvalds <address@hidden
> 
> 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