No, sorry, I tested it, only with a unit test after finding it via static analysis, not the "real" way using the whole kernel build. (Also replied with the test for 0002.)
Found with static analysis, reproduced with a unit test (save attached file to drivers/virtio/test0004.c): % cd drivers/virtio % git checkout virtio_ring.c Updated 0 paths from the index % sed -n '/^void vring_transport_features/,/^}/p' virtio_ring.c > extracted.c && gcc -Wall -Werror -o test0004 test0004.c && ./test0004 FAIL: VIRTIO_F_RING_RESET cleared during negotiation % git apply 0004-virtio_ring-preserve-VIRTIO_F_RING_RESET-in-transpor.patch % sed -n '/^void vring_transport_features/,/^}/p' virtio_ring.c > extracted.c && gcc -Wall -Werror -o test0004 test0004.c && ./test0004 PASS: VIRTIO_F_RING_RESET preserved If this doesn't work, feel free to ditch. On Tue, Apr 7, 2026 at 2:08 PM Michael S. Tsirkin <[email protected]> wrote: > On Tue, Apr 07, 2026 at 01:33:39PM -0400, Andrew Stellman wrote: > > Code analysis only — not observed in practice. And you're right that on > PCI it > > doesn't matter: vp_transport_features() re-sets the bit after > > vring_transport_features() clears it, so PCI never actually loses the > feature. > > Oh. please do test patches, or note they were not tested. > how were rest of patches here tested? > > > > > The gap is for transports that call vring_transport_features() without > > independently re-setting VIRTIO_F_RING_RESET afterward. Whether any > current > > transport hits this in practice I'm not sure — it may just be a whitelist > > consistency fix at this point. > > > > Happy to add a Fixes tag and resend, or drop it if you think the > PCI-level > > re-set makes it unnecessary. > > > > Fixes: 04ca0b0b16f1 ("virtio_pci: support VIRTIO_F_RING_RESET") > > > surely not this because pci is fine. > > > On Tue, Apr 7, 2026 at 12:22 PM Michael S. Tsirkin <[email protected]> > wrote: > > > > On Tue, Apr 07, 2026 at 08:39:04AM -0400, Andrew Stellman wrote: > > > vring_transport_features() whitelists known transport feature bits > and > > > clears the rest via __virtio_clear_bit(). VIRTIO_F_RING_RESET is > > > missing from the whitelist, so it is unconditionally cleared during > > > feature negotiation. Drivers that depend on ring reset capability > > > silently lose the feature. > > > > Hmm was this observed in practice or just from code analysis? > > And on which transport? > > Because > > > > > > static void vp_transport_features(struct virtio_device *vdev, u64 > features) > > { > > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > struct pci_dev *pci_dev = vp_dev->pci_dev; > > > > if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) && > > pci_find_ext_capability(pci_dev, > > PCI_EXT_CAP_ID_SRIOV)) > > __virtio_set_bit(vdev, VIRTIO_F_SR_IOV); > > > > if (features & BIT_ULL(VIRTIO_F_RING_RESET)) > > __virtio_set_bit(vdev, VIRTIO_F_RING_RESET); > > > > > > ... > > > > > > > > } > > > > > > > > > > > Add VIRTIO_F_RING_RESET to the switch statement, matching the other > > > transport-level features. > > > > > > Signed-off-by: Andrew Stellman <[email protected]> > > > --- > > > drivers/virtio/virtio_ring.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/virtio/virtio_ring.c > b/drivers/virtio/virtio_ring.c > > > index fbca7ce..2cb643f 100644 > > > --- a/drivers/virtio/virtio_ring.c > > > +++ b/drivers/virtio/virtio_ring.c > > > @@ -3524,6 +3524,8 @@ void vring_transport_features(struct > virtio_device > > *vdev) > > > break; > > > case VIRTIO_F_IN_ORDER: > > > break; > > > + case VIRTIO_F_RING_RESET: > > > + break; > > > default: > > > /* We don't understand this bit. */ > > > __virtio_clear_bit(vdev, i); > > > -- > > > 2.34.1 > > > > > >
test0004.c
Description: Binary data
