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

[Abhin Parekadan Jose <abhinjoses@gmail.com>]

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
>