From: "Michael S. Tsirkin" <mst@redhat.com>
To: Radu Rendec <radu@rendec.net>
Cc: "Jason Wang" <jasowangio@gmail.com>,
"Xuan Zhuo" <xuanzhuo@linux.alibaba.com>,
"Eugenio Pérez" <eperezma@redhat.com>,
"Bjorn Andersson" <andersson@kernel.org>,
"Mathieu Poirier" <mathieu.poirier@linaro.org>,
virtualization@lists.linux.dev, linux-remoteproc@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] virtio: avoid hanging forever in reset
Date: Wed, 2 Sep 2026 05:30:55 -0400 [thread overview]
Message-ID: <20260902021849-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20260902020741.3686084-1-radu@rendec.net>
On Tue, Sep 01, 2026 at 10:07:41PM -0400, Radu Rendec wrote:
> Under certain conditions(*) vp_modern_get_status() can always return
> 0xff. This has a ripple effect that ultimately makes the system fail to
> boot if the drivers are built-in and the virtio devices are probed
> during boot:
> * vp_reset() for the virtio-net-pci device spins forever in the loop
> around vp_modern_get_status(mdev), which always returns 0xff.
> * vp_reset() is called in the context of the probe() function for the
> virtio-net-pci device, so probe() never returns.
> * the init task gets stuck in wait_for_device_probe() before it
> executes the real init binary.
>
> Add a timeout to the reset function and make the device probing fail if
> the reset function times out.
Generally, I like using gdb stub in qemu to debug kernel
and anything that will make things time out and fail
is unwelcome.
>
> (*) This is not a theoretical problem. It happens on imx8mp-evk under
> Qemu due to a relatively recent change to the dw_pcie driver that's
> incompatible with the dw_pcie emulation in Qemu. I reported that bug
> separately as a follow up to the patch that introduced it:
> https://lore.kernel.org/all/ed86629d74368c6811e9f3b8f169584c0faf71ce.camel@rendec.net/
So presumably it will be fixed in qemu? Or find a different way,
e.g. poke at device/vendor id of virtio. or add a workaround
in the driver if you prefer.
> Signed-off-by: Radu Rendec <radu@rendec.net>
> ---
> drivers/remoteproc/remoteproc_virtio.c | 4 +++-
> drivers/virtio/virtio.c | 8 +++++---
> drivers/virtio/virtio_mmio.c | 5 ++++-
> drivers/virtio/virtio_pci_legacy.c | 4 +++-
> drivers/virtio/virtio_pci_modern.c | 15 ++++++++++++---
> include/linux/virtio.h | 2 +-
> include/linux/virtio_config.h | 2 +-
> 7 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
> index d5e9ff045a28..d103b80b1faf 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -231,7 +231,7 @@ static void rproc_virtio_set_status(struct virtio_device *vdev, u8 status)
> dev_dbg(&vdev->dev, "status: %d\n", status);
> }
>
> -static void rproc_virtio_reset(struct virtio_device *vdev)
> +static int rproc_virtio_reset(struct virtio_device *vdev)
> {
> struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
> struct fw_rsc_vdev *rsc;
> @@ -240,6 +240,8 @@ static void rproc_virtio_reset(struct virtio_device *vdev)
>
> rsc->status = 0;
> dev_dbg(&vdev->dev, "reset !\n");
> +
> + return 0;
> }
>
> /* provide the vdev features as retrieved from the firmware */
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 75bb4ffe3b87..9542e4f7cb31 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -250,7 +250,7 @@ static int virtio_features_ok(struct virtio_device *dev)
> * call/workqueue/bh. Invoking virtio_break_device then flushing any such
> * contexts is one way to handle that.
> * */
> -void virtio_reset_device(struct virtio_device *dev)
> +int virtio_reset_device(struct virtio_device *dev)
> {
> #ifdef CONFIG_VIRTIO_HARDEN_NOTIFICATION
> /*
> @@ -263,7 +263,7 @@ void virtio_reset_device(struct virtio_device *dev)
> virtio_synchronize_cbs(dev);
> #endif
>
> - dev->config->reset(dev);
> + return dev->config->reset(dev);
> }
> EXPORT_SYMBOL_GPL(virtio_reset_device);
>
> @@ -567,7 +567,9 @@ int register_virtio_device(struct virtio_device *dev)
>
> /* We always start by resetting the device, in case a previous
> * driver messed it up. This also tests that code path a little. */
> - virtio_reset_device(dev);
> + err = virtio_reset_device(dev);
> + if (err)
> + goto out_of_node_put;
>
> /* Acknowledge that we've seen the device. */
> virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 316f03b97356..537d38841331 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -249,7 +249,7 @@ static void vm_set_status(struct virtio_device *vdev, u8 status)
> writel(status, vm_dev->base + VIRTIO_MMIO_STATUS);
> }
>
> -static void vm_reset(struct virtio_device *vdev)
> +static int vm_reset(struct virtio_device *vdev)
> {
> struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
>
> @@ -260,7 +260,10 @@ static void vm_reset(struct virtio_device *vdev)
> /* Wait for reset to complete. */
> while (vm_get_status(vdev))
> fsleep(1000);
> + /* TODO: add timeout */
> }
> +
> + return 0;
> }
>
>
> diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
> index d9cbb02b35a1..2882756c7084 100644
> --- a/drivers/virtio/virtio_pci_legacy.c
> +++ b/drivers/virtio/virtio_pci_legacy.c
> @@ -90,7 +90,7 @@ static void vp_set_status(struct virtio_device *vdev, u8 status)
> vp_legacy_set_status(&vp_dev->ldev, status);
> }
>
> -static void vp_reset(struct virtio_device *vdev)
> +static int vp_reset(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> /* 0 status means a reset. */
> @@ -100,6 +100,8 @@ static void vp_reset(struct virtio_device *vdev)
> vp_legacy_get_status(&vp_dev->ldev);
> /* Flush pending VQ/configuration callbacks. */
> vp_synchronize_vectors(vdev);
> +
> + return 0;
> }
>
> static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 6d8ae2a6a8ca..4a6906a18cfa 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -21,6 +21,7 @@
> #include "virtio_pci_common.h"
>
> #define VIRTIO_AVQ_SGS_MAX 4
> +#define VIRTIO_PCI_RESET_TIMEOUT_MS 1000
>
> static void vp_get_features(struct virtio_device *vdev, u64 *features)
> {
> @@ -543,10 +544,11 @@ static void vp_set_status(struct virtio_device *vdev, u8 status)
> vp_modern_avq_activate(vdev);
> }
>
> -static void vp_reset(struct virtio_device *vdev)
> +static int vp_reset(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> + int timeout = VIRTIO_PCI_RESET_TIMEOUT_MS;
>
> /* 0 status means a reset. */
> vp_modern_set_status(mdev, 0);
> @@ -555,13 +557,20 @@ static void vp_reset(struct virtio_device *vdev)
> * This will flush out the status write, and flush in device writes,
> * including MSI-X interrupts, if any.
> */
> - while (vp_modern_get_status(mdev))
> - msleep(1);
> + while (vp_modern_get_status(mdev)) {
> + if (!timeout--) {
> + dev_warn(&vdev->dev, "reset timeout");
> + return -ETIMEDOUT;
> + }
> + msleep(1000);
1 second is a long time. normally devices reset immediately.
> + }
>
> vp_modern_avq_cleanup(vdev);
>
> /* Flush pending VQ/configuration callbacks. */
> vp_synchronize_vectors(vdev);
> +
> + return 0;
> }
>
> static int vp_active_vq(struct virtqueue *vq, u16 msix_vec)
So now if the device does not reset, during cleanup we
previously blocked and now it will corrupt memory.
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index f923e42cfd01..568e1bd95e4c 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -212,7 +212,7 @@ void virtio_config_driver_enable(struct virtio_device *dev);
> int virtio_device_freeze(struct virtio_device *dev);
> int virtio_device_restore(struct virtio_device *dev);
> #endif
> -void virtio_reset_device(struct virtio_device *dev);
> +int virtio_reset_device(struct virtio_device *dev);
> void virtio_device_shutdown(struct virtio_device *dev);
> int virtio_device_reset_prepare(struct virtio_device *dev);
> int virtio_device_reset_done(struct virtio_device *dev);
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 69f84ea85d71..153426b0ae79 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -117,7 +117,7 @@ struct virtio_config_ops {
> u32 (*generation)(struct virtio_device *vdev);
> u8 (*get_status)(struct virtio_device *vdev);
> void (*set_status)(struct virtio_device *vdev, u8 status);
> - void (*reset)(struct virtio_device *vdev);
> + int (*reset)(struct virtio_device *vdev);
> int (*find_vqs)(struct virtio_device *vdev, unsigned int nvqs,
> struct virtqueue *vqs[],
> struct virtqueue_info vqs_info[],
> --
> 2.55.0
next prev parent reply other threads:[~2026-09-02 9:31 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-02 2:07 Radu Rendec
2026-09-02 9:30 ` Michael S. Tsirkin [this message]
2026-09-03 0:29 ` Radu Rendec
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260902021849-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=andersson@kernel.org \
--cc=eperezma@redhat.com \
--cc=jasowangio@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
--cc=radu@rendec.net \
--cc=virtualization@lists.linux.dev \
--cc=xuanzhuo@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®