| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 22 Nov 2022 14:36:56 +0800
From: Jason Wang <jasowang@...hat.com>
To: Shannon Nelson <snelson@...sando.io>, netdev@...r.kernel.org,
davem@...emloft.net, kuba@...nel.org, mst@...hat.com,
virtualization@...ts.linux-foundation.org
Cc: drivers@...sando.io
Subject: Re: [RFC PATCH net-next 15/19] pds_vdpa: virtio bar setup for vdpa
On Tue, Nov 22, 2022 at 11:32 AM Jason Wang <jasowang@...hat.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@...sando.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);
> > +}
Powered by blists - more mailing lists