* [PATCH V2 00/14] vDPA driver for virtio-pci device
@ 2020-11-26 9:25 Jason Wang
2020-11-26 9:25 ` [PATCH V2 01/14] virtio-pci: do not access iomem via virtio_pci_device directly Jason Wang
` (13 more replies)
0 siblings, 14 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:25 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
Hi all:
This series tries to implement a vDPA driver for virtio-pci device
which will bridge between vDPA bus and virtio-pci device.
This could be used for future feature prototyping and testing.
Please review
Changes from V1:
- Split common codes from virito-pci and share it with vDPA driver
- Use dynamic id in order to be less confusing with virtio-pci driver
- No feature whitelist, supporting any features (mq, config etc)
Thanks
Jason Wang (14):
virtio-pci: do not access iomem via virtio_pci_device directly
virtio-pci: switch to use devres for modern devices
virtio-pci: split out modern device
virtio-pci: move the notification sanity check to vp_modern_probe()
virtio-pci-modern: introduce vp_modern_set_queue_vector()
virtio-pci-modern: introduce vp_modern_queue_address()
virtio-pci-modern: introduce helper to set/get queue_enable
virtio-pci-modern: introduce helper for setting/geting queue size
virtio-pci-modern: introduce helper for getting queue nums
virtio-pci-modern: introduce helper to get notification offset
virtio-pci: introduce modern device module
vdpa: set the virtqueue num during register
virtio_vdpa: don't warn when fail to disable vq
vdpa: introduce virtio pci driver
drivers/vdpa/Kconfig | 6 +
drivers/vdpa/Makefile | 1 +
drivers/vdpa/ifcvf/ifcvf_main.c | 5 +-
drivers/vdpa/mlx5/net/mlx5_vnet.c | 5 +-
drivers/vdpa/vdpa.c | 8 +-
drivers/vdpa/vdpa_sim/vdpa_sim.c | 4 +-
drivers/vdpa/virtio_pci/Makefile | 2 +
drivers/vdpa/virtio_pci/vp_vdpa.c | 450 ++++++++++++++++++++++++
drivers/virtio/Kconfig | 10 +-
drivers/virtio/Makefile | 1 +
drivers/virtio/virtio_pci_common.c | 10 -
drivers/virtio/virtio_pci_common.h | 23 +-
drivers/virtio/virtio_pci_legacy.c | 13 +-
drivers/virtio/virtio_pci_modern.c | 442 +++--------------------
drivers/virtio/virtio_pci_modern_dev.c | 462 +++++++++++++++++++++++++
drivers/virtio/virtio_vdpa.c | 3 +-
include/linux/vdpa.h | 7 +-
include/linux/virtio_pci_modern.h | 107 ++++++
18 files changed, 1121 insertions(+), 438 deletions(-)
create mode 100644 drivers/vdpa/virtio_pci/Makefile
create mode 100644 drivers/vdpa/virtio_pci/vp_vdpa.c
create mode 100644 drivers/virtio/virtio_pci_modern_dev.c
create mode 100644 include/linux/virtio_pci_modern.h
--
2.25.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V2 01/14] virtio-pci: do not access iomem via virtio_pci_device directly
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
@ 2020-11-26 9:25 ` Jason Wang
2020-11-26 13:46 ` Michael S. Tsirkin
2020-11-26 9:25 ` [PATCH V2 02/14] virtio-pci: switch to use devres for modern devices Jason Wang
` (12 subsequent siblings)
13 siblings, 1 reply; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:25 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
Instead of accessing iomem via virito_pci_device directly. Add an
indirect level to ease the life of splitting out modern virito-pci
logic.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_modern.c | 76 ++++++++++++++++++------------
1 file changed, 46 insertions(+), 30 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 3d6ae5a5e252..df1481fd400c 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -141,12 +141,13 @@ static void __iomem *map_capability(struct pci_dev *dev, int off,
static u64 vp_get_features(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
u64 features;
- vp_iowrite32(0, &vp_dev->common->device_feature_select);
- features = vp_ioread32(&vp_dev->common->device_feature);
- vp_iowrite32(1, &vp_dev->common->device_feature_select);
- features |= ((u64)vp_ioread32(&vp_dev->common->device_feature) << 32);
+ vp_iowrite32(0, &cfg->device_feature_select);
+ features = vp_ioread32(&cfg->device_feature);
+ vp_iowrite32(1, &cfg->device_feature_select);
+ features |= ((u64)vp_ioread32(&cfg->device_feature) << 32);
return features;
}
@@ -165,6 +166,7 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
static int vp_finalize_features(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
u64 features = vdev->features;
/* Give virtio_ring a chance to accept features. */
@@ -179,10 +181,10 @@ static int vp_finalize_features(struct virtio_device *vdev)
return -EINVAL;
}
- vp_iowrite32(0, &vp_dev->common->guest_feature_select);
- vp_iowrite32((u32)vdev->features, &vp_dev->common->guest_feature);
- vp_iowrite32(1, &vp_dev->common->guest_feature_select);
- vp_iowrite32(vdev->features >> 32, &vp_dev->common->guest_feature);
+ vp_iowrite32(0, &cfg->guest_feature_select);
+ vp_iowrite32((u32)vdev->features, &cfg->guest_feature);
+ vp_iowrite32(1, &cfg->guest_feature_select);
+ vp_iowrite32(vdev->features >> 32, &cfg->guest_feature);
return 0;
}
@@ -192,6 +194,7 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
void *buf, unsigned len)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ void __iomem *device = vp_dev->device;
u8 b;
__le16 w;
__le32 l;
@@ -200,21 +203,21 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
switch (len) {
case 1:
- b = ioread8(vp_dev->device + offset);
+ b = ioread8(device + offset);
memcpy(buf, &b, sizeof b);
break;
case 2:
- w = cpu_to_le16(ioread16(vp_dev->device + offset));
+ w = cpu_to_le16(ioread16(device + offset));
memcpy(buf, &w, sizeof w);
break;
case 4:
- l = cpu_to_le32(ioread32(vp_dev->device + offset));
+ l = cpu_to_le32(ioread32(device + offset));
memcpy(buf, &l, sizeof l);
break;
case 8:
- l = cpu_to_le32(ioread32(vp_dev->device + offset));
+ l = cpu_to_le32(ioread32(device + offset));
memcpy(buf, &l, sizeof l);
- l = cpu_to_le32(ioread32(vp_dev->device + offset + sizeof l));
+ l = cpu_to_le32(ioread32(device + offset + sizeof l));
memcpy(buf + sizeof l, &l, sizeof l);
break;
default:
@@ -228,6 +231,7 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
const void *buf, unsigned len)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ void __iomem *device = vp_dev->device;
u8 b;
__le16 w;
__le32 l;
@@ -237,21 +241,21 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
switch (len) {
case 1:
memcpy(&b, buf, sizeof b);
- iowrite8(b, vp_dev->device + offset);
+ iowrite8(b, device + offset);
break;
case 2:
memcpy(&w, buf, sizeof w);
- iowrite16(le16_to_cpu(w), vp_dev->device + offset);
+ iowrite16(le16_to_cpu(w), device + offset);
break;
case 4:
memcpy(&l, buf, sizeof l);
- iowrite32(le32_to_cpu(l), vp_dev->device + offset);
+ iowrite32(le32_to_cpu(l), device + offset);
break;
case 8:
memcpy(&l, buf, sizeof l);
- iowrite32(le32_to_cpu(l), vp_dev->device + offset);
+ iowrite32(le32_to_cpu(l), device + offset);
memcpy(&l, buf + sizeof l, sizeof l);
- iowrite32(le32_to_cpu(l), vp_dev->device + offset + sizeof l);
+ iowrite32(le32_to_cpu(l), device + offset + sizeof l);
break;
default:
BUG();
@@ -261,35 +265,43 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
static u32 vp_generation(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- return vp_ioread8(&vp_dev->common->config_generation);
+ struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
+
+ return vp_ioread8(&cfg->config_generation);
}
/* config->{get,set}_status() implementations */
static u8 vp_get_status(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- return vp_ioread8(&vp_dev->common->device_status);
+ struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
+
+ return vp_ioread8(&cfg->device_status);
}
static void vp_set_status(struct virtio_device *vdev, u8 status)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
+
/* We should never be setting status to 0. */
BUG_ON(status == 0);
- vp_iowrite8(status, &vp_dev->common->device_status);
+ vp_iowrite8(status, &cfg->device_status);
}
static void vp_reset(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
+
/* 0 status means a reset. */
- vp_iowrite8(0, &vp_dev->common->device_status);
+ vp_iowrite8(0, &cfg->device_status);
/* After writing 0 to device_status, the driver MUST wait for a read of
* device_status to return 0 before reinitializing the device.
* This will flush out the status write, and flush in device writes,
* including MSI-X interrupts, if any.
*/
- while (vp_ioread8(&vp_dev->common->device_status))
+ while (vp_ioread8(&cfg->device_status))
msleep(1);
/* Flush pending VQ/configuration callbacks. */
vp_synchronize_vectors(vdev);
@@ -297,11 +309,13 @@ static void vp_reset(struct virtio_device *vdev)
static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
{
+ struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
+
/* Setup the vector used for configuration events */
- vp_iowrite16(vector, &vp_dev->common->msix_config);
+ vp_iowrite16(vector, &cfg->msix_config);
/* Verify we had enough resources to assign the vector */
/* Will also flush the write out to device */
- return vp_ioread16(&vp_dev->common->msix_config);
+ return vp_ioread16(&cfg->msix_config);
}
static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
@@ -407,6 +421,7 @@ static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned nvqs,
struct irq_affinity *desc)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
struct virtqueue *vq;
int rc = vp_find_vqs(vdev, nvqs, vqs, callbacks, names, ctx, desc);
@@ -417,8 +432,8 @@ static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned nvqs,
* this, there's no way to go back except reset.
*/
list_for_each_entry(vq, &vdev->vqs, list) {
- vp_iowrite16(vq->index, &vp_dev->common->queue_select);
- vp_iowrite16(1, &vp_dev->common->queue_enable);
+ vp_iowrite16(vq->index, &cfg->queue_select);
+ vp_iowrite16(1, &cfg->queue_enable);
}
return 0;
@@ -428,14 +443,15 @@ static void del_vq(struct virtio_pci_vq_info *info)
{
struct virtqueue *vq = info->vq;
struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
+ struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
- vp_iowrite16(vq->index, &vp_dev->common->queue_select);
+ vp_iowrite16(vq->index, &cfg->queue_select);
if (vp_dev->msix_enabled) {
vp_iowrite16(VIRTIO_MSI_NO_VECTOR,
- &vp_dev->common->queue_msix_vector);
+ &cfg->queue_msix_vector);
/* Flush the write out to device */
- vp_ioread16(&vp_dev->common->queue_msix_vector);
+ vp_ioread16(&cfg->queue_msix_vector);
}
if (!vp_dev->notify_base)
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 02/14] virtio-pci: switch to use devres for modern devices
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
2020-11-26 9:25 ` [PATCH V2 01/14] virtio-pci: do not access iomem via virtio_pci_device directly Jason Wang
@ 2020-11-26 9:25 ` Jason Wang
2020-11-26 13:57 ` Michael S. Tsirkin
2020-11-26 9:25 ` [PATCH V2 03/14] virtio-pci: split out modern device Jason Wang
` (11 subsequent siblings)
13 siblings, 1 reply; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:25 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch tries to convert the modern device to use devres to manage
its resources (iomaps). Before this patch the IO address is mapped
individually according to the capability. After this patch, we simply
map the whole BAR.
This simplify the work of splitting modern device logic into an
separate module.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_common.c | 10 --
drivers/virtio/virtio_pci_common.h | 2 +
drivers/virtio/virtio_pci_legacy.c | 13 ++-
drivers/virtio/virtio_pci_modern.c | 141 +++++++++--------------------
4 files changed, 54 insertions(+), 112 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 222d630c41fc..e786701fa1b4 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -527,11 +527,6 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
INIT_LIST_HEAD(&vp_dev->virtqueues);
spin_lock_init(&vp_dev->lock);
- /* enable the device */
- rc = pci_enable_device(pci_dev);
- if (rc)
- goto err_enable_device;
-
if (force_legacy) {
rc = virtio_pci_legacy_probe(vp_dev);
/* Also try modern mode if we can't map BAR0 (no IO space). */
@@ -559,11 +554,8 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
err_register:
if (vp_dev->ioaddr)
virtio_pci_legacy_remove(vp_dev);
- else
- virtio_pci_modern_remove(vp_dev);
err_probe:
pci_disable_device(pci_dev);
-err_enable_device:
if (reg_dev)
put_device(&vp_dev->vdev.dev);
else
@@ -582,8 +574,6 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
if (vp_dev->ioaddr)
virtio_pci_legacy_remove(vp_dev);
- else
- virtio_pci_modern_remove(vp_dev);
pci_disable_device(pci_dev);
put_device(dev);
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index b2f0eb4067cb..1d23420f7ed6 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -49,6 +49,8 @@ struct virtio_pci_device {
u8 __iomem *isr;
/* Modern only fields */
+ /* The IO mapping for the BARs */
+ void __iomem * const *base;
/* The IO mapping for the PCI config space (non-legacy mode) */
struct virtio_pci_common_cfg __iomem *common;
/* Device-specific data (non-legacy mode) */
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index d62e9835aeec..890f155ff48c 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -214,14 +214,19 @@ int virtio_pci_legacy_probe(struct virtio_pci_device *vp_dev)
struct pci_dev *pci_dev = vp_dev->pci_dev;
int rc;
+ rc = pci_enable_device(pci_dev);
+ if (rc)
+ return rc;
+
+ rc = -ENODEV;
/* We only own devices >= 0x1000 and <= 0x103f: leave the rest. */
if (pci_dev->device < 0x1000 || pci_dev->device > 0x103f)
- return -ENODEV;
+ goto err_id;
if (pci_dev->revision != VIRTIO_PCI_ABI_VERSION) {
printk(KERN_ERR "virtio_pci: expected ABI version %d, got %d\n",
VIRTIO_PCI_ABI_VERSION, pci_dev->revision);
- return -ENODEV;
+ goto err_id;
}
rc = dma_set_mask(&pci_dev->dev, DMA_BIT_MASK(64));
@@ -241,7 +246,7 @@ int virtio_pci_legacy_probe(struct virtio_pci_device *vp_dev)
rc = pci_request_region(pci_dev, 0, "virtio-pci-legacy");
if (rc)
- return rc;
+ goto err_id;
rc = -ENOMEM;
vp_dev->ioaddr = pci_iomap(pci_dev, 0, 0);
@@ -267,6 +272,8 @@ int virtio_pci_legacy_probe(struct virtio_pci_device *vp_dev)
err_iomap:
pci_release_region(pci_dev, 0);
+err_id:
+ pci_disable_device(pci_dev);
return rc;
}
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index df1481fd400c..33cc21b818de 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -63,15 +63,15 @@ static void vp_iowrite64_twopart(u64 val,
vp_iowrite32(val >> 32, hi);
}
-static void __iomem *map_capability(struct pci_dev *dev, int off,
+static void __iomem *map_capability(struct virtio_pci_device *vp_dev, int off,
size_t minlen,
u32 align,
- u32 start, u32 size,
+ u32 size,
size_t *len)
{
+ struct pci_dev *dev = vp_dev->pci_dev;
u8 bar;
u32 offset, length;
- void __iomem *p;
pci_read_config_byte(dev, off + offsetof(struct virtio_pci_cap,
bar),
@@ -81,31 +81,13 @@ static void __iomem *map_capability(struct pci_dev *dev, int off,
pci_read_config_dword(dev, off + offsetof(struct virtio_pci_cap, length),
&length);
- if (length <= start) {
- dev_err(&dev->dev,
- "virtio_pci: bad capability len %u (>%u expected)\n",
- length, start);
- return NULL;
- }
-
- if (length - start < minlen) {
+ if (length < 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",
@@ -129,12 +111,7 @@ static void __iomem *map_capability(struct pci_dev *dev, int off,
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);
- return p;
+ return vp_dev->base[bar] + offset;
}
/* virtio config->get_features() implementation */
@@ -369,27 +346,21 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
vp_iowrite64_twopart(virtqueue_get_used_addr(vq),
&cfg->queue_used_lo, &cfg->queue_used_hi);
- if (vp_dev->notify_base) {
- /* offset should not wrap */
- if ((u64)off * vp_dev->notify_offset_multiplier + 2
- > vp_dev->notify_len) {
- dev_warn(&vp_dev->pci_dev->dev,
- "bad notification offset %u (x %u) "
- "for queue %u > %zd",
- off, vp_dev->notify_offset_multiplier,
- index, vp_dev->notify_len);
- err = -EINVAL;
- goto err_map_notify;
- }
- vq->priv = (void __force *)vp_dev->notify_base +
- off * vp_dev->notify_offset_multiplier;
- } else {
- vq->priv = (void __force *)map_capability(vp_dev->pci_dev,
- vp_dev->notify_map_cap, 2, 2,
- off * vp_dev->notify_offset_multiplier, 2,
- NULL);
+ /* offset should not wrap */
+ if ((u64)off * vp_dev->notify_offset_multiplier + 2
+ > vp_dev->notify_len) {
+ dev_warn(&vp_dev->pci_dev->dev,
+ "bad notification offset %u (x %u) "
+ "for queue %u > %zd",
+ off, vp_dev->notify_offset_multiplier,
+ index, vp_dev->notify_len);
+ err = -EINVAL;
+ goto err_map_notify;
}
+ vq->priv = (void __force *)vp_dev->notify_base +
+ off * vp_dev->notify_offset_multiplier;
+
if (!vq->priv) {
err = -ENOMEM;
goto err_map_notify;
@@ -400,15 +371,12 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
msix_vec = vp_ioread16(&cfg->queue_msix_vector);
if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
err = -EBUSY;
- goto err_assign_vector;
+ goto err_map_notify;
}
}
return vq;
-err_assign_vector:
- if (!vp_dev->notify_base)
- pci_iounmap(vp_dev->pci_dev, (void __iomem __force *)vq->priv);
err_map_notify:
vring_del_virtqueue(vq);
return ERR_PTR(err);
@@ -454,9 +422,6 @@ static void del_vq(struct virtio_pci_vq_info *info)
vp_ioread16(&cfg->queue_msix_vector);
}
- if (!vp_dev->notify_base)
- pci_iounmap(vp_dev->pci_dev, (void __force __iomem *)vq->priv);
-
vring_del_virtqueue(vq);
}
@@ -700,6 +665,10 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
check_offsets();
+ err = pcim_enable_device(pci_dev);
+ if (err)
+ return err;
+
/* We only own devices >= 0x1000 and <= 0x107f: leave the rest. */
if (pci_dev->device < 0x1000 || pci_dev->device > 0x107f)
return -ENODEV;
@@ -753,23 +722,24 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
IORESOURCE_IO | IORESOURCE_MEM,
&vp_dev->modern_bars);
- err = pci_request_selected_regions(pci_dev, vp_dev->modern_bars,
- "virtio-pci-modern");
+ err = pcim_iomap_regions(pci_dev, vp_dev->modern_bars,
+ "virtio-pci-modern");
if (err)
return err;
+ vp_dev->base = pcim_iomap_table(pci_dev);
+
err = -EINVAL;
- vp_dev->common = map_capability(pci_dev, common,
+ vp_dev->common = map_capability(vp_dev, common,
sizeof(struct virtio_pci_common_cfg), 4,
- 0, sizeof(struct virtio_pci_common_cfg),
+ sizeof(struct virtio_pci_common_cfg),
NULL);
if (!vp_dev->common)
- goto err_map_common;
- vp_dev->isr = map_capability(pci_dev, isr, sizeof(u8), 1,
- 0, 1,
- NULL);
+ goto err;
+ vp_dev->isr = map_capability(vp_dev, isr, sizeof(u8), 1,
+ 1, NULL);
if (!vp_dev->isr)
- goto err_map_isr;
+ goto err;
/* Read notify_off_multiplier from config space. */
pci_read_config_dword(pci_dev,
@@ -787,29 +757,21 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
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) {
- vp_dev->notify_base = map_capability(pci_dev, notify, 2, 2,
- 0, notify_length,
- &vp_dev->notify_len);
- if (!vp_dev->notify_base)
- goto err_map_notify;
- } else {
- vp_dev->notify_map_cap = notify;
- }
+ vp_dev->notify_base = map_capability(vp_dev, notify, 2, 2,
+ notify_length,
+ &vp_dev->notify_len);
+ if (!vp_dev->notify_base)
+ goto err;
/* Again, we don't know how much we should map, but PAGE_SIZE
* is more than enough for all existing devices.
*/
if (device) {
- vp_dev->device = map_capability(pci_dev, device, 0, 4,
- 0, PAGE_SIZE,
+ vp_dev->device = map_capability(vp_dev, device, 0, 4,
+ PAGE_SIZE,
&vp_dev->device_len);
if (!vp_dev->device)
- goto err_map_device;
+ goto err;
vp_dev->vdev.config = &virtio_pci_config_ops;
} else {
@@ -822,26 +784,7 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
return 0;
-err_map_device:
- if (vp_dev->notify_base)
- pci_iounmap(pci_dev, vp_dev->notify_base);
-err_map_notify:
- pci_iounmap(pci_dev, vp_dev->isr);
-err_map_isr:
- pci_iounmap(pci_dev, vp_dev->common);
-err_map_common:
+err:
return err;
}
-void virtio_pci_modern_remove(struct virtio_pci_device *vp_dev)
-{
- struct pci_dev *pci_dev = vp_dev->pci_dev;
-
- if (vp_dev->device)
- pci_iounmap(pci_dev, vp_dev->device);
- if (vp_dev->notify_base)
- pci_iounmap(pci_dev, vp_dev->notify_base);
- pci_iounmap(pci_dev, vp_dev->isr);
- pci_iounmap(pci_dev, vp_dev->common);
- pci_release_selected_regions(pci_dev, vp_dev->modern_bars);
-}
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 03/14] virtio-pci: split out modern device
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
2020-11-26 9:25 ` [PATCH V2 01/14] virtio-pci: do not access iomem via virtio_pci_device directly Jason Wang
2020-11-26 9:25 ` [PATCH V2 02/14] virtio-pci: switch to use devres for modern devices Jason Wang
@ 2020-11-26 9:25 ` Jason Wang
2020-11-26 9:25 ` [PATCH V2 04/14] virtio-pci: move the notification sanity check to vp_modern_probe() Jason Wang
` (10 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:25 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch splits out the virtio-pci modern device only attributes
into another structure. While at it, a dedicated probe method for
modern only attributes is introduced. This may help for split the
logic into a dedicated module.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_common.h | 33 +++--
drivers/virtio/virtio_pci_modern.c | 224 ++++++++++++++++++-----------
2 files changed, 158 insertions(+), 99 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index 1d23420f7ed6..d32af8ff56f9 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -39,37 +39,43 @@ struct virtio_pci_vq_info {
unsigned msix_vector;
};
-/* Our device structure */
-struct virtio_pci_device {
- struct virtio_device vdev;
+struct virtio_pci_modern_device {
struct pci_dev *pci_dev;
- /* In legacy mode, these two point to within ->legacy. */
- /* Where to read and clear interrupt */
- u8 __iomem *isr;
-
- /* Modern only fields */
- /* The IO mapping for the BARs */
+ /* The IO mapping for the PCI BARs */
void __iomem * const *base;
- /* The IO mapping for the PCI config space (non-legacy mode) */
+
+ /* The IO mapping for the PCI config space */
struct virtio_pci_common_cfg __iomem *common;
/* Device-specific data (non-legacy mode) */
void __iomem *device;
/* Base of vq notifications (non-legacy mode). */
void __iomem *notify_base;
+ /* Where to read and clear interrupt */
+ u8 __iomem *isr;
/* So we can sanity-check accesses. */
size_t notify_len;
size_t device_len;
- /* Capability for when we need to map notifications per-vq. */
- int notify_map_cap;
-
/* Multiply queue_notify_off by this value. (non-legacy mode). */
u32 notify_offset_multiplier;
int modern_bars;
+ struct virtio_device_id id;
+};
+
+/* Our device structure */
+struct virtio_pci_device {
+ struct virtio_device vdev;
+ struct pci_dev *pci_dev;
+ struct virtio_pci_modern_device mdev;
+
+ /* In legacy mode, these two point to within ->legacy. */
+ /* Where to read and clear interrupt */
+ u8 __iomem *isr;
+
/* Legacy only field */
/* the IO mapping for the PCI config space */
void __iomem *ioaddr;
@@ -157,6 +163,5 @@ static inline void virtio_pci_legacy_remove(struct virtio_pci_device *vp_dev)
}
#endif
int virtio_pci_modern_probe(struct virtio_pci_device *);
-void virtio_pci_modern_remove(struct virtio_pci_device *);
#endif
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 33cc21b818de..02688c3b3fbd 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -63,13 +63,11 @@ static void vp_iowrite64_twopart(u64 val,
vp_iowrite32(val >> 32, hi);
}
-static void __iomem *map_capability(struct virtio_pci_device *vp_dev, int off,
- size_t minlen,
- u32 align,
- u32 size,
- size_t *len)
+static void __iomem *map_capability(struct virtio_pci_modern_device *mdev,
+ int off, size_t minlen, u32 align,
+ u32 size, size_t *len)
{
- struct pci_dev *dev = vp_dev->pci_dev;
+ struct pci_dev *dev = mdev->pci_dev;
u8 bar;
u32 offset, length;
@@ -111,14 +109,13 @@ static void __iomem *map_capability(struct virtio_pci_device *vp_dev, int off,
return NULL;
}
- return vp_dev->base[bar] + offset;
+ return mdev->base[bar] + offset;
}
-/* virtio config->get_features() implementation */
-static u64 vp_get_features(struct virtio_device *vdev)
+static u64 vp_modern_get_features(struct virtio_pci_modern_device *mdev)
{
- struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
u64 features;
vp_iowrite32(0, &cfg->device_feature_select);
@@ -129,6 +126,14 @@ static u64 vp_get_features(struct virtio_device *vdev)
return features;
}
+/* virtio config->get_features() implementation */
+static u64 vp_get_features(struct virtio_device *vdev)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+
+ return vp_modern_get_features(&vp_dev->mdev);
+}
+
static void vp_transport_features(struct virtio_device *vdev, u64 features)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
@@ -139,11 +144,21 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
__virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
}
+static void vp_modern_set_features(struct virtio_pci_modern_device *mdev,
+ u64 features)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ vp_iowrite32(0, &cfg->guest_feature_select);
+ vp_iowrite32((u32)features, &cfg->guest_feature);
+ vp_iowrite32(1, &cfg->guest_feature_select);
+ vp_iowrite32(features >> 32, &cfg->guest_feature);
+}
+
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
u64 features = vdev->features;
/* Give virtio_ring a chance to accept features. */
@@ -158,10 +173,7 @@ static int vp_finalize_features(struct virtio_device *vdev)
return -EINVAL;
}
- vp_iowrite32(0, &cfg->guest_feature_select);
- vp_iowrite32((u32)vdev->features, &cfg->guest_feature);
- vp_iowrite32(1, &cfg->guest_feature_select);
- vp_iowrite32(vdev->features >> 32, &cfg->guest_feature);
+ vp_modern_set_features(&vp_dev->mdev, vdev->features);
return 0;
}
@@ -171,12 +183,13 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
void *buf, unsigned len)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- void __iomem *device = vp_dev->device;
+ struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
+ void __iomem *device = mdev->device;
u8 b;
__le16 w;
__le32 l;
- BUG_ON(offset + len > vp_dev->device_len);
+ BUG_ON(offset + len > mdev->device_len);
switch (len) {
case 1:
@@ -208,12 +221,13 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
const void *buf, unsigned len)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- void __iomem *device = vp_dev->device;
+ struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
+ void __iomem *device = mdev->device;
u8 b;
__le16 w;
__le32 l;
- BUG_ON(offset + len > vp_dev->device_len);
+ BUG_ON(offset + len > mdev->device_len);
switch (len) {
case 1:
@@ -239,54 +253,71 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
}
}
+static u32 vp_modern_generation(struct virtio_pci_modern_device *mdev)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ return vp_ioread8(&cfg->config_generation);
+}
+
static u32 vp_generation(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
- return vp_ioread8(&cfg->config_generation);
+ return vp_modern_generation(&vp_dev->mdev);
}
/* config->{get,set}_status() implementations */
+static u8 vp_modern_get_status(struct virtio_pci_modern_device *mdev)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ return vp_ioread8(&cfg->device_status);
+}
+
static u8 vp_get_status(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
- return vp_ioread8(&cfg->device_status);
+ return vp_modern_get_status(&vp_dev->mdev);
+}
+
+static void vp_modern_set_status(struct virtio_pci_modern_device *mdev,
+ u8 status)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ vp_iowrite8(status, &cfg->device_status);
}
static void vp_set_status(struct virtio_device *vdev, u8 status)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
- /* We should never be setting status to 0. */
- BUG_ON(status == 0);
- vp_iowrite8(status, &cfg->device_status);
+ vp_modern_set_status(&vp_dev->mdev, status);
}
static void vp_reset(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
/* 0 status means a reset. */
- vp_iowrite8(0, &cfg->device_status);
+ vp_modern_set_status(&vp_dev->mdev, 0);
/* After writing 0 to device_status, the driver MUST wait for a read of
* device_status to return 0 before reinitializing the device.
* This will flush out the status write, and flush in device writes,
* including MSI-X interrupts, if any.
*/
- while (vp_ioread8(&cfg->device_status))
+ while (vp_modern_get_status(&vp_dev->mdev))
msleep(1);
/* Flush pending VQ/configuration callbacks. */
vp_synchronize_vectors(vdev);
}
-static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
+static u16 vp_modern_config_vector(struct virtio_pci_modern_device *mdev,
+ u16 vector)
{
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
/* Setup the vector used for configuration events */
vp_iowrite16(vector, &cfg->msix_config);
@@ -295,6 +326,11 @@ static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
return vp_ioread16(&cfg->msix_config);
}
+static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
+{
+ return vp_modern_config_vector(&vp_dev->mdev, vector);
+}
+
static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
struct virtio_pci_vq_info *info,
unsigned index,
@@ -303,7 +339,9 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
bool ctx,
u16 msix_vec)
{
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
+
+ struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
struct virtqueue *vq;
u16 num, off;
int err;
@@ -347,19 +385,18 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
&cfg->queue_used_lo, &cfg->queue_used_hi);
/* offset should not wrap */
- if ((u64)off * vp_dev->notify_offset_multiplier + 2
- > vp_dev->notify_len) {
+ if ((u64)off * mdev->notify_offset_multiplier + 2
+ > mdev->notify_len) {
dev_warn(&vp_dev->pci_dev->dev,
"bad notification offset %u (x %u) "
"for queue %u > %zd",
- off, vp_dev->notify_offset_multiplier,
- index, vp_dev->notify_len);
+ off, mdev->notify_offset_multiplier,
+ index, mdev->notify_len);
err = -EINVAL;
goto err_map_notify;
}
-
- vq->priv = (void __force *)vp_dev->notify_base +
- off * vp_dev->notify_offset_multiplier;
+ vq->priv = (void __force *)mdev->notify_base +
+ off * mdev->notify_offset_multiplier;
if (!vq->priv) {
err = -ENOMEM;
@@ -389,7 +426,7 @@ static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned nvqs,
struct irq_affinity *desc)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
+ struct virtio_pci_common_cfg __iomem *cfg = vp_dev->mdev.common;
struct virtqueue *vq;
int rc = vp_find_vqs(vdev, nvqs, vqs, callbacks, names, ctx, desc);
@@ -411,7 +448,7 @@ static void del_vq(struct virtio_pci_vq_info *info)
{
struct virtqueue *vq = info->vq;
struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
+ struct virtio_pci_common_cfg __iomem *cfg = vp_dev->mdev.common;
vp_iowrite16(vq->index, &cfg->queue_select);
@@ -655,20 +692,13 @@ static inline void check_offsets(void)
offsetof(struct virtio_pci_common_cfg, queue_used_hi));
}
-/* the PCI probing function */
-int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
+static int vp_modern_probe(struct virtio_pci_modern_device *mdev)
{
- struct pci_dev *pci_dev = vp_dev->pci_dev;
+ struct pci_dev *pci_dev = mdev->pci_dev;
int err, common, isr, notify, device;
u32 notify_length;
u32 notify_offset;
- check_offsets();
-
- err = pcim_enable_device(pci_dev);
- if (err)
- return err;
-
/* We only own devices >= 0x1000 and <= 0x107f: leave the rest. */
if (pci_dev->device < 0x1000 || pci_dev->device > 0x107f)
return -ENODEV;
@@ -677,17 +707,21 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
/* Transitional devices: use the PCI subsystem device id as
* virtio device id, same as legacy driver always did.
*/
- vp_dev->vdev.id.device = pci_dev->subsystem_device;
+ mdev->id.device = pci_dev->subsystem_device;
} else {
/* Modern devices: simply use PCI device id, but start from 0x1040. */
- vp_dev->vdev.id.device = pci_dev->device - 0x1040;
+ mdev->id.device = pci_dev->device - 0x1040;
}
- vp_dev->vdev.id.vendor = pci_dev->subsystem_vendor;
+ mdev->id.vendor = pci_dev->subsystem_vendor;
+
+ err = pcim_enable_device(pci_dev);
+ if (err)
+ return err;
/* 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,
- &vp_dev->modern_bars);
+ &mdev->modern_bars);
if (!common) {
dev_info(&pci_dev->dev,
"virtio_pci: leaving for legacy driver\n");
@@ -697,10 +731,10 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
/* If common is there, these should be too... */
isr = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_ISR_CFG,
IORESOURCE_IO | IORESOURCE_MEM,
- &vp_dev->modern_bars);
+ &mdev->modern_bars);
notify = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_NOTIFY_CFG,
IORESOURCE_IO | IORESOURCE_MEM,
- &vp_dev->modern_bars);
+ &mdev->modern_bars);
if (!isr || !notify) {
dev_err(&pci_dev->dev,
"virtio_pci: missing capabilities %i/%i/%i\n",
@@ -720,32 +754,31 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
*/
device = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_DEVICE_CFG,
IORESOURCE_IO | IORESOURCE_MEM,
- &vp_dev->modern_bars);
+ &mdev->modern_bars);
- err = pcim_iomap_regions(pci_dev, vp_dev->modern_bars,
+ err = pcim_iomap_regions(pci_dev, mdev->modern_bars,
"virtio-pci-modern");
if (err)
return err;
- vp_dev->base = pcim_iomap_table(pci_dev);
+ mdev->base = pcim_iomap_table(pci_dev);
err = -EINVAL;
- vp_dev->common = map_capability(vp_dev, common,
- sizeof(struct virtio_pci_common_cfg), 4,
- sizeof(struct virtio_pci_common_cfg),
- NULL);
- if (!vp_dev->common)
+ mdev->common = map_capability(mdev, common,
+ sizeof(struct virtio_pci_common_cfg), 4,
+ sizeof(struct virtio_pci_common_cfg),
+ NULL);
+ if (!mdev->common)
goto err;
- vp_dev->isr = map_capability(vp_dev, isr, sizeof(u8), 1,
- 1, NULL);
- if (!vp_dev->isr)
+ mdev->isr = map_capability(mdev, isr, sizeof(u8), 1, 1, NULL);
+ if (!mdev->isr)
goto err;
/* Read notify_off_multiplier from config space. */
pci_read_config_dword(pci_dev,
notify + offsetof(struct virtio_pci_notify_cap,
notify_off_multiplier),
- &vp_dev->notify_offset_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,
@@ -757,34 +790,55 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
cap.offset),
¬ify_offset);
- vp_dev->notify_base = map_capability(vp_dev, notify, 2, 2,
- notify_length,
- &vp_dev->notify_len);
- if (!vp_dev->notify_base)
+ mdev->notify_base = map_capability(mdev, notify, 2, 2,
+ notify_length,
+ &mdev->notify_len);
+ if (!mdev->notify_base)
goto err;
- /* Again, we don't know how much we should map, but PAGE_SIZE
+ /* We don't know how much we should map, but PAGE_SIZE
* is more than enough for all existing devices.
*/
if (device) {
- vp_dev->device = map_capability(vp_dev, device, 0, 4,
- PAGE_SIZE,
- &vp_dev->device_len);
- if (!vp_dev->device)
+ mdev->device = map_capability(mdev, device, 0, 4,
+ PAGE_SIZE,
+ &mdev->device_len);
+ if (!mdev->device)
goto err;
+ }
+ return 0;
+
+err:
+ return err;
+}
+
+/* the PCI probing function */
+int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
+{
+ struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
+ struct pci_dev *pci_dev = vp_dev->pci_dev;
+ int err;
+
+ check_offsets();
+
+ mdev->pci_dev = pci_dev;
+
+ err = vp_modern_probe(mdev);
+ if (err)
+ return err;
+
+ vp_dev->vdev.id = mdev->id;
+ vp_dev->isr = mdev->isr;
+
+ if (mdev->device)
vp_dev->vdev.config = &virtio_pci_config_ops;
- } else {
+ else
vp_dev->vdev.config = &virtio_pci_config_nodev_ops;
- }
vp_dev->config_vector = vp_config_vector;
vp_dev->setup_vq = setup_vq;
vp_dev->del_vq = del_vq;
return 0;
-
-err:
- return err;
}
-
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 04/14] virtio-pci: move the notification sanity check to vp_modern_probe()
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
` (2 preceding siblings ...)
2020-11-26 9:25 ` [PATCH V2 03/14] virtio-pci: split out modern device Jason Wang
@ 2020-11-26 9:25 ` Jason Wang
2020-11-26 9:25 ` [PATCH V2 05/14] virtio-pci-modern: introduce vp_modern_set_queue_vector() Jason Wang
` (9 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:25 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch moves the notification sanity check to
vp_modern_probe(). This can make sure the logic could be reused by
modules other than virtio-pci.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_modern.c | 34 +++++++++++++++++++-----------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 02688c3b3fbd..d001c74beefe 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -384,17 +384,6 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
vp_iowrite64_twopart(virtqueue_get_used_addr(vq),
&cfg->queue_used_lo, &cfg->queue_used_hi);
- /* offset should not wrap */
- if ((u64)off * mdev->notify_offset_multiplier + 2
- > mdev->notify_len) {
- dev_warn(&vp_dev->pci_dev->dev,
- "bad notification offset %u (x %u) "
- "for queue %u > %zd",
- off, mdev->notify_offset_multiplier,
- index, mdev->notify_len);
- err = -EINVAL;
- goto err_map_notify;
- }
vq->priv = (void __force *)mdev->notify_base +
off * mdev->notify_offset_multiplier;
@@ -695,9 +684,11 @@ static inline void check_offsets(void)
static int vp_modern_probe(struct virtio_pci_modern_device *mdev)
{
struct pci_dev *pci_dev = mdev->pci_dev;
- int err, common, isr, notify, device;
+ int err, common, isr, notify, device, i;
+ unsigned int num_queues;
u32 notify_length;
u32 notify_offset;
+ u16 off;
/* We only own devices >= 0x1000 and <= 0x107f: leave the rest. */
if (pci_dev->device < 0x1000 || pci_dev->device > 0x107f)
@@ -796,6 +787,25 @@ static int vp_modern_probe(struct virtio_pci_modern_device *mdev)
if (!mdev->notify_base)
goto err;
+ num_queues = vp_ioread16(&mdev->common->num_queues);
+
+ /* offset should not wrap */
+ for (i = 0; i < num_queues; i++) {
+ vp_iowrite16(i, &mdev->common->queue_select);
+ off = vp_ioread16(&mdev->common->queue_notify_off);
+
+ if ((u64)off * mdev->notify_offset_multiplier + 2
+ > mdev->notify_len) {
+ dev_warn(&pci_dev->dev,
+ "bad notification offset %u (x %u) "
+ "for queue %u > %zd",
+ off, mdev->notify_offset_multiplier,
+ i, mdev->notify_len);
+ err = -EINVAL;
+ goto err;
+ }
+ }
+
/* We don't know how much we should map, but PAGE_SIZE
* is more than enough for all existing devices.
*/
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 05/14] virtio-pci-modern: introduce vp_modern_set_queue_vector()
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
` (3 preceding siblings ...)
2020-11-26 9:25 ` [PATCH V2 04/14] virtio-pci: move the notification sanity check to vp_modern_probe() Jason Wang
@ 2020-11-26 9:25 ` Jason Wang
2020-11-26 9:25 ` [PATCH V2 06/14] virtio-pci-modern: introduce vp_modern_queue_address() Jason Wang
` (8 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:25 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch introduces a helper to set virtqueue MSI vector.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_modern.c | 35 ++++++++++++++++++++----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index d001c74beefe..bacc05cbc762 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -155,6 +155,25 @@ static void vp_modern_set_features(struct virtio_pci_modern_device *mdev,
vp_iowrite32(features >> 32, &cfg->guest_feature);
}
+/*
+ * vp_modern_queue_vector - set the MSIX vector for a specific virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: queue index
+ * @vector: the config vector
+ *
+ * Returns the config vector read from the device
+ */
+static u16 vp_modern_queue_vector(struct virtio_pci_modern_device *mdev,
+ u16 index, u16 vector)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ vp_iowrite16(index, &cfg->queue_select);
+ vp_iowrite16(vector, &cfg->queue_msix_vector);
+ /* Flush the write out to device */
+ return vp_ioread16(&cfg->queue_msix_vector);
+}
+
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
@@ -393,8 +412,7 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
}
if (msix_vec != VIRTIO_MSI_NO_VECTOR) {
- vp_iowrite16(msix_vec, &cfg->queue_msix_vector);
- msix_vec = vp_ioread16(&cfg->queue_msix_vector);
+ msix_vec = vp_modern_queue_vector(mdev, index, msix_vec);
if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
err = -EBUSY;
goto err_map_notify;
@@ -437,16 +455,11 @@ static void del_vq(struct virtio_pci_vq_info *info)
{
struct virtqueue *vq = info->vq;
struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->mdev.common;
-
- vp_iowrite16(vq->index, &cfg->queue_select);
+ struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
- if (vp_dev->msix_enabled) {
- vp_iowrite16(VIRTIO_MSI_NO_VECTOR,
- &cfg->queue_msix_vector);
- /* Flush the write out to device */
- vp_ioread16(&cfg->queue_msix_vector);
- }
+ if (vp_dev->msix_enabled)
+ vp_modern_queue_vector(mdev, vq->index,
+ VIRTIO_MSI_NO_VECTOR);
vring_del_virtqueue(vq);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 06/14] virtio-pci-modern: introduce vp_modern_queue_address()
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
` (4 preceding siblings ...)
2020-11-26 9:25 ` [PATCH V2 05/14] virtio-pci-modern: introduce vp_modern_set_queue_vector() Jason Wang
@ 2020-11-26 9:25 ` Jason Wang
2020-11-26 9:25 ` [PATCH V2 07/14] virtio-pci-modern: introduce helper to set/get queue_enable Jason Wang
` (7 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:25 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch introduce a helper to set virtqueue address for modern address.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_modern.c | 33 ++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index bacc05cbc762..3125987973d3 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -174,6 +174,30 @@ static u16 vp_modern_queue_vector(struct virtio_pci_modern_device *mdev,
return vp_ioread16(&cfg->queue_msix_vector);
}
+/*
+ * vp_modern_queue_address - set the virtqueue address
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ * @desc_addr: address of the descriptor area
+ * @driver_addr: address of the driver area
+ * @device_addr: address of the device area
+ */
+static void vp_modern_queue_address(struct virtio_pci_modern_device *mdev,
+ u16 index, u64 desc_addr, u64 driver_addr,
+ u64 device_addr)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ vp_iowrite16(index, &cfg->queue_select);
+
+ vp_iowrite64_twopart(desc_addr, &cfg->queue_desc_lo,
+ &cfg->queue_desc_hi);
+ vp_iowrite64_twopart(driver_addr, &cfg->queue_avail_lo,
+ &cfg->queue_avail_hi);
+ vp_iowrite64_twopart(device_addr, &cfg->queue_used_lo,
+ &cfg->queue_used_hi);
+}
+
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
@@ -396,12 +420,9 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
/* activate the queue */
vp_iowrite16(virtqueue_get_vring_size(vq), &cfg->queue_size);
- vp_iowrite64_twopart(virtqueue_get_desc_addr(vq),
- &cfg->queue_desc_lo, &cfg->queue_desc_hi);
- vp_iowrite64_twopart(virtqueue_get_avail_addr(vq),
- &cfg->queue_avail_lo, &cfg->queue_avail_hi);
- vp_iowrite64_twopart(virtqueue_get_used_addr(vq),
- &cfg->queue_used_lo, &cfg->queue_used_hi);
+ vp_modern_queue_address(mdev, index, virtqueue_get_desc_addr(vq),
+ virtqueue_get_avail_addr(vq),
+ virtqueue_get_used_addr(vq));
vq->priv = (void __force *)mdev->notify_base +
off * mdev->notify_offset_multiplier;
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 07/14] virtio-pci-modern: introduce helper to set/get queue_enable
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
` (5 preceding siblings ...)
2020-11-26 9:25 ` [PATCH V2 06/14] virtio-pci-modern: introduce vp_modern_queue_address() Jason Wang
@ 2020-11-26 9:25 ` Jason Wang
2020-11-26 9:25 ` [PATCH V2 08/14] virtio-pci-modern: introduce helper for setting/geting queue size Jason Wang
` (6 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:25 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch introduces a helper to set/get queue_enable for modern device.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_modern.c | 37 +++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 3125987973d3..dcdda32b6182 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -198,6 +198,34 @@ static void vp_modern_queue_address(struct virtio_pci_modern_device *mdev,
&cfg->queue_used_hi);
}
+/*
+ * vp_modern_set_queue_enable - enable a virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ * @enable: whether the virtqueue is enable or not
+ */
+static void vp_modern_set_queue_enable(struct virtio_pci_modern_device *mdev,
+ u16 index, bool enable)
+{
+ vp_iowrite16(index, &mdev->common->queue_select);
+ vp_iowrite16(enable, &mdev->common->queue_enable);
+}
+
+/*
+ * vp_modern_get_queue_enable - enable a virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ *
+ * Returns whether a virtqueue is enabled or not
+ */
+static bool vp_modern_get_queue_enable(struct virtio_pci_modern_device *mdev,
+ u16 index)
+{
+ vp_iowrite16(index, &mdev->common->queue_select);
+
+ return vp_ioread16(&mdev->common->queue_enable);
+}
+
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
@@ -397,7 +425,7 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
/* Check if queue is either not available or already active. */
num = vp_ioread16(&cfg->queue_size);
- if (!num || vp_ioread16(&cfg->queue_enable))
+ if (!num || vp_modern_get_queue_enable(mdev, index))
return ERR_PTR(-ENOENT);
if (num & (num - 1)) {
@@ -454,7 +482,6 @@ static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned nvqs,
struct irq_affinity *desc)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_common_cfg __iomem *cfg = vp_dev->mdev.common;
struct virtqueue *vq;
int rc = vp_find_vqs(vdev, nvqs, vqs, callbacks, names, ctx, desc);
@@ -464,10 +491,8 @@ static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned nvqs,
/* Select and activate all queues. Has to be done last: once we do
* this, there's no way to go back except reset.
*/
- list_for_each_entry(vq, &vdev->vqs, list) {
- vp_iowrite16(vq->index, &cfg->queue_select);
- vp_iowrite16(1, &cfg->queue_enable);
- }
+ list_for_each_entry(vq, &vdev->vqs, list)
+ vp_modern_set_queue_enable(&vp_dev->mdev, vq->index, true);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 08/14] virtio-pci-modern: introduce helper for setting/geting queue size
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
` (6 preceding siblings ...)
2020-11-26 9:25 ` [PATCH V2 07/14] virtio-pci-modern: introduce helper to set/get queue_enable Jason Wang
@ 2020-11-26 9:25 ` Jason Wang
2020-11-26 9:25 ` [PATCH V2 09/14] virtio-pci-modern: introduce helper for getting queue nums Jason Wang
` (5 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:25 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch introduces helper for setting/getting queue size for modern
device.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_modern.c | 34 ++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index dcdda32b6182..f85216ccc6df 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -226,6 +226,36 @@ static bool vp_modern_get_queue_enable(struct virtio_pci_modern_device *mdev,
return vp_ioread16(&mdev->common->queue_enable);
}
+/*
+ * vp_modern_set_queue_size - set size for a virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ * @size: the size of the virtqueue
+ */
+static void vp_modern_set_queue_size(struct virtio_pci_modern_device *mdev,
+ u16 index, u16 size)
+{
+ vp_iowrite16(index, &mdev->common->queue_select);
+ vp_iowrite16(size, &mdev->common->queue_size);
+
+}
+
+/*
+ * vp_modern_get_queue_size - get size for a virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ *
+ * Returns the size of the virtqueue
+ */
+static u16 vp_modern_get_queue_size(struct virtio_pci_modern_device *mdev,
+ u16 index)
+{
+ vp_iowrite16(index, &mdev->common->queue_select);
+
+ return vp_ioread16(&mdev->common->queue_size);
+
+}
+
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
@@ -424,7 +454,7 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
vp_iowrite16(index, &cfg->queue_select);
/* Check if queue is either not available or already active. */
- num = vp_ioread16(&cfg->queue_size);
+ num = vp_modern_get_queue_size(mdev, index);
if (!num || vp_modern_get_queue_enable(mdev, index))
return ERR_PTR(-ENOENT);
@@ -447,7 +477,7 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
return ERR_PTR(-ENOMEM);
/* activate the queue */
- vp_iowrite16(virtqueue_get_vring_size(vq), &cfg->queue_size);
+ vp_modern_set_queue_size(mdev, index, virtqueue_get_vring_size(vq));
vp_modern_queue_address(mdev, index, virtqueue_get_desc_addr(vq),
virtqueue_get_avail_addr(vq),
virtqueue_get_used_addr(vq));
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 09/14] virtio-pci-modern: introduce helper for getting queue nums
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
` (7 preceding siblings ...)
2020-11-26 9:25 ` [PATCH V2 08/14] virtio-pci-modern: introduce helper for setting/geting queue size Jason Wang
@ 2020-11-26 9:25 ` Jason Wang
2020-11-26 9:26 ` [PATCH V2 10/14] virtio-pci-modern: introduce helper to get notification offset Jason Wang
` (4 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:25 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch introduces helper for getting queue num of modern device.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_modern.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index f85216ccc6df..0b86a36998c8 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -256,6 +256,17 @@ static u16 vp_modern_get_queue_size(struct virtio_pci_modern_device *mdev,
}
+/*
+ * vp_modern_get_num_queues - get the number of virtqueues
+ * @mdev: the modern virtio-pci device
+ *
+ * Returns the number of virtqueues
+ */
+static u16 vp_modern_get_num_queues(struct virtio_pci_modern_device *mdev)
+{
+ return vp_ioread16(&mdev->common->num_queues);
+}
+
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
@@ -447,7 +458,7 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
u16 num, off;
int err;
- if (index >= vp_ioread16(&cfg->num_queues))
+ if (index >= vp_modern_get_num_queues(mdev))
return ERR_PTR(-ENOENT);
/* Select the queue we're interested in */
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 10/14] virtio-pci-modern: introduce helper to get notification offset
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
` (8 preceding siblings ...)
2020-11-26 9:25 ` [PATCH V2 09/14] virtio-pci-modern: introduce helper for getting queue nums Jason Wang
@ 2020-11-26 9:26 ` Jason Wang
2020-11-26 9:26 ` [PATCH V2 11/14] virtio-pci: introduce modern device module Jason Wang
` (3 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:26 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch introduces help to get notification offset of modern device.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_modern.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 0b86a36998c8..8f1f274724be 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -267,6 +267,21 @@ static u16 vp_modern_get_num_queues(struct virtio_pci_modern_device *mdev)
return vp_ioread16(&mdev->common->num_queues);
}
+/*
+ * vp_modern_get_queue_notify_off - get notification offset for a virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ *
+ * Returns the notification offset for a virtqueue
+ */
+static u16 vp_modern_get_queue_notify_off(struct virtio_pci_modern_device *mdev,
+ u16 index)
+{
+ vp_iowrite16(index, &mdev->common->queue_select);
+
+ return vp_ioread16(&mdev->common->queue_notify_off);
+}
+
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
@@ -453,7 +468,6 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
{
struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
- struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
struct virtqueue *vq;
u16 num, off;
int err;
@@ -461,9 +475,6 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
if (index >= vp_modern_get_num_queues(mdev))
return ERR_PTR(-ENOENT);
- /* Select the queue we're interested in */
- vp_iowrite16(index, &cfg->queue_select);
-
/* Check if queue is either not available or already active. */
num = vp_modern_get_queue_size(mdev, index);
if (!num || vp_modern_get_queue_enable(mdev, index))
@@ -475,7 +486,7 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
}
/* get offset of notification word for this vq */
- off = vp_ioread16(&cfg->queue_notify_off);
+ off = vp_modern_get_queue_notify_off(mdev, index);
info->msix_vector = msix_vec;
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 11/14] virtio-pci: introduce modern device module
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
` (9 preceding siblings ...)
2020-11-26 9:26 ` [PATCH V2 10/14] virtio-pci-modern: introduce helper to get notification offset Jason Wang
@ 2020-11-26 9:26 ` Jason Wang
2020-11-26 9:26 ` [PATCH V2 12/14] vdpa: set the virtqueue num during register Jason Wang
` (2 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:26 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch introduce an separate module that implement the low level
device probe and access logic for modern device. The goal is let the
module to be reused by other driver (e.g vDPA driver that will be
introduced soon).
Note that, the shared memory cap is not converted since there's no
user currently. We can do that in the future if necessary.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/Kconfig | 10 +-
drivers/virtio/Makefile | 1 +
drivers/virtio/virtio_pci_common.h | 28 +-
drivers/virtio/virtio_pci_modern.c | 462 -------------------------
drivers/virtio/virtio_pci_modern_dev.c | 462 +++++++++++++++++++++++++
include/linux/virtio_pci_modern.h | 107 ++++++
6 files changed, 580 insertions(+), 490 deletions(-)
create mode 100644 drivers/virtio/virtio_pci_modern_dev.c
create mode 100644 include/linux/virtio_pci_modern.h
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index e76e9b9ba93c..26491b6e7e10 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -6,6 +6,14 @@ config VIRTIO
bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_RPMSG
or CONFIG_S390_GUEST.
+config VIRTIO_PCI_MODERN
+ tristate "Modern Virtio PCI Device"
+ depends on PCI
+ help
+ Modern PCI device implementation. This module implement the
+ basic probe and control for devices which is based on modern
+ PCI device with possible vendor specific extensions.
+
menuconfig VIRTIO_MENU
bool "Virtio drivers"
default y
@@ -14,7 +22,7 @@ if VIRTIO_MENU
config VIRTIO_PCI
tristate "PCI driver for virtio devices"
- depends on PCI
+ depends on PCI && VIRTIO_PCI_MODERN
select VIRTIO
help
This driver provides support for virtio based paravirtual device
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index 591e6f72aa54..f097578aaa8f 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o
+obj-$(CONFIG_VIRTIO_PCI_MODERN) += virtio_pci_modern_dev.o
obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o
obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index d32af8ff56f9..4025b940f74e 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -25,6 +25,7 @@
#include <linux/virtio_config.h>
#include <linux/virtio_ring.h>
#include <linux/virtio_pci.h>
+#include <linux/virtio_pci_modern.h>
#include <linux/highmem.h>
#include <linux/spinlock.h>
@@ -39,33 +40,6 @@ struct virtio_pci_vq_info {
unsigned msix_vector;
};
-struct virtio_pci_modern_device {
- struct pci_dev *pci_dev;
-
- /* The IO mapping for the PCI BARs */
- void __iomem * const *base;
-
- /* The IO mapping for the PCI config space */
- struct virtio_pci_common_cfg __iomem *common;
- /* Device-specific data (non-legacy mode) */
- void __iomem *device;
- /* Base of vq notifications (non-legacy mode). */
- void __iomem *notify_base;
- /* Where to read and clear interrupt */
- u8 __iomem *isr;
-
- /* So we can sanity-check accesses. */
- size_t notify_len;
- size_t device_len;
-
- /* Multiply queue_notify_off by this value. (non-legacy mode). */
- u32 notify_offset_multiplier;
-
- int modern_bars;
-
- struct virtio_device_id id;
-};
-
/* Our device structure */
struct virtio_pci_device {
struct virtio_device vdev;
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 8f1f274724be..8dfdc3b57502 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -19,113 +19,6 @@
#define VIRTIO_RING_NO_LEGACY
#include "virtio_pci_common.h"
-/*
- * Type-safe wrappers for io accesses.
- * Use these to enforce at compile time the following spec requirement:
- *
- * The driver MUST access each field using the “natural” access
- * method, i.e. 32-bit accesses for 32-bit fields, 16-bit accesses
- * for 16-bit fields and 8-bit accesses for 8-bit fields.
- */
-static inline u8 vp_ioread8(const u8 __iomem *addr)
-{
- return ioread8(addr);
-}
-static inline u16 vp_ioread16 (const __le16 __iomem *addr)
-{
- return ioread16(addr);
-}
-
-static inline u32 vp_ioread32(const __le32 __iomem *addr)
-{
- return ioread32(addr);
-}
-
-static inline void vp_iowrite8(u8 value, u8 __iomem *addr)
-{
- iowrite8(value, addr);
-}
-
-static inline void vp_iowrite16(u16 value, __le16 __iomem *addr)
-{
- iowrite16(value, addr);
-}
-
-static inline void vp_iowrite32(u32 value, __le32 __iomem *addr)
-{
- iowrite32(value, addr);
-}
-
-static void vp_iowrite64_twopart(u64 val,
- __le32 __iomem *lo, __le32 __iomem *hi)
-{
- vp_iowrite32((u32)val, lo);
- vp_iowrite32(val >> 32, hi);
-}
-
-static void __iomem *map_capability(struct virtio_pci_modern_device *mdev,
- int off, size_t minlen, u32 align,
- u32 size, size_t *len)
-{
- struct pci_dev *dev = mdev->pci_dev;
- u8 bar;
- u32 offset, length;
-
- 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);
-
- if (length < minlen) {
- dev_err(&dev->dev,
- "virtio_pci: bad capability len %u (>=%zu expected)\n",
- length, minlen);
- return NULL;
- }
-
- 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;
- }
-
- return mdev->base[bar] + offset;
-}
-
-static u64 vp_modern_get_features(struct virtio_pci_modern_device *mdev)
-{
- struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
-
- u64 features;
-
- vp_iowrite32(0, &cfg->device_feature_select);
- features = vp_ioread32(&cfg->device_feature);
- vp_iowrite32(1, &cfg->device_feature_select);
- features |= ((u64)vp_ioread32(&cfg->device_feature) << 32);
-
- return features;
-}
-
/* virtio config->get_features() implementation */
static u64 vp_get_features(struct virtio_device *vdev)
{
@@ -144,144 +37,6 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
__virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
}
-static void vp_modern_set_features(struct virtio_pci_modern_device *mdev,
- u64 features)
-{
- struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
-
- vp_iowrite32(0, &cfg->guest_feature_select);
- vp_iowrite32((u32)features, &cfg->guest_feature);
- vp_iowrite32(1, &cfg->guest_feature_select);
- vp_iowrite32(features >> 32, &cfg->guest_feature);
-}
-
-/*
- * vp_modern_queue_vector - set the MSIX vector for a specific virtqueue
- * @mdev: the modern virtio-pci device
- * @index: queue index
- * @vector: the config vector
- *
- * Returns the config vector read from the device
- */
-static u16 vp_modern_queue_vector(struct virtio_pci_modern_device *mdev,
- u16 index, u16 vector)
-{
- struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
-
- vp_iowrite16(index, &cfg->queue_select);
- vp_iowrite16(vector, &cfg->queue_msix_vector);
- /* Flush the write out to device */
- return vp_ioread16(&cfg->queue_msix_vector);
-}
-
-/*
- * vp_modern_queue_address - set the virtqueue address
- * @mdev: the modern virtio-pci device
- * @index: the queue index
- * @desc_addr: address of the descriptor area
- * @driver_addr: address of the driver area
- * @device_addr: address of the device area
- */
-static void vp_modern_queue_address(struct virtio_pci_modern_device *mdev,
- u16 index, u64 desc_addr, u64 driver_addr,
- u64 device_addr)
-{
- struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
-
- vp_iowrite16(index, &cfg->queue_select);
-
- vp_iowrite64_twopart(desc_addr, &cfg->queue_desc_lo,
- &cfg->queue_desc_hi);
- vp_iowrite64_twopart(driver_addr, &cfg->queue_avail_lo,
- &cfg->queue_avail_hi);
- vp_iowrite64_twopart(device_addr, &cfg->queue_used_lo,
- &cfg->queue_used_hi);
-}
-
-/*
- * vp_modern_set_queue_enable - enable a virtqueue
- * @mdev: the modern virtio-pci device
- * @index: the queue index
- * @enable: whether the virtqueue is enable or not
- */
-static void vp_modern_set_queue_enable(struct virtio_pci_modern_device *mdev,
- u16 index, bool enable)
-{
- vp_iowrite16(index, &mdev->common->queue_select);
- vp_iowrite16(enable, &mdev->common->queue_enable);
-}
-
-/*
- * vp_modern_get_queue_enable - enable a virtqueue
- * @mdev: the modern virtio-pci device
- * @index: the queue index
- *
- * Returns whether a virtqueue is enabled or not
- */
-static bool vp_modern_get_queue_enable(struct virtio_pci_modern_device *mdev,
- u16 index)
-{
- vp_iowrite16(index, &mdev->common->queue_select);
-
- return vp_ioread16(&mdev->common->queue_enable);
-}
-
-/*
- * vp_modern_set_queue_size - set size for a virtqueue
- * @mdev: the modern virtio-pci device
- * @index: the queue index
- * @size: the size of the virtqueue
- */
-static void vp_modern_set_queue_size(struct virtio_pci_modern_device *mdev,
- u16 index, u16 size)
-{
- vp_iowrite16(index, &mdev->common->queue_select);
- vp_iowrite16(size, &mdev->common->queue_size);
-
-}
-
-/*
- * vp_modern_get_queue_size - get size for a virtqueue
- * @mdev: the modern virtio-pci device
- * @index: the queue index
- *
- * Returns the size of the virtqueue
- */
-static u16 vp_modern_get_queue_size(struct virtio_pci_modern_device *mdev,
- u16 index)
-{
- vp_iowrite16(index, &mdev->common->queue_select);
-
- return vp_ioread16(&mdev->common->queue_size);
-
-}
-
-/*
- * vp_modern_get_num_queues - get the number of virtqueues
- * @mdev: the modern virtio-pci device
- *
- * Returns the number of virtqueues
- */
-static u16 vp_modern_get_num_queues(struct virtio_pci_modern_device *mdev)
-{
- return vp_ioread16(&mdev->common->num_queues);
-}
-
-/*
- * vp_modern_get_queue_notify_off - get notification offset for a virtqueue
- * @mdev: the modern virtio-pci device
- * @index: the queue index
- *
- * Returns the notification offset for a virtqueue
- */
-static u16 vp_modern_get_queue_notify_off(struct virtio_pci_modern_device *mdev,
- u16 index)
-{
- vp_iowrite16(index, &mdev->common->queue_select);
-
- return vp_ioread16(&mdev->common->queue_notify_off);
-}
-
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
@@ -380,13 +135,6 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
}
}
-static u32 vp_modern_generation(struct virtio_pci_modern_device *mdev)
-{
- struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
-
- return vp_ioread8(&cfg->config_generation);
-}
-
static u32 vp_generation(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
@@ -394,14 +142,6 @@ static u32 vp_generation(struct virtio_device *vdev)
return vp_modern_generation(&vp_dev->mdev);
}
-/* config->{get,set}_status() implementations */
-static u8 vp_modern_get_status(struct virtio_pci_modern_device *mdev)
-{
- struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
-
- return vp_ioread8(&cfg->device_status);
-}
-
static u8 vp_get_status(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
@@ -409,14 +149,6 @@ static u8 vp_get_status(struct virtio_device *vdev)
return vp_modern_get_status(&vp_dev->mdev);
}
-static void vp_modern_set_status(struct virtio_pci_modern_device *mdev,
- u8 status)
-{
- struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
-
- vp_iowrite8(status, &cfg->device_status);
-}
-
static void vp_set_status(struct virtio_device *vdev, u8 status)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
@@ -441,18 +173,6 @@ static void vp_reset(struct virtio_device *vdev)
vp_synchronize_vectors(vdev);
}
-static u16 vp_modern_config_vector(struct virtio_pci_modern_device *mdev,
- u16 vector)
-{
- struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
-
- /* Setup the vector used for configuration events */
- vp_iowrite16(vector, &cfg->msix_config);
- /* Verify we had enough resources to assign the vector */
- /* Will also flush the write out to device */
- return vp_ioread16(&cfg->msix_config);
-}
-
static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
{
return vp_modern_config_vector(&vp_dev->mdev, vector);
@@ -689,46 +409,6 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
.get_shm_region = vp_get_shm_region,
};
-/**
- * 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 > 0x5)
- 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;
-}
-
/* This is part of the ABI. Don't screw with it. */
static inline void check_offsets(void)
{
@@ -792,148 +472,6 @@ static inline void check_offsets(void)
offsetof(struct virtio_pci_common_cfg, queue_used_hi));
}
-static int vp_modern_probe(struct virtio_pci_modern_device *mdev)
-{
- struct pci_dev *pci_dev = mdev->pci_dev;
- int err, common, isr, notify, device, i;
- unsigned int num_queues;
- u32 notify_length;
- u32 notify_offset;
- u16 off;
-
- /* We only own devices >= 0x1000 and <= 0x107f: leave the rest. */
- if (pci_dev->device < 0x1000 || pci_dev->device > 0x107f)
- return -ENODEV;
-
- if (pci_dev->device < 0x1040) {
- /* Transitional devices: use the PCI subsystem device id as
- * virtio device id, same as legacy driver always did.
- */
- mdev->id.device = pci_dev->subsystem_device;
- } else {
- /* Modern devices: simply use PCI device id, but start from 0x1040. */
- mdev->id.device = pci_dev->device - 0x1040;
- }
- mdev->id.vendor = pci_dev->subsystem_vendor;
-
- err = pcim_enable_device(pci_dev);
- if (err)
- return err;
-
- /* 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: leaving for legacy driver\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;
- }
-
- err = dma_set_mask_and_coherent(&pci_dev->dev, DMA_BIT_MASK(64));
- if (err)
- err = dma_set_mask_and_coherent(&pci_dev->dev,
- DMA_BIT_MASK(32));
- if (err)
- dev_warn(&pci_dev->dev, "Failed to enable 64-bit or 32-bit DMA. Trying to continue, but this might not work.\n");
-
- /* 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 = pcim_iomap_regions(pci_dev, mdev->modern_bars,
- "virtio-pci-modern");
- if (err)
- return err;
-
- mdev->base = pcim_iomap_table(pci_dev);
-
- err = -EINVAL;
- mdev->common = map_capability(mdev, common,
- sizeof(struct virtio_pci_common_cfg), 4,
- sizeof(struct virtio_pci_common_cfg),
- NULL);
- if (!mdev->common)
- goto err;
- mdev->isr = map_capability(mdev, isr, sizeof(u8), 1, 1, NULL);
- if (!mdev->isr)
- goto err;
-
- /* 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);
-
- mdev->notify_base = map_capability(mdev, notify, 2, 2,
- notify_length,
- &mdev->notify_len);
- if (!mdev->notify_base)
- goto err;
-
- num_queues = vp_ioread16(&mdev->common->num_queues);
-
- /* offset should not wrap */
- for (i = 0; i < num_queues; i++) {
- vp_iowrite16(i, &mdev->common->queue_select);
- off = vp_ioread16(&mdev->common->queue_notify_off);
-
- if ((u64)off * mdev->notify_offset_multiplier + 2
- > mdev->notify_len) {
- dev_warn(&pci_dev->dev,
- "bad notification offset %u (x %u) "
- "for queue %u > %zd",
- off, mdev->notify_offset_multiplier,
- i, mdev->notify_len);
- err = -EINVAL;
- goto err;
- }
- }
-
- /* We don't know how much we should map, but PAGE_SIZE
- * is more than enough for all existing devices.
- */
- if (device) {
- mdev->device = map_capability(mdev, device, 0, 4,
- PAGE_SIZE,
- &mdev->device_len);
- if (!mdev->device)
- goto err;
- }
-
- return 0;
-
-err:
- return err;
-}
-
/* the PCI probing function */
int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
{
diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
new file mode 100644
index 000000000000..90215f25d936
--- /dev/null
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -0,0 +1,462 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/virtio_pci_modern.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+
+static void __iomem *map_capability(struct virtio_pci_modern_device *mdev,
+ int off, size_t minlen, u32 align,
+ u32 size, size_t *len)
+{
+ struct pci_dev *dev = mdev->pci_dev;
+ u8 bar;
+ u32 offset, length;
+
+ 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);
+
+ if (length < minlen) {
+ dev_err(&dev->dev,
+ "virtio_pci: bad capability len %u (>=%zu expected)\n",
+ length, minlen);
+ return NULL;
+ }
+
+ 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;
+ }
+
+ return mdev->base[bar] + offset;
+}
+
+/**
+ * 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 > 0x5)
+ 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;
+}
+
+/*
+ * vp_modern_get_features - get features from device
+ * @mdev: the modern virtio-pci device
+ *
+ * Returns the features read from the device
+ */
+u64 vp_modern_get_features(struct virtio_pci_modern_device *mdev)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ u64 features;
+
+ vp_iowrite32(0, &cfg->device_feature_select);
+ features = vp_ioread32(&cfg->device_feature);
+ vp_iowrite32(1, &cfg->device_feature_select);
+ features |= ((u64)vp_ioread32(&cfg->device_feature) << 32);
+
+ return features;
+}
+EXPORT_SYMBOL_GPL(vp_modern_get_features);
+
+/*
+ * vp_modern_set_features - set features to device
+ * @mdev: the modern virtio-pci device
+ * @features: the features set to device
+ */
+void vp_modern_set_features(struct virtio_pci_modern_device *mdev,
+ u64 features)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ vp_iowrite32(0, &cfg->guest_feature_select);
+ vp_iowrite32((u32)features, &cfg->guest_feature);
+ vp_iowrite32(1, &cfg->guest_feature_select);
+ vp_iowrite32(features >> 32, &cfg->guest_feature);
+}
+EXPORT_SYMBOL_GPL(vp_modern_set_features);
+
+/*
+ * vp_modern_get_generation - get the device genreation
+ * @mdev: the modern virtio-pci device
+ *
+ * Returns the genreation read from device
+ */
+u32 vp_modern_generation(struct virtio_pci_modern_device *mdev)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ return vp_ioread8(&cfg->config_generation);
+}
+EXPORT_SYMBOL_GPL(vp_modern_generation);
+
+/*
+ * vp_modern_get_status - get the device status
+ * @mdev: the modern virtio-pci device
+ *
+ * Returns the status read from device
+ */
+u8 vp_modern_get_status(struct virtio_pci_modern_device *mdev)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ return vp_ioread8(&cfg->device_status);
+}
+EXPORT_SYMBOL_GPL(vp_modern_get_status);
+
+/*
+ * vp_modern_set_status - set status to device
+ * @mdev: the modern virtio-pci device
+ * @status: the status set to device
+ */
+void vp_modern_set_status(struct virtio_pci_modern_device *mdev,
+ u8 status)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ vp_iowrite8(status, &cfg->device_status);
+}
+EXPORT_SYMBOL_GPL(vp_modern_set_status);
+
+/*
+ * vp_modern_queue_vector - set the MSIX vector for a specific virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: queue index
+ * @vector: the config vector
+ *
+ * Returns the config vector read from the device
+ */
+u16 vp_modern_queue_vector(struct virtio_pci_modern_device *mdev,
+ u16 index, u16 vector)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ vp_iowrite16(index, &cfg->queue_select);
+ vp_iowrite16(vector, &cfg->queue_msix_vector);
+ /* Flush the write out to device */
+ return vp_ioread16(&cfg->queue_msix_vector);
+}
+EXPORT_SYMBOL_GPL(vp_modern_queue_vector);
+
+/*
+ * vp_modern_config_vector - set the vector for config interrupt
+ * @mdev: the modern virtio-pci device
+ * @vector: the config vector
+ *
+ * Returns the config vector read from the device
+ */
+u16 vp_modern_config_vector(struct virtio_pci_modern_device *mdev,
+ u16 vector)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ /* Setup the vector used for configuration events */
+ vp_iowrite16(vector, &cfg->msix_config);
+ /* Verify we had enough resources to assign the vector */
+ /* Will also flush the write out to device */
+ return vp_ioread16(&cfg->msix_config);
+}
+EXPORT_SYMBOL_GPL(vp_modern_config_vector);
+
+/*
+ * vp_modern_queue_address - set the virtqueue address
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ * @desc_addr: address of the descriptor area
+ * @driver_addr: address of the driver area
+ * @device_addr: address of the device area
+ */
+void vp_modern_queue_address(struct virtio_pci_modern_device *mdev,
+ u16 index, u64 desc_addr, u64 driver_addr,
+ u64 device_addr)
+{
+ struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+ vp_iowrite16(index, &cfg->queue_select);
+
+ vp_iowrite64_twopart(desc_addr, &cfg->queue_desc_lo,
+ &cfg->queue_desc_hi);
+ vp_iowrite64_twopart(driver_addr, &cfg->queue_avail_lo,
+ &cfg->queue_avail_hi);
+ vp_iowrite64_twopart(device_addr, &cfg->queue_used_lo,
+ &cfg->queue_used_hi);
+}
+EXPORT_SYMBOL_GPL(vp_modern_queue_address);
+
+/*
+ * vp_modern_set_queue_enable - enable a virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ * @enable: whether the virtqueue is enable or not
+ */
+void vp_modern_set_queue_enable(struct virtio_pci_modern_device *mdev,
+ u16 index, bool enable)
+{
+ vp_iowrite16(index, &mdev->common->queue_select);
+ vp_iowrite16(enable, &mdev->common->queue_enable);
+}
+EXPORT_SYMBOL_GPL(vp_modern_set_queue_enable);
+
+/*
+ * vp_modern_get_queue_enable - enable a virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ *
+ * Returns whether a virtqueue is enabled or not
+ */
+bool vp_modern_get_queue_enable(struct virtio_pci_modern_device *mdev,
+ u16 index)
+{
+ vp_iowrite16(index, &mdev->common->queue_select);
+
+ return vp_ioread16(&mdev->common->queue_enable);
+}
+EXPORT_SYMBOL_GPL(vp_modern_get_queue_enable);
+
+/*
+ * vp_modern_set_queue_size - set size for a virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ * @size: the size of the virtqueue
+ */
+void vp_modern_set_queue_size(struct virtio_pci_modern_device *mdev,
+ u16 index, u16 size)
+{
+ vp_iowrite16(index, &mdev->common->queue_select);
+ vp_iowrite16(size, &mdev->common->queue_size);
+
+}
+EXPORT_SYMBOL_GPL(vp_modern_set_queue_size);
+
+/*
+ * vp_modern_get_queue_size - get size for a virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ *
+ * Returns the size of the virtqueue
+ */
+u16 vp_modern_get_queue_size(struct virtio_pci_modern_device *mdev,
+ u16 index)
+{
+ vp_iowrite16(index, &mdev->common->queue_select);
+
+ return vp_ioread16(&mdev->common->queue_size);
+
+}
+EXPORT_SYMBOL_GPL(vp_modern_get_queue_size);
+
+/*
+ * vp_modern_get_num_queues - get the number of virtqueues
+ * @mdev: the modern virtio-pci device
+ *
+ * Returns the number of virtqueues
+ */
+u16 vp_modern_get_num_queues(struct virtio_pci_modern_device *mdev)
+{
+ return vp_ioread16(&mdev->common->num_queues);
+}
+EXPORT_SYMBOL_GPL(vp_modern_get_num_queues);
+
+/*
+ * vp_modern_get_queue_notify_off - get notification offset for a virtqueue
+ * @mdev: the modern virtio-pci device
+ * @index: the queue index
+ *
+ * Returns the notification offset for a virtqueue
+ */
+u16 vp_modern_get_queue_notify_off(struct virtio_pci_modern_device *mdev,
+ u16 index)
+{
+ vp_iowrite16(index, &mdev->common->queue_select);
+
+ return vp_ioread16(&mdev->common->queue_notify_off);
+}
+EXPORT_SYMBOL_GPL(vp_modern_get_queue_notify_off);
+
+/*
+ * vp_modern_probe: probe the modern virtio pci device
+ * @mdev: the modern virtio-pci device
+ *
+ * Return 0 on succeed otherwise fail
+ */
+int vp_modern_probe(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;
+
+ /* We only own devices >= 0x1000 and <= 0x107f: leave the rest. */
+ if (pci_dev->device < 0x1000 || pci_dev->device > 0x107f)
+ return -ENODEV;
+
+ if (pci_dev->device < 0x1040) {
+ /* Transitional devices: use the PCI subsystem device id as
+ * virtio device id, same as legacy driver always did.
+ */
+ mdev->id.device = pci_dev->subsystem_device;
+ } else {
+ /* Modern devices: simply use PCI device id, but start from 0x1040. */
+ mdev->id.device = pci_dev->device - 0x1040;
+ }
+ mdev->id.vendor = pci_dev->subsystem_vendor;
+
+ err = pcim_enable_device(pci_dev);
+ if (err)
+ return err;
+
+ /* 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: leaving for legacy driver\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;
+ }
+
+ err = dma_set_mask_and_coherent(&pci_dev->dev, DMA_BIT_MASK(64));
+ if (err)
+ err = dma_set_mask_and_coherent(&pci_dev->dev,
+ DMA_BIT_MASK(32));
+ if (err)
+ dev_warn(&pci_dev->dev, "Failed to enable 64-bit or 32-bit DMA. Trying to continue, but this might not work.\n");
+
+ /* 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 = pcim_iomap_regions(pci_dev, mdev->modern_bars,
+ "virtio-pci-modern");
+ if (err)
+ return err;
+
+ mdev->base = pcim_iomap_table(pci_dev);
+
+ err = -EINVAL;
+ mdev->common = map_capability(mdev, common,
+ sizeof(struct virtio_pci_common_cfg), 4,
+ sizeof(struct virtio_pci_common_cfg),
+ NULL);
+ if (!mdev->common)
+ goto err;
+ mdev->isr = map_capability(mdev, isr, sizeof(u8), 1, 1, NULL);
+ if (!mdev->isr)
+ goto err;
+
+ /* 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);
+
+ mdev->notify_base = map_capability(mdev, notify, 2, 2,
+ notify_length,
+ &mdev->notify_len);
+ if (!mdev->notify_base)
+ goto err;
+
+ /* We don't know how much we should map, but PAGE_SIZE
+ * is more than enough for all existing devices.
+ */
+ if (device) {
+ mdev->device = map_capability(mdev, device, 0, 4,
+ PAGE_SIZE,
+ &mdev->device_len);
+ if (!mdev->device)
+ goto err;
+ }
+
+ return 0;
+
+err:
+ return err;
+}
+EXPORT_SYMBOL_GPL(vp_modern_probe);
+
+MODULE_VERSION("0.1");
+MODULE_DESCRIPTION("Modern Virtio PCI Device");
+MODULE_AUTHOR("Jason Wang <jasowang@redhat.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
new file mode 100644
index 000000000000..a50261b2a17d
--- /dev/null
+++ b/include/linux/virtio_pci_modern.h
@@ -0,0 +1,107 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_VIRTIO_PCI_MODERN_H
+#define _LINUX_VIRTIO_PCI_MODERN_H
+
+#include <linux/pci.h>
+#include <linux/virtio_pci.h>
+
+struct virtio_pci_modern_device {
+ struct pci_dev *pci_dev;
+
+ /* The IO mapping for the PCI BARs */
+ void __iomem * const *base;
+
+ /* The IO mapping for the PCI config space */
+ struct virtio_pci_common_cfg __iomem *common;
+ /* Device-specific data (non-legacy mode) */
+ void __iomem *device;
+ /* Base of vq notifications (non-legacy mode). */
+ void __iomem *notify_base;
+ /* Where to read and clear interrupt */
+ u8 __iomem *isr;
+
+ /* So we can sanity-check accesses. */
+ size_t notify_len;
+ size_t device_len;
+
+ /* Multiply queue_notify_off by this value. (non-legacy mode). */
+ u32 notify_offset_multiplier;
+
+ int modern_bars;
+
+ struct virtio_device_id id;
+};
+
+/*
+ * Type-safe wrappers for io accesses.
+ * Use these to enforce at compile time the following spec requirement:
+ *
+ * The driver MUST access each field using the “natural” access
+ * method, i.e. 32-bit accesses for 32-bit fields, 16-bit accesses
+ * for 16-bit fields and 8-bit accesses for 8-bit fields.
+ */
+static inline u8 vp_ioread8(const u8 __iomem *addr)
+{
+ return ioread8(addr);
+}
+static inline u16 vp_ioread16 (const __le16 __iomem *addr)
+{
+ return ioread16(addr);
+}
+
+static inline u32 vp_ioread32(const __le32 __iomem *addr)
+{
+ return ioread32(addr);
+}
+
+static inline void vp_iowrite8(u8 value, u8 __iomem *addr)
+{
+ iowrite8(value, addr);
+}
+
+static inline void vp_iowrite16(u16 value, __le16 __iomem *addr)
+{
+ iowrite16(value, addr);
+}
+
+static inline void vp_iowrite32(u32 value, __le32 __iomem *addr)
+{
+ iowrite32(value, addr);
+}
+
+static inline void vp_iowrite64_twopart(u64 val,
+ __le32 __iomem *lo,
+ __le32 __iomem *hi)
+{
+ vp_iowrite32((u32)val, lo);
+ vp_iowrite32(val >> 32, hi);
+}
+
+u64 vp_modern_get_features(struct virtio_pci_modern_device *mdev);
+void vp_modern_set_features(struct virtio_pci_modern_device *mdev,
+ u64 features);
+u32 vp_modern_generation(struct virtio_pci_modern_device *mdev);
+u8 vp_modern_get_status(struct virtio_pci_modern_device *mdev);
+void vp_modern_set_status(struct virtio_pci_modern_device *mdev,
+ u8 status);
+u16 vp_modern_queue_vector(struct virtio_pci_modern_device *mdev,
+ u16 idx, u16 vector);
+u16 vp_modern_config_vector(struct virtio_pci_modern_device *mdev,
+ u16 vector);
+void vp_modern_queue_address(struct virtio_pci_modern_device *mdev,
+ u16 index, u64 desc_addr, u64 driver_addr,
+ u64 device_addr);
+void vp_modern_set_queue_enable(struct virtio_pci_modern_device *mdev,
+ u16 idx, bool enable);
+bool vp_modern_get_queue_enable(struct virtio_pci_modern_device *mdev,
+ u16 idx);
+void vp_modern_set_queue_size(struct virtio_pci_modern_device *mdev,
+ u16 idx, u16 size);
+u16 vp_modern_get_queue_size(struct virtio_pci_modern_device *mdev,
+ u16 idx);
+u16 vp_modern_get_num_queues(struct virtio_pci_modern_device *mdev);
+u16 vp_modern_get_queue_notify_off(struct virtio_pci_modern_device *mdev,
+ u16 idx);
+int vp_modern_probe(struct virtio_pci_modern_device *mdev);
+
+#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 12/14] vdpa: set the virtqueue num during register
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
` (10 preceding siblings ...)
2020-11-26 9:26 ` [PATCH V2 11/14] virtio-pci: introduce modern device module Jason Wang
@ 2020-11-26 9:26 ` Jason Wang
2020-11-26 9:26 ` [PATCH V2 13/14] virtio_vdpa: don't warn when fail to disable vq Jason Wang
2020-11-26 9:26 ` [PATCH V2 14/14] vdpa: introduce virtio pci driver Jason Wang
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:26 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch delay the queue number setting to vDPA device
registering. This allows us to probe the virtqueue numbers between
device allocation and registering.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vdpa/ifcvf/ifcvf_main.c | 5 ++---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 5 ++---
drivers/vdpa/vdpa.c | 8 ++++----
drivers/vdpa/vdpa_sim/vdpa_sim.c | 4 ++--
include/linux/vdpa.h | 7 +++----
5 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index 8b4028556cb6..d65f3221d8ed 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -438,8 +438,7 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
}
adapter = vdpa_alloc_device(struct ifcvf_adapter, vdpa,
- dev, &ifc_vdpa_ops,
- IFCVF_MAX_QUEUE_PAIRS * 2);
+ dev, &ifc_vdpa_ops);
if (adapter == NULL) {
IFCVF_ERR(pdev, "Failed to allocate vDPA structure");
return -ENOMEM;
@@ -463,7 +462,7 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++)
vf->vring[i].irq = -EINVAL;
- ret = vdpa_register_device(&adapter->vdpa);
+ ret = vdpa_register_device(&adapter->vdpa, IFCVF_MAX_QUEUE_PAIRS * 2);
if (ret) {
IFCVF_ERR(pdev, "Failed to register ifcvf to vdpa bus");
goto err;
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 74264e590695..baa6be16f3e5 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1932,8 +1932,7 @@ void *mlx5_vdpa_add_dev(struct mlx5_core_dev *mdev)
max_vqs = MLX5_CAP_DEV_VDPA_EMULATION(mdev, max_num_virtio_queues);
max_vqs = min_t(u32, max_vqs, MLX5_MAX_SUPPORTED_VQS);
- ndev = vdpa_alloc_device(struct mlx5_vdpa_net, mvdev.vdev, mdev->device, &mlx5_vdpa_ops,
- 2 * mlx5_vdpa_max_qps(max_vqs));
+ ndev = vdpa_alloc_device(struct mlx5_vdpa_net, mvdev.vdev, mdev->device, &mlx5_vdpa_ops);
if (IS_ERR(ndev))
return ndev;
@@ -1960,7 +1959,7 @@ void *mlx5_vdpa_add_dev(struct mlx5_core_dev *mdev)
if (err)
goto err_res;
- err = vdpa_register_device(&mvdev->vdev);
+ err = vdpa_register_device(&mvdev->vdev, 2 * mlx5_vdpa_max_qps(max_vqs));
if (err)
goto err_reg;
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index a69ffc991e13..ba89238f9898 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -61,7 +61,6 @@ static void vdpa_release_dev(struct device *d)
* initialized but before registered.
* @parent: the parent device
* @config: the bus operations that is supported by this device
- * @nvqs: number of virtqueues supported by this device
* @size: size of the parent structure that contains private data
*
* Driver should use vdpa_alloc_device() wrapper macro instead of
@@ -72,7 +71,6 @@ static void vdpa_release_dev(struct device *d)
*/
struct vdpa_device *__vdpa_alloc_device(struct device *parent,
const struct vdpa_config_ops *config,
- int nvqs,
size_t size)
{
struct vdpa_device *vdev;
@@ -99,7 +97,6 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent,
vdev->index = err;
vdev->config = config;
vdev->features_valid = false;
- vdev->nvqs = nvqs;
err = dev_set_name(&vdev->dev, "vdpa%u", vdev->index);
if (err)
@@ -122,11 +119,14 @@ EXPORT_SYMBOL_GPL(__vdpa_alloc_device);
* vdpa_register_device - register a vDPA device
* Callers must have a succeed call of vdpa_alloc_device() before.
* @vdev: the vdpa device to be registered to vDPA bus
+ * @nvqs: number of virtqueues supported by this device
*
* Returns an error when fail to add to vDPA bus
*/
-int vdpa_register_device(struct vdpa_device *vdev)
+int vdpa_register_device(struct vdpa_device *vdev, int nvqs)
{
+ vdev->nvqs = nvqs;
+
return device_add(&vdev->dev);
}
EXPORT_SYMBOL_GPL(vdpa_register_device);
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index fb3e7d46870f..e3108bd77610 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -352,7 +352,7 @@ static struct vdpasim *vdpasim_create(void)
else
ops = &vdpasim_net_config_ops;
- vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, VDPASIM_VQ_NUM);
+ vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops);
if (!vdpasim)
goto err_alloc;
@@ -378,7 +378,7 @@ static struct vdpasim *vdpasim_create(void)
vringh_set_iotlb(&vdpasim->vqs[1].vring, vdpasim->iommu);
vdpasim->vdpa.dma_dev = dev;
- ret = vdpa_register_device(&vdpasim->vdpa);
+ ret = vdpa_register_device(&vdpasim->vdpa, VDPASIM_VQ_NUM);
if (ret)
goto err_iommu;
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 30bc7a7223bb..d9e9d17b9083 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -244,18 +244,17 @@ struct vdpa_config_ops {
struct vdpa_device *__vdpa_alloc_device(struct device *parent,
const struct vdpa_config_ops *config,
- int nvqs,
size_t size);
-#define vdpa_alloc_device(dev_struct, member, parent, config, nvqs) \
+#define vdpa_alloc_device(dev_struct, member, parent, config) \
container_of(__vdpa_alloc_device( \
- parent, config, nvqs, \
+ parent, config, \
sizeof(dev_struct) + \
BUILD_BUG_ON_ZERO(offsetof( \
dev_struct, member))), \
dev_struct, member)
-int vdpa_register_device(struct vdpa_device *vdev);
+int vdpa_register_device(struct vdpa_device *vdev, int nvqs);
void vdpa_unregister_device(struct vdpa_device *vdev);
/**
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 13/14] virtio_vdpa: don't warn when fail to disable vq
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
` (11 preceding siblings ...)
2020-11-26 9:26 ` [PATCH V2 12/14] vdpa: set the virtqueue num during register Jason Wang
@ 2020-11-26 9:26 ` Jason Wang
2020-11-26 9:26 ` [PATCH V2 14/14] vdpa: introduce virtio pci driver Jason Wang
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:26 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
There's no guarantee that the device can disable a specific virtqueue
through set_vq_ready(). One example is the modern virtio-pci
device. So this patch removes the warning.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_vdpa.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index 4a9ddb44b2a7..e28acf482e0c 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -225,9 +225,8 @@ static void virtio_vdpa_del_vq(struct virtqueue *vq)
list_del(&info->node);
spin_unlock_irqrestore(&vd_dev->lock, flags);
- /* Select and deactivate the queue */
+ /* Select and deactivate the queue (best effort) */
ops->set_vq_ready(vdpa, index, 0);
- WARN_ON(ops->get_vq_ready(vdpa, index));
vring_del_virtqueue(vq);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V2 14/14] vdpa: introduce virtio pci driver
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
` (12 preceding siblings ...)
2020-11-26 9:26 ` [PATCH V2 13/14] virtio_vdpa: don't warn when fail to disable vq Jason Wang
@ 2020-11-26 9:26 ` Jason Wang
13 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-26 9:26 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel; +Cc: shahafs
This patch introduce a vDPA driver for virtio-pci device. It bridges
the virtio-pci control command to the vDPA bus. This will be used for
features prototyping and testing.
Note that get/restore virtqueue state is not supported which needs
extension on the virtio specification.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vdpa/Kconfig | 6 +
drivers/vdpa/Makefile | 1 +
drivers/vdpa/virtio_pci/Makefile | 2 +
drivers/vdpa/virtio_pci/vp_vdpa.c | 450 ++++++++++++++++++++++++++++++
4 files changed, 459 insertions(+)
create mode 100644 drivers/vdpa/virtio_pci/Makefile
create mode 100644 drivers/vdpa/virtio_pci/vp_vdpa.c
diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
index d7d32b656102..4cca53114cc4 100644
--- a/drivers/vdpa/Kconfig
+++ b/drivers/vdpa/Kconfig
@@ -47,4 +47,10 @@ config MLX5_VDPA_NET
be executed by the hardware. It also supports a variety of stateless
offloads depending on the actual device used and firmware version.
+config VP_VDPA
+ tristate "Virtio PCI bridge vDPA driver"
+ depends on PCI_MSI && VIRTIO_PCI_MODERN
+ help
+ This kernel module that bridges virtio PCI device to vDPA bus.
+
endif # VDPA
diff --git a/drivers/vdpa/Makefile b/drivers/vdpa/Makefile
index d160e9b63a66..67fe7f3d6943 100644
--- a/drivers/vdpa/Makefile
+++ b/drivers/vdpa/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_VDPA) += vdpa.o
obj-$(CONFIG_VDPA_SIM) += vdpa_sim/
obj-$(CONFIG_IFCVF) += ifcvf/
obj-$(CONFIG_MLX5_VDPA) += mlx5/
+obj-$(CONFIG_VP_VDPA) += virtio_pci/
diff --git a/drivers/vdpa/virtio_pci/Makefile b/drivers/vdpa/virtio_pci/Makefile
new file mode 100644
index 000000000000..231088d3af7d
--- /dev/null
+++ b/drivers/vdpa/virtio_pci/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_VP_VDPA) += vp_vdpa.o
diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c
new file mode 100644
index 000000000000..6458fa470566
--- /dev/null
+++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
@@ -0,0 +1,450 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * vDPA bridge driver for modern virtio-pci device
+ *
+ * Copyright (c) 2020, Red Hat Inc. All rights reserved.
+ * Author: Jason Wang <jasowang@redhat.com>
+ *
+ * Based on virtio_pci_modern.c.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/vdpa.h>
+#include <linux/virtio.h>
+#include <linux/virtio_config.h>
+#include <linux/virtio_ring.h>
+#include <linux/virtio_pci.h>
+#include <linux/virtio_pci_modern.h>
+
+#define VP_VDPA_QUEUE_MAX 256
+#define VP_VDPA_DRIVER_NAME "vp_vdpa"
+
+struct vp_vring {
+ void __iomem *notify;
+ char msix_name[256];
+ struct vdpa_callback cb;
+ int irq;
+};
+
+struct vp_vdpa {
+ struct vdpa_device vdpa;
+ struct virtio_pci_modern_device mdev;
+ struct vp_vring *vring;
+ struct vdpa_callback cb;
+ char msix_name[256];
+ int config_irq;
+ int queues;
+ int vectors;
+};
+
+static struct vp_vdpa *vdpa_to_vp(struct vdpa_device *vdpa)
+{
+ return container_of(vdpa, struct vp_vdpa, vdpa);
+}
+
+static struct virtio_pci_modern_device *vdpa_to_mdev(struct vdpa_device *vdpa)
+{
+ struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
+
+ return &vp_vdpa->mdev;
+}
+
+static u64 vp_vdpa_get_features(struct vdpa_device *vdpa)
+{
+ struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
+
+ return vp_modern_get_features(mdev);
+}
+
+static int vp_vdpa_set_features(struct vdpa_device *vdpa, u64 features)
+{
+ struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
+
+ vp_modern_set_features(mdev, features);
+
+ return 0;
+}
+
+static u8 vp_vdpa_get_status(struct vdpa_device *vdpa)
+{
+ struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
+
+ return vp_modern_get_status(mdev);
+}
+
+static void vp_vdpa_free_irq(struct vp_vdpa *vp_vdpa)
+{
+ struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+ struct pci_dev *pdev = mdev->pci_dev;
+ int i;
+
+ for (i = 0; i < vp_vdpa->queues; i++) {
+ if (vp_vdpa->vring[i].irq != VIRTIO_MSI_NO_VECTOR) {
+ vp_modern_queue_vector(mdev, i, VIRTIO_MSI_NO_VECTOR);
+ devm_free_irq(&pdev->dev, vp_vdpa->vring[i].irq,
+ &vp_vdpa->vring[i]);
+ vp_vdpa->vring[i].irq = VIRTIO_MSI_NO_VECTOR;
+ }
+ }
+
+ if (vp_vdpa->config_irq != VIRTIO_MSI_NO_VECTOR) {
+ vp_modern_config_vector(mdev, VIRTIO_MSI_NO_VECTOR);
+ devm_free_irq(&pdev->dev, vp_vdpa->config_irq, vp_vdpa);
+ vp_vdpa->config_irq = VIRTIO_MSI_NO_VECTOR;
+ }
+
+ if (vp_vdpa->vectors) {
+ pci_free_irq_vectors(pdev);
+ vp_vdpa->vectors = 0;
+ }
+}
+
+static irqreturn_t vp_vdpa_vq_handler(int irq, void *arg)
+{
+ struct vp_vring *vring = arg;
+
+ if (vring->cb.callback)
+ return vring->cb.callback(vring->cb.private);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t vp_vdpa_config_handler(int irq, void *arg)
+{
+ struct vp_vdpa *vp_vdpa = arg;
+
+ if (vp_vdpa->cb.callback)
+ return vp_vdpa->cb.callback(vp_vdpa->cb.private);
+
+ return IRQ_HANDLED;
+}
+
+static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa)
+{
+ struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+ struct pci_dev *pdev = mdev->pci_dev;
+ int i, ret, irq;
+ int queues = vp_vdpa->queues;
+ int vectors = queues + 1;
+
+ ret = pci_alloc_irq_vectors(pdev, vectors, vectors, PCI_IRQ_MSIX);
+ if (ret != vectors) {
+ dev_err(&pdev->dev,
+ "vp_vdpa: fail to allocate irq vectors want %d but %d\n",
+ vectors, ret);
+ return ret;
+ }
+
+ vp_vdpa->vectors = vectors;
+
+ for (i = 0; i < queues; i++) {
+ snprintf(vp_vdpa->vring[i].msix_name, 256,
+ "vp-vdpa[%s]-%d\n", pci_name(pdev), i);
+ irq = pci_irq_vector(pdev, i);
+ ret = devm_request_irq(&pdev->dev, irq,
+ vp_vdpa_vq_handler,
+ 0, vp_vdpa->vring[i].msix_name,
+ &vp_vdpa->vring[i]);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "vp_vdpa: fail to request irq for vq %d\n", i);
+ goto err;
+ }
+ vp_modern_queue_vector(mdev, i, i);
+ vp_vdpa->vring[i].irq = irq;
+ }
+
+ snprintf(vp_vdpa->msix_name, 256, "vp-vdpa[%s]-config\n",
+ pci_name(pdev));
+ irq = pci_irq_vector(pdev, queues);
+ ret = devm_request_irq(&pdev->dev, irq, vp_vdpa_config_handler, 0,
+ vp_vdpa->msix_name, vp_vdpa);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "vp_vdpa: fail to request irq for vq %d\n", i);
+ goto err;
+ }
+ vp_modern_config_vector(mdev, queues);
+ vp_vdpa->config_irq = irq;
+
+ return 0;
+err:
+ vp_vdpa_free_irq(vp_vdpa);
+ return ret;
+}
+
+static void vp_vdpa_set_status(struct vdpa_device *vdpa, u8 status)
+{
+ struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
+ struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+ u8 s = vp_vdpa_get_status(vdpa);
+
+ if (status & VIRTIO_CONFIG_S_DRIVER_OK &&
+ !(s & VIRTIO_CONFIG_S_DRIVER_OK)) {
+ vp_vdpa_request_irq(vp_vdpa);
+ }
+
+ vp_modern_set_status(mdev, status);
+
+ if (!(status & VIRTIO_CONFIG_S_DRIVER_OK) &&
+ (s & VIRTIO_CONFIG_S_DRIVER_OK))
+ vp_vdpa_free_irq(vp_vdpa);
+}
+
+static u16 vp_vdpa_get_vq_num_max(struct vdpa_device *vdpa)
+{
+ return VP_VDPA_QUEUE_MAX;
+}
+
+static int vp_vdpa_get_vq_state(struct vdpa_device *vdpa, u16 qid,
+ struct vdpa_vq_state *state)
+{
+ /* Note that this is not supported by virtio specification, so
+ * we return -EOPNOTSUPP here. This means we can't support live
+ * migration, vhost device start/stop.
+ */
+ return -EOPNOTSUPP;
+}
+
+static int vp_vdpa_set_vq_state(struct vdpa_device *vdpa, u16 qid,
+ const struct vdpa_vq_state *state)
+{
+ /* Note that this is not supported by virtio specification, so
+ * we return -ENOPOTSUPP here. This means we can't support live
+ * migration, vhost device start/stop.
+ */
+ return -EOPNOTSUPP;
+}
+
+static void vp_vdpa_set_vq_cb(struct vdpa_device *vdpa, u16 qid,
+ struct vdpa_callback *cb)
+{
+ struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
+
+ vp_vdpa->vring[qid].cb = *cb;
+}
+
+static void vp_vdpa_set_vq_ready(struct vdpa_device *vdpa,
+ u16 qid, bool ready)
+{
+ struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
+
+ vp_modern_set_queue_enable(mdev, qid, ready);
+}
+
+static bool vp_vdpa_get_vq_ready(struct vdpa_device *vdpa, u16 qid)
+{
+ struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
+
+ return vp_modern_get_queue_enable(mdev, qid);
+}
+
+static void vp_vdpa_set_vq_num(struct vdpa_device *vdpa, u16 qid,
+ u32 num)
+{
+ struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
+
+ vp_modern_set_queue_size(mdev, qid, num);
+}
+
+static int vp_vdpa_set_vq_address(struct vdpa_device *vdpa, u16 qid,
+ u64 desc_area, u64 driver_area,
+ u64 device_area)
+{
+ struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
+
+ vp_modern_queue_address(mdev, qid, desc_area,
+ driver_area, device_area);
+
+ return 0;
+}
+
+static void vp_vdpa_kick_vq(struct vdpa_device *vdpa, u16 qid)
+{
+ struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
+
+ vp_iowrite16(qid, vp_vdpa->vring[qid].notify);
+}
+
+static u32 vp_vdpa_get_generation(struct vdpa_device *vdpa)
+{
+ struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
+
+ return vp_ioread8(&mdev->common->config_generation);
+}
+
+static u32 vp_vdpa_get_device_id(struct vdpa_device *vdpa)
+{
+ struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
+
+ return mdev->id.device;
+}
+
+static u32 vp_vdpa_get_vendor_id(struct vdpa_device *vdpa)
+{
+ struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
+
+ return mdev->id.vendor;
+}
+
+static u32 vp_vdpa_get_vq_align(struct vdpa_device *vdpa)
+{
+ return PAGE_SIZE;
+}
+
+static void vp_vdpa_get_config(struct vdpa_device *vdpa,
+ unsigned int offset,
+ void *buf, unsigned int len)
+{
+ struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
+ struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+ u8 old, new;
+ u8 *p;
+ int i;
+
+ do {
+ old = vp_ioread8(&mdev->common->config_generation);
+ p = buf;
+ for (i = 0; i < len; i++)
+ *p++ = vp_ioread8(mdev->device + offset + i);
+
+ new = vp_ioread8(&mdev->common->config_generation);
+ } while (old != new);
+}
+
+static void vp_vdpa_set_config(struct vdpa_device *vdpa,
+ unsigned int offset, const void *buf,
+ unsigned int len)
+{
+ struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
+ struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+ const u8 *p = buf;
+ int i;
+
+ for (i = 0; i < len; i++)
+ vp_iowrite8(*p++, mdev->device + offset + i);
+}
+
+static void vp_vdpa_set_config_cb(struct vdpa_device *vdpa,
+ struct vdpa_callback *cb)
+{
+ struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
+
+ vp_vdpa->cb = *cb;
+}
+
+static const struct vdpa_config_ops vp_vdpa_ops = {
+ .get_features = vp_vdpa_get_features,
+ .set_features = vp_vdpa_set_features,
+ .get_status = vp_vdpa_get_status,
+ .set_status = vp_vdpa_set_status,
+ .get_vq_num_max = vp_vdpa_get_vq_num_max,
+ .get_vq_state = vp_vdpa_get_vq_state,
+ .set_vq_state = vp_vdpa_set_vq_state,
+ .set_vq_cb = vp_vdpa_set_vq_cb,
+ .set_vq_ready = vp_vdpa_set_vq_ready,
+ .get_vq_ready = vp_vdpa_get_vq_ready,
+ .set_vq_num = vp_vdpa_set_vq_num,
+ .set_vq_address = vp_vdpa_set_vq_address,
+ .kick_vq = vp_vdpa_kick_vq,
+ .get_generation = vp_vdpa_get_generation,
+ .get_device_id = vp_vdpa_get_device_id,
+ .get_vendor_id = vp_vdpa_get_vendor_id,
+ .get_vq_align = vp_vdpa_get_vq_align,
+ .get_config = vp_vdpa_get_config,
+ .set_config = vp_vdpa_set_config,
+ .set_config_cb = vp_vdpa_set_config_cb,
+};
+
+static void vp_vdpa_free_irq_vectors(void *data)
+{
+ pci_free_irq_vectors(data);
+}
+
+static int vp_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+ struct virtio_pci_modern_device *mdev;
+ struct device *dev = &pdev->dev;
+ struct vp_vdpa *vp_vdpa;
+ u16 notify_off;
+ int ret, i;
+
+ vp_vdpa = vdpa_alloc_device(struct vp_vdpa, vdpa,
+ dev, &vp_vdpa_ops);
+ if (vp_vdpa == NULL) {
+ dev_err(dev, "vp_vdpa: Failed to allocate vDPA structure\n");
+ return -ENOMEM;
+ }
+
+ mdev = &vp_vdpa->mdev;
+ mdev->pci_dev = pdev;
+
+ if (vp_modern_probe(mdev)) {
+ dev_err(&pdev->dev, "Failed to probe modern PCI device\n");
+ goto err;
+ }
+
+ pci_set_master(pdev);
+ pci_set_drvdata(pdev, vp_vdpa);
+
+ vp_vdpa->vdpa.dma_dev = &pdev->dev;
+ vp_vdpa->queues = vp_modern_get_num_queues(mdev);
+
+ ret = devm_add_action_or_reset(dev, vp_vdpa_free_irq_vectors, pdev);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Failed for adding devres for freeing irq vectors\n");
+ goto err;
+ }
+
+ vp_vdpa->vring = devm_kcalloc(&pdev->dev, vp_vdpa->queues,
+ sizeof(*vp_vdpa->vring),
+ GFP_KERNEL);
+ if (!vp_vdpa->vring) {
+ dev_err(&pdev->dev, "Fail to allocate virtqueues\n");
+ goto err;
+ }
+
+ for (i = 0; i < vp_vdpa->queues; i++) {
+ notify_off = vp_modern_get_queue_notify_off(mdev, i);
+ vp_vdpa->vring[i].irq = VIRTIO_MSI_NO_VECTOR;
+ vp_vdpa->vring[i].notify = mdev->notify_base +
+ notify_off * mdev->notify_offset_multiplier;
+ }
+ vp_vdpa->config_irq = VIRTIO_MSI_NO_VECTOR;
+
+ ret = vdpa_register_device(&vp_vdpa->vdpa, vp_vdpa->queues);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register to vdpa bus\n");
+ goto err;
+ }
+
+ return 0;
+
+err:
+ put_device(&vp_vdpa->vdpa.dev);
+ return ret;
+}
+
+static void vp_vdpa_remove(struct pci_dev *pdev)
+{
+ struct vp_vdpa *vp_vdpa = pci_get_drvdata(pdev);
+
+ vdpa_unregister_device(&vp_vdpa->vdpa);
+}
+
+static struct pci_driver vp_vdpa_driver = {
+ .name = "vp-vdpa",
+ .id_table = NULL, /* only dynamic ids */
+ .probe = vp_vdpa_probe,
+ .remove = vp_vdpa_remove,
+};
+
+module_pci_driver(vp_vdpa_driver);
+
+MODULE_AUTHOR("Jason Wang <jasowang@redhat.com>");
+MODULE_DESCRIPTION("vp-vdpa");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1");
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH V2 01/14] virtio-pci: do not access iomem via virtio_pci_device directly
2020-11-26 9:25 ` [PATCH V2 01/14] virtio-pci: do not access iomem via virtio_pci_device directly Jason Wang
@ 2020-11-26 13:46 ` Michael S. Tsirkin
2020-11-27 2:50 ` Jason Wang
0 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2020-11-26 13:46 UTC (permalink / raw)
To: Jason Wang; +Cc: virtualization, linux-kernel, shahafs
On Thu, Nov 26, 2020 at 05:25:51PM +0800, Jason Wang wrote:
> Instead of accessing iomem via virito_pci_device directly. Add an
> indirect level
well this patch does not add any indirection it's just refactoring.
which is ok of course let's just say it as is.
> to ease the life of splitting out modern virito-pci
typo
> logic.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/virtio/virtio_pci_modern.c | 76 ++++++++++++++++++------------
> 1 file changed, 46 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 3d6ae5a5e252..df1481fd400c 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -141,12 +141,13 @@ static void __iomem *map_capability(struct pci_dev *dev, int off,
> static u64 vp_get_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
> u64 features;
>
> - vp_iowrite32(0, &vp_dev->common->device_feature_select);
> - features = vp_ioread32(&vp_dev->common->device_feature);
> - vp_iowrite32(1, &vp_dev->common->device_feature_select);
> - features |= ((u64)vp_ioread32(&vp_dev->common->device_feature) << 32);
> + vp_iowrite32(0, &cfg->device_feature_select);
> + features = vp_ioread32(&cfg->device_feature);
> + vp_iowrite32(1, &cfg->device_feature_select);
> + features |= ((u64)vp_ioread32(&cfg->device_feature) << 32);
>
> return features;
> }
> @@ -165,6 +166,7 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
> u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> @@ -179,10 +181,10 @@ static int vp_finalize_features(struct virtio_device *vdev)
> return -EINVAL;
> }
>
> - vp_iowrite32(0, &vp_dev->common->guest_feature_select);
> - vp_iowrite32((u32)vdev->features, &vp_dev->common->guest_feature);
> - vp_iowrite32(1, &vp_dev->common->guest_feature_select);
> - vp_iowrite32(vdev->features >> 32, &vp_dev->common->guest_feature);
> + vp_iowrite32(0, &cfg->guest_feature_select);
> + vp_iowrite32((u32)vdev->features, &cfg->guest_feature);
> + vp_iowrite32(1, &cfg->guest_feature_select);
> + vp_iowrite32(vdev->features >> 32, &cfg->guest_feature);
>
> return 0;
> }
> @@ -192,6 +194,7 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
> void *buf, unsigned len)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + void __iomem *device = vp_dev->device;
> u8 b;
> __le16 w;
> __le32 l;
> @@ -200,21 +203,21 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
>
> switch (len) {
> case 1:
> - b = ioread8(vp_dev->device + offset);
> + b = ioread8(device + offset);
> memcpy(buf, &b, sizeof b);
> break;
> case 2:
> - w = cpu_to_le16(ioread16(vp_dev->device + offset));
> + w = cpu_to_le16(ioread16(device + offset));
> memcpy(buf, &w, sizeof w);
> break;
> case 4:
> - l = cpu_to_le32(ioread32(vp_dev->device + offset));
> + l = cpu_to_le32(ioread32(device + offset));
> memcpy(buf, &l, sizeof l);
> break;
> case 8:
> - l = cpu_to_le32(ioread32(vp_dev->device + offset));
> + l = cpu_to_le32(ioread32(device + offset));
> memcpy(buf, &l, sizeof l);
> - l = cpu_to_le32(ioread32(vp_dev->device + offset + sizeof l));
> + l = cpu_to_le32(ioread32(device + offset + sizeof l));
> memcpy(buf + sizeof l, &l, sizeof l);
> break;
> default:
> @@ -228,6 +231,7 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
> const void *buf, unsigned len)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + void __iomem *device = vp_dev->device;
> u8 b;
> __le16 w;
> __le32 l;
> @@ -237,21 +241,21 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
> switch (len) {
> case 1:
> memcpy(&b, buf, sizeof b);
> - iowrite8(b, vp_dev->device + offset);
> + iowrite8(b, device + offset);
> break;
> case 2:
> memcpy(&w, buf, sizeof w);
> - iowrite16(le16_to_cpu(w), vp_dev->device + offset);
> + iowrite16(le16_to_cpu(w), device + offset);
> break;
> case 4:
> memcpy(&l, buf, sizeof l);
> - iowrite32(le32_to_cpu(l), vp_dev->device + offset);
> + iowrite32(le32_to_cpu(l), device + offset);
> break;
> case 8:
> memcpy(&l, buf, sizeof l);
> - iowrite32(le32_to_cpu(l), vp_dev->device + offset);
> + iowrite32(le32_to_cpu(l), device + offset);
> memcpy(&l, buf + sizeof l, sizeof l);
> - iowrite32(le32_to_cpu(l), vp_dev->device + offset + sizeof l);
> + iowrite32(le32_to_cpu(l), device + offset + sizeof l);
> break;
> default:
> BUG();
> @@ -261,35 +265,43 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
> static u32 vp_generation(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> - return vp_ioread8(&vp_dev->common->config_generation);
> + struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
> +
> + return vp_ioread8(&cfg->config_generation);
> }
>
> /* config->{get,set}_status() implementations */
> static u8 vp_get_status(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> - return vp_ioread8(&vp_dev->common->device_status);
> + struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
> +
> + return vp_ioread8(&cfg->device_status);
> }
>
> static void vp_set_status(struct virtio_device *vdev, u8 status)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
> +
> /* We should never be setting status to 0. */
> BUG_ON(status == 0);
> - vp_iowrite8(status, &vp_dev->common->device_status);
> + vp_iowrite8(status, &cfg->device_status);
> }
>
> static void vp_reset(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
> +
> /* 0 status means a reset. */
> - vp_iowrite8(0, &vp_dev->common->device_status);
> + vp_iowrite8(0, &cfg->device_status);
> /* After writing 0 to device_status, the driver MUST wait for a read of
> * device_status to return 0 before reinitializing the device.
> * This will flush out the status write, and flush in device writes,
> * including MSI-X interrupts, if any.
> */
> - while (vp_ioread8(&vp_dev->common->device_status))
> + while (vp_ioread8(&cfg->device_status))
> msleep(1);
> /* Flush pending VQ/configuration callbacks. */
> vp_synchronize_vectors(vdev);
> @@ -297,11 +309,13 @@ static void vp_reset(struct virtio_device *vdev)
>
> static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
> {
> + struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
> +
> /* Setup the vector used for configuration events */
> - vp_iowrite16(vector, &vp_dev->common->msix_config);
> + vp_iowrite16(vector, &cfg->msix_config);
> /* Verify we had enough resources to assign the vector */
> /* Will also flush the write out to device */
> - return vp_ioread16(&vp_dev->common->msix_config);
> + return vp_ioread16(&cfg->msix_config);
> }
>
> static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
> @@ -407,6 +421,7 @@ static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> struct irq_affinity *desc)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
> struct virtqueue *vq;
> int rc = vp_find_vqs(vdev, nvqs, vqs, callbacks, names, ctx, desc);
>
> @@ -417,8 +432,8 @@ static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> * this, there's no way to go back except reset.
> */
> list_for_each_entry(vq, &vdev->vqs, list) {
> - vp_iowrite16(vq->index, &vp_dev->common->queue_select);
> - vp_iowrite16(1, &vp_dev->common->queue_enable);
> + vp_iowrite16(vq->index, &cfg->queue_select);
> + vp_iowrite16(1, &cfg->queue_enable);
> }
>
> return 0;
> @@ -428,14 +443,15 @@ static void del_vq(struct virtio_pci_vq_info *info)
> {
> struct virtqueue *vq = info->vq;
> struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
> + struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
>
> - vp_iowrite16(vq->index, &vp_dev->common->queue_select);
> + vp_iowrite16(vq->index, &cfg->queue_select);
>
> if (vp_dev->msix_enabled) {
> vp_iowrite16(VIRTIO_MSI_NO_VECTOR,
> - &vp_dev->common->queue_msix_vector);
> + &cfg->queue_msix_vector);
> /* Flush the write out to device */
> - vp_ioread16(&vp_dev->common->queue_msix_vector);
> + vp_ioread16(&cfg->queue_msix_vector);
> }
>
> if (!vp_dev->notify_base)
> --
> 2.25.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V2 02/14] virtio-pci: switch to use devres for modern devices
2020-11-26 9:25 ` [PATCH V2 02/14] virtio-pci: switch to use devres for modern devices Jason Wang
@ 2020-11-26 13:57 ` Michael S. Tsirkin
2020-11-27 2:54 ` Jason Wang
0 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2020-11-26 13:57 UTC (permalink / raw)
To: Jason Wang; +Cc: virtualization, linux-kernel, shahafs
On Thu, Nov 26, 2020 at 05:25:52PM +0800, Jason Wang wrote:
> This patch tries to convert the modern device to use devres to manage
> its resources (iomaps). Before this patch the IO address is mapped
> individually according to the capability. After this patch, we simply
> map the whole BAR.
I think the point of mapping capability was e.g. for devices with
huge BARs. We don't want to waste virtual memory for e.g. 32 bit guests.
And in particular the spec says:
The drivers SHOULD only map part of configuration structure large enough for device operation. The drivers
MUST handle an unexpectedly large length, but MAY check that length is large enough for device operation.
I also wonder how would this interact with cases where device memory is
mapped for different reasons, such as for MSI table access, into userspace
as it has resources such as virtio mem, etc.
E.g. don't e.g. intel CPUs disallow mapping the same address twice
with different attributes?
> This simplify the work of splitting modern device logic into an
> separate module.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/virtio/virtio_pci_common.c | 10 --
> drivers/virtio/virtio_pci_common.h | 2 +
> drivers/virtio/virtio_pci_legacy.c | 13 ++-
> drivers/virtio/virtio_pci_modern.c | 141 +++++++++--------------------
> 4 files changed, 54 insertions(+), 112 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 222d630c41fc..e786701fa1b4 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -527,11 +527,6 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
> INIT_LIST_HEAD(&vp_dev->virtqueues);
> spin_lock_init(&vp_dev->lock);
>
> - /* enable the device */
> - rc = pci_enable_device(pci_dev);
> - if (rc)
> - goto err_enable_device;
> -
> if (force_legacy) {
> rc = virtio_pci_legacy_probe(vp_dev);
> /* Also try modern mode if we can't map BAR0 (no IO space). */
> @@ -559,11 +554,8 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
> err_register:
> if (vp_dev->ioaddr)
> virtio_pci_legacy_remove(vp_dev);
> - else
> - virtio_pci_modern_remove(vp_dev);
> err_probe:
> pci_disable_device(pci_dev);
> -err_enable_device:
> if (reg_dev)
> put_device(&vp_dev->vdev.dev);
> else
> @@ -582,8 +574,6 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
>
> if (vp_dev->ioaddr)
> virtio_pci_legacy_remove(vp_dev);
> - else
> - virtio_pci_modern_remove(vp_dev);
>
> pci_disable_device(pci_dev);
> put_device(dev);
> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> index b2f0eb4067cb..1d23420f7ed6 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -49,6 +49,8 @@ struct virtio_pci_device {
> u8 __iomem *isr;
>
> /* Modern only fields */
> + /* The IO mapping for the BARs */
> + void __iomem * const *base;
> /* The IO mapping for the PCI config space (non-legacy mode) */
> struct virtio_pci_common_cfg __iomem *common;
> /* Device-specific data (non-legacy mode) */
> diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
> index d62e9835aeec..890f155ff48c 100644
> --- a/drivers/virtio/virtio_pci_legacy.c
> +++ b/drivers/virtio/virtio_pci_legacy.c
> @@ -214,14 +214,19 @@ int virtio_pci_legacy_probe(struct virtio_pci_device *vp_dev)
> struct pci_dev *pci_dev = vp_dev->pci_dev;
> int rc;
>
> + rc = pci_enable_device(pci_dev);
> + if (rc)
> + return rc;
> +
> + rc = -ENODEV;
> /* We only own devices >= 0x1000 and <= 0x103f: leave the rest. */
> if (pci_dev->device < 0x1000 || pci_dev->device > 0x103f)
> - return -ENODEV;
> + goto err_id;
>
> if (pci_dev->revision != VIRTIO_PCI_ABI_VERSION) {
> printk(KERN_ERR "virtio_pci: expected ABI version %d, got %d\n",
> VIRTIO_PCI_ABI_VERSION, pci_dev->revision);
> - return -ENODEV;
> + goto err_id;
> }
>
> rc = dma_set_mask(&pci_dev->dev, DMA_BIT_MASK(64));
> @@ -241,7 +246,7 @@ int virtio_pci_legacy_probe(struct virtio_pci_device *vp_dev)
>
> rc = pci_request_region(pci_dev, 0, "virtio-pci-legacy");
> if (rc)
> - return rc;
> + goto err_id;
>
> rc = -ENOMEM;
> vp_dev->ioaddr = pci_iomap(pci_dev, 0, 0);
> @@ -267,6 +272,8 @@ int virtio_pci_legacy_probe(struct virtio_pci_device *vp_dev)
>
> err_iomap:
> pci_release_region(pci_dev, 0);
> +err_id:
> + pci_disable_device(pci_dev);
> return rc;
> }
>
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index df1481fd400c..33cc21b818de 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -63,15 +63,15 @@ static void vp_iowrite64_twopart(u64 val,
> vp_iowrite32(val >> 32, hi);
> }
>
> -static void __iomem *map_capability(struct pci_dev *dev, int off,
> +static void __iomem *map_capability(struct virtio_pci_device *vp_dev, int off,
> size_t minlen,
> u32 align,
> - u32 start, u32 size,
> + u32 size,
> size_t *len)
> {
> + struct pci_dev *dev = vp_dev->pci_dev;
> u8 bar;
> u32 offset, length;
> - void __iomem *p;
>
> pci_read_config_byte(dev, off + offsetof(struct virtio_pci_cap,
> bar),
> @@ -81,31 +81,13 @@ static void __iomem *map_capability(struct pci_dev *dev, int off,
> pci_read_config_dword(dev, off + offsetof(struct virtio_pci_cap, length),
> &length);
>
> - if (length <= start) {
> - dev_err(&dev->dev,
> - "virtio_pci: bad capability len %u (>%u expected)\n",
> - length, start);
> - return NULL;
> - }
> -
> - if (length - start < minlen) {
> + if (length < minlen) {
> dev_err(&dev->dev,
> "virtio_pci: bad capability len %u (>=%zu expected)\n",
> length, minlen);
> return NULL;
> }
>
> - length -= start;
> -
> - if (start + offset < offset) {
why kill wrap around?
> - 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",
> @@ -129,12 +111,7 @@ static void __iomem *map_capability(struct pci_dev *dev, int off,
> 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);
> - return p;
> + return vp_dev->base[bar] + offset;
> }
>
> /* virtio config->get_features() implementation */
> @@ -369,27 +346,21 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
> vp_iowrite64_twopart(virtqueue_get_used_addr(vq),
> &cfg->queue_used_lo, &cfg->queue_used_hi);
>
> - if (vp_dev->notify_base) {
> - /* offset should not wrap */
> - if ((u64)off * vp_dev->notify_offset_multiplier + 2
> - > vp_dev->notify_len) {
> - dev_warn(&vp_dev->pci_dev->dev,
> - "bad notification offset %u (x %u) "
> - "for queue %u > %zd",
> - off, vp_dev->notify_offset_multiplier,
> - index, vp_dev->notify_len);
> - err = -EINVAL;
> - goto err_map_notify;
> - }
> - vq->priv = (void __force *)vp_dev->notify_base +
> - off * vp_dev->notify_offset_multiplier;
> - } else {
> - vq->priv = (void __force *)map_capability(vp_dev->pci_dev,
> - vp_dev->notify_map_cap, 2, 2,
> - off * vp_dev->notify_offset_multiplier, 2,
> - NULL);
> + /* offset should not wrap */
> + if ((u64)off * vp_dev->notify_offset_multiplier + 2
> + > vp_dev->notify_len) {
> + dev_warn(&vp_dev->pci_dev->dev,
> + "bad notification offset %u (x %u) "
> + "for queue %u > %zd",
> + off, vp_dev->notify_offset_multiplier,
> + index, vp_dev->notify_len);
> + err = -EINVAL;
> + goto err_map_notify;
> }
>
> + vq->priv = (void __force *)vp_dev->notify_base +
> + off * vp_dev->notify_offset_multiplier;
> +
> if (!vq->priv) {
> err = -ENOMEM;
> goto err_map_notify;
> @@ -400,15 +371,12 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
> msix_vec = vp_ioread16(&cfg->queue_msix_vector);
> if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
> err = -EBUSY;
> - goto err_assign_vector;
> + goto err_map_notify;
> }
> }
>
> return vq;
>
> -err_assign_vector:
> - if (!vp_dev->notify_base)
> - pci_iounmap(vp_dev->pci_dev, (void __iomem __force *)vq->priv);
> err_map_notify:
> vring_del_virtqueue(vq);
> return ERR_PTR(err);
> @@ -454,9 +422,6 @@ static void del_vq(struct virtio_pci_vq_info *info)
> vp_ioread16(&cfg->queue_msix_vector);
> }
>
> - if (!vp_dev->notify_base)
> - pci_iounmap(vp_dev->pci_dev, (void __force __iomem *)vq->priv);
> -
> vring_del_virtqueue(vq);
> }
>
> @@ -700,6 +665,10 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
>
> check_offsets();
>
> + err = pcim_enable_device(pci_dev);
> + if (err)
> + return err;
> +
> /* We only own devices >= 0x1000 and <= 0x107f: leave the rest. */
> if (pci_dev->device < 0x1000 || pci_dev->device > 0x107f)
> return -ENODEV;
> @@ -753,23 +722,24 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
> IORESOURCE_IO | IORESOURCE_MEM,
> &vp_dev->modern_bars);
>
> - err = pci_request_selected_regions(pci_dev, vp_dev->modern_bars,
> - "virtio-pci-modern");
> + err = pcim_iomap_regions(pci_dev, vp_dev->modern_bars,
> + "virtio-pci-modern");
> if (err)
> return err;
>
> + vp_dev->base = pcim_iomap_table(pci_dev);
> +
> err = -EINVAL;
> - vp_dev->common = map_capability(pci_dev, common,
> + vp_dev->common = map_capability(vp_dev, common,
> sizeof(struct virtio_pci_common_cfg), 4,
> - 0, sizeof(struct virtio_pci_common_cfg),
> + sizeof(struct virtio_pci_common_cfg),
> NULL);
> if (!vp_dev->common)
> - goto err_map_common;
> - vp_dev->isr = map_capability(pci_dev, isr, sizeof(u8), 1,
> - 0, 1,
> - NULL);
> + goto err;
> + vp_dev->isr = map_capability(vp_dev, isr, sizeof(u8), 1,
> + 1, NULL);
> if (!vp_dev->isr)
> - goto err_map_isr;
> + goto err;
>
> /* Read notify_off_multiplier from config space. */
> pci_read_config_dword(pci_dev,
> @@ -787,29 +757,21 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
> 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) {
> - vp_dev->notify_base = map_capability(pci_dev, notify, 2, 2,
> - 0, notify_length,
> - &vp_dev->notify_len);
> - if (!vp_dev->notify_base)
> - goto err_map_notify;
> - } else {
> - vp_dev->notify_map_cap = notify;
> - }
> + vp_dev->notify_base = map_capability(vp_dev, notify, 2, 2,
> + notify_length,
> + &vp_dev->notify_len);
> + if (!vp_dev->notify_base)
> + goto err;
>
> /* Again, we don't know how much we should map, but PAGE_SIZE
> * is more than enough for all existing devices.
> */
> if (device) {
> - vp_dev->device = map_capability(pci_dev, device, 0, 4,
> - 0, PAGE_SIZE,
> + vp_dev->device = map_capability(vp_dev, device, 0, 4,
> + PAGE_SIZE,
> &vp_dev->device_len);
> if (!vp_dev->device)
> - goto err_map_device;
> + goto err;
>
> vp_dev->vdev.config = &virtio_pci_config_ops;
> } else {
> @@ -822,26 +784,7 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
>
> return 0;
>
> -err_map_device:
> - if (vp_dev->notify_base)
> - pci_iounmap(pci_dev, vp_dev->notify_base);
> -err_map_notify:
> - pci_iounmap(pci_dev, vp_dev->isr);
> -err_map_isr:
> - pci_iounmap(pci_dev, vp_dev->common);
> -err_map_common:
> +err:
> return err;
> }
>
> -void virtio_pci_modern_remove(struct virtio_pci_device *vp_dev)
> -{
> - struct pci_dev *pci_dev = vp_dev->pci_dev;
> -
> - if (vp_dev->device)
> - pci_iounmap(pci_dev, vp_dev->device);
> - if (vp_dev->notify_base)
> - pci_iounmap(pci_dev, vp_dev->notify_base);
> - pci_iounmap(pci_dev, vp_dev->isr);
> - pci_iounmap(pci_dev, vp_dev->common);
> - pci_release_selected_regions(pci_dev, vp_dev->modern_bars);
> -}
> --
> 2.25.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V2 01/14] virtio-pci: do not access iomem via virtio_pci_device directly
2020-11-26 13:46 ` Michael S. Tsirkin
@ 2020-11-27 2:50 ` Jason Wang
0 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-27 2:50 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization, linux-kernel, shahafs
On 2020/11/26 下午9:46, Michael S. Tsirkin wrote:
> On Thu, Nov 26, 2020 at 05:25:51PM +0800, Jason Wang wrote:
>> Instead of accessing iomem via virito_pci_device directly. Add an
>> indirect level
> well this patch does not add any indirection it's just refactoring.
> which is ok of course let's just say it as is.
>
>> to ease the life of splitting out modern virito-pci
> typo
Will fix.
Thanks
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V2 02/14] virtio-pci: switch to use devres for modern devices
2020-11-26 13:57 ` Michael S. Tsirkin
@ 2020-11-27 2:54 ` Jason Wang
0 siblings, 0 replies; 19+ messages in thread
From: Jason Wang @ 2020-11-27 2:54 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization, linux-kernel, shahafs
On 2020/11/26 下午9:57, Michael S. Tsirkin wrote:
> On Thu, Nov 26, 2020 at 05:25:52PM +0800, Jason Wang wrote:
>> This patch tries to convert the modern device to use devres to manage
>> its resources (iomaps). Before this patch the IO address is mapped
>> individually according to the capability. After this patch, we simply
>> map the whole BAR.
> I think the point of mapping capability was e.g. for devices with
> huge BARs. We don't want to waste virtual memory for e.g. 32 bit guests.
>
> And in particular the spec says:
>
> The drivers SHOULD only map part of configuration structure large enough for device operation. The drivers
> MUST handle an unexpectedly large length, but MAY check that length is large enough for device operation.
Good point, so I will stick to devres but not use the shortcut like
whole BAR mapping.
>
> I also wonder how would this interact with cases where device memory is
> mapped for different reasons, such as for MSI table access, into userspace
> as it has resources such as virtio mem, etc.
I think it depends on the driver, e.g for virtio-pci and vDPA, the upper
layer driver (virtio bus or vDPA bus) know nothing about transport
specific thing. It should be ok.
> E.g. don't e.g. intel CPUs disallow mapping the same address twice
> with different attributes?
Do you mean it doesn't allow one VA is mapped as UC but the other is
not? I don't know. But anyhow my understanding is that
virtio-pci/vp_vdpa tries to hide the details so we can not have two
mappings here.
Thanks
>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2020-11-27 2:54 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-26 9:25 [PATCH V2 00/14] vDPA driver for virtio-pci device Jason Wang
2020-11-26 9:25 ` [PATCH V2 01/14] virtio-pci: do not access iomem via virtio_pci_device directly Jason Wang
2020-11-26 13:46 ` Michael S. Tsirkin
2020-11-27 2:50 ` Jason Wang
2020-11-26 9:25 ` [PATCH V2 02/14] virtio-pci: switch to use devres for modern devices Jason Wang
2020-11-26 13:57 ` Michael S. Tsirkin
2020-11-27 2:54 ` Jason Wang
2020-11-26 9:25 ` [PATCH V2 03/14] virtio-pci: split out modern device Jason Wang
2020-11-26 9:25 ` [PATCH V2 04/14] virtio-pci: move the notification sanity check to vp_modern_probe() Jason Wang
2020-11-26 9:25 ` [PATCH V2 05/14] virtio-pci-modern: introduce vp_modern_set_queue_vector() Jason Wang
2020-11-26 9:25 ` [PATCH V2 06/14] virtio-pci-modern: introduce vp_modern_queue_address() Jason Wang
2020-11-26 9:25 ` [PATCH V2 07/14] virtio-pci-modern: introduce helper to set/get queue_enable Jason Wang
2020-11-26 9:25 ` [PATCH V2 08/14] virtio-pci-modern: introduce helper for setting/geting queue size Jason Wang
2020-11-26 9:25 ` [PATCH V2 09/14] virtio-pci-modern: introduce helper for getting queue nums Jason Wang
2020-11-26 9:26 ` [PATCH V2 10/14] virtio-pci-modern: introduce helper to get notification offset Jason Wang
2020-11-26 9:26 ` [PATCH V2 11/14] virtio-pci: introduce modern device module Jason Wang
2020-11-26 9:26 ` [PATCH V2 12/14] vdpa: set the virtqueue num during register Jason Wang
2020-11-26 9:26 ` [PATCH V2 13/14] virtio_vdpa: don't warn when fail to disable vq Jason Wang
2020-11-26 9:26 ` [PATCH V2 14/14] vdpa: introduce virtio pci driver Jason Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).