[PATCH 0/2] virtio_pci_modern: fix vp_reset() hang on unresponsive device

[PATCH 0/2] virtio_pci_modern: fix vp_reset() hang on unresponsive device

[Abhin Parekadan Jose]

While investigating a syzbot report of a WARN_ON_ONCE firing in
virtio_dev_remove() [1], I found a related but more serious issue:
vp_reset() in the modern virtio-pci transport can hang indefinitely
if PCI_COMMAND memory-space decode is disabled while the device is
bound (e.g. surprise removal, hardware fault, or -- as reproduced
here -- a direct write to the PCI_COMMAND register). The status
register poll loop has no way to distinguish "device still resetting"
from "device unreachable," so it never terminates.

Patch 1 adds a VIRTIO_STATUS_ERROR() check that recognizes an
all-ones status read as invalid (per spec, bits 4-5 are reserved and
can never legitimately be set) and warns once at the point the bad
read actually happens.

Patch 2 uses that check to break out of vp_reset()'s poll loop
instead of spinning forever.

Reproduced on a modern-transport virtio-blk-pci device:

  # printf '\x00\x00' | dd of=/sys/bus/pci/devices/0000:01:00.0/config \
    bs=1 seek=4 count=2 conv=notrunc
  # echo 1 > /sys/bus/pci/devices/0000:01:00.0/remove

The second command hangs indefinitely without this series; gdb
confirms the CPU is stuck in vp_reset()'s status-polling loop, with
vp_modern_get_status() consistently returning 0xff.

[1] https://syzbot.org/bug?extid=a1c8effc62c569d4bd25

Abhin Parekadan Jose (2):
  virtio_pci_modern_dev: warn once on invalid status
  virtio_pci_modern: avoid infinite loop in vp_reset() on invalid status

 drivers/virtio/virtio_pci_modern.c     |  6 +++++-
 drivers/virtio/virtio_pci_modern_dev.c |  8 +++++++-
 include/uapi/linux/virtio_config.h     | 16 ++++++++++++++++
 3 files changed, 28 insertions(+), 2 deletions(-)

--
2.51.1

[PATCH 1/2] virtio_pci_modern_dev: warn once on invalid status

[Abhin Parekadan Jose]

vp_modern_get_status() returns the raw device_status byte as read
from the common configuration structure (struct virtio_pci_common_cfg,
mapped via the VIRTIO_PCI_CAP_COMMON_CFG capability). That byte should
only ever contain some combination of the status bits defined by the
virtio spec (bits 0-3, 6-7); bits 4 and 5 are reserved and a
spec-compliant device must never set them. A value with any other bit
set means either the device is violating the spec, or the read never
reached real device state at all -- e.g. because a write of 0x0000 to
the PCI_COMMAND register (config space offset 4) clears the Memory
Space Enable bit, causing the device to stop responding to
memory-mapped register accesses -- effectively simulating an
unresponsive/removed device without a real hot-unplug. In that case
the MMIO read returns the bus's synthesized all-ones response instead
of real device state.

Add VIRTIO_STATUS_ERROR() to the uapi header to recognize such values,
and warn once from vp_modern_get_status() when it sees one, so the
bogus status is visible at its source rather than only showing up as
confusing behavior in callers.

Signed-off-by: Abhin Parekadan Jose <abhinjoses@gmail.com>
---
 drivers/virtio/virtio_pci_modern_dev.c |  8 +++++++-
 include/uapi/linux/virtio_config.h     | 16 ++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
index 413a8c353463..60dd8acf1c28 100644
--- a/drivers/virtio/virtio_pci_modern_dev.c
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -480,8 +480,14 @@ EXPORT_SYMBOL_GPL(vp_modern_generation);
 u8 vp_modern_get_status(struct virtio_pci_modern_device *mdev)
 {
 	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+	u8 status = vp_ioread8(&cfg->device_status);
 
-	return vp_ioread8(&cfg->device_status);
+	if (VIRTIO_STATUS_ERROR(status)) {
+		WARN_ONCE(1, "virtio: device returned error status: %#x\n",
+			  status);
+	}
+
+	return status;
 }
 EXPORT_SYMBOL_GPL(vp_modern_get_status);
 
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 2445f365bce7..6f458914c0ba 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -45,6 +45,22 @@
 /* We've given up on this device. */
 #define VIRTIO_CONFIG_S_FAILED		0x80
 
+/*
+ * Check if a status value indicates an error
+ * All device_status bits currently defined by the virtio spec (bits
+ * 0,1,2,3,6,7). Bits 4 and 5 (0x10, 0x20) are reserved/undefined -- a
+ * real device must never set them. A status byte with any bit outside
+ * this mask set cannot be a legitimate value: either the device is
+ * violating the spec, or the read never actually reached it (e.g.
+ * PCI_COMMAND memory decode is disabled and this is a synthesized
+ * all-ones bus response instead of real device state).
+ */
+#define VIRTIO_STATUS_ERROR(val) \
+	(((u8)(val)) & \
+	 ~(VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER | \
+	   VIRTIO_CONFIG_S_DRIVER_OK | VIRTIO_CONFIG_S_FEATURES_OK | \
+	   VIRTIO_CONFIG_S_NEEDS_RESET | VIRTIO_CONFIG_S_FAILED))
+
 /*
  * Virtio feature bits VIRTIO_TRANSPORT_F_START through
  * VIRTIO_TRANSPORT_F_END are reserved for the transport
-- 
2.51.1

[PATCH 2/2] virtio_pci_modern: avoid infinite loop in vp_reset() on invalid status

[Abhin Parekadan Jose]

vp_reset() polls device_status in a tight loop, waiting for it to read
back as 0 after the reset write. device_status is read via MMIO from
the common configuration structure, which requires the PCI_COMMAND
Memory Space Enable bit to be set. If that bit is cleared while the
device is bound -- e.g. by writing 0x0000 to PCI_COMMAND (config space
offset 4) -- the MMIO read no longer reaches the device and returns
the bus's synthesized all-ones response instead. Since that value can
never legitimately clear to 0, the loop spins forever and hangs the
caller.

Use VIRTIO_STATUS_ERROR() to recognize such values and bail out of the
poll loop instead of looping indefinitely.

Signed-off-by: Abhin Parekadan Jose <abhinjoses@gmail.com>
---
 drivers/virtio/virtio_pci_modern.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 6d8ae2a6a8ca..209fa3b36c90 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -547,6 +547,7 @@ static void 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;
+	u8 status;
 
 	/* 0 status means a reset. */
 	vp_modern_set_status(mdev, 0);
@@ -555,8 +556,11 @@ 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))
+	while ((status = vp_modern_get_status(mdev))) {
+		if (VIRTIO_STATUS_ERROR(status))
+			break;
 		msleep(1);
+	}
 
 	vp_modern_avq_cleanup(vdev);
 
-- 
2.51.1

Re: [PATCH 0/2] virtio_pci_modern: fix vp_reset() hang on unresponsive device

[Michael S. Tsirkin]

On Sun, Aug 02, 2026 at 05:40:57PM +0000, Abhin Parekadan Jose wrote:
> While investigating a syzbot report of a WARN_ON_ONCE firing in
> virtio_dev_remove() [1],


And I responded to that syzbot report, and I quote:

So it writes 0 into pci command, effectively killing the device,
and then is unhappy that the driver prints warnings?
Who thought it's a good idea? Why?


> I found a related but more serious issue:
> vp_reset() in the modern virtio-pci transport can hang indefinitely
> if PCI_COMMAND memory-space decode is disabled while the device is
> bound (e.g. surprise removal, hardware fault, or -- as reproduced
> here -- a direct write to the PCI_COMMAND register). The status
> register poll loop has no way to distinguish "device still resetting"
> from "device unreachable," so it never terminates.
> 
> Patch 1 adds a VIRTIO_STATUS_ERROR() check that recognizes an
> all-ones status read as invalid (per spec, bits 4-5 are reserved and
> can never legitimately be set) and warns once at the point the bad
> read actually happens.
> 
> Patch 2 uses that check to break out of vp_reset()'s poll loop
> instead of spinning forever.

Was all this including the cover letter written with ai assistance?
if yes pls disclose this.

> Reproduced on a modern-transport virtio-blk-pci device:
> 
>   # printf '\x00\x00' | dd of=/sys/bus/pci/devices/0000:01:00.0/config \
>     bs=1 seek=4 count=2 conv=notrunc
>   # echo 1 > /sys/bus/pci/devices/0000:01:00.0/remove
> 
> The second command hangs indefinitely without this series; gdb
> confirms the CPU is stuck in vp_reset()'s status-polling loop, with
> vp_modern_get_status() consistently returning 0xff.
> 
> [1] https://syzbot.org/bug?extid=a1c8effc62c569d4bd25
> 
> Abhin Parekadan Jose (2):
>   virtio_pci_modern_dev: warn once on invalid status
>   virtio_pci_modern: avoid infinite loop in vp_reset() on invalid status
> 
>  drivers/virtio/virtio_pci_modern.c     |  6 +++++-
>  drivers/virtio/virtio_pci_modern_dev.c |  8 +++++++-
>  include/uapi/linux/virtio_config.h     | 16 ++++++++++++++++
>  3 files changed, 28 insertions(+), 2 deletions(-)
> 
> --
> 2.51.1

Re: [PATCH 2/2] virtio_pci_modern: avoid infinite loop in vp_reset() on invalid status

[Michael S. Tsirkin]

On Sun, Aug 02, 2026 at 05:40:59PM +0000, Abhin Parekadan Jose wrote:
> vp_reset() polls device_status in a tight loop, waiting for it to read
> back as 0 after the reset write. device_status is read via MMIO from
> the common configuration structure, which requires the PCI_COMMAND
> Memory Space Enable bit to be set. If that bit is cleared while the
> device is bound -- e.g. by writing 0x0000 to PCI_COMMAND (config space
> offset 4)

So don't do it?

> -- the MMIO read no longer reaches the device and returns
> the bus's synthesized all-ones response instead. Since that value can
> never legitimately clear to 0, the loop spins forever and hangs the
> caller.
> 
> Use VIRTIO_STATUS_ERROR() to recognize such values and bail out of the
> poll loop instead of looping indefinitely.

If you want to work on suprise removal, that is great, but
with actual surprise removal testing, please. I'm not
inclined to include changes when testing amounted
to illegally poking at pci command, and without much in the way
of what effect this has on the drivers.

In particular, please read cover.1752094439.git.mst@redhat.com - a thread
where we seem to have come to the conclusion that hangs where
surprise removal happens while the remove callback is in progress
are fundamentally unfixable without pci (and likely acpi) core
changes.

> 
> Signed-off-by: Abhin Parekadan Jose <abhinjoses@gmail.com>
> ---
>  drivers/virtio/virtio_pci_modern.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 6d8ae2a6a8ca..209fa3b36c90 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -547,6 +547,7 @@ static void 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;
> +	u8 status;
>  
>  	/* 0 status means a reset. */
>  	vp_modern_set_status(mdev, 0);
> @@ -555,8 +556,11 @@ 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))
> +	while ((status = vp_modern_get_status(mdev))) {
> +		if (VIRTIO_STATUS_ERROR(status))
> +			break;
>  		msleep(1);
> +	}


I am not convinced we'll never use all status bits eventually.
Currently a single bit (32, bit 5) is unused.

And then this test will give false positives.

>  
>  	vp_modern_avq_cleanup(vdev);
>  
> -- 
> 2.51.1

Re: [PATCH 1/2] virtio_pci_modern_dev: warn once on invalid status

[Michael S. Tsirkin]

On Sun, Aug 02, 2026 at 05:40:58PM +0000, Abhin Parekadan Jose wrote:
> vp_modern_get_status() returns the raw device_status byte as read
> from the common configuration structure (struct virtio_pci_common_cfg,
> mapped via the VIRTIO_PCI_CAP_COMMON_CFG capability). That byte should
> only ever contain some combination of the status bits defined by the
> virtio spec (bits 0-3, 6-7); bits 4 and 5 are reserved and a
> spec-compliant device must never set them.

Future versions of the spec are likely to use these bits,
I prefer to make the drivers future proof. In fact, things
like kexec mean that we don't know what drove the
device earlier, so we do not know that the previous
driver did not write 0xFF there.



> A value with any other bit
> set means either the device is violating the spec, or the read never
> reached real device state at all -- e.g. because a write of 0x0000 to
> the PCI_COMMAND register (config space offset 4) clears the Memory
> Space Enable bit, causing the device to stop responding to
> memory-mapped register accesses -- effectively simulating an
> unresponsive/removed device without a real hot-unplug. In that case
> the MMIO read returns the bus's synthesized all-ones response instead
> of real device state.
> 
> Add VIRTIO_STATUS_ERROR() to the uapi header to recognize such values,
> and warn once from vp_modern_get_status() when it sees one, so the
> bogus status is visible at its source rather than only showing up as
> confusing behavior in callers.
> 
> Signed-off-by: Abhin Parekadan Jose <abhinjoses@gmail.com>
> ---
>  drivers/virtio/virtio_pci_modern_dev.c |  8 +++++++-
>  include/uapi/linux/virtio_config.h     | 16 ++++++++++++++++
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
> index 413a8c353463..60dd8acf1c28 100644
> --- a/drivers/virtio/virtio_pci_modern_dev.c
> +++ b/drivers/virtio/virtio_pci_modern_dev.c
> @@ -480,8 +480,14 @@ EXPORT_SYMBOL_GPL(vp_modern_generation);
>  u8 vp_modern_get_status(struct virtio_pci_modern_device *mdev)
>  {
>  	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
> +	u8 status = vp_ioread8(&cfg->device_status);
>  
> -	return vp_ioread8(&cfg->device_status);
> +	if (VIRTIO_STATUS_ERROR(status)) {
> +		WARN_ONCE(1, "virtio: device returned error status: %#x\n",
> +			  status);
> +	}
> +
> +	return status;
>  }
>  EXPORT_SYMBOL_GPL(vp_modern_get_status);
>  
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 2445f365bce7..6f458914c0ba 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -45,6 +45,22 @@
>  /* We've given up on this device. */
>  #define VIRTIO_CONFIG_S_FAILED		0x80
>  
> +/*
> + * Check if a status value indicates an error
> + * All device_status bits currently defined by the virtio spec (bits
> + * 0,1,2,3,6,7). Bits 4 and 5 (0x10, 0x20) are reserved/undefined -- a
> + * real device must never set them. A status byte with any bit outside
> + * this mask set cannot be a legitimate value: either the device is
> + * violating the spec, or the read never actually reached it (e.g.
> + * PCI_COMMAND memory decode is disabled and this is a synthesized
> + * all-ones bus response instead of real device state).
> + */
> +#define VIRTIO_STATUS_ERROR(val) \
> +	(((u8)(val)) & \
> +	 ~(VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER | \
> +	   VIRTIO_CONFIG_S_DRIVER_OK | VIRTIO_CONFIG_S_FEATURES_OK | \
> +	   VIRTIO_CONFIG_S_NEEDS_RESET | VIRTIO_CONFIG_S_FAILED))
> +


Not clear what this does in UAPI, even if useful.

>  /*
>   * Virtio feature bits VIRTIO_TRANSPORT_F_START through
>   * VIRTIO_TRANSPORT_F_END are reserved for the transport
> -- 
> 2.51.1

Re: [PATCH 0/2] virtio_pci_modern: fix vp_reset() hang on unresponsive device

[Abhin Parekadan Jose]

On Sun, Aug 02, 2026 at 01:47:01PM -0400, Michael S. Tsirkin wrote:
> On Sun, Aug 02, 2026 at 05:40:57PM +0000, Abhin Parekadan Jose wrote:
> > While investigating a syzbot report of a WARN_ON_ONCE firing in
> > virtio_dev_remove() [1],
>
>
> And I responded to that syzbot report, and I quote:
>
> So it writes 0 into pci command, effectively killing the device,
> and then is unhappy that the driver prints warnings?
> Who thought it's a good idea? Why?

I was learning how to reproduce syzbot bugs when I found this
issue by writing 0 to PCI_COMMAND to simulate an unresponsive
device. While doing that I noticed that echo 1 > /sys/../remove
hung completely rather than just printing the warning. Since the
device_status register lives in the virtio common config MMIO
space and has defined values(based on the bits set) in the spec.
I thought it made sense for virtio to detect this and handle it
gracefully rather than spin forever, so I wrote up a small fix
for that.

> > I found a related but more serious issue:
> > vp_reset() in the modern virtio-pci transport can hang indefinitely
> > if PCI_COMMAND memory-space decode is disabled while the device is
> > bound (e.g. surprise removal, hardware fault, or -- as reproduced
> > here -- a direct write to the PCI_COMMAND register). The status
> > register poll loop has no way to distinguish "device still resetting"
> > from "device unreachable," so it never terminates.
> >
> > Patch 1 adds a VIRTIO_STATUS_ERROR() check that recognizes an
> > all-ones status read as invalid (per spec, bits 4-5 are reserved and
> > can never legitimately be set) and warns once at the point the bad
> > read actually happens.
> >
> > Patch 2 uses that check to break out of vp_reset()'s poll loop
> > instead of spinning forever.
>
> Was all this including the cover letter written with ai assistance?
> if yes pls disclose this.

Yes, I used AI assistance (Claude). The commit messages were written
by me and then refined with AI for spelling and grammar; the cover
letter was generated by Claude and reviewed by me. The code, testing,
and debugging were done by me -- I reproduced the hang in QEMU,
debugged to reach the hanging loop, and wrote the actual fix.

I should have disclosed this upfront. I'll do so in future
submissions.

Do I need to add Assisted-by: Claude <claude-4-6-sonnet> to the
commit messages?

P.S. This is my first kernel patch set.

Re: [PATCH 0/2] virtio_pci_modern: fix vp_reset() hang on unresponsive device

[Michael S. Tsirkin]

On Sun, Aug 02, 2026 at 06:28:03PM +0000, Abhin Parekadan Jose wrote:
> On Sun, Aug 02, 2026 at 01:47:01PM -0400, Michael S. Tsirkin wrote:
> > On Sun, Aug 02, 2026 at 05:40:57PM +0000, Abhin Parekadan Jose wrote:
> > > While investigating a syzbot report of a WARN_ON_ONCE firing in
> > > virtio_dev_remove() [1],
> >
> >
> > And I responded to that syzbot report, and I quote:
> >
> > So it writes 0 into pci command, effectively killing the device,
> > and then is unhappy that the driver prints warnings?
> > Who thought it's a good idea? Why?
> 
> I was learning how to reproduce syzbot bugs when I found this
> issue by writing 0 to PCI_COMMAND to simulate an unresponsive
> device.

Yea I have no idea where does this syzbot "bug report"
come from. Poking at random at device registers is ... not
a very good idea.

> While doing that I noticed that echo 1 > /sys/../remove
> hung completely rather than just printing the warning. Since the
> device_status register lives in the virtio common config MMIO
> space and has defined values(based on the bits set) in the spec.
> I thought it made sense for virtio to detect this and handle it
> gracefully rather than spin forever, so I wrote up a small fix
> for that.
> 
> > > I found a related but more serious issue:
> > > vp_reset() in the modern virtio-pci transport can hang indefinitely
> > > if PCI_COMMAND memory-space decode is disabled while the device is
> > > bound (e.g. surprise removal, hardware fault, or -- as reproduced
> > > here -- a direct write to the PCI_COMMAND register). The status
> > > register poll loop has no way to distinguish "device still resetting"
> > > from "device unreachable," so it never terminates.
> > >
> > > Patch 1 adds a VIRTIO_STATUS_ERROR() check that recognizes an
> > > all-ones status read as invalid (per spec, bits 4-5 are reserved and
> > > can never legitimately be set) and warns once at the point the bad
> > > read actually happens.
> > >
> > > Patch 2 uses that check to break out of vp_reset()'s poll loop
> > > instead of spinning forever.
> >
> > Was all this including the cover letter written with ai assistance?
> > if yes pls disclose this.
> 
> Yes, I used AI assistance (Claude). The commit messages were written
> by me and then refined with AI for spelling and grammar; the cover
> letter was generated by Claude and reviewed by me.

I suggest limiting it to fixing spelling and grammar exclusively.  It
tends to do things like dramatize, e.g. "more serious issue", like it
did here.

> The code, testing,
> and debugging were done by me -- I reproduced the hang in QEMU,
> debugged to reach the hanging loop, and wrote the actual fix.
> 
> I should have disclosed this upfront. I'll do so in future
> submissions.
> 
> Do I need to add Assisted-by: Claude <claude-4-6-sonnet> to the
> commit messages?

    Assisted-by: Claude:claude-sonnet-4-6


> 
> P.S. This is my first kernel patch set.


Thanks, keep at it. Bonus points if you find a real fix for
issues raised in thread about surprise removal, see e.g. here
cover.1752094439.git.mst@redhat.com
but don't expect it to be easy.


-- 
MST

Re: [PATCH 0/2] virtio_pci_modern: fix vp_reset() hang on unresponsive device

[Abhin Parekadan Jose]

On Sun, Aug 02, 2026 at 03:08:12PM -0400, Michael S. Tsirkin wrote:
> On Sun, Aug 02, 2026 at 06:28:03PM +0000, Abhin Parekadan Jose wrote:
> > On Sun, Aug 02, 2026 at 01:47:01PM -0400, Michael S. Tsirkin wrote:
> > > On Sun, Aug 02, 2026 at 05:40:57PM +0000, Abhin Parekadan Jose wrote:
> > > > While investigating a syzbot report of a WARN_ON_ONCE firing in
> > > > virtio_dev_remove() [1],
> > >
> > >
> > > And I responded to that syzbot report, and I quote:
> > >
> > > So it writes 0 into pci command, effectively killing the device,
> > > and then is unhappy that the driver prints warnings?
> > > Who thought it's a good idea? Why?
> > 
> > I was learning how to reproduce syzbot bugs when I found this
> > issue by writing 0 to PCI_COMMAND to simulate an unresponsive
> > device.
> 
> Yea I have no idea where does this syzbot "bug report"
> come from. Poking at random at device registers is ... not
> a very good idea.
> 
> > While doing that I noticed that echo 1 > /sys/../remove
> > hung completely rather than just printing the warning. Since the
> > device_status register lives in the virtio common config MMIO
> > space and has defined values(based on the bits set) in the spec.
> > I thought it made sense for virtio to detect this and handle it
> > gracefully rather than spin forever, so I wrote up a small fix
> > for that.
> > 
> > > > I found a related but more serious issue:
> > > > vp_reset() in the modern virtio-pci transport can hang indefinitely
> > > > if PCI_COMMAND memory-space decode is disabled while the device is
> > > > bound (e.g. surprise removal, hardware fault, or -- as reproduced
> > > > here -- a direct write to the PCI_COMMAND register). The status
> > > > register poll loop has no way to distinguish "device still resetting"
> > > > from "device unreachable," so it never terminates.
> > > >
> > > > Patch 1 adds a VIRTIO_STATUS_ERROR() check that recognizes an
> > > > all-ones status read as invalid (per spec, bits 4-5 are reserved and
> > > > can never legitimately be set) and warns once at the point the bad
> > > > read actually happens.
> > > >
> > > > Patch 2 uses that check to break out of vp_reset()'s poll loop
> > > > instead of spinning forever.
> > >
> > > Was all this including the cover letter written with ai assistance?
> > > if yes pls disclose this.
> > 
> > Yes, I used AI assistance (Claude). The commit messages were written
> > by me and then refined with AI for spelling and grammar; the cover
> > letter was generated by Claude and reviewed by me.
> 
> I suggest limiting it to fixing spelling and grammar exclusively.  It
> tends to do things like dramatize, e.g. "more serious issue", like it
> did here.
> 
> > The code, testing,
> > and debugging were done by me -- I reproduced the hang in QEMU,
> > debugged to reach the hanging loop, and wrote the actual fix.
> > 
> > I should have disclosed this upfront. I'll do so in future
> > submissions.
> > 
> > Do I need to add Assisted-by: Claude <claude-4-6-sonnet> to the
> > commit messages?
> 
>     Assisted-by: Claude:claude-sonnet-4-6
> 
> 
> > 
> > P.S. This is my first kernel patch set.
> 
> 
> Thanks, keep at it. Bonus points if you find a real fix for
> issues raised in thread about surprise removal, see e.g. here
> cover.1752094439.git.mst@redhat.com
> but don't expect it to be easy.
> 

That looks interesting (haven't gone through in detail but got a gist of it).
I'll try it out and make suggestions if I find a good solution.

As for the current patch set, does it make sense to drop macro
`VIRTIO_STATUS_ERROR` and use `PCI_POSSIBLE_ERROR` to break out of the loop,
I could test it by actually doing a surprise removal on qemu via the monitor
or just drop this patch set and try on cover.1752094439.git.mst@redhat.com patch set?

> -- 
> MST
> 

Re: [PATCH 0/2] virtio_pci_modern: fix vp_reset() hang on unresponsive device

[Michael S. Tsirkin]

On Sun, Aug 02, 2026 at 09:48:26PM +0200, Abhin Parekadan Jose wrote:
> On Sun, Aug 02, 2026 at 03:08:12PM -0400, Michael S. Tsirkin wrote:
> > On Sun, Aug 02, 2026 at 06:28:03PM +0000, Abhin Parekadan Jose wrote:
> > > On Sun, Aug 02, 2026 at 01:47:01PM -0400, Michael S. Tsirkin wrote:
> > > > On Sun, Aug 02, 2026 at 05:40:57PM +0000, Abhin Parekadan Jose wrote:
> > > > > While investigating a syzbot report of a WARN_ON_ONCE firing in
> > > > > virtio_dev_remove() [1],
> > > >
> > > >
> > > > And I responded to that syzbot report, and I quote:
> > > >
> > > > So it writes 0 into pci command, effectively killing the device,
> > > > and then is unhappy that the driver prints warnings?
> > > > Who thought it's a good idea? Why?
> > > 
> > > I was learning how to reproduce syzbot bugs when I found this
> > > issue by writing 0 to PCI_COMMAND to simulate an unresponsive
> > > device.
> > 
> > Yea I have no idea where does this syzbot "bug report"
> > come from. Poking at random at device registers is ... not
> > a very good idea.
> > 
> > > While doing that I noticed that echo 1 > /sys/../remove
> > > hung completely rather than just printing the warning. Since the
> > > device_status register lives in the virtio common config MMIO
> > > space and has defined values(based on the bits set) in the spec.
> > > I thought it made sense for virtio to detect this and handle it
> > > gracefully rather than spin forever, so I wrote up a small fix
> > > for that.
> > > 
> > > > > I found a related but more serious issue:
> > > > > vp_reset() in the modern virtio-pci transport can hang indefinitely
> > > > > if PCI_COMMAND memory-space decode is disabled while the device is
> > > > > bound (e.g. surprise removal, hardware fault, or -- as reproduced
> > > > > here -- a direct write to the PCI_COMMAND register). The status
> > > > > register poll loop has no way to distinguish "device still resetting"
> > > > > from "device unreachable," so it never terminates.
> > > > >
> > > > > Patch 1 adds a VIRTIO_STATUS_ERROR() check that recognizes an
> > > > > all-ones status read as invalid (per spec, bits 4-5 are reserved and
> > > > > can never legitimately be set) and warns once at the point the bad
> > > > > read actually happens.
> > > > >
> > > > > Patch 2 uses that check to break out of vp_reset()'s poll loop
> > > > > instead of spinning forever.
> > > >
> > > > Was all this including the cover letter written with ai assistance?
> > > > if yes pls disclose this.
> > > 
> > > Yes, I used AI assistance (Claude). The commit messages were written
> > > by me and then refined with AI for spelling and grammar; the cover
> > > letter was generated by Claude and reviewed by me.
> > 
> > I suggest limiting it to fixing spelling and grammar exclusively.  It
> > tends to do things like dramatize, e.g. "more serious issue", like it
> > did here.
> > 
> > > The code, testing,
> > > and debugging were done by me -- I reproduced the hang in QEMU,
> > > debugged to reach the hanging loop, and wrote the actual fix.
> > > 
> > > I should have disclosed this upfront. I'll do so in future
> > > submissions.
> > > 
> > > Do I need to add Assisted-by: Claude <claude-4-6-sonnet> to the
> > > commit messages?
> > 
> >     Assisted-by: Claude:claude-sonnet-4-6
> > 
> > 
> > > 
> > > P.S. This is my first kernel patch set.
> > 
> > 
> > Thanks, keep at it. Bonus points if you find a real fix for
> > issues raised in thread about surprise removal, see e.g. here
> > cover.1752094439.git.mst@redhat.com
> > but don't expect it to be easy.
> > 
> 
> That looks interesting (haven't gone through in detail but got a gist of it).
> I'll try it out and make suggestions if I find a good solution.
> 
> As for the current patch set, does it make sense to drop macro
> `VIRTIO_STATUS_ERROR` and use `PCI_POSSIBLE_ERROR` to break out of the loop,
> I could test it by actually doing a surprise removal on qemu via the monitor
> or just drop this patch set

I'd drop this, I'm not interested in working around one source of hangs
if others in the same exact path remain unfixable.

> and try on cover.1752094439.git.mst@redhat.com patch set?

it's not a question of "trying it on" it's a question of the fact that
the pci core serializes probe/removal events so a driver inside
probe/remove never sees the removal event. in this instance it
is polling so it can check (at the cost of adding cpu overhead,
mostly for nothing) but in most places it can't, we need the event
to reach it.

> > -- 
> > MST
> >