| Message ID | 20221118225656.48309-16-snelson@pensando.io (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | pds core and vdpa drivers | expand |
在 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. Thanks > > Signed-off-by: Shannon Nelson <snelson@pensando.io> > --- > 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), > + ¬ify_length); > + > + pci_read_config_dword(pci_dev, > + notify + offsetof(struct virtio_pci_notify_cap, > + cap.offset), > + ¬ify_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); > +}
On Tue, Nov 22, 2022 at 11:32 AM Jason Wang <jasowang@redhat.com> 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@pensando.io> > > --- > > 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), > > + ¬ify_length); > > + > > + pci_read_config_dword(pci_dev, > > + notify + offsetof(struct virtio_pci_notify_cap, > > + cap.offset), > > + ¬ify_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); > > +}
On 11/21/22 10:36 PM, Jason Wang wrote: > On Tue, Nov 22, 2022 at 11:32 AM Jason Wang <jasowang@redhat.com> 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. I'll look into possible mods for it, although I'm not sure how quickly I can get to that... maybe a v+1 along the way. > > Btw, I found only isr/notification were used but not the others? If > this is true, we can avoid mapping those capabilities. I'll keep this in mind. sln
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), + ¬ify_length); + + pci_read_config_dword(pci_dev, + notify + offsetof(struct virtio_pci_notify_cap, + cap.offset), + ¬ify_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); +}
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. Signed-off-by: Shannon Nelson <snelson@pensando.io> --- 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