Re: [RFC PATCH net-next 15/19] pds_vdpa: virtio bar setup for vdpa

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

 



On Tue, Nov 22, 2022 at 11:32 AM Jason Wang <jasowang@xxxxxxxxxx> wrote:
>
>
> 在 2022/11/19 06:56, Shannon Nelson 写道:
> > The PDS vDPA device has a virtio BAR for describing itself, and
> > the pds_vdpa driver needs to access it.  Here we copy liberally
> > from the existing drivers/virtio/virtio_pci_modern_dev.c as it
> > has what we need, but we need to modify it so that it can work
> > with our device id and so we can use our own DMA mask.
> >
> > We suspect there is room for discussion here about making the
> > existing code a little more flexible, but we thought we'd at
> > least start the discussion here.
>
>
> Exactly, since the virtio_pci_modern_dev.c is a library, we could tweak
> it to allow the caller to pass the device_id with the DMA mask. Then we
> can avoid code/bug duplication here.

Btw, I found only isr/notification were used but not the others? If
this is true, we can avoid mapping those capabilities.

Thanks

>
> Thanks
>
>
> >
> > Signed-off-by: Shannon Nelson <snelson@xxxxxxxxxxx>
> > ---
> >   drivers/vdpa/pds/Makefile     |   3 +-
> >   drivers/vdpa/pds/pci_drv.c    |  10 ++
> >   drivers/vdpa/pds/pci_drv.h    |   2 +
> >   drivers/vdpa/pds/virtio_pci.c | 283 ++++++++++++++++++++++++++++++++++
> >   4 files changed, 297 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/vdpa/pds/virtio_pci.c
> >
> > diff --git a/drivers/vdpa/pds/Makefile b/drivers/vdpa/pds/Makefile
> > index 3ba28a875574..b8376ab165bc 100644
> > --- a/drivers/vdpa/pds/Makefile
> > +++ b/drivers/vdpa/pds/Makefile
> > @@ -4,4 +4,5 @@
> >   obj-$(CONFIG_PDS_VDPA) := pds_vdpa.o
> >
> >   pds_vdpa-y := pci_drv.o     \
> > -           debugfs.o
> > +           debugfs.o \
> > +           virtio_pci.o
> > diff --git a/drivers/vdpa/pds/pci_drv.c b/drivers/vdpa/pds/pci_drv.c
> > index 369e11153f21..10491e22778c 100644
> > --- a/drivers/vdpa/pds/pci_drv.c
> > +++ b/drivers/vdpa/pds/pci_drv.c
> > @@ -44,6 +44,14 @@ pds_vdpa_pci_probe(struct pci_dev *pdev,
> >               goto err_out_free_mem;
> >       }
> >
> > +     vdpa_pdev->vd_mdev.pci_dev = pdev;
> > +     err = pds_vdpa_probe_virtio(&vdpa_pdev->vd_mdev);
> > +     if (err) {
> > +             dev_err(dev, "Unable to probe for virtio configuration: %pe\n",
> > +                     ERR_PTR(err));
> > +             goto err_out_free_mem;
> > +     }
> > +
> >       pci_enable_pcie_error_reporting(pdev);
> >
> >       /* Use devres management */
> > @@ -74,6 +82,7 @@ pds_vdpa_pci_probe(struct pci_dev *pdev,
> >   err_out_pci_release_device:
> >       pci_disable_device(pdev);
> >   err_out_free_mem:
> > +     pds_vdpa_remove_virtio(&vdpa_pdev->vd_mdev);
> >       pci_disable_pcie_error_reporting(pdev);
> >       kfree(vdpa_pdev);
> >       return err;
> > @@ -88,6 +97,7 @@ pds_vdpa_pci_remove(struct pci_dev *pdev)
> >       pci_clear_master(pdev);
> >       pci_disable_pcie_error_reporting(pdev);
> >       pci_disable_device(pdev);
> > +     pds_vdpa_remove_virtio(&vdpa_pdev->vd_mdev);
> >       kfree(vdpa_pdev);
> >
> >       dev_info(&pdev->dev, "Removed\n");
> > diff --git a/drivers/vdpa/pds/pci_drv.h b/drivers/vdpa/pds/pci_drv.h
> > index 747809e0df9e..15f3b34fafa9 100644
> > --- a/drivers/vdpa/pds/pci_drv.h
> > +++ b/drivers/vdpa/pds/pci_drv.h
> > @@ -43,4 +43,6 @@ struct pds_vdpa_pci_device {
> >       struct virtio_pci_modern_device vd_mdev;
> >   };
> >
> > +int pds_vdpa_probe_virtio(struct virtio_pci_modern_device *mdev);
> > +void pds_vdpa_remove_virtio(struct virtio_pci_modern_device *mdev);
> >   #endif /* _PCI_DRV_H */
> > diff --git a/drivers/vdpa/pds/virtio_pci.c b/drivers/vdpa/pds/virtio_pci.c
> > new file mode 100644
> > index 000000000000..0f4ac9467199
> > --- /dev/null
> > +++ b/drivers/vdpa/pds/virtio_pci.c
> > @@ -0,0 +1,283 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +/*
> > + * adapted from drivers/virtio/virtio_pci_modern_dev.c, v6.0-rc1
> > + */
> > +
> > +#include <linux/virtio_pci_modern.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/delay.h>
> > +
> > +#include "pci_drv.h"
> > +
> > +/*
> > + * pds_vdpa_map_capability - map a part of virtio pci capability
> > + * @mdev: the modern virtio-pci device
> > + * @off: offset of the capability
> > + * @minlen: minimal length of the capability
> > + * @align: align requirement
> > + * @start: start from the capability
> > + * @size: map size
> > + * @len: the length that is actually mapped
> > + * @pa: physical address of the capability
> > + *
> > + * Returns the io address of for the part of the capability
> > + */
> > +static void __iomem *
> > +pds_vdpa_map_capability(struct virtio_pci_modern_device *mdev, int off,
> > +                      size_t minlen, u32 align, u32 start, u32 size,
> > +                      size_t *len, resource_size_t *pa)
> > +{
> > +     struct pci_dev *dev = mdev->pci_dev;
> > +     u8 bar;
> > +     u32 offset, length;
> > +     void __iomem *p;
> > +
> > +     pci_read_config_byte(dev, off + offsetof(struct virtio_pci_cap,
> > +                                              bar),
> > +                          &bar);
> > +     pci_read_config_dword(dev, off + offsetof(struct virtio_pci_cap, offset),
> > +                          &offset);
> > +     pci_read_config_dword(dev, off + offsetof(struct virtio_pci_cap, length),
> > +                           &length);
> > +
> > +     /* Check if the BAR may have changed since we requested the region. */
> > +     if (bar >= PCI_STD_NUM_BARS || !(mdev->modern_bars & (1 << bar))) {
> > +             dev_err(&dev->dev,
> > +                     "virtio_pci: bar unexpectedly changed to %u\n", bar);
> > +             return NULL;
> > +     }
> > +
> > +     if (length <= start) {
> > +             dev_err(&dev->dev,
> > +                     "virtio_pci: bad capability len %u (>%u expected)\n",
> > +                     length, start);
> > +             return NULL;
> > +     }
> > +
> > +     if (length - start < minlen) {
> > +             dev_err(&dev->dev,
> > +                     "virtio_pci: bad capability len %u (>=%zu expected)\n",
> > +                     length, minlen);
> > +             return NULL;
> > +     }
> > +
> > +     length -= start;
> > +
> > +     if (start + offset < offset) {
> > +             dev_err(&dev->dev,
> > +                     "virtio_pci: map wrap-around %u+%u\n",
> > +                     start, offset);
> > +             return NULL;
> > +     }
> > +
> > +     offset += start;
> > +
> > +     if (offset & (align - 1)) {
> > +             dev_err(&dev->dev,
> > +                     "virtio_pci: offset %u not aligned to %u\n",
> > +                     offset, align);
> > +             return NULL;
> > +     }
> > +
> > +     if (length > size)
> > +             length = size;
> > +
> > +     if (len)
> > +             *len = length;
> > +
> > +     if (minlen + offset < minlen ||
> > +         minlen + offset > pci_resource_len(dev, bar)) {
> > +             dev_err(&dev->dev,
> > +                     "virtio_pci: map virtio %zu@%u out of range on bar %i length %lu\n",
> > +                     minlen, offset,
> > +                     bar, (unsigned long)pci_resource_len(dev, bar));
> > +             return NULL;
> > +     }
> > +
> > +     p = pci_iomap_range(dev, bar, offset, length);
> > +     if (!p)
> > +             dev_err(&dev->dev,
> > +                     "virtio_pci: unable to map virtio %u@%u on bar %i\n",
> > +                     length, offset, bar);
> > +     else if (pa)
> > +             *pa = pci_resource_start(dev, bar) + offset;
> > +
> > +     return p;
> > +}
> > +
> > +/**
> > + * virtio_pci_find_capability - walk capabilities to find device info.
> > + * @dev: the pci device
> > + * @cfg_type: the VIRTIO_PCI_CAP_* value we seek
> > + * @ioresource_types: IORESOURCE_MEM and/or IORESOURCE_IO.
> > + * @bars: the bitmask of BARs
> > + *
> > + * Returns offset of the capability, or 0.
> > + */
> > +static inline int virtio_pci_find_capability(struct pci_dev *dev, u8 cfg_type,
> > +                                          u32 ioresource_types, int *bars)
> > +{
> > +     int pos;
> > +
> > +     for (pos = pci_find_capability(dev, PCI_CAP_ID_VNDR);
> > +          pos > 0;
> > +          pos = pci_find_next_capability(dev, pos, PCI_CAP_ID_VNDR)) {
> > +             u8 type, bar;
> > +
> > +             pci_read_config_byte(dev, pos + offsetof(struct virtio_pci_cap,
> > +                                                      cfg_type),
> > +                                  &type);
> > +             pci_read_config_byte(dev, pos + offsetof(struct virtio_pci_cap,
> > +                                                      bar),
> > +                                  &bar);
> > +
> > +             /* Ignore structures with reserved BAR values */
> > +             if (bar >= PCI_STD_NUM_BARS)
> > +                     continue;
> > +
> > +             if (type == cfg_type) {
> > +                     if (pci_resource_len(dev, bar) &&
> > +                         pci_resource_flags(dev, bar) & ioresource_types) {
> > +                             *bars |= (1 << bar);
> > +                             return pos;
> > +                     }
> > +             }
> > +     }
> > +     return 0;
> > +}
> > +
> > +/*
> > + * pds_vdpa_probe_virtio: probe the modern virtio pci device, note that the
> > + * caller is required to enable PCI device before calling this function.
> > + * @mdev: the modern virtio-pci device
> > + *
> > + * Return 0 on succeed otherwise fail
> > + */
> > +int pds_vdpa_probe_virtio(struct virtio_pci_modern_device *mdev)
> > +{
> > +     struct pci_dev *pci_dev = mdev->pci_dev;
> > +     int err, common, isr, notify, device;
> > +     u32 notify_length;
> > +     u32 notify_offset;
> > +
> > +     /* check for a common config: if not, use legacy mode (bar 0). */
> > +     common = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_COMMON_CFG,
> > +                                         IORESOURCE_IO | IORESOURCE_MEM,
> > +                                         &mdev->modern_bars);
> > +     if (!common) {
> > +             dev_info(&pci_dev->dev,
> > +                      "virtio_pci: missing common config\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     /* If common is there, these should be too... */
> > +     isr = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_ISR_CFG,
> > +                                      IORESOURCE_IO | IORESOURCE_MEM,
> > +                                      &mdev->modern_bars);
> > +     notify = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_NOTIFY_CFG,
> > +                                         IORESOURCE_IO | IORESOURCE_MEM,
> > +                                         &mdev->modern_bars);
> > +     if (!isr || !notify) {
> > +             dev_err(&pci_dev->dev,
> > +                     "virtio_pci: missing capabilities %i/%i/%i\n",
> > +                     common, isr, notify);
> > +             return -EINVAL;
> > +     }
> > +
> > +     /* Device capability is only mandatory for devices that have
> > +      * device-specific configuration.
> > +      */
> > +     device = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_DEVICE_CFG,
> > +                                         IORESOURCE_IO | IORESOURCE_MEM,
> > +                                         &mdev->modern_bars);
> > +
> > +     err = pci_request_selected_regions(pci_dev, mdev->modern_bars,
> > +                                        "virtio-pci-modern");
> > +     if (err)
> > +             return err;
> > +
> > +     err = -EINVAL;
> > +     mdev->common = pds_vdpa_map_capability(mdev, common,
> > +                                   sizeof(struct virtio_pci_common_cfg), 4,
> > +                                   0, sizeof(struct virtio_pci_common_cfg),
> > +                                   NULL, NULL);
> > +     if (!mdev->common)
> > +             goto err_map_common;
> > +     mdev->isr = pds_vdpa_map_capability(mdev, isr, sizeof(u8), 1,
> > +                                          0, 1,
> > +                                          NULL, NULL);
> > +     if (!mdev->isr)
> > +             goto err_map_isr;
> > +
> > +     /* Read notify_off_multiplier from config space. */
> > +     pci_read_config_dword(pci_dev,
> > +                           notify + offsetof(struct virtio_pci_notify_cap,
> > +                                             notify_off_multiplier),
> > +                           &mdev->notify_offset_multiplier);
> > +     /* Read notify length and offset from config space. */
> > +     pci_read_config_dword(pci_dev,
> > +                           notify + offsetof(struct virtio_pci_notify_cap,
> > +                                             cap.length),
> > +                           &notify_length);
> > +
> > +     pci_read_config_dword(pci_dev,
> > +                           notify + offsetof(struct virtio_pci_notify_cap,
> > +                                             cap.offset),
> > +                           &notify_offset);
> > +
> > +     /* We don't know how many VQs we'll map, ahead of the time.
> > +      * If notify length is small, map it all now.
> > +      * Otherwise, map each VQ individually later.
> > +      */
> > +     if ((u64)notify_length + (notify_offset % PAGE_SIZE) <= PAGE_SIZE) {
> > +             mdev->notify_base = pds_vdpa_map_capability(mdev, notify,
> > +                                                          2, 2,
> > +                                                          0, notify_length,
> > +                                                          &mdev->notify_len,
> > +                                                          &mdev->notify_pa);
> > +             if (!mdev->notify_base)
> > +                     goto err_map_notify;
> > +     } else {
> > +             mdev->notify_map_cap = notify;
> > +     }
> > +
> > +     /* Again, we don't know how much we should map, but PAGE_SIZE
> > +      * is more than enough for all existing devices.
> > +      */
> > +     if (device) {
> > +             mdev->device = pds_vdpa_map_capability(mdev, device, 0, 4,
> > +                                                     0, PAGE_SIZE,
> > +                                                     &mdev->device_len,
> > +                                                     NULL);
> > +             if (!mdev->device)
> > +                     goto err_map_device;
> > +     }
> > +
> > +     return 0;
> > +
> > +err_map_device:
> > +     if (mdev->notify_base)
> > +             pci_iounmap(pci_dev, mdev->notify_base);
> > +err_map_notify:
> > +     pci_iounmap(pci_dev, mdev->isr);
> > +err_map_isr:
> > +     pci_iounmap(pci_dev, mdev->common);
> > +err_map_common:
> > +     pci_release_selected_regions(pci_dev, mdev->modern_bars);
> > +     return err;
> > +}
> > +
> > +void pds_vdpa_remove_virtio(struct virtio_pci_modern_device *mdev)
> > +{
> > +     struct pci_dev *pci_dev = mdev->pci_dev;
> > +
> > +     if (mdev->device)
> > +             pci_iounmap(pci_dev, mdev->device);
> > +     if (mdev->notify_base)
> > +             pci_iounmap(pci_dev, mdev->notify_base);
> > +     pci_iounmap(pci_dev, mdev->isr);
> > +     pci_iounmap(pci_dev, mdev->common);
> > +     pci_release_selected_regions(pci_dev, mdev->modern_bars);
> > +}

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux