Re: [PATCH net-next v3 00/11] VSOCK: add vsock_test test suite
From: Stefano Garzarella
Date: Wed, 18 Dec 2019 19:06:57 +0100
> The vsock_diag.ko module already has a test suite but the core AF_VSOCK
> functionality has no tests. This patch series adds several test cases that
> exercise AF_VSOCK SOCK_STREAM socket semantics (send/recv, connect/accept,
> half-closed connections, simultaneous connections).
>
> The v1 of this series was originally sent by Stefan.
>
> v3:
> - Patch 6:
> * check the byte received in the recv_byte()
> * use send(2)/recv(2) instead of write(2)/read(2) to test also flags
> (e.g. MSG_PEEK)
> - Patch 8:
> * removed unnecessary control_expectln("CLOSED") [Stefan].
> - removed patches 9,10,11 added in the v2
> - new Patch 9 add parameters to list and skip tests (e.g. useful for vmci
> that doesn't support half-closed socket in the host)
> - new Patch 10 prints a list of options in the help
> - new Patch 11 tests MSG_PEEK flags of recv(2)
>
> v2: https://patchwork.ozlabs.org/cover/1140538/
> v1: https://patchwork.ozlabs.org/cover/847998/
Series applied, thanks.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net] virtio-net: Skip set_features on non-cvq devices
On Fri, Dec 20, 2019 at 4:22 PM Alistair Delva wrote:
>
> On devices without control virtqueue support, such as the virtio_net
> implementation in crosvm[1], attempting to configure LRO will panic the
> kernel:
>
> kernel BUG at drivers/net/virtio_net.c:1591!
> invalid opcode: [#1] PREEMPT SMP PTI
> CPU: 1 PID: 483 Comm: Binder:330_1 Not tainted 5.4.5-01326-g19463e9acaac #1
> Hardware name: ChromiumOS crosvm, BIOS 0
> RIP: 0010:virtnet_send_command+0x15d/0x170 [virtio_net]
> Code: d8 00 00 00 80 78 02 00 0f 94 c0 65 48 8b 0c 25 28 00 00 00 48 3b 4c 24
> 70 75 11 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 <0f> 0b e8 ec a4 12 c8
> 66 90 66 2e 0f 1f 84 00 00 00 00 00 55 48 89
> RSP: 0018:b97940e7bb50 EFLAGS: 00010246
> RAX: c0596020 RBX: a0e1fc8ea840 RCX: 0017
> RDX: c0596110 RSI: 0011 RDI: 000d
> RBP: b97940e7bbf8 R08: a0e1fc8ea0b0 R09: a0e1fc8ea0b0
> R10: R11: c0590940 R12: 0005
> R13: a0e1ffad2c00 R14: b97940e7bc08 R15:
> FS: () GS:a0e1fd10(006b) knlGS:e5ef7494
> CS: 0010 DS: 002b ES: 002b CR0: 80050033
> CR2: e5eeb82c CR3: 79b06001 CR4: 00360ee0
> DR0: DR1: DR2:
> DR3: DR6: fffe0ff0 DR7: 0400
> Call Trace:
> ? preempt_count_add+0x58/0xb0
> ? _raw_spin_lock_irqsave+0x36/0x70
> ? _raw_spin_unlock_irqrestore+0x1a/0x40
> ? __wake_up+0x70/0x190
> virtnet_set_features+0x90/0xf0 [virtio_net]
> __netdev_update_features+0x271/0x980
> ? nlmsg_notify+0x5b/0xa0
> dev_disable_lro+0x2b/0x190
> ? inet_netconf_notify_devconf+0xe2/0x120
> devinet_sysctl_forward+0x176/0x1e0
> proc_sys_call_handler+0x1f0/0x250
> proc_sys_write+0xf/0x20
> __vfs_write+0x3e/0x190
> ? __sb_start_write+0x6d/0xd0
> vfs_write+0xd3/0x190
> ksys_write+0x68/0xd0
> __ia32_sys_write+0x14/0x20
> do_fast_syscall_32+0x86/0xe0
> entry_SYSENTER_compat+0x7c/0x8e
>
> This happens because virtio_set_features() does not check the presence
> of the control virtqueue feature, which is sanity checked by a BUG_ON
> in virtnet_send_command().
>
> Fix this by skipping any feature processing if the control virtqueue is
> missing. This should be OK for any future feature that is added, as
> presumably all of them would require control virtqueue support to notify
> the endpoint that offload etc. should begin.
>
> [1] https://chromium.googlesource.com/chromiumos/platform/crosvm/
>
> Fixes: a02e8964eaf9 ("virtio-net: ethtool configurable LRO")
> Cc: sta...@vger.kernel.org [4.20+]
> Cc: Michael S. Tsirkin
> Cc: Jason Wang
> Cc: David S. Miller
> Cc: kernel-t...@android.com
> Cc: virtualization@lists.linux-foundation.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Alistair Delva
Thanks for debugging this, Alistair.
> ---
> drivers/net/virtio_net.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 4d7d5434cc5d..709bcd34e485 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2560,6 +2560,9 @@ static int virtnet_set_features(struct net_device *dev,
> u64 offloads;
> int err;
>
> + if (!vi->has_cvq)
> + return 0;
> +
Instead of checking for this in virtnet_set_features, how about we
make configurability contingent on cvq in virtnet_probe:
- if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS))
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) &&
+ virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
dev->hw_features |= NETIF_F_LRO;
Based on this logic a little below in the same function
if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
vi->has_cvq = true;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Spice-devel] [PATCH 4/4] drm/qxl: add drm_driver.release callback.
On Fri, Dec 20, 2019 at 07:09:20AM -0500, Frediano Ziglio wrote:
> >
> > Move final cleanups to qxl_drm_release() callback.
>
> Can you explain in the commit why this is better or preferable?
It gets called when the drm device refcount goes down to zero.
It's needed for a proper cleanup in the correct order.
> > Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
>
> I suppose this is to replace the former manual cleanup calls,
> which were moved to qxl_drm_release, I think this could be
> added in the commit message ("why"), I don't see much value
> in describing "how" this was done.
The call is part of the shutdown sequence for atomic drm drivers
and wasn't present in qxl for some reason.
> > Reorder calls in qxl_device_fini(). Cleaning up gem & ttm
> > might trigger qxl commands, so we should do that before
> > releaseing command rings.
>
> Typo: releaseing -> releasing
> Why not putting this in a separate commit? Was this behaviour
> changed? It does not seem so to me.
Yes, I can make that a separate commit.
No, behavior didn't change. qxl_device_fini() is simply broken
without this.
cheers,
Gerd
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Spice-devel] [PATCH 4/4] drm/qxl: add drm_driver.release callback.
>
> Move final cleanups to qxl_drm_release() callback.
Can you explain in the commit why this is better or preferable?
> Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
I suppose this is to replace the former manual cleanup calls,
which were moved to qxl_drm_release, I think this could be
added in the commit message ("why"), I don't see much value
in describing "how" this was done.
>
> Reorder calls in qxl_device_fini(). Cleaning up gem & ttm
> might trigger qxl commands, so we should do that before
> releaseing command rings.
Typo: releaseing -> releasing
Why not putting this in a separate commit? Was this behaviour
changed? It does not seem so to me.
>
> Signed-off-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/qxl/qxl_drv.c | 21 ++---
> drivers/gpu/drm/qxl/qxl_kms.c | 8
> 2 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 1d601f57a6ba..8044363ba0f2 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -34,6 +34,7 @@
> #include
>
> #include
> +#include
> #include
> #include
> #include
> @@ -132,21 +133,25 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> return ret;
> }
>
> +static void qxl_drm_release(struct drm_device *dev)
> +{
> + struct qxl_device *qdev = dev->dev_private;
> +
> + qxl_modeset_fini(qdev);
> + qxl_device_fini(qdev);
> + dev->dev_private = NULL;
> + kfree(qdev);
> +}
> +
> static void
> qxl_pci_remove(struct pci_dev *pdev)
> {
> struct drm_device *dev = pci_get_drvdata(pdev);
> - struct qxl_device *qdev = dev->dev_private;
>
> drm_dev_unregister(dev);
> -
> - qxl_modeset_fini(qdev);
> - qxl_device_fini(qdev);
> + drm_atomic_helper_shutdown(dev);
> if (is_vga(pdev))
> vga_put(pdev, VGA_RSRC_LEGACY_IO);
> -
> - dev->dev_private = NULL;
> - kfree(qdev);
> drm_dev_put(dev);
> }
>
> @@ -279,6 +284,8 @@ static struct drm_driver qxl_driver = {
> .major = 0,
> .minor = 1,
> .patchlevel = 0,
> +
> + .release = qxl_drm_release,
> };
>
> static int __init qxl_init(void)
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index bfc1631093e9..70b20ee4741a 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -299,12 +299,12 @@ void qxl_device_fini(struct qxl_device *qdev)
> {
> qxl_bo_unref(>current_release_bo[0]);
> qxl_bo_unref(>current_release_bo[1]);
> - flush_work(>gc_work);
> - qxl_ring_free(qdev->command_ring);
> - qxl_ring_free(qdev->cursor_ring);
> - qxl_ring_free(qdev->release_ring);
> qxl_gem_fini(qdev);
> qxl_bo_fini(qdev);
> + flush_work(>gc_work);
> + qxl_ring_free(qdev->command_ring);
> + qxl_ring_free(qdev->cursor_ring);
> + qxl_ring_free(qdev->release_ring);
> io_mapping_free(qdev->surface_mapping);
> io_mapping_free(qdev->vram_mapping);
> iounmap(qdev->ram_header);
Frediano
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 4/4] drm/qxl: add drm_driver.release callback.
Move final cleanups to qxl_drm_release() callback.
Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
Reorder calls in qxl_device_fini(). Cleaning up gem & ttm
might trigger qxl commands, so we should do that before
releaseing command rings.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_drv.c | 21 ++---
drivers/gpu/drm/qxl/qxl_kms.c | 8
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 1d601f57a6ba..8044363ba0f2 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -34,6 +34,7 @@
#include
#include
+#include
#include
#include
#include
@@ -132,21 +133,25 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
return ret;
}
+static void qxl_drm_release(struct drm_device *dev)
+{
+ struct qxl_device *qdev = dev->dev_private;
+
+ qxl_modeset_fini(qdev);
+ qxl_device_fini(qdev);
+ dev->dev_private = NULL;
+ kfree(qdev);
+}
+
static void
qxl_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
- struct qxl_device *qdev = dev->dev_private;
drm_dev_unregister(dev);
-
- qxl_modeset_fini(qdev);
- qxl_device_fini(qdev);
+ drm_atomic_helper_shutdown(dev);
if (is_vga(pdev))
vga_put(pdev, VGA_RSRC_LEGACY_IO);
-
- dev->dev_private = NULL;
- kfree(qdev);
drm_dev_put(dev);
}
@@ -279,6 +284,8 @@ static struct drm_driver qxl_driver = {
.major = 0,
.minor = 1,
.patchlevel = 0,
+
+ .release = qxl_drm_release,
};
static int __init qxl_init(void)
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index bfc1631093e9..70b20ee4741a 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -299,12 +299,12 @@ void qxl_device_fini(struct qxl_device *qdev)
{
qxl_bo_unref(>current_release_bo[0]);
qxl_bo_unref(>current_release_bo[1]);
- flush_work(>gc_work);
- qxl_ring_free(qdev->command_ring);
- qxl_ring_free(qdev->cursor_ring);
- qxl_ring_free(qdev->release_ring);
qxl_gem_fini(qdev);
qxl_bo_fini(qdev);
+ flush_work(>gc_work);
+ qxl_ring_free(qdev->command_ring);
+ qxl_ring_free(qdev->cursor_ring);
+ qxl_ring_free(qdev->release_ring);
io_mapping_free(qdev->surface_mapping);
io_mapping_free(qdev->vram_mapping);
iounmap(qdev->ram_header);
--
2.18.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 2/4] drm/cirrus: add drm_driver.release callback.
Move final cleanups from cirrus_pci_remove() to the new callback.
Add drm_atomic_helper_shutdown() call to cirrus_pci_remove().
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/cirrus/cirrus.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index 248c9f765c45..a4ca66f9a423 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -510,6 +510,16 @@ static void cirrus_mode_config_init(struct cirrus_device
*cirrus)
/* -- */
+static void cirrus_release(struct drm_device *dev)
+{
+ struct cirrus_device *cirrus = dev->dev_private;
+
+ drm_mode_config_cleanup(dev);
+ iounmap(cirrus->mmio);
+ iounmap(cirrus->vram);
+ kfree(cirrus);
+}
+
DEFINE_DRM_GEM_FOPS(cirrus_fops);
static struct drm_driver cirrus_driver = {
@@ -523,6 +533,7 @@ static struct drm_driver cirrus_driver = {
.fops= _fops,
DRM_GEM_SHMEM_DRIVER_OPS,
+ .release = cirrus_release,
};
static int cirrus_pci_probe(struct pci_dev *pdev,
@@ -604,14 +615,10 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
static void cirrus_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
- struct cirrus_device *cirrus = dev->dev_private;
drm_dev_unregister(dev);
- drm_mode_config_cleanup(dev);
- iounmap(cirrus->mmio);
- iounmap(cirrus->vram);
+ drm_atomic_helper_shutdown(dev);
drm_dev_put(dev);
- kfree(cirrus);
pci_release_regions(pdev);
}
--
2.18.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 1/4] drm/bochs: add drm_driver.release callback.
From: Gurchetan Singh
Move bochs_unload call from bochs_remove() to the new bochs_release()
callback. Also call drm_dev_unregister() first in bochs_remove().
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/bochs/bochs_drv.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bochs/bochs_drv.c
b/drivers/gpu/drm/bochs/bochs_drv.c
index 10460878414e..87ee1eb21a4d 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -60,6 +60,11 @@ static int bochs_load(struct drm_device *dev)
DEFINE_DRM_GEM_FOPS(bochs_fops);
+static void bochs_release(struct drm_device *dev)
+{
+ bochs_unload(dev);
+}
+
static struct drm_driver bochs_driver = {
.driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
.fops = _fops,
@@ -69,6 +74,7 @@ static struct drm_driver bochs_driver = {
.major = 1,
.minor = 0,
DRM_GEM_VRAM_DRIVER,
+ .release= bochs_release,
};
/* -- */
@@ -148,9 +154,8 @@ static void bochs_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
- drm_atomic_helper_shutdown(dev);
drm_dev_unregister(dev);
- bochs_unload(dev);
+ drm_atomic_helper_shutdown(dev);
drm_dev_put(dev);
}
--
2.18.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 3/4] drm/virtio: add drm_driver.release callback.
Split virtio_gpu_deinit(), move the drm shutdown and release to
virtio_gpu_release(). Also free vbufs in case we can't queue them.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 1 +
drivers/gpu/drm/virtio/virtgpu_drv.c | 4
drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +
drivers/gpu/drm/virtio/virtgpu_vq.c | 8 ++--
4 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 7e69c06e168e..227f7c07e61c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -216,6 +216,7 @@ extern struct drm_ioctl_desc
virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
/* virtio_kms.c */
int virtio_gpu_init(struct drm_device *dev);
void virtio_gpu_deinit(struct drm_device *dev);
+void virtio_gpu_release(struct drm_device *dev);
int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file);
void virtio_gpu_driver_postclose(struct drm_device *dev, struct drm_file
*file);
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c
b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 8cf27af3ad53..664a741a3b0b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -31,6 +31,7 @@
#include
#include
+#include
#include
#include
@@ -136,6 +137,7 @@ static void virtio_gpu_remove(struct virtio_device *vdev)
struct drm_device *dev = vdev->priv;
drm_dev_unregister(dev);
+ drm_atomic_helper_shutdown(dev);
virtio_gpu_deinit(dev);
drm_dev_put(dev);
}
@@ -214,4 +216,6 @@ static struct drm_driver driver = {
.major = DRIVER_MAJOR,
.minor = DRIVER_MINOR,
.patchlevel = DRIVER_PATCHLEVEL,
+
+ .release = virtio_gpu_release,
};
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 2f5773e43557..387f05a9cc18 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -237,6 +237,11 @@ void virtio_gpu_deinit(struct drm_device *dev)
flush_work(>config_changed_work);
vgdev->vdev->config->reset(vgdev->vdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
+}
+
+void virtio_gpu_release(struct drm_device *dev)
+{
+ struct virtio_gpu_device *vgdev = dev->dev_private;
virtio_gpu_modeset_fini(vgdev);
virtio_gpu_free_vbufs(vgdev);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5914e79d3429..619c6108d07e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -319,8 +319,10 @@ static bool virtio_gpu_queue_ctrl_buffer_locked(struct
virtio_gpu_device *vgdev,
bool notify = false;
int ret;
- if (!vgdev->vqs_ready)
+ if (!vgdev->vqs_ready) {
+ free_vbuf(vgdev, vbuf);
return notify;
+ }
sg_init_one(, vbuf->buf, vbuf->size);
sgs[outcnt + incnt] =
@@ -447,8 +449,10 @@ static void virtio_gpu_queue_cursor(struct
virtio_gpu_device *vgdev,
int ret;
int outcnt;
- if (!vgdev->vqs_ready)
+ if (!vgdev->vqs_ready) {
+ free_vbuf(vgdev, vbuf);
return;
+ }
sg_init_one(, vbuf->buf, vbuf->size);
sgs[0] =
--
2.18.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
