From e117d9b6e79a2466faf382cb7e8e8823e82d6bd5 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Fri, 8 Mar 2024 09:51:20 +0100 Subject: virtio-mmio: Convert to platform remove callback returning void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König Message-Id: Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_mmio.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 59892a31cf76..173596589c71 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -696,12 +696,10 @@ free_vm_dev: return rc; } -static int virtio_mmio_remove(struct platform_device *pdev) +static void virtio_mmio_remove(struct platform_device *pdev) { struct virtio_mmio_device *vm_dev = platform_get_drvdata(pdev); unregister_virtio_device(&vm_dev->vdev); - - return 0; } @@ -847,7 +845,7 @@ MODULE_DEVICE_TABLE(acpi, virtio_mmio_acpi_match); static struct platform_driver virtio_mmio_driver = { .probe = virtio_mmio_probe, - .remove = virtio_mmio_remove, + .remove_new = virtio_mmio_remove, .driver = { .name = "virtio-mmio", .of_match_table = virtio_mmio_match, -- cgit v1.2.3 From e4544c550eb1857845bb7114bac53edd04dc078f Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Mon, 18 Mar 2024 13:06:45 +0100 Subject: virtio-mem: support suspend+resume With virtio-mem, primarily hibernation is problematic: as the machine shuts down, the virtio-mem device loses its state. Powering the machine back up is like losing a bunch of DIMMs. While there would be ways to add limited support, suspend+resume is more commonly used for VMs and "easier" to support cleanly. s2idle can be supported without any device dependencies. Similarly, one would expect suspend-to-ram (i.e., S3) to work out of the box. However, QEMU currently unplugs all device memory when resuming the VM, using a cold reset on the "wakeup" path. In order to support S3, we need a feature flag for the device to tell us if memory remains plugged when waking up. In the future, QEMU will implement this feature. So let's always support s2idle and support S3 with plugged memory only if the device indicates support. Block hibernation early using the PM notifier. Trying to hibernate now fails early: # echo disk > /sys/power/state [ 26.455369] PM: hibernation: hibernation entry [ 26.458271] virtio_mem virtio0: hibernation is not supported. [ 26.462498] PM: hibernation: hibernation exit -bash: echo: write error: Operation not permitted s2idle works even without the new feature bit: # echo s2idle > /sys/power/mem_sleep # echo mem > /sys/power/state [ 52.083725] PM: suspend entry (s2idle) [ 52.095950] Filesystems sync: 0.010 seconds [ 52.101493] Freezing user space processes [ 52.104213] Freezing user space processes completed (elapsed 0.001 seconds) [ 52.106520] OOM killer disabled. [ 52.107655] Freezing remaining freezable tasks [ 52.110880] Freezing remaining freezable tasks completed (elapsed 0.001 seconds) [ 52.113296] printk: Suspending console(s) (use no_console_suspend to debug) S3 does not work without the feature bit when memory is plugged: # echo deep > /sys/power/mem_sleep # echo mem > /sys/power/state [ 32.788281] PM: suspend entry (deep) [ 32.816630] Filesystems sync: 0.027 seconds [ 32.820029] Freezing user space processes [ 32.823870] Freezing user space processes completed (elapsed 0.001 seconds) [ 32.827756] OOM killer disabled. [ 32.829608] Freezing remaining freezable tasks [ 32.833842] Freezing remaining freezable tasks completed (elapsed 0.001 seconds) [ 32.837953] printk: Suspending console(s) (use no_console_suspend to debug) [ 32.916172] virtio_mem virtio0: suspend+resume with plugged memory is not supported [ 32.916181] virtio-pci 0000:00:02.0: PM: pci_pm_suspend(): virtio_pci_freeze+0x0/0x50 returns -1 [ 32.916197] virtio-pci 0000:00:02.0: PM: dpm_run_callback(): pci_pm_suspend+0x0/0x170 returns -1 [ 32.916210] virtio-pci 0000:00:02.0: PM: failed to suspend async: error -1 But S3 works with the new feature bit when memory is plugged (patched QEMU): # echo deep > /sys/power/mem_sleep # echo mem > /sys/power/state [ 33.983694] PM: suspend entry (deep) [ 34.009828] Filesystems sync: 0.024 seconds [ 34.013589] Freezing user space processes [ 34.016722] Freezing user space processes completed (elapsed 0.001 seconds) [ 34.019092] OOM killer disabled. [ 34.020291] Freezing remaining freezable tasks [ 34.023549] Freezing remaining freezable tasks completed (elapsed 0.001 seconds) [ 34.026090] printk: Suspending console(s) (use no_console_suspend to debug) Cc: "Michael S. Tsirkin" Cc: Jason Wang Cc: Xuan Zhuo Signed-off-by: David Hildenbrand Message-Id: <20240318120645.105664-1-david@redhat.com> Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_mem.c | 68 +++++++++++++++++++++++++++++++++++++---- include/uapi/linux/virtio_mem.h | 2 ++ 2 files changed, 64 insertions(+), 6 deletions(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c index 8e3223294442..51088d02de32 100644 --- a/drivers/virtio/virtio_mem.c +++ b/drivers/virtio/virtio_mem.c @@ -21,6 +21,7 @@ #include #include #include +#include #include @@ -252,6 +253,9 @@ struct virtio_mem { /* Memory notifier (online/offline events). */ struct notifier_block memory_notifier; + /* Notifier to block hibernation image storing/reloading. */ + struct notifier_block pm_notifier; + #ifdef CONFIG_PROC_VMCORE /* vmcore callback for /proc/vmcore handling in kdump mode */ struct vmcore_cb vmcore_cb; @@ -1111,6 +1115,25 @@ static int virtio_mem_memory_notifier_cb(struct notifier_block *nb, return rc; } +static int virtio_mem_pm_notifier_cb(struct notifier_block *nb, + unsigned long action, void *arg) +{ + struct virtio_mem *vm = container_of(nb, struct virtio_mem, + pm_notifier); + switch (action) { + case PM_HIBERNATION_PREPARE: + case PM_RESTORE_PREPARE: + /* + * When restarting the VM, all memory is unplugged. Don't + * allow to hibernate and restore from an image. + */ + dev_err(&vm->vdev->dev, "hibernation is not supported.\n"); + return NOTIFY_BAD; + default: + return NOTIFY_OK; + } +} + /* * Set a range of pages PG_offline. Remember pages that were never onlined * (via generic_online_page()) using PageDirty(). @@ -2615,11 +2638,19 @@ static int virtio_mem_init_hotplug(struct virtio_mem *vm) rc = register_memory_notifier(&vm->memory_notifier); if (rc) goto out_unreg_group; - rc = register_virtio_mem_device(vm); + /* Block hibernation as early as possible. */ + vm->pm_notifier.priority = INT_MAX; + vm->pm_notifier.notifier_call = virtio_mem_pm_notifier_cb; + rc = register_pm_notifier(&vm->pm_notifier); if (rc) goto out_unreg_mem; + rc = register_virtio_mem_device(vm); + if (rc) + goto out_unreg_pm; return 0; +out_unreg_pm: + unregister_pm_notifier(&vm->pm_notifier); out_unreg_mem: unregister_memory_notifier(&vm->memory_notifier); out_unreg_group: @@ -2897,6 +2928,7 @@ static void virtio_mem_deinit_hotplug(struct virtio_mem *vm) /* unregister callbacks */ unregister_virtio_mem_device(vm); + unregister_pm_notifier(&vm->pm_notifier); unregister_memory_notifier(&vm->memory_notifier); /* @@ -2960,17 +2992,40 @@ static void virtio_mem_config_changed(struct virtio_device *vdev) #ifdef CONFIG_PM_SLEEP static int virtio_mem_freeze(struct virtio_device *vdev) { + struct virtio_mem *vm = vdev->priv; + /* - * When restarting the VM, all memory is usually unplugged. Don't - * allow to suspend/hibernate. + * We block hibernation using the PM notifier completely. The workqueue + * is already frozen by the PM core at this point, so we simply + * reset the device and cleanup the queues. */ - dev_err(&vdev->dev, "save/restore not supported.\n"); - return -EPERM; + if (pm_suspend_target_state != PM_SUSPEND_TO_IDLE && + vm->plugged_size && + !virtio_has_feature(vm->vdev, VIRTIO_MEM_F_PERSISTENT_SUSPEND)) { + dev_err(&vm->vdev->dev, + "suspending with plugged memory is not supported\n"); + return -EPERM; + } + + virtio_reset_device(vdev); + vdev->config->del_vqs(vdev); + vm->vq = NULL; + return 0; } static int virtio_mem_restore(struct virtio_device *vdev) { - return -EPERM; + struct virtio_mem *vm = vdev->priv; + int ret; + + ret = virtio_mem_init_vq(vm); + if (ret) + return ret; + virtio_device_ready(vdev); + + /* Let's check if anything changed. */ + virtio_mem_config_changed(vdev); + return 0; } #endif @@ -2979,6 +3034,7 @@ static unsigned int virtio_mem_features[] = { VIRTIO_MEM_F_ACPI_PXM, #endif VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE, + VIRTIO_MEM_F_PERSISTENT_SUSPEND, }; static const struct virtio_device_id virtio_mem_id_table[] = { diff --git a/include/uapi/linux/virtio_mem.h b/include/uapi/linux/virtio_mem.h index e9122f1d0e0c..6e4b2cf6b7f1 100644 --- a/include/uapi/linux/virtio_mem.h +++ b/include/uapi/linux/virtio_mem.h @@ -90,6 +90,8 @@ #define VIRTIO_MEM_F_ACPI_PXM 0 /* unplugged memory must not be accessed */ #define VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE 1 +/* plugged memory will remain plugged when suspending+resuming */ +#define VIRTIO_MEM_F_PERSISTENT_SUSPEND 2 /* --- virtio-mem: guest -> host requests --- */ -- cgit v1.2.3 From 810d831bbbf3cbd86e5aa91c8485b4d35186144d Mon Sep 17 00:00:00 2001 From: David Stevens Date: Thu, 21 Mar 2024 10:24:44 +0900 Subject: virtio_balloon: Give the balloon its own wakeup source Wakeup sources don't support nesting multiple events, so sharing a single object between multiple drivers can result in one driver overriding the wakeup event processing period specified by another driver. Have the virtio balloon driver use the wakeup source of the device it is bound to rather than the wakeup source of the parent device, to avoid conflicts with the transport layer. Note that although the virtio balloon's virtio_device itself isn't what actually wakes up the device, it is responsible for processing wakeup events. In the same way that EPOLLWAKEUP uses a dedicated wakeup_source to prevent suspend when userspace is processing wakeup events, a dedicated wakeup_source is necessary when processing wakeup events in a higher layer in the kernel. Fixes: b12fbc3f787e ("virtio_balloon: stay awake while adjusting balloon") Signed-off-by: David Stevens Acked-by: David Hildenbrand Message-Id: <20240321012445.1593685-2-stevensd@google.com> Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_balloon.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 1f5b3dd31fcf..89bc8da80519 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -450,7 +450,7 @@ static void start_update_balloon_size(struct virtio_balloon *vb) vb->adjustment_signal_pending = true; if (!vb->adjustment_in_progress) { vb->adjustment_in_progress = true; - pm_stay_awake(vb->vdev->dev.parent); + pm_stay_awake(&vb->vdev->dev); } spin_unlock_irqrestore(&vb->adjustment_lock, flags); @@ -462,7 +462,7 @@ static void end_update_balloon_size(struct virtio_balloon *vb) spin_lock_irq(&vb->adjustment_lock); if (!vb->adjustment_signal_pending && vb->adjustment_in_progress) { vb->adjustment_in_progress = false; - pm_relax(vb->vdev->dev.parent); + pm_relax(&vb->vdev->dev); } spin_unlock_irq(&vb->adjustment_lock); } @@ -1029,6 +1029,15 @@ static int virtballoon_probe(struct virtio_device *vdev) spin_lock_init(&vb->adjustment_lock); + /* + * The virtio balloon itself can't wake up the device, but it is + * responsible for processing wakeup events passed up from the transport + * layer. Wakeup sources don't support nesting/chaining calls, so we use + * our own wakeup source to ensure wakeup events are properly handled + * without trampling on the transport layer's wakeup source. + */ + device_set_wakeup_capable(&vb->vdev->dev, true); + virtio_device_ready(vdev); if (towards_target(vb)) -- cgit v1.2.3 From c578123e63da31d9d3876c33a8e3415597288e7d Mon Sep 17 00:00:00 2001 From: David Stevens Date: Thu, 21 Mar 2024 10:24:45 +0900 Subject: virtio_balloon: Treat stats requests as wakeup events Treat stats requests as wakeup events to ensure that the driver responds to device requests in a timely manner. Signed-off-by: David Stevens Acked-by: David Hildenbrand Message-Id: <20240321012445.1593685-3-stevensd@google.com> Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_balloon.c | 75 +++++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 29 deletions(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 89bc8da80519..b09e8e3c62e5 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -121,11 +121,14 @@ struct virtio_balloon { struct page_reporting_dev_info pr_dev_info; /* State for keeping the wakeup_source active while adjusting the balloon */ - spinlock_t adjustment_lock; - bool adjustment_signal_pending; - bool adjustment_in_progress; + spinlock_t wakeup_lock; + bool processing_wakeup_event; + u32 wakeup_signal_mask; }; +#define VIRTIO_BALLOON_WAKEUP_SIGNAL_ADJUST (1 << 0) +#define VIRTIO_BALLOON_WAKEUP_SIGNAL_STATS (1 << 1) + static const struct virtio_device_id id_table[] = { { VIRTIO_ID_BALLOON, VIRTIO_DEV_ANY_ID }, { 0 }, @@ -140,6 +143,36 @@ static u32 page_to_balloon_pfn(struct page *page) return pfn * VIRTIO_BALLOON_PAGES_PER_PAGE; } +static void start_wakeup_event(struct virtio_balloon *vb, u32 mask) +{ + unsigned long flags; + + spin_lock_irqsave(&vb->wakeup_lock, flags); + vb->wakeup_signal_mask |= mask; + if (!vb->processing_wakeup_event) { + vb->processing_wakeup_event = true; + pm_stay_awake(&vb->vdev->dev); + } + spin_unlock_irqrestore(&vb->wakeup_lock, flags); +} + +static void process_wakeup_event(struct virtio_balloon *vb, u32 mask) +{ + spin_lock_irq(&vb->wakeup_lock); + vb->wakeup_signal_mask &= ~mask; + spin_unlock_irq(&vb->wakeup_lock); +} + +static void finish_wakeup_event(struct virtio_balloon *vb) +{ + spin_lock_irq(&vb->wakeup_lock); + if (!vb->wakeup_signal_mask && vb->processing_wakeup_event) { + vb->processing_wakeup_event = false; + pm_relax(&vb->vdev->dev); + } + spin_unlock_irq(&vb->wakeup_lock); +} + static void balloon_ack(struct virtqueue *vq) { struct virtio_balloon *vb = vq->vdev->priv; @@ -370,8 +403,10 @@ static void stats_request(struct virtqueue *vq) struct virtio_balloon *vb = vq->vdev->priv; spin_lock(&vb->stop_update_lock); - if (!vb->stop_update) + if (!vb->stop_update) { + start_wakeup_event(vb, VIRTIO_BALLOON_WAKEUP_SIGNAL_STATS); queue_work(system_freezable_wq, &vb->update_balloon_stats_work); + } spin_unlock(&vb->stop_update_lock); } @@ -444,29 +479,10 @@ static void virtio_balloon_queue_free_page_work(struct virtio_balloon *vb) static void start_update_balloon_size(struct virtio_balloon *vb) { - unsigned long flags; - - spin_lock_irqsave(&vb->adjustment_lock, flags); - vb->adjustment_signal_pending = true; - if (!vb->adjustment_in_progress) { - vb->adjustment_in_progress = true; - pm_stay_awake(&vb->vdev->dev); - } - spin_unlock_irqrestore(&vb->adjustment_lock, flags); - + start_wakeup_event(vb, VIRTIO_BALLOON_WAKEUP_SIGNAL_ADJUST); queue_work(system_freezable_wq, &vb->update_balloon_size_work); } -static void end_update_balloon_size(struct virtio_balloon *vb) -{ - spin_lock_irq(&vb->adjustment_lock); - if (!vb->adjustment_signal_pending && vb->adjustment_in_progress) { - vb->adjustment_in_progress = false; - pm_relax(&vb->vdev->dev); - } - spin_unlock_irq(&vb->adjustment_lock); -} - static void virtballoon_changed(struct virtio_device *vdev) { struct virtio_balloon *vb = vdev->priv; @@ -495,7 +511,10 @@ static void update_balloon_stats_func(struct work_struct *work) vb = container_of(work, struct virtio_balloon, update_balloon_stats_work); + + process_wakeup_event(vb, VIRTIO_BALLOON_WAKEUP_SIGNAL_STATS); stats_handle_request(vb); + finish_wakeup_event(vb); } static void update_balloon_size_func(struct work_struct *work) @@ -506,9 +525,7 @@ static void update_balloon_size_func(struct work_struct *work) vb = container_of(work, struct virtio_balloon, update_balloon_size_work); - spin_lock_irq(&vb->adjustment_lock); - vb->adjustment_signal_pending = false; - spin_unlock_irq(&vb->adjustment_lock); + process_wakeup_event(vb, VIRTIO_BALLOON_WAKEUP_SIGNAL_ADJUST); diff = towards_target(vb); @@ -523,7 +540,7 @@ static void update_balloon_size_func(struct work_struct *work) if (diff) queue_work(system_freezable_wq, work); else - end_update_balloon_size(vb); + finish_wakeup_event(vb); } static int init_vqs(struct virtio_balloon *vb) @@ -1027,7 +1044,7 @@ static int virtballoon_probe(struct virtio_device *vdev) goto out_unregister_oom; } - spin_lock_init(&vb->adjustment_lock); + spin_lock_init(&vb->wakeup_lock); /* * The virtio balloon itself can't wake up the device, but it is -- cgit v1.2.3 From 7cc5d6caaf403c730ea1aac0b708dea47758877d Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Sun, 31 Mar 2024 10:43:49 +0200 Subject: virtio: balloon: drop owner assignment virtio core already sets the .owner, so driver does not need to. Signed-off-by: Krzysztof Kozlowski Message-Id: <20240331-module-owner-virtio-v2-2-98f04bfaf46a@linaro.org> Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_balloon.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index b09e8e3c62e5..c0a63638f95e 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -1181,7 +1181,6 @@ static struct virtio_driver virtio_balloon_driver = { .feature_table = features, .feature_table_size = ARRAY_SIZE(features), .driver.name = KBUILD_MODNAME, - .driver.owner = THIS_MODULE, .id_table = id_table, .validate = virtballoon_validate, .probe = virtballoon_probe, -- cgit v1.2.3 From ebfb92a96715df5bf95195ed60e6d91f51d36127 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Sun, 31 Mar 2024 10:43:50 +0200 Subject: virtio: input: drop owner assignment virtio core already sets the .owner, so driver does not need to. Signed-off-by: Krzysztof Kozlowski Message-Id: <20240331-module-owner-virtio-v2-3-98f04bfaf46a@linaro.org> Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_input.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c index 3aa46703872d..1a730d6c0b55 100644 --- a/drivers/virtio/virtio_input.c +++ b/drivers/virtio/virtio_input.c @@ -394,7 +394,6 @@ static const struct virtio_device_id id_table[] = { static struct virtio_driver virtio_input_driver = { .driver.name = KBUILD_MODNAME, - .driver.owner = THIS_MODULE, .feature_table = features, .feature_table_size = ARRAY_SIZE(features), .id_table = id_table, -- cgit v1.2.3 From b0e55a950ee6145e0debfab941b057046e6d87c2 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Sun, 31 Mar 2024 10:43:51 +0200 Subject: virtio: mem: drop owner assignment virtio core already sets the .owner, so driver does not need to. Signed-off-by: Krzysztof Kozlowski Message-Id: <20240331-module-owner-virtio-v2-4-98f04bfaf46a@linaro.org> Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_mem.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c index 51088d02de32..6d4dfbc53a66 100644 --- a/drivers/virtio/virtio_mem.c +++ b/drivers/virtio/virtio_mem.c @@ -3046,7 +3046,6 @@ static struct virtio_driver virtio_mem_driver = { .feature_table = virtio_mem_features, .feature_table_size = ARRAY_SIZE(virtio_mem_features), .driver.name = KBUILD_MODNAME, - .driver.owner = THIS_MODULE, .id_table = virtio_mem_id_table, .probe = virtio_mem_probe, .remove = virtio_mem_remove, -- cgit v1.2.3 From 89875151fccdd024d571aa884ea97a0128b968b6 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 26 Apr 2024 17:08:45 +0200 Subject: virtio: delete vq in vp_find_vqs_msix() when request_irq() fails When request_irq() fails, error path calls vp_del_vqs(). There, as vq is present in the list, free_irq() is called for the same vector. That causes following splat: [ 0.414355] Trying to free already-free IRQ 27 [ 0.414403] WARNING: CPU: 1 PID: 1 at kernel/irq/manage.c:1899 free_irq+0x1a1/0x2d0 [ 0.414510] Modules linked in: [ 0.414540] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc4+ #27 [ 0.414540] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014 [ 0.414540] RIP: 0010:free_irq+0x1a1/0x2d0 [ 0.414540] Code: 1e 00 48 83 c4 08 48 89 e8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 90 8b 74 24 04 48 c7 c7 98 80 6c b1 e8 00 c9 f7 ff 90 <0f> 0b 90 90 48 89 ee 4c 89 ef e8 e0 20 b8 00 49 8b 47 40 48 8b 40 [ 0.414540] RSP: 0000:ffffb71480013ae0 EFLAGS: 00010086 [ 0.414540] RAX: 0000000000000000 RBX: ffffa099c2722000 RCX: 0000000000000000 [ 0.414540] RDX: 0000000000000000 RSI: ffffb71480013998 RDI: 0000000000000001 [ 0.414540] RBP: 0000000000000246 R08: 00000000ffffdfff R09: 0000000000000001 [ 0.414540] R10: 00000000ffffdfff R11: ffffffffb18729c0 R12: ffffa099c1c91760 [ 0.414540] R13: ffffa099c1c916a4 R14: ffffa099c1d2f200 R15: ffffa099c1c91600 [ 0.414540] FS: 0000000000000000(0000) GS:ffffa099fec40000(0000) knlGS:0000000000000000 [ 0.414540] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 0.414540] CR2: 0000000000000000 CR3: 0000000008e3e001 CR4: 0000000000370ef0 [ 0.414540] Call Trace: [ 0.414540] [ 0.414540] ? __warn+0x80/0x120 [ 0.414540] ? free_irq+0x1a1/0x2d0 [ 0.414540] ? report_bug+0x164/0x190 [ 0.414540] ? handle_bug+0x3b/0x70 [ 0.414540] ? exc_invalid_op+0x17/0x70 [ 0.414540] ? asm_exc_invalid_op+0x1a/0x20 [ 0.414540] ? free_irq+0x1a1/0x2d0 [ 0.414540] vp_del_vqs+0xc1/0x220 [ 0.414540] vp_find_vqs_msix+0x305/0x470 [ 0.414540] vp_find_vqs+0x3e/0x1a0 [ 0.414540] vp_modern_find_vqs+0x1b/0x70 [ 0.414540] init_vqs+0x387/0x600 [ 0.414540] virtnet_probe+0x50a/0xc80 [ 0.414540] virtio_dev_probe+0x1e0/0x2b0 [ 0.414540] really_probe+0xc0/0x2c0 [ 0.414540] ? __pfx___driver_attach+0x10/0x10 [ 0.414540] __driver_probe_device+0x73/0x120 [ 0.414540] driver_probe_device+0x1f/0xe0 [ 0.414540] __driver_attach+0x88/0x180 [ 0.414540] bus_for_each_dev+0x85/0xd0 [ 0.414540] bus_add_driver+0xec/0x1f0 [ 0.414540] driver_register+0x59/0x100 [ 0.414540] ? __pfx_virtio_net_driver_init+0x10/0x10 [ 0.414540] virtio_net_driver_init+0x90/0xb0 [ 0.414540] do_one_initcall+0x58/0x230 [ 0.414540] kernel_init_freeable+0x1a3/0x2d0 [ 0.414540] ? __pfx_kernel_init+0x10/0x10 [ 0.414540] kernel_init+0x1a/0x1c0 [ 0.414540] ret_from_fork+0x31/0x50 [ 0.414540] ? __pfx_kernel_init+0x10/0x10 [ 0.414540] ret_from_fork_asm+0x1a/0x30 [ 0.414540] Fix this by calling deleting the current vq when request_irq() fails. Fixes: 0b0f9dc52ed0 ("Revert "virtio_pci: use shared interrupts for virtqueues"") Signed-off-by: Jiri Pirko Message-Id: <20240426150845.3999481-1-jiri@resnulli.us> Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_pci_common.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index b655fccaf773..584af7816532 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -348,8 +348,10 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, vring_interrupt, 0, vp_dev->msix_names[msix_vec], vqs[i]); - if (err) + if (err) { + vp_del_vq(vqs[i]); goto error_find; + } } return 0; -- cgit v1.2.3 From c8fae27d141a32a1624d0d0d5419d94252824498 Mon Sep 17 00:00:00 2001 From: Li Zhang Date: Sat, 16 Mar 2024 13:25:54 +0800 Subject: virtio-pci: Check if is_avq is NULL [bug] In the virtio_pci_common.c function vp_del_vqs, vp_dev->is_avq is involved to determine whether it is admin virtqueue, but this function vp_dev->is_avq may be empty. For installations, virtio_pci_legacy does not assign a value to vp_dev->is_avq. [fix] Check whether it is vp_dev->is_avq before use. [test] Test with virsh Attach device Before this patch, the following command would crash the guest system After applying the patch, everything seems to be working fine. Signed-off-by: Li Zhang Message-Id: <1710566754-3532-1-git-send-email-zhanglikernel@gmail.com> Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_pci_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index 584af7816532..f6b0b00e4599 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -236,7 +236,7 @@ void vp_del_vqs(struct virtio_device *vdev) int i; list_for_each_entry_safe(vq, n, &vdev->vqs, list) { - if (vp_dev->is_avq(vdev, vq->index)) + if (vp_dev->is_avq && vp_dev->is_avq(vdev, vq->index)) continue; if (vp_dev->per_vq_vectors) { -- cgit v1.2.3