| 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
| ||
|
Message-ID: <CACGkMEt5Jbsp=+st8aG_0kXD+OSSp+FX9vYE+gTkywp2ZN4LTw@mail.gmail.com>
Date: Wed, 15 Mar 2023 15:05:23 +0800
From: Jason Wang <jasowang@...hat.com>
To: Shannon Nelson <shannon.nelson@....com>
Cc: mst@...hat.com, virtualization@...ts.linux-foundation.org,
brett.creeley@....com, davem@...emloft.net, netdev@...r.kernel.org,
kuba@...nel.org, drivers@...sando.io
Subject: Re: [PATCH RFC v2 virtio 3/7] pds_vdpa: virtio bar setup for vdpa
On Thu, Mar 9, 2023 at 9:31 AM Shannon Nelson <shannon.nelson@....com> wrote:
>
> 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.
By passing a pointer to a customized id probing routine to vp_modern_probe()?
Thanks
>
> 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 <shannon.nelson@....com>
> ---
> drivers/vdpa/pds/Makefile | 1 +
> drivers/vdpa/pds/aux_drv.c | 14 ++
> drivers/vdpa/pds/aux_drv.h | 1 +
> drivers/vdpa/pds/debugfs.c | 1 +
> drivers/vdpa/pds/vdpa_dev.c | 1 +
> drivers/vdpa/pds/virtio_pci.c | 281 ++++++++++++++++++++++++++++++++++
> drivers/vdpa/pds/virtio_pci.h | 8 +
> 7 files changed, 307 insertions(+)
> create mode 100644 drivers/vdpa/pds/virtio_pci.c
> create mode 100644 drivers/vdpa/pds/virtio_pci.h
>
> diff --git a/drivers/vdpa/pds/Makefile b/drivers/vdpa/pds/Makefile
> index 13b50394ec64..ca2efa8c6eb5 100644
> --- a/drivers/vdpa/pds/Makefile
> +++ b/drivers/vdpa/pds/Makefile
> @@ -4,6 +4,7 @@
> obj-$(CONFIG_PDS_VDPA) := pds_vdpa.o
>
> pds_vdpa-y := aux_drv.o \
> + virtio_pci.o \
> vdpa_dev.o
>
> pds_vdpa-$(CONFIG_DEBUG_FS) += debugfs.o
> diff --git a/drivers/vdpa/pds/aux_drv.c b/drivers/vdpa/pds/aux_drv.c
> index 63e40ae68211..28158d0d98a5 100644
> --- a/drivers/vdpa/pds/aux_drv.c
> +++ b/drivers/vdpa/pds/aux_drv.c
> @@ -4,6 +4,7 @@
> #include <linux/auxiliary_bus.h>
> #include <linux/pci.h>
> #include <linux/vdpa.h>
> +#include <linux/virtio_pci_modern.h>
>
> #include <linux/pds/pds_core.h>
> #include <linux/pds/pds_auxbus.h>
> @@ -12,6 +13,7 @@
> #include "aux_drv.h"
> #include "debugfs.h"
> #include "vdpa_dev.h"
> +#include "virtio_pci.h"
>
> static const struct auxiliary_device_id pds_vdpa_id_table[] = {
> { .name = PDS_VDPA_DEV_NAME, },
> @@ -49,8 +51,19 @@ static int pds_vdpa_probe(struct auxiliary_device *aux_dev,
> if (err)
> goto err_aux_unreg;
>
> + /* Find the virtio configuration */
> + vdpa_aux->vd_mdev.pci_dev = padev->vf->pdev;
> + err = pds_vdpa_probe_virtio(&vdpa_aux->vd_mdev);
> + if (err) {
> + dev_err(dev, "Unable to probe for virtio configuration: %pe\n",
> + ERR_PTR(err));
> + goto err_free_mgmt_info;
> + }
> +
> return 0;
>
> +err_free_mgmt_info:
> + pci_free_irq_vectors(padev->vf->pdev);
> err_aux_unreg:
> padev->ops->unregister_client(padev);
> err_free_mem:
> @@ -65,6 +78,7 @@ static void pds_vdpa_remove(struct auxiliary_device *aux_dev)
> struct pds_vdpa_aux *vdpa_aux = auxiliary_get_drvdata(aux_dev);
> struct device *dev = &aux_dev->dev;
>
> + pds_vdpa_remove_virtio(&vdpa_aux->vd_mdev);
> pci_free_irq_vectors(vdpa_aux->padev->vf->pdev);
>
> vdpa_aux->padev->ops->unregister_client(vdpa_aux->padev);
> diff --git a/drivers/vdpa/pds/aux_drv.h b/drivers/vdpa/pds/aux_drv.h
> index 94ba7abcaa43..87ac3c01c476 100644
> --- a/drivers/vdpa/pds/aux_drv.h
> +++ b/drivers/vdpa/pds/aux_drv.h
> @@ -16,6 +16,7 @@ struct pds_vdpa_aux {
>
> int vf_id;
> struct dentry *dentry;
> + struct virtio_pci_modern_device vd_mdev;
>
> int nintrs;
> };
> diff --git a/drivers/vdpa/pds/debugfs.c b/drivers/vdpa/pds/debugfs.c
> index 7b7e90fd6578..aa5e9677fe74 100644
> --- a/drivers/vdpa/pds/debugfs.c
> +++ b/drivers/vdpa/pds/debugfs.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /* Copyright(c) 2023 Advanced Micro Devices, Inc */
>
> +#include <linux/virtio_pci_modern.h>
> #include <linux/vdpa.h>
>
> #include <linux/pds/pds_core.h>
> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
> index bd840688503c..15d623297203 100644
> --- a/drivers/vdpa/pds/vdpa_dev.c
> +++ b/drivers/vdpa/pds/vdpa_dev.c
> @@ -4,6 +4,7 @@
> #include <linux/pci.h>
> #include <linux/vdpa.h>
> #include <uapi/linux/vdpa.h>
> +#include <linux/virtio_pci_modern.h>
>
> #include <linux/pds/pds_core.h>
> #include <linux/pds/pds_adminq.h>
> diff --git a/drivers/vdpa/pds/virtio_pci.c b/drivers/vdpa/pds/virtio_pci.c
> new file mode 100644
> index 000000000000..cb879619dac3
> --- /dev/null
> +++ b/drivers/vdpa/pds/virtio_pci.c
> @@ -0,0 +1,281 @@
> +// 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/pci.h>
> +
> +#include "virtio_pci.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);
> +}
> diff --git a/drivers/vdpa/pds/virtio_pci.h b/drivers/vdpa/pds/virtio_pci.h
> new file mode 100644
> index 000000000000..f017cfa1173c
> --- /dev/null
> +++ b/drivers/vdpa/pds/virtio_pci.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright(c) 2023 Advanced Micro Devices, Inc */
> +
> +#ifndef _PDS_VIRTIO_PCI_H_
> +#define _PDS_VIRTIO_PCI_H_
> +int pds_vdpa_probe_virtio(struct virtio_pci_modern_device *mdev);
> +void pds_vdpa_remove_virtio(struct virtio_pci_modern_device *mdev);
> +#endif /* _PDS_VIRTIO_PCI_H_ */
> --
> 2.17.1
>
Powered by blists - more mailing lists