lkml.org 
[lkml]   [2010]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH 3/3] VFIO V4: VFIO driver: Non-privileged user level PCI drivers
    From
    Date
    On Wed, 2010-09-22 at 14:18 -0700, Tom Lyon wrote:
    > +ssize_t vfio_mem_readwrite(
    > + int write,
    > + struct vfio_dev *vdev,
    > + char __user *buf,
    > + size_t count,
    > + loff_t *ppos)
    > +{
    > + struct pci_dev *pdev = vdev->pdev;
    > + resource_size_t end;
    > + void __iomem *io;
    > + loff_t pos;
    > + int pci_space;
    > +
    > + pci_space = vfio_offset_to_pci_space(*ppos);
    > + pos = vfio_offset_to_pci_offset(*ppos);
    > +
    > + if (!pci_resource_start(pdev, pci_space))
    > + return -EINVAL;
    > + end = pci_resource_len(pdev, pci_space);
    > + if (vdev->barmap[pci_space] == NULL)
    > + vdev->barmap[pci_space] = pci_iomap(pdev, pci_space, 0);
    > + io = vdev->barmap[pci_space];
    > +

    So we do a pci_iomap, but never do corresponding pci_iounmap. This also
    only works for the first 6 BARs since the ROM BAR needs pci_map_rom. I
    wonder if we should be doing all the BAR mapping at open and unmap at
    close so that we can fail if the device can't get basic resources. I
    believe we should also be calling pci_request_regions in here somewhere.
    Perhaps something like this:

    diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
    index a18e39a..d3886d9 100644
    --- a/drivers/vfio/vfio_main.c
    +++ b/drivers/vfio/vfio_main.c
    @@ -85,6 +85,53 @@ static inline int overlap(int a1, int b1, int a2, int b2)
    return !(b2 <= a1 || b1 <= a2);
    }

    +static int vfio_setup_pci(struct vfio_dev *vdev)
    +{
    + int ret, bar;
    +
    + ret = pci_enable_device(vdev->pdev);
    + if (ret)
    + return ret;
    +
    + ret = pci_request_regions(vdev->pdev, "VFIO");
    + if (ret) {
    + pci_disable_device(vdev->pdev);
    + return ret;
    + }
    +
    + for (bar = PCI_STD_RESOURCES; bar <= PCI_ROM_RESOURCE; bar++) {
    + if (!pci_resource_len(vdev->pdev, bar))
    + continue;
    + if (bar != PCI_ROM_RESOURCE) {
    + if (!pci_resource_start(vdev->pdev, bar))
    + continue;
    + vdev->barmap[bar] = pci_iomap(vdev->pdev, bar, 0);
    + } else {
    + size_t size;
    + vdev->barmap[bar] = pci_map_rom(vdev->pdev, &size);
    + }
    + }
    + return ret;
    +}
    +
    +static void vfio_disable_pci(struct vfio_dev *vdev)
    +{
    + int bar;
    +
    + for (bar = PCI_STD_RESOURCES; bar <= PCI_ROM_RESOURCE; bar++) {
    + if (!vdev->barmap[bar])
    + continue;
    + if (bar != PCI_ROM_RESOURCE)
    + pci_iounmap(vdev->pdev, vdev->barmap[bar]);
    + else
    + pci_unmap_rom(vdev->pdev, vdev->barmap[bar]);
    + vdev->barmap[bar] = NULL;
    + }
    +
    + pci_release_regions(vdev->pdev);
    + pci_disable_device(vdev->pdev);
    +}
    +
    static int vfio_open(struct inode *inode, struct file *filep)
    {
    struct vfio_dev *vdev;
    @@ -110,7 +157,7 @@ static int vfio_open(struct inode *inode, struct file *filep)
    INIT_LIST_HEAD(&listener->dm_list);
    filep->private_data = listener;
    if (vdev->listeners == 0)
    - ret = pci_enable_device(vdev->pdev);
    + ret = vfio_setup_pci(vdev);
    if (ret == 0)
    vdev->listeners++;
    mutex_unlock(&vdev->lgate);
    @@ -151,7 +198,7 @@ static int vfio_release(struct inode *inode, struct file *filep)
    vdev->vconfig = NULL;
    kfree(vdev->pci_config_map);
    vdev->pci_config_map = NULL;
    - pci_disable_device(vdev->pdev);
    + vfio_disable_pci(vdev);
    vfio_domain_unset(vdev);
    wake_up(&vdev->dev_idle_q);
    }
    diff --git a/drivers/vfio/vfio_rdwr.c b/drivers/vfio/vfio_rdwr.c
    index 1fd50a6..7705b45 100644
    --- a/drivers/vfio/vfio_rdwr.c
    +++ b/drivers/vfio/vfio_rdwr.c
    @@ -64,7 +64,7 @@ ssize_t vfio_io_readwrite(
    if (pos + count > end)
    return -EINVAL;
    if (vdev->barmap[pci_space] == NULL)
    - vdev->barmap[pci_space] = pci_iomap(pdev, pci_space, 0);
    + return -EINVAL;
    io = vdev->barmap[pci_space];

    while (count > 0) {
    @@ -137,7 +137,12 @@ ssize_t vfio_mem_readwrite(
    return -EINVAL;
    end = pci_resource_len(pdev, pci_space);
    if (vdev->barmap[pci_space] == NULL)
    - vdev->barmap[pci_space] = pci_iomap(pdev, pci_space, 0);
    + return -EINVAL;
    + if (pci_space == PCI_ROM_RESOURCE) {
    + u32 rom = *(u32 *)(vdev->vconfig + PCI_ROM_ADDRESS);
    + if (!(rom & PCI_ROM_ADDRESS_ENABLE))
    + return -EINVAL;
    + }
    io = vdev->barmap[pci_space];

    if (pos > end)



    \
     
     \ /
      Last update: 2010-09-27 22:49    [W:0.030 / U:122.952 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site